Repository navigation
Dev - #43
Dev#43
Conversation
…y lands `otx install` wrote `extraKnownMarketplaces` into settings.json and then shelled out to `claude plugin install`. But that declaration is not what the install reads: Claude Code turns it into a record in plugins/known_marketplaces.json when a session starts, and `claude plugin install` resolves the plugin through the record. On a machine where no session had yet registered it, the install failed with `Plugin "opentrace" not found in marketplace "opentrace"` — and the fallback we printed, `Run claude plugin install opentrace@opentrace`, failed exactly the same way. Onboarding reported `✓ added` and left nothing installed. So register the marketplace first when it is not already recorded, and stop rather than installing blind if that fails. Also surface the reason: the note used to give a command and no cause, which is what made the dead end hard to read. The harness stub was complicit — it accepted any `plugin install` and exited 1 for everything else, so it green-lit an install the real binary rejects. It now writes the marketplace record on `marketplace add` and refuses an install from a marketplace that was never registered, which is the contract the real CLI has. Verified by removing the fix: two unit tests and two integration tests fail without it. Endpoint line in the summary now prints the no-slash form. buildMcpUrl keeps the trailing slash for the keychain and header-auth entries, but the plugin authenticates over OAuth against the form the server advertises, and that is the endpoint a user might paste. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…onnector The bridge wrote this into claude_desktop_config.json: npx -y mcp-remote <url> --header "Authorization: Bearer otk_…" which put the raw CLI key in a process argument, readable by any local process through `ps` or /proc/<pid>/cmdline. It also needed Node on PATH, ran a proxy process per session, depended on a third-party package to hold our credential, and — because a plugin's server is scoped `plugin:opentrace:opentrace` — made local Code-tab sessions list OpenTrace twice, which the code called unavoidable. It was aimed at the wrong file. The chat surface takes connectors from the claude.ai account, not from disk, and a custom connector accepts a remote MCP URL directly on the same runtime, transport and auth a directory listing uses. So otx now prints the prefilled add-custom-connector link and writes nothing: OAuth in the browser, no key on disk, no npx, no duplicate, and it reaches claude.ai and mobile rather than desktop alone. Answering the question this started from: for Claude Code — terminal and desktop Code tab alike — the MCP route was already redundant, because the plugin bundles its own server and supersedes the bare entry. The bridge was the only local MCP write aimed at Claude, so removing it leaves the Code tab covered by the plugin and the chat surface covered by a link. The per-editor MCP route still matters for Cursor, VS Code, Windsurf, Zed and JetBrains, which have no plugin equivalent. Migration: entries written by older versions are live stdio servers still holding a key, so `install` removes one when it finds it and says why, and `remove()` stays for `disconnect`. KeyClient gains `writable?: false` for exactly this shape — a client otx cleans up after but no longer configures — and `connect --client claude-desktop` prints the link instead of failing on a flag that used to work. BREAKING CHANGE: `otx connect --client claude-desktop` and `otx login --client claude-desktop` no longer write claude_desktop_config.json. They print the connector link. `disconnect --client claude-desktop` is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A `git add -A` in the previous commit swept in `.claude/settings.json` and `.claude/worktrees/plugin-hook-prewarm` — the latter as a gitlink, since it is its own checkout. Neither belongs here: the worktree is local scratch space and the settings file was untracked by choice. Ignore the worktrees directory and settings.local.json so this cannot recur. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Express and the happy path do the plugin, and the plugin already covers the desktop Code tab through ~/.claude — so onboarding has nothing left to say about the Claude app. It was printing an add-custom-connector link whenever the app was detected, which advertised a product surface the user had not asked about and that the directory listing will serve anyway once approved. So `install` no longer mentions chat at all: no line in the Express plan, no link in the summary. The connector is reached only by asking for it — `otx connect --client claude-desktop` still prints the link. Kept unconditional: removing an `mcp-remote` bridge left by an older version. That is a key sitting in a process argument, not a feature, so it goes whether or not the user cares about chat. Its note now points at the flag rather than at a link that is no longer printed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(install): register the marketplace, so the plugin install actually lands
What this PR doesWhat this PR doesRemoves the
Risk: High fan-in on Flagged because it touches a contract (buildMcpUrl) — callers elsewhere may be affected. ⚡ Blast radius17 files · 109 symbols touched · 75 call sites elsewhere in the codebase
Explore this PR's blast radius → · graph: 💰 Agent economics50 agent sessions reported in the last 30 days, but none matched this PR's commits. A session matches on the author email it reports and a commit landing within ~2h of it — so check the agent's email matches the commit author, and that the work was committed in the same sitting. Sessions → Full economics → · by CodeHub |
![opentrace[bot]](https://avatars.githubusercontent.com/u/160156268?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔧 Changes Requested
Good change to ship. Removing the mcp-remote bridge was the right call — key-in-argv is a real exposure, and the account-level connector link is a cleaner answer for that surface.
The one thing to fix before merging is the orphaned JSDoc in src/util/claude-plugins.ts — see the inline suggestion. It's a two-line fix.
Everything else holds up: the writable: false pattern is a clean way to keep remove() alive without re-exposing write(), the marketplace-registration-before-install fix addresses the actual root cause of the silent failure, and the tests cover the right things (no file written, no key in output, bridge cleanup, marketplace sequencing). The unconditional stale-bridge removal on any install run is intentional and correct — no reason to scope it to Claude Code targets only.
| /** Is the plugin in the records `/plugin` reads? */ | ||
| /** True when the marketplace is in Claude Code's own record, not merely declared. */ |
There was a problem hiding this comment.
The old JSDoc for isPluginInstalled was left above marketplaceRegistered when the new function was inserted. Two consecutive JSDoc comments means the first one (/** Is the plugin in the records… */) is orphaned — it documents nothing and will confuse readers.
| /** Is the plugin in the records `/plugin` reads? */ | |
| /** True when the marketplace is in Claude Code's own record, not merely declared. */ | |
| /** True when the marketplace is in Claude Code's own record, not merely declared. */ |
opentrace
Bot
added
OpenTrace::ChangesRequested
and removed
OpenTrace::ReviewInProgress
labels
Drop mcp-remote bridge; fix plugin marketplace registration
🔒 Security · 🐛 Bug Fix · ♻️ Refactor · 🧪 Tests · 📝 Docs
Removes the
mcp-remotestdio bridge thatotxwrote intoclaude_desktop_config.json— it exposed the raw API key in annpxprocess's argv, readable viaps. The Claude Desktop chat surface takes account-level connectors instead, sootxnow prints a prefilledclaude.ailink. This PR also fixes a silent plugin install failure:claude plugin installreadsknown_marketplaces.json, not theextraKnownMarketplacesdeclaration insettings.json, so the marketplace must be explicitly registered first viaclaude plugin marketplace add.Both fixes are active migrations:
installremoves any legacy bridge it finds on disk, andensurePluginInstallednow runsmarketplace addbeforeplugin install.Complexity
🟢 Low ·
17 files changed, 381 insertions(+), 218 deletions(-)All changes are contained within the onboarding/key-attachment layer. The
writable?flag onKeyClientis a small interface evolution with one new call-site guard. The two substantive fixes (bridge removal, marketplace registration) are independent of each other and straightforward to verify in isolation.Tests
🧪 Adds a new
ensurePluginInstalledunit test suite (4 cases) covering marketplace registration ordering, idempotency, and failure propagation — an area that previously had no direct unit tests. Integration tests are fully updated to assert the new behaviour.Note
mcp-remotebridge putotk_…in process argv — readable by any local process viapsor/proc/<pid>/cmdline. This is the primary motivation for the change and should be treated as a security fix, not just a UX improvement.Review focus
Pay particular attention to the following areas:
write?optionality at call sites —attachClientKeyguards against a missingwritewith a thrown error, but the contract relies on callers filtering viawritableKeyClients()first; verify no call site bypasses this.installremoves a legacymcp-remoteentry fromclaude_desktop_config.jsonwithout prompting, regardless of which tool the current run targets; confirm this is acceptable UX for users who didn't ask to touch the Claude app.--client claude-desktopbehaviour change — the flag now exits 0 and prints a link instead of writing a file; any automation that relied on the old behaviour will silently get different output.Additional details
How the pieces fit together
The two fixes are independent but ship together because they both affect the same onboarding flow.
Bridge removal.
claude-desktopstays inKEY_CLIENTSwithwritable: falsesofindKeyClient("claude-desktop")still resolves — needed for the cleanup path ininstalland for the graceful redirect inconnect --client claude-desktop.detectKeyClients()andwritableKeyClients()filter it out of writable contexts. Thewrite()method is gone;attachClientKeythrows if called on awritable: falseclient (programming-error guard, not user-facing). The newcustomConnectorUrl()inconstants.tsgenerates the prefilled link and strips the trailing slash to match the server's OAuth resource identifier.Marketplace registration. The root cause was a two-record system:
settings.json'sextraKnownMarketplacesis a declaration that Claude Code materialises intoknown_marketplaces.jsononly when a session starts.claude plugin installreads the record, not the declaration.ensurePluginInstallednow checksmarketplaceRegistered()(readsknown_marketplaces.jsondirectly) and runsmarketplace addfirst if needed. The check is idempotent — already-registered marketplaces are skipped. The harness stub is updated to enforce the same two-step, so integration tests would have caught this bug going forward.Error reporting improvement. Previously, a failed plugin install printed only the command to run — which was the same failing command. Now
pluginInstallErrorcaptures the last line of stderr/stdout and surfaces it inline, alongside the corrected two-step recovery command (marketplace add && plugin install).