Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The new command works as described. I ran it from source against a test project with JSONC comments, current-style mcp.servers entries and an old-style top-level entry. With --yes it removed each one and kept the other servers and comments. Without a terminal, a missing name or a missing --yes fails and the file is left unchanged. --local --global is rejected. bun test test/mcp-remove.test.ts (8 tests) and bun typecheck in packages/cli both pass. I didn't drive the interactive chooser myself.
Public API: adds the CLI subcommand opencode mcp remove [name] with the flags --local, --global and --yes. Purely additive and backwards compatible; config keys and formats are unchanged.
Three things worth fixing, noted inline:
- Removing an unknown name exits 0, so scripts can't tell that nothing was removed.
- The handler keeps its own copy of core's config-file search, which can drift out of sync with core.
- The test imports the
tmpdirfixture from the core package instead of the CLI's own fixture.
Share configuration filenames with core through Util and use the CLI's own temporary-directory fixture.
There was a problem hiding this comment.
My earlier concerns are addressed. Removing an unknown name now fails: mcp remove nope --yes exits 1 and leaves the config unchanged, which I checked by running the CLI from source. The test now uses the CLI's own tmpdir fixture. Core and the CLI now share one list of config file names (@opencode/util/config-file). It holds the same values in the same order as before, so core's config discovery is unchanged. The walk up from the current folder and the symlink check are still copied in remove.ts, but I don't consider that blocking. bun test test/mcp-remove.test.ts (8 tests) passes, and bun typecheck passes in packages/cli, packages/core and packages/util.
Public API: adds the CLI subcommand opencode mcp remove [name] with --local, --global and --yes, plus the internal shared module @opencode/util/config-file. Nothing existing changes, so it is backwards compatible.
No further concerns.
|
Thanks, both look good to me at 0da8037. Keeping the folder search separate makes sense: |
Issue for this PR
Closes #49812
Type of change
What does this PR do?
opencode mcp remove [name]for local and remote servers, including legacy config entries.--localand--globalnarrow the choices.--yesfor scripts. Noninteractive calls require a name and reject ambiguity without editing anything.The command deletes only the selected entry and separator in the selected file, preserving unrelated settings, servers, and JSONC comments. Unlike
mcp logout, it removes configuration, not OAuth credentials. It does not delete scripts or change/mcps.How did you verify your code works?
packages/cli:bun test --timeout 30000 test/mcp-remove.test.ts --rerun-each 10 --randomize— 80 passed.packages/cli:bun test --timeout 30000 --only-failures— 338 passed, 9 skipped, 0 failed (124.50s).packages/cli:bun typecheck— passed.bun run check— passed, with non-blocking lint warnings.services/www:bun typecheck,bun run check:generated, andbun run build— passed.Screenshots / recordings
CLI-only change; manually verified the prompts as described above. No recording attached.
Checklist
Added by the opencode bot (AI-generated).
Public API: adds the
opencode mcp remove [name]CLI command with--local,--globaland--yesflags (backwards compatible)