Repository navigation
fix(cli): complete servers/add|edit|remove and --output-format values - #2634
Merged
Merged
Conversation
…tput-format values (#2629) --completion (#2434) offered --method values from a hard-coded CATALOG_METHODS of servers/list and servers/show, so the catalog writes added in the same milestone (#2433) were never completed, and --output-format (#2431) offered no values at all. CATALOG_METHODS now spreads CATALOG_WRITE_METHODS from servers-write, and parseArgs validates --method against that same list, so completion and validation share one source. --output-format completes from OUTPUT_FILE_FORMATS. New tests read the write methods from servers-write rather than from the completion list (the old test compared the list to itself, which is how the gap shipped), and check no offered --method is rejected as unsupported. Found by the 2.10.0 release smoke (#2623). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation satisfies the linked issue and includes appropriate drift and shell-completion coverage.
0 open findings
What changed in this PR
Completes CLI shell suggestions for catalog write methods and output formats while sharing validation constants.
Changes:
- Adds
servers/add,servers/edit, andservers/removecompletions. - Adds
rawandjsonoutput-format completions. - Adds parser and Bash completion regression tests.
| File | Description |
|---|---|
clients/cli/src/completion.ts |
Defines shared catalog methods and output-format choices. |
clients/cli/src/cli.ts |
Reuses shared catalog validation and error-list generation. |
clients/cli/__tests__/completion.test.ts |
Covers write-method and output-format completions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
Copilot review loop closed: round 1 was clean (0 findings — no inline comments, nothing in the headline or a suppressed block), so no further round was requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2629
Found by the 2.10.0 release smoke (#2623).
What changed
--completion(#2434) offered--methodvalues from a hard-codedCATALOG_METHODSofservers/listandservers/show, so the catalog writes added in the same milestone (#2433) never completed.--output-format(#2431) offered no values at all.CATALOG_METHODSnow spreadsCATALOG_WRITE_METHODSfromhandlers/servers-write.ts.parseArgsvalidates--methodagainst the same list (isCatalogMethod), so completion and validation share one source. The "Unsupported method" message lists the same methods in the same order as before.--output-formatcompletes fromOUTPUT_FILE_FORMATS(raw,json). The existing drift guard already runs every stated choice through the real option parser, so this value set is checked too.servers-write, not from the completion list. The old test compared the list with itself, which is how the gap shipped. Another new test checks that no offered--methodis rejected as unsupported, with a bogus-method control. The bash shell test now covers--method servers/and--output-format.Verification
--method servers/givesservers/list servers/show, and--output-formatgives nothingservers/list servers/show servers/add servers/edit servers/remove, andraw jsonnpm run local:gate: every stage passes (CLI 583 + 2 skipped, DCO 1/1,smoke:mcpdopassed) exceptsmoke:web:firefox, which cannot launch Playwright's Firefox on macOS 27 (smoke:web:firefox cannot launch Playwright's Firefox on macOS 27 #2625). CI does not run the Firefox pass, and this diff is CLI-only.local:storybookwas run separately and passed 529.🤖 Generated with Claude Code