Repository navigation
docs(llm): fix m365 force flag, atl attachment/unarchive syntax, safety labels - #131
Conversation
…ty labels - m365: v11 has no --confirm; name -f/--force as the prompt-skipping flag - atl: --download --id, --download-all, --upload; page archive --unarchive; drop deprecated-alias note and v1.12 pins (doc already needs >= v1.13) - atl: add assign, changelog, doctor, auth refresh, transition --comment, page edit --append - n8nctl: add project list and workflow transfer - gh: fetch the repo's default branch instead of main - Safety labels replace CRITICAL; lower shouting caps where the line carries its reason
There was a problem hiding this comment.
Code Review
This pull request updates the documentation in lib/llm/index.js for several CLI tools, including sqlcmd, git, atl, n8nctl, gcx, and m365. Key updates include renaming 'CRITICAL' safety headers to 'Safety', replacing hardcoded branch names with placeholders, adding new commands for Jira, Confluence, and n8nctl, and clarifying safety guidelines regarding the -f/--force flag. The review feedback suggests enclosing the transition name 'Done' in double quotes in the newly added Jira transition example to maintain consistency and prevent potential shell parsing issues.
With -S the server flag wins over the current context, so showing the context displayed the wrong target for production MI writes.
|
Ratatoskr reviewed this pull request. Changes requested on Finished 2026-10-06 13:13 UTC. |
fank
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES: 1 blocking finding. The new confluence page archive --unarchive guidance documents a command that always fails in every atl-cli release.
Review details
Reviewed head 22c4afd6b107c8fbe076a11d8a7c51581328360f.
Blocking
1. archive --unarchive is a stub that always fails (item 27): lib/llm/index.js:382, :403, and the removed note at old line 388
The --unarchive flag exists in --help, but the implementation never calls an API. In atl-cli v1.13.0, v1.14.0 and current main, internal/api/confluence.go contains:
// UnarchivePage restores an archived page.
// NOTE: Confluence Cloud has no REST API for unarchiving pages.
// The v1 workaround using PUT /content/{id} was deprecated (410 Gone).
// Users must restore archived pages via the Confluence web UI.
func (s *ConfluenceService) UnarchivePage(ctx context.Context, pageID string) error {
return fmt.Errorf("unarchive is not supported via API - ... Please use the Confluence web UI to restore archived pages")
}runArchive in internal/cmd/confluence/page/archive.go calls it for each page ID and reports Failed to unarchive page …. The text this PR removes ("410 Gone (unarchive removed - use web UI)" and "reversible (via web UI only - no restore API)") was correct. The replacement is wrong in two ways:
- It sends agents to a command that cannot succeed.
- The tip "Archive is reversible (
archive --unarchive)" tells the model that it can undo an archive itself. An agent that believes this may archive pages more readily. In practice, only a human in the web UI can restore them.
Checking --help alone can't catch this. The flag is registered, but nothing behind it calls an API.
Fix: drop the --unarchive example line and restore both the API note and the "via web UI only" tip. Option: keep a line that says --unarchive exists but returns an error, so agents don't try it.
Verified correct (against atl-cli v1.13.0 source, the floor named in the doc header, and n8n-cli v1.3.0)
jira issue attachment:--downloadrequires--id(attachment.go:70-71).--download-all,--upload(stringArray, repeatable) and--outputall exist.jira issue assign --assignee(@me, a user, or-),jira issue changelog --field,transition --comment/-c,confluence page edit --append/-a,doctorandauth refresh --hostnameall exist in v1.13.0. Removing thev1.12.0+pins therefore loses nothing.- The deprecated top-level aliases are still registered (hidden) in v1.14.0 (
internal/cmd/root.go:75-79). Dropping the sentence only removes a pointer to a deprecated form, so that change is fine. - n8nctl
workflow transfer <workflow-id> <project-id>with--skip-credentials, andproject list, both exist in v1.3.0 (internal/cmd/workflow/workflow.go:540-578,internal/cmd/project/project.go). - m365
-f/--forcereplacing--confirm: I did not check this against the m365 binary because it isn't installed here. It does match the PnP CLI's documented change from--confirmto--forcesince v7, and no--confirmremains in the file. - sqlcmd write rule: step 1 now names the
-Sserver first. That fixes the real mismatch wherecurrent-contextshowedstagewhile a-Swrite went elsewhere. The doc and the generated block (:65,:1239) are in sync. - Label softening: every rule and step is still present. Only the emphasis changed.
Optional, non-blocking
n8nctl workflow transfer(:639-640) moves credentials across projects by default. The n8nctl section has no safety rule, which matches howactivateis documented today. Still, a short "confirm before transferring" note may be worth adding, because this command changes who can use a credential.
Tests and checks
- This is a docs-only change in a template string, and no test covers
CLI_DOCScontent. That is consistent with the repository today, so it is not a blocker. - I could not run
node --testlocally because dependencies (chalk) are not installed in the review checkout, and I did not runnpm installthere. CI lint, format and CodeQL pass. There is no test job in the PR checks.
Earlier discussions
- One review thread exists (gemini-code-assist, quoting
"Done"). It is already resolved, and the current head quotes the name at:265. No action needed. I did not author it, so I did not touch it.
…ow transfer atl's confluence page archive --unarchive is registered but returns an error without calling any API, since Confluence Cloud has no restore endpoint.
|
Ratatoskr reviewed this pull request. Approved on Finished 2026-10-07 06:18 UTC. |
There was a problem hiding this comment.
APPROVE: 0 blocking findings. The earlier --unarchive blocker is fixed at 6ae37a9, and the new flags match the tool sources.
Review details
Reviewed head 6ae37a9484572b79247db2353f52e549b90d58e9. It is up to date with main, and the only changed file is lib/llm/index.js.
Earlier blocker: resolved
- The
confluence page archive --unarchiveexample is removed.lib/llm/index.js:396again has the "410 Gone (unarchive removed - use web UI)" note, and:403again has "Archive is reversible (via web UI only - no restore API), delete is not". Both lines now matchmain.
Verified against source
- atl-cli
v1.14.0:jira issue attachmenthas-d/--download("requires --id"),--id,-a/--download-all,-o/--output(default.) and a repeatable-u/--upload(internal/cmd/issue/attachment.go:78-83).jira issue assign --assigneeaccepts@me, and-unassigns it (assign.go:53,79).jira issue changelog --fieldexists (changelog.go:68).transition -c/--commentexists (transition.go:61).confluence page edit -a/--appendexists (confluence/page/edit.go:60).auth refresh --hostnameexists (auth/refresh.go:48), and so doesdoctor(internal/cmd/doctor/).
- n8n-cli
v1.3.0:workflow transfer <workflow-id> <project-id>transfers the credentials unless--skip-credentialsis passed (internal/cmd/workflow/workflow.go:540-578). The new "confirm the target project with the user first" note fits that behavior. - m365 (public
pnp/cli-microsoft365source):spo file removedocuments-f, --force("Don't prompt for confirming…"). Commands such asspo cdn origin removedefine onlyforce(aliasf) and noconfirmoption. That supports replacing--confirmwith-f/--forcein step 3 and in the tip (:782,:878,:1251).--confirmno longer appears anywhere in the file. - sqlcmd: the doc rule (
:65-67) and the generated block (:1240-1242) now both resolve the-Sserver first. The two copies match each other. - Wording changes: I compared every changed
CRITICAL/NEVER/DO NOT/NOTline with the old text. Each rule and step is still there; only the emphasis changed.
Optional, non-blocking
:639-640: in "Credentials move too unless --skip-credentials, which changes who can use them", the "which" could be read as referring to--skip-credentialsinstead of the credential move. Something like "Credentials move too (changing who can use them) unless --skip-credentials is passed" would remove the ambiguity.
Tests and checks
- This is a docs-only change inside a template string. No test pins the
CLI_DOCScontent, which is how the repository already works, so it does not block this PR. node --check lib/llm/index.jspasses. I could not runnpm testbecausenode_modulesis not installed in the review checkout, and I did not runnpm install. CI Lint, Check format and CodeQL pass. The License Check was skipped, and the PR checks have no test job.- The atl, n8nctl and m365 binaries are not installed here, so I checked flags against source at the tagged versions, not with
--help.
Earlier discussions
- r4195690670 (mine,
--unarchiveexample): verified fixed, replied and resolved. - r4195690676 (mine, archive-reversible tip): verified fixed, replied and resolved.
- r4184646231 (gemini-code-assist, quoting
"Done"): already resolved, and the current head quotes it at:265. I left it alone.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the LLM system prompt and tool documentation in lib/llm/index.js. Key changes include refining safety rules for sqlcmd and m365 (such as resolving the target server/database and warning against -f/--force instead of --confirm), updating various CLI commands for atl (adding token refresh, doctor, issue assignment, transition comments, and attachment options), documenting the --append flag for Confluence, and adding n8nctl workflow transfer and project listing commands. Additionally, several instances of uppercase emphasis (e.g., 'CRITICAL', 'NEVER', 'DO NOT') were toned down to standard casing or replaced with 'Safety'. There are no review comments, and I have no additional feedback to provide.
… atl version gates - atl: require v1.14.0, add 'jira assets attributes' and 'jira api --api-version 2', document --reply-to without the pre-1.14 branch - n8nctl: --project for project-scoped variables; clarify transfer credential note - sqlcmd: define the context resolved when -S is absent - m365: drop the Tips bullet that restated Safety rule 3
|
/gemini review |
|
Dispositions for the Ratatoskr reviews (changes-requested summary, changes-requested review, approve summary):
11ed9dd also fixes what the independent readiness reviews found:
|
There was a problem hiding this comment.
Code Review
This pull request updates the documentation and instructions for various CLI tools (such as sqlcmd, atl, n8nctl, and m365) in lib/llm/index.js, adding new commands, updating safety rules, and softening capitalization. A review comment suggests improving the Git documentation by using origin/HEAD instead of the placeholder to make the commands fully copy-pasteable and directly executable.
|
🔏 Readiness attested — the Readiness summaryPR Readiness Check (#131, 11ed9dd) |
Problem
An audit of the generated CLI docs (
lib/llm/index.js) found flags and subcommands that don't exist in the installed tools. Target models follow the docs literally, so a wrong flag produces a wrong command. The audit also found shouting labels (**CRITICAL**,NEVER,DO NOT) that add pressure without adding information.Changes
--confirm; the prompt-skipping flag is-f, --force. Step 3 of the m365 rule (cli-m365 doc and generated CLAUDE.md block) and the cli-m365 tip now name-f/--force. Steps 1–2 are unchanged.-Sserver if the command passes one, otherwise the context set bysqlcmd config use-contextearlier in the same command, else the current context. Before, they showedsqlcmd config current-context, which showsstage/localwhile a-Swrite goes to the production MI. Step 3 (explicit confirmation) is unchanged.**CRITICAL**:→**Safety**:in cli-sqlcmd, cli-m365, cli-hcloud and cli-ovhcloud, which matches the generated block. Steps are unchanged; the sqlcmd write-safety steps are untouched. Caps were lowered only where the line already gives its reason (gh reviews endpoint, ADF mention syntax, gcx stack-login restart, gcx--cloud-token, playwright Basic Auth, Confluence--body). No constraint was removed.--download <id>→--download --id <id>; added--download-alland--upload.page archive --unarchiveis listed in--help, butUnarchivePage(internal/api/confluence.go, v1.14.0 and main) returns an error without calling any API, because Confluence Cloud has no restore endpoint. The existing "410 Gone (unarchive removed - use web UI)" note and the "via web UI only - no restore API" tip are kept unchanged.jira …command forms. Removed thev1.12.0+pins: every example passes--context, which needs ≥ v1.13.0. The header now requires v1.14.0, so--reply-to(which notifies the author since v1.14.0) is documented without the old v1.13.0 branches.workflow transfermoves the workflow's credentials unless--skip-credentialsis passed: "Credentials move too (changing who can use them) unless --skip-credentials", and the doc says to confirm the target project with the user first.jira assets attributes <object-type-id>,jira api --api-version 2(v3 stays the default), and n8nctlvariable … --project <id>for project-scoped variables. The m365 Tips bullet that repeated Safety rule 3 is removed.git fetch origin main→git fetch origin <default-branch>(masterormain).jira issue assign,jira issue changelog,doctor,auth refresh,transition --comment,confluence page edit --append,attachment --upload. n8nctl:project list,workflow transfer. Skipped: n8nctlvariable(already documented). Skipped:gh attach list/get, because gh attach is not documented in this package. Its doc (negsoft-pr-screenshots.md) comes from environment-setupsrc/llm_internal.js, so it is a follow-up there.Verification
Each flag and subcommand was checked against the installed binaries with read-only
--help.go version -mreports atl-cli v1.14.0, n8n-cli v1.3.0 and m365 v11.11.0:spo file remove --help,spo list remove --helpandspo site remove --helpall show-f, --force — Don't prompt for confirm…. None of them has--confirm.jira issue attachment --helpshows-d, --download Download a specific attachment (requires --id),--id,-a, --download-all,-u, --upload stringArrayand-o, --output.jira issue assign --helpshows--assignee(@me, or-to unassign).jira issue changelog --helpshows--fieldand--limit.jira issue transition --helpshows-c, --comment.confluence page edit --helpshows-a, --append.doctor --helpandauth refresh --help(--hostname) both exist.n8nctl project --helplists onlylist.n8nctl workflow transfer --helpshows<workflow-id> <project-id>and--skip-credentials.CLI_DOCScontent contains no leftover--confirm,CRITICALorNEVER, and no stray${.11ed9dd:npm test6 pass, 0 fail; eslint andprettier --checkclean. The atl 1.14.0 and n8nctl 1.3.0 additions were checked against the tagged sources (gh api …/contents/…?ref=<tag>).Rollout
environment-setup uses the published
@enthus-appdev/llm-cli-setuppackage, so these docs reach developer machines only after a release of this package and a bump in environment-setup.