Uh oh!
There was an error while loading. Please reload this page.
chore(runtime): retire the Claude subscription OAuth path - #2965
Conversation
The subscription path authenticated as the Claude Code client and shaped every request to look like that client: its client id, its user agent, a `You are Claude Code` system prefix injected ahead of the caller's own, and a synthesized `x-anthropic-billing-header`. The module that assembled it was named for what it did — `subscription-cloaked-request`. Removed, in the order a request met them: - the OAuth login contract, so no authorize URL carries that client id; - the login provider from the wire enum, so a Client asking to start one is refused by the protocol rather than by a later guard; - the paste-code presentation flow, which served no other provider; - the request cloak and its headers; - the account-usage probe, which read quota under the same identity; - the runtime adapter, so no Session can select or send with the provider; - the credential refresh, so a stored token is inert. `connection.test` also stopped reporting a resolved token as a verified connection. That shortcut is why a workspace could show "Claude OAuth 已登录" against a connection that could not answer a single turn. The provider type stays registered. `connection-catalog.json` decodes its connections with a plain `map`, so an unregistered type does not fail one entry — it fails the document, and a workspace that ever signed in would lose every other connection with it. `gemini-cli` already models this: registered, unwired, unusable. The settings row stays for the same reason, now carrying what happened and where to go instead, because a row that simply disappears leaves the user to work out on their own why their model stopped answering. Callers keep Claude models through an Anthropic API key connection. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
Astro-Han
left a comment
There was a problem hiding this comment.
The direction is right, and I checked it rather than taking it on faith: the verification table in #2949 shows the 200 required the whole cloak — Bearer auth plus the oauth-2025-04-20 / claude-code-20250219 betas, a claude-cli UA, and a You are Claude Code system prefix. There is no "keep it but de-cloak it" version of this path, so deleting it is the honest call. Keeping the provider type registered is load-bearing and also right: decodeProviderType throws on an unregistered provider and connection-catalog-document.ts maps over every entry, so unregistering would take the entire catalog down with it. gemini-cli is the same-shape precedent.
One P0 first — the test suite in this PR was never run.subscription-credentials.test.ts renamed the provider and slug on the call but left the credential store keyed on 'claude-subscription' on both sides of it. Resolve finds nothing, returns logged-out, fetchFn is never called, and the await refreshStarted below it never settles. node --test has no default timeout, so the file does not terminate. I reproduced it: on this branch the run hangs indefinitely (with --test-timeout it surfaces as 20 passed / 1 timed out); on main the same file is 21/21 in 0.46s. Four characters, three places.
The root cause behind the rest. This PR asks "which code implemented Claude subscription OAuth?" and deletes the answer, thoroughly. But retiring a capability that shipped leaves four things behind, and only one of them is code: the credentials already on users' disks, the settings surface a user sees, the documents describing the capability, and the public record of the decision. That single gap accounts for nearly every comment below — they are less independent defects than three unanswered questions.
The clearest symptom is the tombstone. The Claude card sat behind MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL, so no released build ever showed it to anyone. This PR deletes the gate, and now every user who opens Settings → Models → OAuth finds a "Claude Code" card announcing that a login they never had is retired. Worth avoiding on its own, and it also states publicly that Maka once offered this login — which is what the gate existed to defer pending product and legal sign-off.
The other half is the credential. The refresh token stays in the store indefinitely: the logout IPC channel went away with the enum, the card has no sign-out, and isWiredOAuthProvider now returns false so the delete affordance downgrades from "disconnect and delete" to plain "delete". Deleting the connection is the only way to clear it, and nothing tells the user that. The Cursor retirement (61a0ca1f5, Part of #1982) shipped a cleanup module and updated the docs — same shape of change.
Minimal path. Fix the P0. Render the Claude card only when the catalog actually holds a claude-subscription connection, so the tombstone reaches the people it is for and nobody else — that also disposes of the stale "Sign in with Claude Pro / Max" subtitle sitting directly above the retirement banner, and of the unclickable <Text> CTA. Add one sentence to the banner pointing at "delete this connection to clear the stored credentials". Then the two residue items below.
Cleanest path. Treat this as a migration rather than a deletion: open the issue that records the decision, as the Cursor retirement did; clear the stored refresh token as part of retirement instead of leaving it to the user; drop 'claude-subscription' from isInteractiveOAuthLoginProvider in storage so the protocol enum is the single truth rather than a second check layered over the first; and land the README and SECURITY.md edits in the same PR.
Either way, two items sit outside this diff and want picking up. packages/runtime/package.json:44 still exports "./claude-subscription-usage", whose source file this PR deletes, so dist/claude-subscription-usage.js no longer exists after a build — nothing imports it today, but it is a dangling entry in a published exports map, and neither typecheck nor knip looks there. And README.md:192 still lists Claude among the providers holding subscription OAuth tokens. (SECURITY.md:167 cites claude-subscription-ipc-boundary.test.ts, which does not exist on main either — pre-existing, but this is the natural PR to fix it in.)
Two things I checked and found clean, so you do not need to re-verify them. Users with a stored credential are safe: the catalog decodes, isConnectionReady returns oauth_subscription_not_wired, buildChatModelChoices filters the model out before anything can throw, and refresh terminates in one call rather than looping. And removing the wire enum is not a breaking change — oauth.login.start takes { attemptId, connectionId }, provider only appears in the host→client projection, and an old client hitting it gets operation_unavailable, an error code already declared in that operation's contract.
The remaining deletion residue I have not listed individually — eight items, grouped into one comment on preload.ts.
Review assistance: Claude Code (Opus) ran four parallel passes over correctness, deletion completeness, architecture and test quality, and drafted this comment. I independently verified the parts it turns on: the P0 by reading the test and reproducing the hang, the deleted experimental gate, the dangling exports entry, the unreachable block in oauth-login.ts, the two-sided login check, and the README / SECURITY.md text. The catalog-decode argument, the stored-credential walkthrough and the residue inventory are AI output that I have not re-derived line by line.
中文
方向是对的,而且我核过而不是照单全收:#2949 自己的验证表说明那个 200 依赖整套伪装——Bearer 认证,加上 oauth-2025-04-20 / claude-code-20250219 两个 beta 头、claude-cli UA,以及 You are Claude Code 的 system 前缀。这条路径不存在「保留但脱掉伪装」的版本,所以删掉是诚实的选择。保留 provider type 注册这一点是承重的,也是对的:decodeProviderType 对未注册 provider 直接抛错,而 connection-catalog-document.ts 会 map 整份条目,注销它会连带让整份 catalog 解码失败。gemini-cli 是同形态先例。
先说 P0——这个 PR 的测试从未被运行过。subscription-credentials.test.ts 把调用处的 provider 与 slug 改了名,但两侧的凭据 store 仍以 'claude-subscription' 为 key。resolve 找不到凭据、返回 logged-out、fetchFn 永不被调用,下面那句 await refreshStarted 永远不会 settle。node --test 默认没有超时,所以这个文件不会结束。我复现了:本分支运行无限挂起(加 --test-timeout 才暴露成 20 pass / 1 timed out),main 上同一文件 21/21、0.46 秒。四个字符,三个位置。
其余问题的根因。 这个 PR 问的是「哪些代码实现了 Claude 订阅 OAuth」,并且很彻底地删掉了答案。但退役一个已经发布过的能力会留下四样东西,其中只有一样是代码:用户磁盘上已存的凭据、用户看得见的设置界面、描述该能力的文档,以及这个决定的公开记录。下面几乎每条评论都源自这一个缺口——它们与其说是独立缺陷,不如说是三个没被回答的问题。
最明显的症状是那块墓碑。Claude 卡片原本在 MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL 后面,也就是说没有任何发布版本向任何人展示过它。本 PR 删掉了这道门,于是每个用户打开「设置 → 模型 → OAuth」都会看到一张「Claude Code」卡片,宣告一个他们从未拥有过的登录方式已停用。这件事本身就该避免;而且它等于公开声明 Maka 曾提供过这种登录——而那道门当初存在的理由,正是要把这件事推迟到 product 与 legal 签字之后。
另一半是凭据。refresh token 会无限期留在凭据库里:logout IPC 通道随 enum 一起消失,卡片没有登出按钮,isWiredOAuthProvider 现在返回 false,删除文案也从「断开并删除」降级为「删除」。删除这条连接是清除它的唯一途径,而没有任何地方告诉用户这一点。Cursor 那次退役(61a0ca1f5,Part of #1982)带了凭据清理模块,也更新了文档——是同一形状的改动。
最小方案。 修掉 P0。只在 catalog 里确实存在 claude-subscription 连接时才渲染 Claude 卡片,让墓碑只出现在它面向的那批人面前——这同时消掉了停用 Banner 正上方那句过期的「登录 Claude Pro / Max」副标题,以及那个点不动的 <Text> CTA。在 Banner 里加一句,指向「删除此连接以清除已保存的凭据」。然后处理下面两条残留。
最彻底方案。 把它当成一次迁移而不是一次删除:像 Cursor 退役那样,先开一个 issue 记录这个决定;把已存的 refresh token 作为退役的一部分清掉,而不是留给用户;把 'claude-subscription' 从 storage 的 isInteractiveOAuthLoginProvider 里移除,让协议 enum 成为唯一真相,而不是在原判定之上再叠一道;README 与 SECURITY.md 的改动也放进同一个 PR。
两条路都要顺手捡起两件不在本 diff 内的事。packages/runtime/package.json:44 仍然导出 "./claude-subscription-usage",而它的源文件被本 PR 删了,构建后 dist/claude-subscription-usage.js 不存在——今天没人 import,但这是发布产物 exports map 里的一个悬空入口,typecheck 与 knip 都不看这里。另外 README.md:192 仍把 Claude 列在持有订阅 OAuth token 的 provider 中。(SECURITY.md:167 引用的 claude-subscription-ipc-boundary.test.ts 在 main 上也不存在——这是既有问题,但本 PR 是顺手修掉它最自然的地方。)
有两件事我核过、是干净的,你不用再验。已存凭据的用户是安全的:catalog 正常解码,isConnectionReady 返回 oauth_subscription_not_wired,buildChatModelChoices 在任何东西抛错之前就把该模型过滤掉了,刷新单次调用即终止、不会死循环。移除 wire enum 也不是破坏性变更:oauth.login.start 的入参是 { attemptId, connectionId },provider 只出现在 host→client 的 projection 里,旧客户端打过来会拿到 operation_unavailable——这是该操作契约里已声明的错误码。
剩下的删除残留我没有逐条列出——共八项,合并成 preload.ts 上的一条评论。
评审协助说明:Claude Code (Opus) 并行跑了正确性、删除彻底性、架构与测试质量四轮审查,并起草了本条评论。结论所依赖的部分我逐条独立核实过:P0 是我自己读测试并复现挂起,被删掉的实验开关,悬空的 exports 入口,oauth-login.ts 里的不可达块,两侧的登录判定,以及 README / SECURITY.md 的原文。catalog 解码论证、已存凭据走查与残留清单属于 AI 输出,我没有逐行重新推导。
| providerType: 'claude-subscription', | ||
| slug: 'claude-subscription', | ||
| providerType: 'openai-codex', | ||
| slug: 'codex-subscription', |
There was a problem hiding this comment.
P0 — this rename only landed on the call. The credential store on both sides of it is still keyed on 'claude-subscription': refreshingStore.setSecret('claude-subscription', ...) above, logoutStore.deleteSecret('claude-subscription', ...) and the assertion below.
So getSecret('codex-subscription') returns null, resolve short-circuits to logged-out, fetchFn is never called, markRefreshStarted() never fires, and await refreshStarted never settles. node --test has no default timeout, so the whole file hangs — CI would report a job timeout rather than a failing assertion, and every later case in the file is masked.
Reproduced: this branch hangs indefinitely (--test-timeout=20000 surfaces it as 20 passed / 1 timed out); main is 21/21 in 0.46s. Fix is the three remaining slugs.
| const visibleCards: OAuthCard[] = cards | ||
| .filter((card) => card.id !== 'claude' || claudeCatalogEnabled === true) | ||
| .filter(matchesQuery) |
There was a problem hiding this comment.
P1 — the deleted line was .filter((card) => card.id !== 'claude' || claudeCatalogEnabled === true), so the Claude card only ever appeared under MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL. No released build has shown it to anyone. Without the gate, { id: 'claude', name: 'Claude Code' } in modelOAuthCards reaches every user's Settings → Models → OAuth as a tombstone for a login they never had — and states publicly that Maka once offered it, which is precisely what the gate deferred pending product and legal sign-off.
Suggest gating on the catalog instead of the env flag: render the card only when a claude-subscription connection actually exists. That reaches the users the notice is for, and it also resolves oauthPanelSubtitle('claude') at :161 still returning "Sign in with Claude Pro / Max…" — rendered directly above the banner saying the login is retired.
| title={copy.claudeRetiredTitle} | ||
| description={copy.claudeRetiredBody} | ||
| /> | ||
| <Text>{copy.claudeRetiredAction}</Text> |
There was a problem hiding this comment.
P1 — two things here. This CTA is a bare <Text>, so "go add an API key connection" is not clickable.
The larger one: the stored refresh token outlives this PR with no way to clear it that the user can find. The logout IPC channel went with the enum, this card has no sign-out, and isWiredOAuthProvider('claude-subscription') is now false so use-connection-detail.ts:562 downgrades the affordance from "disconnect and delete" to plain "delete". Deleting the connection is the only path, and nothing says so.
Minimal fix: one sentence in claudeRetiredBody pointing at it. Cleaner: clear the credential as part of retirement — the Cursor retirement (61a0ca1f5) shipped a cleanup module for exactly this.
| // A retired provider keeps its persisted connections readable, but it can | ||
| // no longer be signed into. | ||
| const provider = admitted.connection.providerType; | ||
| if (!OAUTH_LOGIN_PROVIDERS.includes(provider as OAuthLoginProvider)) { |
There was a problem hiding this comment.
P2 — this is a second answer to a question storage already answers. isInteractiveOAuthLoginProvider in packages/storage/src/runtime-policy/coordinator.ts:1522 still returns true for 'claude-subscription', so #prepareStart successfully takes an admission ticket at :284 and then rejects at :286, dropping the ticket unreleased.
That drop matches the existing admissionClosed / isProviderEnabled shape, so it is not a new regression and I have not marked it higher. The concern is the seam: AGENTS.md asks for extending the closest existing check rather than layering a parallel one. Removing 'claude-subscription' from isInteractiveOAuthLoginProvider makes the protocol enum the single truth and lets this new check go away.
| // Account usage was only ever reported for the retired subscription | ||
| // provider, and reading it required that vendor's own client identity. | ||
| // The operation stays on the wire so older clients keep a defined answer. | ||
| const catalog = await this.#runtimePolicy.connectionCatalog.getSnapshot(); |
There was a problem hiding this comment.
P2 — the rewrite dropped #acquireResidency() and the RuntimePolicyStoreError catch. A store error from getSnapshot() now propagates to the catch-all in operation-dispatcher.ts:295 and is flattened to internal_failure, which means persistence_failed — declared in ACCOUNT_USAGE_ERRORS at protocol/oauth.ts:49 — can no longer be produced by this path.
Since the operation exists only to give old clients a defined answer, the simpler shape is to return unsupported_provider without reading the catalog at all. The not_found branch carries no information for an operation that is unconditionally unavailable.
| const config = OAUTH_LOGIN_PROVIDER_CONFIG[input.provider]; | ||
| const redirectUri = resolveRedirectUri(input.provider, input.redirectUri); | ||
| if (input.provider !== 'claude-subscription') { | ||
| { |
There was a problem hiding this comment.
P2 — removing the if (input.provider !== 'claude-subscription') condition left the block behind. This is now an unconditional bare block that always returns, so everything from const url = new URL(config.authorizationEndpoint) at :110 through the return at :119 is unreachable, and config at :93 is used only by that dead tail (loopbackConfig on the next line is the same lookup).
Neither tsc nor biome flags it — I ran both, green — so CI will not catch this. The function collapses to the loopback path: drop the block, delete :110-119, and keep one of the two identical config bindings.
| }, | ||
| }, | ||
| // PR-MODEL-OAUTH-ALL-0: Codex / Antigravity subscription | ||
| // bridges. Same shape as `claudeSubscription` (no token-shaped |
There was a problem hiding this comment.
P3 — grouping the remaining deletion residue here rather than filing eight comments. None of these block anything; they are the tail of an otherwise thorough deletion.
- This comment block still describes the
claudeSubscriptionbridge it was written for, now deleted. bridge-contract.d.ts:701—openAiCodex: {carries four extra spaces of indentation left by the removed block.oauth-login.ts:19—OAuthInitialTokenProviderstill includes'claude-subscription', making the claude branch inreadOAuthTokenPayloadat :242 unreachable.oauth-execution-authority.ts:325-331, :353-360— claude-specific branches remain, andSubscriptionModelFetchInput.claude(subscription-model-fetch.ts:16-20) now has no consumer.packages/core/src/oauth-subscription.ts—buildClaudeAuthorizationUrl,parsePastedAuthorization,constantTimeStringEqual, the two TTL constants andOAuthSubscriptionProviderare referenced only by their own unit tests. Therequest_authorization_codepresentation channel likewise has no production caller left.claude-subscription-runtime.test.ts:82— theclaudeOAuthConnection()helper has no callers.settings-provider-copy.ts— the wholeclaudecopy block (startFailed/quotaFailed/pasteAria/fiveHour…) is unreferenced.provider-settings.stories.tsx:224andsettings-pages.stories.tsx:579still mock the deletedwindow.maka.claudeSubscriptionbridge;docs/astryx-surface-file-inventory.md:68still records this card as usingBanner, Buttonwhen it now uses onlyBanner.
Switching these fixtures to a provider that still refreshes left the stored secret under the old slug, so the resolve never found a credential, never started a refresh, and awaited a `refreshStarted` promise that nothing would settle. The file hung, and the five tests behind it never ran — the summary still read "0 fail", because a test that never starts fails nothing. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
Addresses review on apache#2965. Gate the retired card on a stored `claude-subscription` connection instead of the deleted env flag, so a workspace that never enrolled is not told Maka once offered this login. Make the protocol enum the single truth: `isInteractiveOAuthLoginProvider` no longer admits the retired provider, so the coordinator's duplicate guard goes away. `oauth.account.usage.fetch` now answers `unsupported_provider` without reading the catalog — it kept a `not_found` path that no longer carried information. Remove what the retirement left unreachable: the claude branches in `createHostOAuthModelFetch` and the `claudeDeviceId` / `initialTokens` plumbing that fed them, `SubscriptionModelFetchInput.claude`, the claude arm of `decodeOAuthInitialTokenPayload`, and the loopback authorization-code path in `oauth-login` — both live providers enroll through device authorization, so the paste-code flow was its only caller. Its bounded-read and error-classification tests move to `requestOAuthTokenEndpointJson`, which the device flows still use. Also drops the dead PKCE/authorization half of `@maka/core/oauth-subscription`, the `claude` copy block behind the deleted 491-line card, the Storybook fixtures, the dangling `./claude-subscription-usage` export, and the README / SECURITY references (§4's contract-test list cited two files deleted by apache#1851; repointed at the live ones). Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
Addressed in P1 card visibility. The card now renders only when the catalog actually holds a Protocol enum as the single truth. Residue. The One thing I found while there. SECURITY.md §4 cited two contract-test files that #1851 deleted; repointed at ones that exist. Net -658 lines. Build, lint, format and the full suite pass locally (5 ripgrep-related failures reproduce on a clean tree — compared). |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks — this is a real second pass, not a patch over the first review. I re-read it fresh rather than diffing against my own comments, and checked the things most likely to go wrong in a deletion this size.
Confirmed closed. The half-renamed test terminates: subscription-credentials.test.js is 21/21 in 0.53s, and every provider/slug pair in that file is now consistent. The card is catalog-gated, the enum is the single truth, #fetchAccountUsage no longer reads the catalog, and the docs and dangling export are handled. I also checked the direction you went further in — buildOAuthLoginAuthorization and exchangeOAuthAuthorizationCode really had no production caller left once the paste-code flow went, and the seven bounded-read and error-classification assertions did land on requestOAuthTokenEndpointJson rather than quietly disappearing. That was the right call.
Two things block merge.
P1 — CI is red. Two provider-settings stories die on Cannot read properties of undefined (reading 'list'). The catalog gate reads the globalwindow.maka.connections, while every other connection read on this surface goes through the injected ConnectionsBridge (providers-panel.tsx:69, :109) — which is what the story actually provides. Detail inline; threading the existing bridge fixes the story for free and keeps one bridge per surface. This one is on me: the catalog gate was my suggestion and I didn't think about where this hook gets its data.
P1 — a retired connection's detail page still offers to sign in.oauthLoginServiceFor (use-connection-detail.ts:46-65) has no claude-subscription case, so it returns null and the page falls through to the generic banner: "OAuth 已登录 / 若请求提示需要重新登录,请到账号连接重新授权" (settings-provider-copy.ts:52-53, English at :191-192). The credential is still on disk so hasSecret is true and this renders for exactly the users this PR is for. Following that instruction lands on the retirement notice — a dead end that contradicts claudeRetiredBody. deriveProviderAuthContract reaches the same place by another route: isWiredOAuthProvider is now false, so it falls into oauth_preview and the sheet reads "账号登录预览". Worth noting a81c12c3d's own message named this display as the thing it was eliminating.
Everything below is P2 — nothing wrong in main, but worth doing. They share one cause: this PR deletes 17 test cases and adds none. The four +test( / +describe( lines in the diff are all renames of existing cases. So each point that enforces the retirement is now unpinned, and I mutation-tested rather than assuming:
provider-registry.ts:1799—runtimeAdapter: { kind: 'unavailable' }is the single line making an existing Claude connection unsendable, and it reverse-determinesisWiredOAuthProvider. Reverted to{ kind: 'anthropic', … }, core and runtime stay green (8 and 120 passing).coordinator.ts:1521— putting'claude-subscription'back in the allow list leaves storage 39/39 green, while the negative-assertion pattern forgithub-copilotalready sits at:2645in the same file.oauth-coordinator.ts:175— changing the constantreasonleaves runtime-host 22/22 green.- The catalog gate on the card has no test, no story and no e2e at all.
One assertion pinning runtimeAdapter.kind === 'unavailable' and isWiredOAuthProvider('claude-subscription') === false would cover the load-bearing one.
Separately, oauth-coordinator.test.ts dropped "OAuth credential commit excludes overlapping backend activations in both directions" along with its Claude fixture — but oauth-coordinator.ts:346 wraps commit in #activation.runMutation for every provider, and runBackendActivation no longer appears anywhere in that file. Worth re-landing on a Codex fixture; concurrency invariants are the expensive kind to lose quietly.
The rest, briefly, all inline: the as OAuthLoginProvider cast trades a compile-time enum cross-check for prose; SECURITY.md's "static-analysis contract tests" name two files that do neither; README.zh-CN.md:189 still lists Claude while README.md:193 no longer does; the epoch was not bumped; the quota pipeline has no writer left; the PR body describes one of three commits and its verification numbers predate 8048b9f4a.
One correction to my earlier review: I said removing the wire enum was not a breaking change. That was only true in one direction. A new Client decoding an old Host's claude-subscription login projection now throws invalidProtocolFrame rather than returning a declared error — and this repo bumps RUNTIME_HOST_COMPATIBILITY_EPOCH for that class of change. Either bump it or say why this one is exempt.
Review assistance: Claude Code (Opus) ran four independent fresh-eye passes — deletion completeness, behavior of what remains, tests and coverage, docs and user-visible surfaces — with no access to the previous round's findings, and drafted this. I verified the load-bearing parts myself: the zero net test cases, the mutex test's provider-agnostic production path, the missing README.zh-CN edit, the SECURITY.md citations by grepping them, the new cast against origin/main, and the epoch precedent. The mutation runs are AI output I have not re-run.
中文
谢谢——这是一次真正的第二轮,不是在第一轮意见上打补丁。我这次是重新读的,没有对着自己上一轮的评论逐条勾,重点看这种规模的删除最容易出事的地方。
已确认闭合。 那个改名改一半的测试正常终止了:subscription-credentials.test.js 21/21、0.53 秒,文件里每一对 provider/slug 现在都一致。卡片改成按目录门控,enum 成了唯一真相,#fetchAccountUsage 不再读目录,文档和悬空 export 也都处理了。你多走的那一步我也核了——paste-code 流程去掉后,buildOAuthLoginAuthorization 和 exchangeOAuthAuthorizationCode 确实再无生产调用方,那七条 bounded-read 与错误分类断言也确实落到了 requestOAuthTokenEndpointJson 上,没有悄悄消失。这一步做得对。
有两件事挡着合并。
P1 —— CI 是红的。 两个 provider-settings story 挂在 Cannot read properties of undefined (reading 'list')。目录门控读的是全局window.maka.connections,而这个界面其余所有连接读取走的都是注入的 ConnectionsBridge(providers-panel.tsx:69、:109)——story 提供的也正是后者。细节在行内;把现成的 bridge 透进去,story 自动就好了,一个界面也只保留一个 bridge。这条得算我头上:目录门控是我建议的,我没考虑这个 hook 的数据从哪来。
P1 —— 退役连接的详情页仍然在邀请用户登录。oauthLoginServiceFor(use-connection-detail.ts:46-65)没有 claude-subscription 分支,返回 null,页面落到通用兜底 Banner:「OAuth 已登录 / 若请求提示需要重新登录,请到账号连接重新授权」(settings-provider-copy.ts:52-53,英文在 :191-192)。凭据还在盘上,hasSecret 为真,所以这段恰好会渲染给本 PR 面向的那批用户。照着这句话走会走到停用告示——一条死路,且与 claudeRetiredBody 直接矛盾。deriveProviderAuthContract 从另一条路到达同一处:isWiredOAuthProvider 现在是 false,于是落进 oauth_preview,详情页写着「账号登录预览」。顺带一提,a81c12c3d 自己的说明里点名要消灭的正是这块显示。
下面全部是 P2 —— 合进去主干上不会有东西是错的,但值得做。 它们有同一个成因:这个 PR 删了 17 个测试用例,一个没加。diff 里那四行 +test( / +describe( 全是既有用例的改名。于是每一个执行退役的点现在都没有钉子,而且我是变异实测而不是推断:
provider-registry.ts:1799——runtimeAdapter: { kind: 'unavailable' }是让已存 Claude 连接不能发送的唯一一行,并反向决定isWiredOAuthProvider。改回{ kind: 'anthropic', … },core 与 runtime 依然全绿(8 与 120 通过)。coordinator.ts:1521—— 把'claude-subscription'加回允许列表,storage 39/39 全绿;而同一文件:2645就有现成的github-copilot否定断言模式。oauth-coordinator.ts:175—— 改掉那个常量reason,runtime-host 22/22 全绿。- 卡片的目录门控完全没有测试、没有 story、也没有 e2e。
补一条钉住 runtimeAdapter.kind === 'unavailable' 且 isWiredOAuthProvider('claude-subscription') === false 的断言,就能盖住最承重的那个。
另外,oauth-coordinator.test.ts 把「提交与后端激活双向互斥」连同它的 Claude fixture 一起删掉了——但 oauth-coordinator.ts:346 对每个 provider 都把 commit 包在 #activation.runMutation 里,而 runBackendActivation 现在在该文件里一次都不出现。建议按 Codex fixture 重新落回来;并发不变量是丢了最难察觉的那一类。
其余简述,都在行内:as OAuthLoginProvider 这个 cast 把编译期的 enum 交叉检查换成了一句注释;SECURITY.md 的「static-analysis contract tests」点名的两个文件两样都不做;README.zh-CN.md:189 仍列着 Claude 而 README.md:193 已经删掉;epoch 没有 bump;quota 管道已无写入方;PR 描述只覆盖三个提交里的一个,验证数字也早于 8048b9f4a。
对我上一轮的一处更正:我说移除 wire enum 不是破坏性变更——那只在一个方向上成立。新 Client 解旧 Host 返回的 claude-subscription login projection,现在会抛 invalidProtocolFrame,而不是返回已声明的错误码;而这个仓库为这类变更是会 bump RUNTIME_HOST_COMPATIBILITY_EPOCH 的。要么 bump,要么写明这次为什么豁免。
评审协助说明:Claude Code (Opus) 跑了四轮相互独立的 fresh-eye 审查——删除完整性、保留代码的行为、测试与覆盖、文档与用户可见面——都没有拿到上一轮的结论,并起草了本条评论。承重的部分我自己核实了:净增测试用例为零、被删互斥用例对应的生产路径与 provider 无关、README.zh-CN 漏改、逐个 grep 了 SECURITY.md 引用的文件、对照 origin/main 确认新增的 cast,以及 epoch 的历史惯例。变异测试的运行结果属于 AI 输出,我没有自己重跑。
| // state: clearing the search then revealed signed-in accounts rendering as | ||
| // "可用". The retired row has no account state to read. | ||
| const cardsToRefresh = cards.filter((card) => card.id !== 'claude'); | ||
| const retired = await window.maka.connections |
There was a problem hiding this comment.
P1 — this is the storybook failure. window.maka.connections is the global bridge, but every other connection read on this surface goes through the injected ConnectionsBridge: providers-panel.tsx:69 declares it and :109 calls bridge.list(). The story passes that bridge as a prop, so the global is undefined and undefined.list() throws.
.catch(() => false) cannot help here — the TypeError fires synchronously on the property access, before any promise exists, which is why the story dies with an uncaught page error instead of degrading. useOAuthCards({ query }) has no bridge today, which is presumably why the global was the easy reach; threading the existing one in fixes the story without a fixture and keeps a single bridge per surface.
Two smaller things in the same block: the .catch(() => false) silently hides the retirement notice on a transient list failure while every other card failure goes through setRefreshError, and the await at :86 runs before Promise.all rather than alongside it, adding a round trip to first paint.
| // keeps the provider type registered — a persisted connection must still | ||
| // decode, or one retired entry takes the whole catalog down with it — while | ||
| // removing it from the models a Session can select or send with. | ||
| runtimeAdapter: { kind: 'unavailable' }, |
There was a problem hiding this comment.
P2 — this line is the whole retirement. It is what makes an existing Claude connection unsendable (model-factory.ts:111, test-connection.ts:212 both throw off it) and it reverse-determines isWiredOAuthProvider at :1863. Reverting it to { kind: 'anthropic', auth: 'bearer', normalizeBaseUrl: true } and rebuilding leaves provider-catalog-contract.test.js + onboarding.test.js at 8/8 and provider-contract-matrix.test.js at 120/120.
So a retired provider could be silently made sendable again and nothing would notice. One assertion in provider-catalog-contract.test.ts — runtimeAdapter.kind === 'unavailable' and isWiredOAuthProvider('claude-subscription') === false — pins it.
While here: the six fallbackModels at :1783-1789 are retained and nothing filters on runtimeAdapter: 'unavailable' in the pickers, so the PR body's "No Session can select or send with it" holds for send but not for select. Worth narrowing the claim or filtering.
| @@ -1521,9 +1521,5 @@ function requiresNetworkProxyCredential(networkProxy: RuntimePolicy['networkProx | |||
| function isInteractiveOAuthLoginProvider( | |||
There was a problem hiding this comment.
P2 — no regression protection. Adding 'claude-subscription' back to this list and rebuilding leaves runtime-policy-stores.test.js at 39/39.
The pattern to copy is already in that file: runtime-policy-stores.test.ts:2645 asserts { kind: 'provider_action_unavailable', availability: 'hidden' } for github-copilot. The PR switched the fixture at :2588 from claude to codex rather than adding the claude negative case.
The behavior itself is right, and I checked it: the provider test at :410 runs before prepareConnectionMaterial and before the ticket is issued, so nothing is admitted and there is no ticket to leak.
| @@ -763,102 +638,6 @@ test('OAuth login rejects an experimentally disabled provider before presentatio | |||
| } | |||
| }); | |||
There was a problem hiding this comment.
P2 — the deletion here took a provider-agnostic invariant with it. OAuth credential commit excludes overlapping backend activations in both directions used a Claude fixture, but what it asserted is oauth-coordinator.ts:346, which wraps commit in #activation.runMutation for every provider — Codex and xAI included. runBackendActivation now appears nowhere in this file, and runtime-policy-activation-gate.test.ts only covers the gate primitive, not the coordinator's use of it. Worth re-landing on a Codex fixture.
Same hunk removed the only caller of handlers['oauth.account.usage.fetch'], so the constant this PR newly defines on the wire has no test: changing reason to 'credential_unavailable' leaves this file plus oauth-protocol.test.js and oauth-execution-authority.test.js at 22/22.
Also worth a look while here: model-catalog-choices.test.ts:81 still passes, but now because claude-sub is filtered out of choices entirely rather than because the email was redacted — the fixture no longer checks anything.
| if (!this.#isProviderEnabled(admitted.connection.providerType)) { | ||
| // Storage already refuses to admit a retired provider, so the ticket above | ||
| // cannot belong to one by the time it reaches here. | ||
| const provider = admitted.connection.providerType as OAuthLoginProvider; |
There was a problem hiding this comment.
P2 — origin/main passed admitted.connection.providerType straight through, with no cast. The compiler was checking that the storage provider union and the protocol OAuthLoginProvider union stayed compatible; the cast replaces that with the comment above it.
It is a no-op today because both unions were narrowed together — which is exactly the property worth keeping a compiler check on, in a PR whose entire job is narrowing two enums in lockstep. If they drift, #isProviderEnabled and the projection take a wrong-typed value silently. Dropping the cast is one line.
| residency.release(); | ||
| } | ||
| async #fetchAccountUsage(): Promise<OperationOutcome<'oauth.account.usage.fetch'>> { | ||
| // Account usage was only ever reported for the retired subscription |
There was a problem hiding this comment.
P2 — no objection to the shape; returning a constant without touching the catalog is the right call for an operation that is unconditionally unavailable, and dropping residency is fine since there is no longer any I/O to fail.
What it leaves behind is dead protocol surface: not_found and persistence_failed in ACCOUNT_USAGE_ERRORS (protocol/oauth.ts:50) now have no producer, three of the four OAuthAccountUsageUnavailableReason members are unreachable, and the entire kind: 'available' variant is unproducible.
Downstream the pipeline is dead too: runtime-host-oauth-ipc-main.ts:77's accountUsage map lost its only writer with the refresh-quota handler, so :178 always reads undefined and :209 deletes nothing; fetchOAuthAccountUsage on the client (runtime-host-client.ts:372) has no caller. Either retire the pipeline with it or note why it stays.
| export const OAUTH_PRESENTATION_URL_MAX_LENGTH = 8_192; | ||
| export const OAUTH_PRESENTATION_STATE_HINT_MAX_LENGTH = 1_024; | ||
| export const OAUTH_LOGIN_PROVIDERS = ['claude-subscription', 'openai-codex', 'xai-oauth'] as const; | ||
| export const OAUTH_LOGIN_PROVIDERS = ['openai-codex', 'xai-oauth'] as const; |
There was a problem hiding this comment.
P2 — narrowing this closed schema is a cross-version decode change and the epoch was not bumped. decodeOAuthLoginProjection (:200-216) and decodeOAuthAccountUsageFetchResult (:181-193) validate against this list, so a new Client against a same-epoch older Host now throws invalidProtocolFrame on a claude-subscription projection where it used to decode.
I got this wrong in my first review — I only considered old Client against new Host, where the declared operation_unavailable does cover it. The other direction is the one that breaks. git log -L on RUNTIME_HOST_COMPATIBILITY_EPOCH shows this repo bumps for this class (#2633, #2625). Blast radius is small pre-release, so either bump it or say in the body why this narrowing is exempt.
| @@ -214,8 +213,8 @@ user can verify it before saving. The main process accepts it for that | |||
| single request and does not echo it in the response. | |||
| The static-analysis contract tests for this policy: | |||
There was a problem hiding this comment.
P2 — the previous references were dead (both files went in #1851), so re-pointing them is right. But these two do not do what the sentence says. Neither is a static-analysis test — #1332 replaced the source-grep tests with behavior tests, and there is no static-analysis test for the token boundary left in the repo. And runtime-host-connections-ipc-main.test.ts has zero occurrences of apiKey, secret or token; it covers channel registration and delete retries. web-search-credentials.test.ts tests resolveTavilyApiKey precedence and asserts the function returns the cleartext key.
§2.5's subject is "API keys / OAuth tokens / bot tokens", and §3 invites reports about cleartext secrets crossing IPC — so pointing at tests that assert neither could lead a reporter to assume a defence that is not there. Either narrow the wording to what these files actually cover, or add a real boundary test and cite that.
Related, at :92: Claude was removed from the list of subscription OAuth tokens living in credentials.json, but for an upgrading user the token is still there until they delete the connection. Worth a clause saying so.
| - Sessions, messages, execution ledgers, workflows, usage, Automations, and Daily Review live in `runtime.sqlite`; | ||
| - Runtime credentials such as API keys, bot tokens, and proxy passwords currently live in local plaintext `credentials.json`, behind the OS account boundary, with POSIX directory mode `0700` and file mode `0600` enforced; | ||
| - Subscription OAuth tokens (Claude, Codex, GitHub Copilot, xAI, and the Antigravity preview) live in the same `credentials.json` — the single authority for Runtime Host clients. Pre-existing Electron `safeStorage` credential/token files are not imported; affected users must re-authenticate; | ||
| - Subscription OAuth tokens (Codex, GitHub Copilot, xAI, and the Antigravity preview) live in the same `credentials.json` — the single authority for Runtime Host clients. Pre-existing Electron `safeStorage` credential/token files are not imported; affected users must re-authenticate; |
There was a problem hiding this comment.
P2 — README.zh-CN.md:189 still reads 订阅 OAuth token(Claude、Codex、GitHub Copilot、xAI 以及 Antigravity preview), so the two READMEs now contradict each other on a security-relevant fact. The Cursor retirement (61a0ca1f5, #2037) updated README.md, README.zh-CN.md and SECURITY.md together. One line, worth riding along with the rest rather than deferring.
| @@ -286,12 +281,8 @@ function GitHubCopilotLoginPanel() { | |||
| async function getSubscriptionSnapshot(serviceId: OAuthCardId): Promise<SubscriptionSnapshot> { | |||
| if (serviceId === 'claude') { | |||
There was a problem hiding this comment.
P3 — grouping the residue here rather than filing a dozen comments. None of it blocks anything.
- This
serviceId === 'claude'branch is unreachable::85filters claude out ofcardsToRefreshbefore any snapshot is read. - Dead symbols:
randomOpaqueValueand itsrandomBytesimport (oauth-coordinator.ts:1,:549),assertOpaqueValue(oauth-login.ts:116),isNonEmptyString(subscription-model-fetch.ts:98),claudeOAuthConnection(claude-subscription-runtime.test.ts:81). - The whole
request_authorization_codechain is now unreachable but fully retained: the#presentoverload (oauth-coordinator.ts:450-453),DEFAULT_AUTHORIZATION_TIMEOUT_MS/MAX_AUTHORIZATION_TIMEOUT_MS/PRESENTATION_TIMEOUT_MARGIN_MS,#authorizationTimeoutMs,authorizationTimeout(), plusOAuthPresentationMethodand its shapes in the protocol and both desktop handlers. Retaining it while deleting theOAUTH_LOGIN_PROVIDERSmember is two opposite compatibility calls in one file. - Unreachable union members still typed:
ProviderModelDiscovery.auth's'claude-subscription'(provider-registry.ts:46),model-factory.ts:399's case,SubscriptionAccountStateimported unused atpreload.ts:87. - Stale comments describing deleted IPC channels and the paste-code flow:
use-connection-detail.ts:38-40,core/oauth-subscription.ts:92,:138,:143. docs/astryx-surface-file-inventory.md:68still records this card as usingBanner, Button; it is Banner only now.test-connection.ts:323-332keeps the "skip the synthetic probe" branch for Codex with a comment referencing the deleted Claude path — and the deleted test was the only one counting fetches, so that invariant is now unguarded for Codex too.
The retired Claude row renders only when the workspace has a stored `claude-subscription` connection, which the OAuth section reads off `window.maka` rather than through the panel's bridge prop. The provider stories stub `window.maka` with the subscription channels only, so both add-connection stories threw on `connections.list`. Install the story's own bridge as that channel. `ConnectionsProblems` carries a `claude-subscription` entry, so it now shows what a workspace that enrolled before actually sees. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
Addresses the second review on apache#2965. **The retired connection's detail page still invited a sign-in.** `oauthLoginServiceFor` had no case for it, so the page fell through to the generic notice: "若请求提示需要重新登录,请到账号连接重新授权" — an instruction whose only destination is the retirement notice. The credential outlives retirement, so `hasSecret` is true and this rendered for exactly the users this PR is for. `deriveProviderAuthContract` reached the same place by another route, reading the provider as `oauth_preview` — a login that was taken away is not a preview of one that is coming. Both now read a `retired` marker on the registry entry rather than inferring retirement from an unavailable adapter, which an unwired preview provider is indistinguishable from. `ProviderAuthState` gains `retired` and `ProviderAuthSetupMode` gains `oauth_retired`; both are exhaustively switched, so the compiler found every surface that had to answer for the new state. **Tests.** The prior commits deleted 17 cases and added none, leaving every line that enforces the retirement separately revertible with the suite green. Added: a registry contract pinning `runtimeAdapter.kind === 'unavailable'`, `isWiredOAuthProvider === false`, and the auth contract for every retired provider; a storage case asserting a retired connection cannot begin an interactive login; the coordinator's account-usage constant; and the "commit excludes overlapping backend activations" invariant, re-landed on a Codex fixture — what it asserts is provider-agnostic and it left with its Claude fixture. Each was mutation-checked against the revert it is meant to catch. **Epoch.** Narrowing `OAUTH_LOGIN_PROVIDERS` is a decode change in the new-Client-against-old-Host direction, which this repo bumps for (apache#2633, apache#2625). Bumped to 20. Also: dropped the `as OAuthLoginProvider` cast so the compiler keeps storage's union and the protocol's in step; the catalog read now runs alongside the snapshot fetches instead of ahead of them, and a failed read keeps the previous answer rather than retracting the notice; `README.zh-CN.md` matches `README.md`; `SECURITY.md` §4 no longer claims contract tests that do not exist; and the remaining dead symbols and stale comments are gone. Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
P1 storybook. Threaded the injected bridge through as you suggested, no fixture. 126 stories pass locally. I had skipped this gate last round — Playwright had no Chromium on my machine and I moved on instead of installing it. P1 detail page inviting a sign-in. Confirmed, and I had missed it. I went one step further than the literal fix: " Tests. Fair — 17 deleted, 0 added. Four added, each mutation-checked against the revert it is meant to catch:
Epoch. Taking your correction — bumped to 20. Cast. Dropped. After the narrowing both unions are identical, so the compiler takes that cross-check back. Rest. One thing I did not do. The On Build, lint, format, the full 6418-test suite and the Storybook smoke pass locally (5 ripgrep failures reproduce on a clean tree). |
Summary
The Claude subscription path authenticated as the Claude Code client and
shaped every request to look like that client: its
client_id, itsUser-Agent, aYou are Claude Codesystem block injected ahead of thecaller's own prompt, and a synthesized
x-anthropic-billing-header. Themodule that assembled it was named for what it did —
subscription-cloaked-request.Removed, in the order a request met them:
subscription-cloaked-request.tsclaudeSubscriptionHeadersclaude-code-*betasclaude-subscription-usage.tsunavailable)connection.testalso stopped reporting a resolved token as a verifiedconnection. That shortcut is why a workspace could show "Claude OAuth 已登录"
on a connection that could not answer a single turn.
Callers keep Claude models through an Anthropic API key connection.
Why the provider type stays registered
connection-catalog.jsondecodes its connections with a plainmap, anddecodeProviderTypethrows on an unregistered type. Removing the type wouldtherefore not skip one entry — it would fail the whole document, and a
workspace that ever signed in would lose every other connection with it
(API keys, Codex, Copilot).
So the type stays registered and unwired.
gemini-clialready models exactlythis shape: registered,
runtimeAdapter: unavailable, not offered as a usablemodel.
The settings row stays for the same reason, now carrying what happened and
where to go instead. A row that simply disappears leaves an affected user to
work out on their own why their model stopped answering — which is the failure
mode this change is trying not to create.
Verification
lint,format:check,build,typecheck,knip(desktop + ui) andastryx:themepass.@maka/core@maka/storage@maka/runtime-host@maka/desktop@maka/runtimeThose 5 runtime failures are file-tool path containment and Grep sandbox
tests. I ran the suite on this branch and on a clean tree and diffed the
failing test names: identical sets, no new failure.
That diff earned its keep. A first pass showed one extra failure —
OpenCode connection probes follow each selected model protocol— becauseremoving the
claude-subscriptioncase fromtestConnectionModelhad alsotaken the
anthropiccase's return statement, silently routing everyAnthropic API key connection test at
/chat/completions. Fixed here.Tests whose subject was the removed behavior are gone (the cloak shape, the
always-verified test shortcut, the paste-code presentation). Tests that used
this provider only as a convenient OAuth fixture now run on
openai-codex.One in
provider-authwas dropped outright: it asserted the behavior of awired OAuth provider with fallback-only discovery, and after this change the
registry has no such provider.
Retirement is now an explicit registry fact
A retired provider and an unwired preview provider look identical from
runtimeAdapter: 'unavailable', but only one of them was ever available to auser. The registry entry carries
retired: trueandisRetiredProvider()reads it, so the connection detail page and
deriveProviderAuthContractcantell "this was taken away" from "this has not arrived yet". Without it the
detail page told these users to go reauthorize — an instruction whose only
destination is the retirement notice — and the auth sheet read
账号登录预览.ProviderAuthStategainsretiredandProviderAuthSetupModegainsoauth_retired. Both are exhaustively switched, so the compiler named everysurface that had to answer for the new state.
Compatibility
RUNTIME_HOST_COMPATIBILITY_EPOCHgoes 19 → 20. NarrowingOAUTH_LOGIN_PROVIDERSis a decode change in the new-Client-against-old-Hostdirection — a
claude-subscriptionprojection from an older Host now throwsinvalidProtocolFramewhere it used to decode. This repo bumps for that class(#2633, #2625).
oauth.account.usage.fetchstays on the wire and answersunsupported_providerunconditionally, reading no state. Therequest_authorization_codepresentation chain is unreachable after the epochbump but is left in place; removing it is a second protocol narrowing and is
clearer as its own change.
Testing
Each line that enforces the retirement is pinned by an assertion that was
mutation-checked against the revert it is meant to catch:
runtimeAdapter.kind === 'unavailable',isWiredOAuthProvider === falseprovider-catalog-contractfailsretired: trueon the registry entryprovider-catalog-contractfailsruntime-policy-storesfails once both gates are removedoauth.account.usage.fetch's constant reasonoauth-coordinatorfailsoauth-coordinatorfailsThe last one is re-landed from the Claude fixture this PR removed — what it
asserts is provider-agnostic (
oauth-coordinator.tswraps every commit in#activation.runMutation), so it now runs on Codex.Note on the storage row: removing
'claude-subscription'fromisInteractiveOAuthLoginProvideralone leaves the suite green, becausederiveProviderAuthContractnow refuses independently. The assertion pins theend-to-end verdict rather than either gate.
Checklist
Does this PR entail a change in behavior?