Repository navigation
Conversation
The legacy credential import has to re-derive the integration a remote MCP server registers under, so the derivation moves out of the layer into a leaf module both can call.
Upgrading from V1 left every OAuth remote MCP server reporting needs authentication: the V1 token store (`mcp-auth.json`) was never read, so the integration V2 derives from the server's name and url had no credential row and each connect started a fresh dynamic registration instead of refreshing the tokens V1 held. Import those entries once, keyed by the same integration the server registers under, reusing the client registration the tokens were issued to.
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
As submitted, this doesn't work: integration-id.ts imports from @opencode-ai/schema, which doesn't exist in this repo. The CLI server exits on startup with Cannot find package '@opencode-ai/schema', bun run typecheck in packages/core fails with TS2307, and test/database-migration.test.ts can't load.
I ran the reproduction script from #54879 against a real V1 1.19.0 data directory and a mock OAuth MCP server. On the base, the bug reproduces. With only that import changed to @opencode/schema/integration, the approach works: the V1 tokens are written under the right integration id, mcp list shows the server as connected, the mock receives Bearer v1-access-token, the typecheck is clean, and the migration tests pass (22/22). The rest of the design looks right: the id helper is shared with mcp/index.ts, the client registration is kept, and an existing V2 credential wins.
Public API: unchanged. The migration only reads mcp-auth.json and leaves it in place.
Two things to fix (see inline comments):
- the package name in that import
- a corrupt
mcp-auth.jsonfails the migration instead of being skipped
Related: #53607
| @@ -0,0 +1,11 @@ | |||
| import { createHash } from "node:crypto" | |||
| import { Integration } from "@opencode-ai/schema/integration" | |||
There was a problem hiding this comment.
This package is @opencode/schema in this repo; @opencode-ai/schema doesn't exist. As submitted, the CLI server exits on startup (Cannot find package '@opencode-ai/schema'), bun run typecheck fails with TS2307, and test/database-migration.test.ts can't load. With @opencode/schema/integration, everything passes and the #54879 repro connects using the imported V1 token.
| if (content === undefined) return | ||
| const input = Option.getOrUndefined(decodeJson(content)) | ||
| if (typeof input !== "object" || input === null || Array.isArray(input)) { | ||
| return yield* Effect.fail(new Error("Legacy MCP auth file must contain an object")) |
There was a problem hiding this comment.
If mcp-auth.json isn't valid JSON or isn't an object, this fails the migration. The migration would then fail again on every startup, all because of a leftover V1 file that a user can easily fix by running opencode mcp auth. V1 wrote this file without locking, so a corrupt copy is a real possibility (see #29820). I'd return here instead of failing, the same way malformed entries are already skipped. A small test with a corrupt file would cover it.
`@opencode-ai/schema` is not a package in this repo: the workspace packages are named `@opencode/*`. The import only resolved locally because a stale pre-rename symlink survives in node_modules, so a fresh checkout failed to start the CLI server and failed to typecheck.
The import runs on every startup, so failing on a file the user cannot see would keep the database shut. Malformed entries are already skipped; the whole-file case now behaves the same way.
|
Both points are fixed and pushed. 1. The import — Worth recording why this passed every gate locally and still broke on a fresh checkout: 2. The corrupt file — Gates re-run on the pushed head:
|
|
Closing this to stay within GitHub's 3-open-pull-request limit for contributors without write access, which currently blocks every new pull request here. The branch remains on my fork, so I can reopen it on request. |
Issue for this PR
Closes #54879
Type of change
What does this PR do?
V1 kept remote MCP OAuth tokens in
mcp-auth.jsonunder the global data directory. V2 never reads it, so after an upgrade the integration V2 derives for a remote server —mcp_<sha1(name + "\0" + url)>— has no credential row.connectProviderthen finds nothing, falls back to an empty in-memory store, and every OAuth server reportsneeds_autheven though the V1 store still holds a valid refresh token.This imports those entries in the migration that already imports the V1 provider credentials, keyed by the same integration the server registers under:
packages/core/src/mcp/integration-id.ts— the id derivation, moved out of the MCP layer into a leaf module: the import re-derives it and the layer calls the same function, so the two cannot drift apart.packages/core/src/database/migration/20261013150000_import_legacy_mcp_credentials.ts— readsmcp-auth.jsonand writes one OAuth credential per usable entry:access/refresh,expiresconverted from V1's seconds to V2's milliseconds, theserverUrl/tokenType/scopemetadatatoTokensreads back, and the dynamically registered client the tokens were issued to — refreshing against a different client than the refresh token belongs to is rejected by the authorization server.serverUrlor without tokens are skipped: V1 only ever read an entry back through a matching URL, and a login in flight has no tokens yet. An integration that already holds a credential keeps it, mirroring the existing provider-credential import.How did you verify your code works?
packages/core/test/database-migration.test.ts: amcp-auth.jsonholding a full entry (client info, scope, abandoned verifier and state), an entry without client info, and two unusable entries. They assert the two rows the import writes (including the millisecond expiry and the exact integration ids), that a credential written by a later V2 login for the same server wins, and that the source file is left byte-identical. The stored row is then decoded throughMcpOAuth.toTokens/McpOAuth.clientFromCredentialto pin exactly what the connect-time provider reads back.bun test test/database-migration.test.ts: 21 pass, 0 fail.bun run typecheckinpackages/core,bun script/migration.ts --checkand oxlint on the changed files are clean.bun test test/mcp.test.ts test/mcp-oauth.test.ts: 97 pass. Two failures (modern protocol > exposes every connection operation,discovers and reads MCP resources through Code Mode) reproduce with this branch's changes tomcp/index.tsset aside, so they are not from this PR.Checklist
Added by the opencode bot (AI-generated).
Related: #53607