feat(runtime): retire the Claude subscription OAuth provider - #3183
feat(runtime): retire the Claude subscription OAuth provider#3183Joob1n wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWhat problem this solvesThis PR retires the The provider remains registered as Source of truthThe PR extends the existing provider registry, readiness, authentication-contract, onboarding, storage-policy, runtime, and UI paths. It does not create a parallel retirement path. The provider registry is the source of truth for retirement. Runtime and UI behavior derive from that state. Scope and necessityThis is the smallest coherent solution for provider retirement:
The removed settings component, usage module, bridge methods, protocol operations, provider-specific helpers, fixtures, and stale export cannot be removed further without weakening compatibility or regression coverage. The Complexity deltaThe PR removes:
The PR adds:
Total maintenance complexity decreases. The added retirement state is necessary to preserve stored-connection decoding while preventing use. Validation and risksTests cover readiness rejection, provider contracts, onboarding, connection status, OAuth protocol behavior, storage policy, session admission, runtime rejection without network requests, and supported-provider OAuth flows. The author reports passing build, typecheck, lint, formatting, and other listed checks. Environment-related failures match clean Concrete risks include:
Review-relevant risksThe current diff affects user-visible settings and onboarding, public bridge and protocol contracts, runtime compatibility, authentication behavior, and stored-connection handling. Material changes in these areas require independent human review under repository policy. The follow-up for The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughClaude subscription OAuth support is retired across core, runtime, runtime-host, storage, and desktop. Existing connections remain decodable but cannot authenticate, execute models, refresh quota, or initiate OAuth login. The UI directs users to replacement connections. ChangesClaude subscription retirement
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The change retires Claude subscription connections, but a retained retired connection can still be selected as the default and then cause new sessions to fail instead of running. Merge should wait for default-target validation to reject retired providers and for the corresponding mutation-path test. Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoRetire Claude subscription OAuth across runtime and desktop
AI Description
Diagram
High-Level Assessment
Files changed (65)
|
16f779f to
16c9576
Compare
Code Review by Qodo
1.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/runtime-host/src/server/oauth-coordinator.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDelete the unused authorization-code exchange dependency.
Disposition: optional.
After Line 333,
#runLogincalls only#runXaiLoginor#runCodexDeviceLogin. Neither method calls#exchangeCode. DeleteexchangeOAuthAuthorizationCode,HostOAuthCoordinatorInput.exchangeCode, and the private#exchangeCodefield.As per path instructions, “Flag concrete cases where code can be deleted or simplified.”
Also applies to: 333-333
Source: Path instructions
packages/runtime-host/src/__tests__/oauth-execution-authority.test.ts (1)
439-439: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated union member.
'openai-codex' | 'openai-codex'is equivalent to one literal type. DeclareproviderTypeas'openai-codex'.Disposition: optional.
Proposed simplification
- providerType: 'openai-codex' | 'openai-codex', + providerType: 'openai-codex',As per path instructions, “Flag concrete cases where code can be deleted or simplified.”
Source: Path instructions
packages/runtime/src/__tests__/oauth-login.test.ts (1)
104-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDelete the unused Claude environment entry.
MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTALaffects neither assertion. The test only evaluatesopenai-codexandxai-oauth, and xAI ignores environment flags. Remove this entry to keep the test limited to active enrollment behavior. Disposition: optional.As per path instructions, delete an unnecessary path before adding behavior.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 000f121d-fee4-4262-991f-407da136b587
📒 Files selected for processing (71)
apps/desktop/src/main/__tests__/chat-readiness.test.tsapps/desktop/src/main/__tests__/runtime-host-account-connection.test.tsapps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.tsapps/desktop/src/main/chat-readiness.tsapps/desktop/src/main/oauth-connection-identities.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-oauth-ipc-main.tsapps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/locales/conversation-copy.tsapps/desktop/src/renderer/locales/onboarding-copy.tsapps/desktop/src/renderer/locales/settings-provider-copy.tsapps/desktop/src/renderer/onboarding-hero-copy.tsapps/desktop/src/renderer/settings/claude-subscription-card.tsxapps/desktop/src/renderer/settings/provider-connection-detail.tsxapps/desktop/src/renderer/settings/provider-oauth-section.tsxapps/desktop/src/renderer/settings/providers-panel.tsxapps/desktop/src/renderer/settings/use-connection-detail.tsapps/desktop/stories/settings/provider-settings.stories.tsxapps/desktop/stories/settings/settings-pages.stories.tsxdocs/astryx-surface-file-inventory.mddocs/astryx-surface-file-inventory.pathspackages/core/src/__tests__/connection-readiness.test.tspackages/core/src/__tests__/oauth-subscription.test.tspackages/core/src/__tests__/onboarding.test.tspackages/core/src/__tests__/provider-auth.test.tspackages/core/src/__tests__/provider-catalog-contract.test.tspackages/core/src/connection-error-copy.tspackages/core/src/connection-readiness.tspackages/core/src/model-catalog.tspackages/core/src/model-web-search.tspackages/core/src/oauth-subscription.tspackages/core/src/onboarding.tspackages/core/src/provider-auth.tspackages/core/src/provider-registry.tspackages/core/src/task-submission-readiness.tspackages/runtime-host/src/__tests__/execution-model-composition.test.tspackages/runtime-host/src/__tests__/oauth-coordinator.test.tspackages/runtime-host/src/__tests__/oauth-execution-authority.test.tspackages/runtime-host/src/__tests__/oauth-protocol.test.tspackages/runtime-host/src/__tests__/oauth-two-client-uds.test.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/protocol/oauth.tspackages/runtime-host/src/protocol/operations.tspackages/runtime-host/src/server/execution-composition.tspackages/runtime-host/src/server/execution-model-authority.tspackages/runtime-host/src/server/execution-model-composition.tspackages/runtime-host/src/server/oauth-coordinator.tspackages/runtime-host/src/server/oauth-execution-authority.tspackages/runtime/src/__tests__/claude-subscription-runtime.test.tspackages/runtime/src/__tests__/claude-subscription-usage.test.tspackages/runtime/src/__tests__/computer-use-model-loop.test.tspackages/runtime/src/__tests__/model-factory-thinking.test.tspackages/runtime/src/__tests__/oauth-login.test.tspackages/runtime/src/__tests__/provider-contract-matrix.tspackages/runtime/src/__tests__/subscription-credentials.test.tspackages/runtime/src/__tests__/subscription-model-fetch.test.tspackages/runtime/src/claude-subscription-usage.tspackages/runtime/src/model-factory.tspackages/runtime/src/model-fetcher.tspackages/runtime/src/model-runtime.tspackages/runtime/src/oauth-login.tspackages/runtime/src/oauth-provider-contracts.tspackages/runtime/src/subscription-auth.tspackages/runtime/src/subscription-cloaked-request.tspackages/runtime/src/subscription-credentials.tspackages/runtime/src/subscription-model-fetch.tspackages/runtime/src/test-connection.tspackages/storage/src/__tests__/runtime-policy-stores.test.tspackages/storage/src/runtime-policy/coordinator.tspackages/storage/src/runtime-policy/operations.ts
💤 Files with no reviewable changes (21)
- docs/astryx-surface-file-inventory.paths
- apps/desktop/src/main/oauth-connection-identities.ts
- apps/desktop/stories/settings/provider-settings.stories.tsx
- apps/desktop/stories/settings/settings-pages.stories.tsx
- apps/desktop/src/renderer/settings/claude-subscription-card.tsx
- packages/runtime/src/claude-subscription-usage.ts
- packages/runtime/src/tests/computer-use-model-loop.test.ts
- packages/runtime-host/src/protocol/operations.ts
- packages/runtime/src/tests/subscription-model-fetch.test.ts
- packages/runtime-host/src/server/execution-model-composition.ts
- apps/desktop/src/preload/bridge-contract.d.ts
- packages/core/src/model-web-search.ts
- packages/runtime/src/subscription-cloaked-request.ts
- packages/core/src/tests/oauth-subscription.test.ts
- packages/runtime-host/src/server/execution-composition.ts
- packages/runtime-host/src/server/execution-model-authority.ts
- packages/runtime/src/tests/claude-subscription-usage.test.ts
- apps/desktop/src/main/runtime-host-client.ts
- packages/runtime-host/src/server/oauth-execution-authority.ts
- packages/runtime/src/subscription-auth.ts
- packages/runtime/src/subscription-model-fetch.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.
16c9576 to
2ddd92d
Compare
|
All three are real. Fixed in the amended commit. 1. Stale 2. Orphaned Claude settings copy. Removed the whole 3. Login branch not future-exhaustive. Agreed — the ternary was the one place in this PR that fails open. Replaced with a On the alternative approach: agreed, and the reason is stronger than compatibility. |
fd810dd to
1d978df
Compare
|
Thanks for the removal — the impersonation is unambiguous and the decision is right: the deleted Conclusion: PASS with one P2 (epoch) — and the UI/UX screenshot gate must be satisfied before merge. P2 — the epoch bump the PR claims ("RUNTIME_HOST_COMPATIBILITY_EPOCH moves to 23") is not in the diff. This PR changes wire protocol (removes P3-1 (merge gate): no before/after screenshots for the UI/UX changes. This PR visibly changes Settings (retirement banner, removed test/set-default buttons, list-row retired state), onboarding ( P3 (optional): one mutation-claim is not independently reproducible — the storage allow-list test ( AI-assisted review disclosure: this review was produced with AI assistance (two independent pi review subagents on 中文摘要（AI 辅助审查）结论：PASS（1 个 P2 + UI/UX 截图 gate 阻塞）。冒充确凿、移除决定正确（subscription-cloaked-request.ts 全套伪造 Claude Code 身份——UA/x-app:cli/会话头/假系统提示/计费标记/借用的 client_id，风险在用户自己的 Anthropic 账户，gating 只是把违规路径藏起来，移除才消除风险载体）。7 个 enforcement point 全部真实且相互独立（注册表真相源→storage 准入→send 准入→picker→OAuth 登录→wire 枚举→runtime 兜底），逐一核实存在且单独 mutation 会红。删除干净（4 文件整删、共享基建正确裁剪、无悬空 import、stale export 已修）。retained-but-retired 的技术理由基本成立（decodeProviderType 对未注册类型 throw、整个 catalog 文档失败，unregister 会让用户失去全部连接）——但有 gemini-cli 先例（按原始 providerType 字符串先过滤）证明完全 unregister 可行，"不可能"略夸大；真实理由（保留可见可删、让用户清理本机凭据、否则 onboarding 退回 needs_connection）是合理产品选择，建议在 PR 中诚实陈述。breaking-change 叙述除 epoch 段外诚实。P2：PR 声称的 epoch bump（22→23）不在 diff 里——protocol/index.ts 未动，base 已是 23（#3145 升的），main 当前 24（#3165 升的）；"旧客户端被 epoch gate 拒绝"是巧合保护——#3145 与本 PR 之间构建的 client（epoch 23、旧协议）会通过精确相等握手（connection.ts:1290）然后中途在 oauth.account.usage.fetch/login 上报 unknown-operation 而非握手期干净拒绝。实际影响低（无已发布 epoch-23 client），但违反仓库"每个 breaking PR 自己 bump"惯例且描述误导。建议本 PR bump 25（与 #3103 协调）并修正描述，或显式声明骑 #3145 的 bump 并 release-atomic。P3-1（合并 gate）：大量 UI/UX 变化（Settings 退休横幅/移除 test 与 set-default 按钮/list 行 retired 状态/onboarding all_connections_retired 卡片/删 494 行卡片）但无任何截图或录屏——按仓库规则 UI/UX 变化合并前必须 before/after 截图。P3（可选）：storage allow-list 的 mutation 声称不可独立复现（contract 门独立产生同结果，防御纵深生效但表格高估该行判别力）；oauth-coordinator.ts 残留死代码（exchangeOAuthAuthorizationCode 导入、#exchangeCode 字段、HostOAuthCoordinatorInput.exchangeCode 永不被调用）；oauth-login.test.ts:106 仍设置已删除的 MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL env flag；新增 retired UI 状态无 Storybook stories、E2E 未跑（仓库惯例 UI 状态要确定性 Storybook/Playwright 覆盖）。 |
There was a problem hiding this comment.
English
Requesting changes.
Correction: the compatibility-epoch issue was already reported in the top-level review comment from Astro-Han, so I removed my duplicate inline comment. That earlier comment also captures the current-main coordination correctly: main already uses epoch 24, so this PR needs the next distinct epoch rather than another bump to 24.
This review retains two independent actionable findings:
- Host session admission can still persist sessions targeting a retired connection when its credential remains stored.
- The authorization-code/paste-code OAuth path has lost its last production producer but remains across protocol, Host, runtime, and Desktop layers.
The Host admission gap independently warrants changes. The authorization-code comment intentionally covers the full producerless demand chain, extending beyond the narrower unused-#exchangeCode observation in the earlier review.
简体中文
建议 Request changes。
更正：compatibility epoch 问题已经由 Astro-Han 的顶层 review comment先行报告，因此我删除了自己的重复 inline comment。该评论对当前 main 的协调也更准确：main 已经使用 epoch 24，所以本 PR 需要下一个独立 epoch，而不是再次提升到 24。
本 review 保留两项独立且可执行的 finding：
- 当旧 credential 仍被保留时，Host session admission 仍能持久化指向退役 connection 的 session。
- authorization-code/paste-code OAuth 路径已经失去最后一个生产 producer，却仍横跨 protocol、Host、runtime 和 Desktop 层存在。
Host admission 缺口本身足以支持 Request changes。authorization-code 评论针对的是完整的无 producer demand chain，范围超过先前 review 中较窄的 unused #exchangeCode 观察。
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
对抗性复核结论：当前 revision 不可合并。移除 Claude subscription OAuth 的方向正确，删除量也总体干净，但执行准入和协议边界还没有闭合。
- P1：breaking protocol 没有分配新的 compatibility epoch。
PR head 的 packages/runtime-host/src/protocol/index.ts:74 仍是 epoch 23；当前 main 已是 24。这个 PR 又删除了 OAuth provider/operation，所以合并结果会继续使用 24，epoch-24 的旧 Client/Host 能通过握手，再在 removed operation/provider 上中途失败。这里需要占用一个新的独立 epoch，并与其它并行 breaking PR 协调，不能依赖别的 PR 的 bump。
- P1：保留的旧 credential 会让 retired connection 通过 Host session admission。
packages/core/src/provider-auth.ts:93-100 对 retired provider 仍返回 requiresSecret: true；packages/storage/src/runtime-policy/coordinator.ts:562-587 只据此准备 credential，存在旧 token 时返回 ready。packages/runtime-host/src/server/session-catalog-coordinator.ts:653-679 信任该结果，所以 default/explicit session.create 或 configuration update 可以持久化指向 retired Claude connection 的 Session，真正执行时才在更深层失败。
请把 retired/unavailable 作为 execution resolver 的一等结果，在 create/update commit 前拒绝，并覆盖 retained credential 下的 default 与 explicit target。
- P2：升级前的 retired Claude default target 没有释放。
packages/storage/src/runtime-policy/connection-catalog-document.ts:127-135 只清理旧 Gemini preview；Claude connection 保留后，其 defaultTarget 也原样保留。session-catalog-coordinator.ts:716-752 会继续选择它；apps/desktop/src/renderer/settings/providers-panel.tsx:266-270 还会显示“默认”，但不给用户切换该连接的操作。即使另有健康连接，新 Session 默认仍落到退役目标。迁移/读取时应释放 retired default，或提供等价的统一修复路径。
- P2：
request_authorization_code/ paste-code OAuth 链路已无生产 producer，却仍横跨 protocol、Host、runtime 和 Desktop 存在。
当前 Codex 与 xAI 都只产生 open_external，但 packages/runtime-host/src/protocol/oauth.ts:53-74、server/oauth-coordinator.ts:69,124,452-476、Desktop pending-code/IPC、client presenter 以及 packages/runtime/src/oauth-login.ts:139 的 authorization-code exchange 仍被保留。应端到端删除这条无需求的敏感鉴权表面；未来有真实 provider 需要时再随 producer 引入。
对应地，apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.ts:90 把 Codex fixture 伪造成 paste-code 流程，未覆盖真实 open_external 行为，应替换而不是继续维持死接口。
结论：方案目前不最优；需要局部根治，不需要推翻 retained-but-retired registry 设计。可删除完整 paste-code 链路、无调用的 exchange 注入、废弃 Claude env fixture 和重复 union member。CI 全绿，但没有覆盖 retained credential 的 Session admission、retired default migration 和合并后 epoch；这些是 merge blockers。
|
@M4n5ter @hqhq1025 — on the Why split. This PR's claim is "a provider that violates Anthropic's Consumer Terms no longer exists." The paste-code teardown is cleanup that removal enables — it is producerless because of this PR — but it is not needed for that claim to hold, and nothing in the retirement is incorrect while the dead path exists. Verifying them together also means one reviewer has to check both "the provider is gone from seven authorities" and "codex and xai device login still work after an OAuth-presentation teardown," which are unrelated failure modes. It matters for revert granularity too: if the teardown breaks a device flow in a way the tests miss, reverting it should not drag the Consumer Terms fix back with it. This PR is already 73 files; the teardown adds 14 across four packages. I am taking the cost of that honestly: the teardown is another wire change and will need its own epoch, so splitting spends two epochs instead of one. PR B scope — exactly this, nothing else. Removed end to end:
Kept, because the live device flows import them: Also in PR B: Everything else you both raised — the epoch, Host session admission with a retained credential, the retired default target, the override-ordering bypass @coderabbitai found, and the If either of you would rather have the teardown here despite the above, say so and I will fold it in. |
|
Tip For best results, initiate chat on the files or code changes.
The current PR can remain focused on retiring For PR B, remove the wire variants, desktop IPC channel, coordinator exchange path, runtime authorization-code helpers, and path-only tests in the same change. Keep the device-flow helpers that Codex and xAI still import. Please open a tracked follow-up issue for PR B, or link the follow-up PR here when it exists. This will make the remaining protocol cleanup explicit and reviewable. I will review the current-PR fixes after they are pushed. You are interacting with an AI system. |
1d978df to
91c504e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1ff2d6a-840f-468f-a678-dadabaee4355
📒 Files selected for processing (14)
apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/renderer/settings/providers-panel.tsxpackages/runtime-host/src/__tests__/session-catalog-coordinator.test.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/server/session-catalog-coordinator.tspackages/runtime/src/__tests__/claude-subscription-runtime.test.tspackages/runtime/src/__tests__/oauth-login.test.tspackages/runtime/src/__tests__/provider-contract-matrix.tspackages/runtime/src/model-runtime.tspackages/storage/src/__tests__/runtime-policy-stores.test.tspackages/storage/src/runtime-policy/connection-catalog-document.tspackages/storage/src/runtime-policy/coordinator.tspackages/storage/src/runtime-policy/operations.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src/main/runtime-host-client.ts
- packages/runtime/src/tests/claude-subscription-runtime.test.ts
- apps/desktop/src/renderer/settings/providers-panel.tsx
- apps/desktop/src/main/tests/runtime-host-oauth-ipc-main.test.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
|
Pushed. Point by point, and where I was wrong I have said so rather than quietly fixed it. @Astro-Han / @hqhq1025 — P1/P2, the epoch. Fixed: 24 → 25 in @M4n5ter / @hqhq1025 — P1/P2, Host session admission with a retained credential. Fixed, and you were right that my earlier reasoning was too convenient — I had considered guarding
@hqhq1025 — P2, the retired default target. Fixed, and thank you for catching the second half of it, which was mine: I had hidden "set as default" for retired connections but left the branch order such that a connection which already was the default still rendered the 默认 Badge with no control to move it — I removed a dead end and added a new one. Loading the catalog now releases a @coderabbitai — Major, the override-ordering bypass. Fixed: @coderabbitai — the contract matrix and the Codex IPC fixture. Both fixed here. @Astro-Han — P3, the mutation table overstated one row. Correct, and now stated in the description: reverting only the storage interactive-login allow list leaves the suite green, because the auth-contract gate independently produces the same refusal. That is defense in depth working, but the row claimed discriminating power it does not have. @Astro-Han — P3, "impossible to unregister" was overstated. Also corrected in the description. Also in this push: Still deliberately not done here: the Not covered: the new retired UI states have no Storybook stories and I have not run the Playwright E2E suite; both are stated in the description rather than left implied. Before/after screenshots for the UI/UX gate are in the PR body. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Changed files relevant to reported fixes ==='
git diff --name-only origin/main...HEAD | rg \
'(^packages/runtime-host/src/protocol/index\.ts$|^packages/storage/src/runtime-policy/|^packages/runtime/src/model-runtime\.ts$|^packages/runtime/src/__tests__/provider-contract-matrix\.ts$|^apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main\.test\.ts$|^packages/runtime-host/src/server/session-catalog-coordinator\.ts$|^packages/runtime-host/src/__tests__/session-catalog-coordinator\.test\.ts$|^packages/storage/src/__tests__/runtime-policy-stores\.test\.ts$|^packages/storage/src/runtime-policy/connection-catalog-document\.ts$|^apps/desktop/src/renderer/settings/providers-panel\.tsx$|^packages/runtime/src/__tests__/claude-subscription-runtime\.test\.ts$|^packages/runtime/src/__tests__/oauth-login\.test\.ts$)' || true
echo '=== Targeted outlines ==='
ast-grep outline packages/runtime/src/model-runtime.ts --items all --view expanded
ast-grep outline packages/storage/src/runtime-policy/coordinator.ts --items all --view expanded
ast-grep outline packages/runtime-host/src/server/session-catalog-coordinator.ts --items all --view expanded
echo '=== Epoch and retirement enforcement references ==='
rg -n -C 4 \
'RUNTIME_HOST_COMPATIBILITY_EPOCH|provider_retired|isRetiredProvider|lookupModelProviderOverride|wireDimensionCell|reasoning.*[Rr]eplay|openExternal|requestAuthorizationCode|MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL' \
packages/runtime-host/src/protocol/index.ts \
packages/runtime/src/model-runtime.ts \
packages/runtime/src/__tests__/provider-contract-matrix.ts \
packages/storage/src/runtime-policy \
packages/storage/src/__tests__/runtime-policy-stores.test.ts \
packages/runtime-host/src/server/session-catalog-coordinator.ts \
packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts \
apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.ts \
apps/desktop/src/renderer/settings/providers-panel.tsx \
packages/runtime/src/__tests__/claude-subscription-runtime.test.ts \
packages/runtime/src/__tests__/oauth-login.test.ts || true
echo '=== Remaining removed experimental flag references ==='
rg -n -C 2 'MAKA_CLAUDE_SUBSCRIPTION_EXPERIMENTAL' . -g '!node_modules' -g '!dist' -g '!build' || true
echo '=== Exact diff for relevant files ==='
git diff --unified=35 origin/main...HEAD -- \
packages/runtime-host/src/protocol/index.ts \
packages/runtime/src/model-runtime.ts \
packages/runtime/src/__tests__/provider-contract-matrix.ts \
packages/storage/src/runtime-policy/coordinator.ts \
packages/storage/src/runtime-policy/operations.ts \
packages/storage/src/runtime-policy/connection-catalog-document.ts \
packages/storage/src/__tests__/runtime-policy-stores.test.ts \
packages/runtime-host/src/server/session-catalog-coordinator.ts \
packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts \
apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.ts \
apps/desktop/src/renderer/settings/providers-panel.tsx \
packages/runtime/src/__tests__/claude-subscription-runtime.test.ts \
packages/runtime/src/__tests__/oauth-login.test.tsLength of output: 156942 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR body ==='
gh pr view 3183 --repo maka-agent/maka-agent --json body,url --jq '"URL: \(.url)\n\n\(.body)"'
echo '=== Candidate tracked follow-ups ==='
gh search issues \
--repo maka-agent/maka-agent \
--state open \
--limit 30 \
--json number,title,url,body \
--jq '.[] | select(
((.title + "\n" + .body) | test(
"request_authorization_code|complete-authorization|exchangeOAuthAuthorizationCode|paste-code";
"i"
))
) | "#\(.number) \(.title)\n\(.url)"' || trueLength of output: 8507
The Comment image satisfies the before/after UI screenshot requirement. It shows the retired composer state, model-list status, and connection-detail banner with unavailable actions removed. One follow-up remains: the PR body links the teardown scope comment, but not a tracked issue or follow-up PR for ✏️ Learnings added
You are interacting with an AI system. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for taking on the retirement as an end-to-end product change. The core direction is sound: keep the provider identity only so existing rows remain decodable and deletable, remove its OAuth/runtime implementation, and fail closed at execution. Four independent reviewer passes plus a read-only DeepSeek V4 Flash high-effort pass converged on the remaining boundary gap below.
The first-principles invariant should be: a retired provider may be decoded and deleted, but it cannot enter any ordinary mutation or action path. Today the read path clears a retired default and the execution path rejects it, while the catalog mutation API can still create a new claude-subscription row or commit it as a default because the provider remains a valid ProviderType; enabled retired rows can also still surface generic test actions. That creates states which a later read silently rewrites and actions which are guaranteed to fail.
The smallest coherent fix is to enforce retirement once at the storage mutation authority (create, setDefaultTarget, and any update that would make the row executable), then expose the same capability to shared action construction so test/default actions are omitted. Please keep decode/delete as the explicit exceptions. This is simpler than adding more renderer-specific guards and makes the retained registry entry serve exactly one compatibility purpose.
I did not run a local test suite; this review is based on the latest head, focused static tracing, the existing CI results, four independent @reviewer reviews, and a read-only ollama-cloud/deepseek-v4-flash high-effort review. AI-assisted review; I verified the reported control flow and discarded findings that were only artifacts of comparing this old branch head directly with current main.
中文评论
感谢把 Claude subscription 的退役做成端到端收口。核心方向是对的：只保留 provider 身份用于解码和删除旧连接，移除 OAuth/Runtime 实现，并在执行边界 fail closed。
剩余问题是 authority 没有完全统一：退休 provider 仍是合法 ProviderType，所以 mutation API 仍可新建该连接、把它设为默认，通用 action 层也仍可能提供测试入口；但读取时又会清空默认值，执行时必然拒绝。这会制造“写入成功、读回被改写”以及必然失败的操作。
更符合第一性原理和奥卡姆剃刀的方案，是在 storage mutation authority 一次性禁止退休 provider 的新建、设默认和重新启用，并让共享 action 构造消费同一 capability；仅把解码和删除保留为明确例外。这样不需要继续在各个 renderer 补零散判断。
本次未运行本地测试；结论来自最新 head 的静态追踪、现有 CI、四个独立 reviewer 和一次只读的 DeepSeek V4 Flash high-effort 审查。AI 辅助审查；我已人工复核控制流，并剔除了由旧分支与当前 main 直接比较产生的误报。
| protocol: 'anthropic', | ||
| runtimeAdapter: { kind: 'claude-subscription' }, | ||
| runtimeAdapter: { kind: 'unavailable' }, | ||
| retired: true, |
There was a problem hiding this comment.
P2 — Reject retired providers at the catalog mutation authority. Keeping this registry entry is necessary for decoding existing rows, but it also keeps claude-subscription valid for CreateCatalogConnectionInput. ConnectionCatalogDocumentOwner.create() does not check isRetiredProvider, so an IPC/protocol caller can still create a brand-new enabled connection that can never execute. The same gap lets setDefaultTarget() return committed for a retired row, after which read() silently normalizes that default back to null. Please make decode/delete the explicit exceptions and reject create/default/re-enable mutations for retired providers in the storage owner; add focused tests for create and set-default rejection.
ace2ca2 to
330cc83
Compare
Maka could sign in with a Claude Pro/Max subscription and send inference through it. Anthropic's Consumer Terms permit programmatic access to the consumer Claude services only through an API key or explicit permission, and this path had neither: it presented itself as Claude Code — borrowing that client_id, its User-Agent, its beta header set and an `x-app: cli` marker — to get requests accepted. The account carrying that risk is the user's, not Maka's. Remove the capability rather than gate it. `claude-subscription` keeps its registry entry so a stored connection still decodes and renders, and is marked `retired`, which is distinct from a provider that was never wired: both have no Runtime adapter, but only one used to work. Retirement is refused at each authority that could otherwise admit the connection, so no single revert makes it sendable again: - the auth contract hides every action, which is what makes the storage layer refuse a model fetch or a connection test - the readiness gate reports `provider_retired` before the send is admitted, instead of letting it fail inside model construction - the model catalog resolves every model to `provider_removed`, so the pickers stop offering them - the interactive-login allow list and the Host wire enum no longer name it - `getAIModel` and `resolveModelRuntime` throw as the last backstop Settings explains the state instead of pointing at a sign-in that no longer exists, and stops offering "set as default" and "test connection" for a connection that cannot perform either. Deleting the connection is what clears the credential this machine still holds. The impersonation code goes with it: the cloaked request builder, the Claude token endpoint and its client identity, the cloaked model-fetch headers, and the subscription usage/quota path that needed that same identity to read. `RUNTIME_HOST_COMPATIBILITY_EPOCH` moves to 23: the OAuth login provider enum and the account-usage operation both changed. Generated-by: Claude Code
330cc83 to
66f94be
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest head closes the prior retirement gaps cleanly. The compatibility epoch is now 26; legacy rows remain decodable/deletable but cannot be created, enabled, or selected as defaults; default targets pointing at retired connections are released on read; Host admission rejects provider_retired before credentials or Session persistence; and the auth/model/runtime backstops prevent an override bypass. Current-main integration is clean and all live checks are green.
From first principles, provider retirement now has one enforceable storage/execution contract rather than relying on UI removal. The remaining producerless request_authorization_code / paste-code path can be deleted as a separate, tracked cleanup; it is no longer reachable through a live provider and does not block this retirement. Please also update the PR description's epoch note from 24→25 to the actual 25→26.
Approved. Reviewed with Codex using two independent reviewer agents and an external DeepSeek review; I verified the latest head, mutation/admission/model backstops, compatibility epoch, current-main merge, and live CI.
中文
最新 head 已完整闭合此前的退休缺口：compatibility epoch 现为 26；旧行仍可解码/删除，但不能新建、启用或设为默认；读取时会释放指向退休连接的默认目标；Host admission 在读取凭据或持久化 Session 前返回 provider_retired；auth/model/runtime backstop 也阻止 override 绕过。与当前 main 可干净合并，实时检查全绿。
按第一性原理，provider retirement 现在由统一的存储/执行契约强制，而不是依赖 UI 删除。剩余没有 producer 的 request_authorization_code / paste-code 路径可以作为独立、可跟踪的清理删除；它已无法通过现有 provider 到达，不阻塞本次退休。另请把 PR 描述中的 epoch 从 24→25 修正为实际的 25→26。
批准。本次由 Codex 配合两个独立 reviewer agent 和外部 DeepSeek 审查；我核验了最新 head、mutation/admission/model backstop、compatibility epoch、与当前 main 的合并和实时 CI。
|
复核最新 head 还剩两个合并前建议修正的点：
|
The command palette filtered per-connection commands on `enabled` alone, so a retained retired connection — which stays enabled so its credential remains visible and deletable — still offered "set as default" and "test connection", both refused downstream by the storage default-target gate and the hidden auth actions. The palette now checks the same isRetiredProvider capability Settings uses, including the diag test-default entry for a stale in-memory default. Regression covers a retained-enabled retired row next to a live positive control; reverting the loop filter turns it red. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
@likun666661 — both fixed in P2, the palette. You were right, and it was the one shared surface the sweep missed: P3, the epoch note. The description now states the actual diff, 25 → 26, verified against The teardown promise is now #3219 rather than a comment: full file-by-file scope, what stays because the live device flows import it, and the epoch cost of the split. @M4n5ter @hqhq1025 — your review requests are addressed as of this head (Host admission with a retained credential, the retired default target, the storage mutation gates, and the fixture replacement), and @Astro-Han has approved the retirement chain. Could you take another look when you have a moment? |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest delta closes the remaining Command Palette action surface for retained retired providers. It reuses the existing retirement authority, covers all three connection command paths, and includes a discriminating live-connection positive control. I found no new P0–P3 issue in this head.
The prior epoch, Host admission, retired-default, mutation-authority, and runtime/auth backstop findings remain resolved. The producerless authorization-code path is now explicitly tracked as separate follow-up work.
Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the latest delta and prior-finding state against this exact head and current main.
中文
最新 delta 已关闭 retained retired provider 在 Command Palette 中最后的 action surface。实现复用了现有 retirement authority，覆盖三类 connection command，并用 live connection 正向对照避免过度过滤。当前 head 未发现新的 P0–P3。
此前的 epoch、Host admission、retired default、mutation authority 和 runtime/auth backstop 问题仍保持已解决；producerless authorization-code 路径已明确拆为后续工作。
本次由 Codex 两轮独立审查，并使用 DeepSeek V4 Flash 做外部对抗审查；我核对了当前 head、最新 delta 与 prior findings。
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
Requesting changes on the latest head.
The earlier epoch, default-target, general session-admission, mutation-authority, runtime-backstop, and Command Palette findings remain fixed. This pass found three non-duplicate residual paths:
- Removing
oauth.account.usage.fetchmakes previously issued Remote Host access files fail decoding, preventing the Host from starting. The known Remote Host population is almost nonexistent, so its practical reach is negligible, but the upgrade failure is deterministic for affected profiles. - Configured subagent admission still considers retained retired connections available. Direct spawn and graph provisioning can consequently persist a child/operator that can never execute.
- The CLI
/modelprojection still presents models belonging to the retired connection and only rejects them after selection.
The configured-subagent persistence gap independently warrants changes. Both remaining selection gaps can be closed by consuming the new canonical isRetiredProvider() authority rather than adding renderer- or caller-specific state.
The simplify audit found no additional merge blocker. The producerless authorization-code/paste-code path is already tracked by an existing review and is intentionally not duplicated here. Making retired connections fully read-only tombstones would be a reasonable follow-up simplification.
I verified these paths against head 6fc4447. Relevant targeted suites pass and all current GitHub checks are green; the missing retired-provider cases are not covered by those tests.
I’ll respond quickly to follow-ups on this PR, so please feel free to ping me directly.
简体中文
建议在最新 head 上 Request changes。
此前关于 compatibility epoch、default target、通用 session admission、mutation authority、runtime backstop 和 Command Palette 的问题仍保持已修复。本轮发现三个未与现有 review 重复的剩余入口：
- 删除
oauth.account.usage.fetch会使此前签发的 Remote Host access file 解码失败，阻止 Host 启动。已知 Remote Host 用户几乎不存在，实际影响面可以忽略不计，但对命中的 profile 来说升级故障是确定性的。 - Configured subagent admission 仍把保留的 retired connection 视为可用，因此直接 spawn 和 graph provisioning 可以持久化一个永远无法执行的 child/operator。
- CLI
/model投影仍会展示属于 retired connection 的模型，直到用户选择后才拒绝。
Configured-subagent 的持久化缺口本身足以支持 Request changes。两个剩余选择入口都可以通过消费新的统一 isRetiredProvider() authority 来关闭，不需要增加 renderer 或 caller 专属状态。
简化审计没有发现额外 merge blocker。没有 producer 的 authorization-code/paste-code 路径已经由现有 review 跟踪，本次不重复评论。将 retired connection 进一步收敛为完全只读的 tombstone，可以作为后续简化。
我在 head 6fc4447 上验证了这些路径。相关 targeted suites 通过，当前 GitHub checks 全绿；但现有测试没有覆盖上述 retired-provider 场景。
对于这个 PR，我会尽快回复后续讨论；有需要请直接 ping 我。
Three residual entry points from review, all consuming the canonical isRetiredProvider() authority rather than caller-local state: A previously issued Remote Host access file granting oauth.account.usage.fetch failed decoding once the operation left the protocol, deterministically blocking Host startup for that profile. The grant is now released on decode — the same seam that migrates the legacy transcript-query grant — because there is nothing to migrate a retired operation to, and failing the whole file refuses a Host over a capability it could not serve anyway. Configured subagent admission judged a preset by enabled + model alone, and a retained retired connection stays enabled by design, so a direct spawn or graph provisioning could persist a child that can never execute. provider_retired is now a first-class unavailability reason, checked ahead of connection_disabled — there is no switch that repairs it — refused by resolve() before anything persists, and surfaced with its own copy in the desktop subagent settings list. The CLI /model projection listed a retired connection's models and only refused them after selection; it now filters them out with the same authority. No wire-contract change: the availability union lives in the runtime package, not the protocol, so no epoch bump rides along. Each fix has a regression that turns red when the guard alone is reverted; the runtime suite's failing set is byte-identical to the same suite at the base commit on this machine (environmental ripgrep/filesystem cases). Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
@M4n5ter — all three closed in 1 — the access-file upgrade failure. Real and deterministic, as you said. The fix reuses the seam the store already has for grants that outlived their operation: 2 — configured subagent admission. The gap was that 3 — the CLI No epoch bump rides along: the availability union lives in Suite state: the runtime suite's failing set on this machine is byte-identical between this head and the base commit (stashed, rebuilt, re-ran, diffed programmatically — the environmental ripgrep/filesystem cases), and the desktop suite is 930/930. On the read-only tombstone follow-up you sketched: agreed it is the natural next simplification once #3219 lands, and I would keep it out of this PR for the same revert-granularity reason as the teardown. |
…signature Main's apache#2972 landed a test constructing createHostSessionEffectModel with claudeDeviceId, which this branch removes with the provider that needed it. The merge is otherwise clean; only the field goes. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
EnglishThanks for the detailed follow-up. I verified both One user-facing selection path still remains. The Desktop preset list now reports
A retained retired connection intentionally remains enabled, so the editor still lets users create and save a preset targeting it; the newly saved preset then immediately appears as Please exclude retired providers from the editor's selectable/valid projection using the same canonical helper. An existing retired selection can remain visible but disabled so the user can reroute it. A focused editor regression would distinguish this from the list-badge coverage already added. The access-file thread is satisfied by the implementation. I’ll keep the combined selection thread open for this remaining editor path and re-check promptly after the update. 简体中文感谢详细回复。我核验了 目前还剩一个用户可见的选择入口。Desktop preset 列表现在会正确显示
Retained retired connection 按设计仍保持 enabled，因此 editor 仍允许用户创建并保存一个指向它的 preset；该 preset 保存后会立刻显示为 请使用同一个统一 helper，从 editor 的可选和有效 connection 投影中排除 retired provider。已有的 retired selection 可以继续显示为 disabled，方便用户改路由。建议增加一个 editor 层面的针对性回归测试，以区别于目前仅覆盖列表 badge 的测试。 Access-file thread 的实现已经满足要求。我会暂时保留合并后的 selection thread，等待这个 editor 入口关闭；更新后我会尽快复核。 |
The trust-boundary step failed with insecure_endpoint_directory on a tree whose merge introduced no endpoint or ACL delta, and the same main commit passes the identical check on main. One rerun separates a runner environment fault from something this branch causes. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
…jobs bded591 and 5da258d are byte-identical trees: the first failed only windows_recovery (endpoint ACL) and passed the CLI Windows smoke, the second passed windows_recovery and failed the CLI smoke with INTERNAL_STARTUP_FAILURE during TUI setup. Every check has passed on this tree at least once; this run is for one clean sweep. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
@M4n5ter @hqhq1025 — the three residual paths are fixed as of |
EnglishThanks — I confirmed that all 20 checks are green, and I agree that the two earlier Windows failures were runner flakes; they are now tracked separately in #3237. I also agree that However, this reply does not address the later Desktop preset-editor finding. On the current head No code has changed in that path since the finding was posted. I’m therefore keeping the selection finding open and cannot approve this head yet. Please apply the canonical retirement check to the editor’s selectable/valid projection while keeping an existing retired selection visible but disabled for rerouting. 简体中文谢谢。我已确认当前 20 项检查全部通过，也认同此前两次 Windows 失败属于 runner flake；该问题现已由 #3237 单独跟踪。我也确认 不过，这条回复没有覆盖之后提出的 Desktop preset editor 问题。在当前 head 该问题提出后，这条代码路径没有发生变化。因此我会继续保留 selection finding，目前还不能 approve。请在 editor 的可选及有效 connection 投影中使用统一的 retirement 判断；已有 retired selection 可以保持可见但禁用，方便用户重新选择连接。 |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest delta correctly closes the three retirement gaps raised after the prior approval: released Remote Host grants migrate before validation, configured-subagent runtime admission rejects retired connections before child persistence, and CLI model projection filters retired choices. Deletion safety, protocol epoch 26, execution backstops, and all required checks also hold.
I am not re-approving yet because the existing M4n5ter thread identifies one remaining product projection: the Desktop Subagent editor still treats enabled as sufficient in usableConnections, validConnection, and connectionOptions. A retained enabled Claude connection can therefore be selected and saved into a preset that the runtime correctly refuses as provider_retired. I verified that current head still has this path, so I am leaving one deduplicated overall comment rather than repeating the inline finding.
AI-assisted review by Codex with three independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high); I verified the post-approval delta, existing thread, current head, and live CI.
中文
最新增量已正确关闭上次批准后的三个缺口：旧 Remote Host grant 在校验前迁移；configured-subagent runtime admission 在持久化 child 前拒绝 retired connection；CLI model projection 过滤 retired choices。删除安全、protocol epoch 26、execution backstop 与必需 CI 也都成立。
暂不再次 Approve，因为 M4n5ter 已有线程指出剩余一个产品投影：Desktop Subagent editor 的 usableConnections、validConnection 和 connectionOptions 仍只判断 enabled。保留且 enabled 的 Claude connection 因此仍可被选择并保存成 runtime 必然以 provider_retired 拒绝的 preset。我已确认当前 head 仍存在该路径，因此只发去重后的整体评论，不重复 inline。
本次由 Codex、三个独立 reviewer 和 OpenCode Go DeepSeek V4 Flash high 辅助；已核对批准后的增量、已有线程、当前 head 与实时 CI。
The list badge said provider_retired, but the editor still judged a connection by enabled alone in usableConnections, validConnection, and connectionOptions, so a retained retired row could be selected and saved into a preset the runtime admission is guaranteed to refuse. One predicate — isSelectableSubagentConnection, enabled and not retired — now feeds all of them plus the model derivations, and the option row for an enabled-but-retired connection says why it cannot be picked instead of graying out silently. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
|
@Astro-Han — the editor projection is closed in |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The latest head correctly closes the Desktop Subagent-editor gap: one isSelectableSubagentConnection() predicate now excludes retired providers from usable models, validity, and options, with focused retained/disabled/live coverage.
The existing catalog-mutation authority thread is still only partially closed. Create/default/session admission now reject retired providers, but connection.catalog.update can take a retained disabled claude-subscription row and commit enabled: true; runtime then predictably rejects it as provider_retired. I am not duplicating that inline thread. The smallest coherent fix is to reject re-enabling retired rows at the storage mutation authority (or reject all ordinary updates if retained rows are decode/delete-only), with no revision change on rejection.
All live checks are green, but merge state remains blocked by unresolved review state.
Reviewed with Codex using two independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the exact delta, the prior Desktop finding, storage/runtime retirement invariants, focused tests, and live CI.
中文
最新 head 已正确修复 Desktop Subagent editor 缺口：统一的 isSelectableSubagentConnection() predicate 会在 usable models、validity 和 options 中排除 retired provider，并有 retained/disabled/live 聚焦覆盖。
但现有 catalog mutation authority 线程仍只部分关闭。create/default/session admission 已拒绝 retired provider；connection.catalog.update 仍可把 retained disabled claude-subscription row 提交为 enabled: true，随后 runtime 必然以 provider_retired 拒绝。我不重复发布该行内线程。最小一致修复是在 storage mutation authority 拒绝重新启用 retired row；若 retained row 只用于 decode/delete，则可直接拒绝所有普通 update，并保证 rejection 不增加 revision。
实时检查全绿，但 unresolved review state 仍使 merge state blocked。
本次由 Codex 配合两个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我核验了精确增量、既有 Desktop finding、storage/runtime retirement invariant、聚焦测试和实时 CI。
M4n5ter
left a comment
M4n5ter
left a comment
There was a problem hiding this comment.
English
LGTM. The latest update closes the remaining subagent editor path, and the previously reported findings are resolved. All current checks pass.
简体中文
LGTM。最新更新已关闭剩余的 subagent editor 路径，之前提出的问题均已解决。当前检查全部通过。
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
One new merge-blocking finding is attached inline.
The earlier hqhq1025 blockers around the compatibility epoch, retained-credential Session admission, and retired default targets are fixed on this branch. I did not duplicate the existing unresolved retired-provider update/re-enable thread.
This exact head also conflicts with current main, whose Runtime Host compatibility epoch is already 28. The required rebase must allocate the next distinct epoch for this wire removal, preserve both sides' authority changes, and receive a complete new-head review.
I reviewed the complete revision; required checks are green, build:test passed, and 228 focused local tests passed.
Codex-assisted review performed under the maintainer-approved review workflow.
| // decoding, but nothing may author a new one: it could never execute, and | ||
| // reading the catalog back would immediately release it as a default. | ||
| // Decoding and deleting are the deliberate exceptions to that. | ||
| if (isRetiredProvider(input.connection.providerType)) { |
There was a problem hiding this comment.
[P1] Keep retained retired connections from aborting config restore
This guard is correct for ordinary authoring, but this revision deliberately retains existing claude-subscription connections and credentials so users can see and delete them. Config export includes every catalog connection, while a fresh-profile import saves connections sequentially. After any earlier live connections commit, the retained row reaches createConnection() and throws here, so the import exits before restoring settings, credentials, and memory, leaving a partial configuration.
This is the supported backup/restore path for the users affected by this retirement, not a forged input. Filter retired providers at import planning, count them as skipped, skip their credentials, and keep this storage guard. Add a full-bundle regression with a live connection before a retained retired one.
Config export carries every catalog connection, and this retirement deliberately retains existing claude-subscription rows so the user can see and delete them. A fresh-profile import saves connections one at a time, so that row reached createConnection(), which refuses it — and the refusal aborted the restore partway, after whichever live connections had already committed and before settings, credentials, and memory. That is the supported backup path for exactly the users this retirement affects. The storage guard stays: creating a retired connection is still refused, because it could never execute. Import planning no longer hands it one — the row is planned as skipped, and its credential is skipped with it, since only a created or overwritten slug gets a secret written. The regression restores a full bundle whose live connection is ordered before the retained retired one, so an aborted import would look like a partial success rather than a clean failure; reverting the filter turns it red. Reported by @hqhq1025. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_014ajaRxC4jydavY9nYUFj5J
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
The previous config-restore P1 is fixed on this revision. Retired-provider connections are removed from the import plan before any connection save, their credentials remain unapplied, and the rest of the selected bundle continues normally. Exact-head build:test, 149 focused retirement/import tests, lint, format, and the Astryx surface inventory all passed.
I am leaving a COMMENT rather than approving because both hosted GitHub Actions suites are currently action_required, and the existing current-code update/re-enable thread remains unresolved. I found no new unique finding and did not duplicate that thread.
Codex-assisted review performed under the maintainer-approved review workflow.
|
Could a committer approve the workflow runs on What changed since the last review, so the new head is not a black box: @hqhq1025's P1 — config restore. You were right, and it was the worst kind of break: the users this retirement affects are exactly the ones whose backup carries a The guard stays, because creating a retired connection is still wrong. Import planning no longer hands it one: the row is planned as skipped, and its credential is skipped with it, since only a created or overwritten slug gets a secret written. The regression restores a full bundle whose live connection is ordered before the retained retired one — so a regression would look like a partial success rather than a clean failure — and reverting the filter turns it red. The rebase you asked for. Current
Epoch is 30, not the 24→25 an earlier description claimed and not the 26 this branch carried before the rebase. Main reached 29 while this sat, and main also adopted a changelog comment format for that constant, which the new entry follows. Verified with Local state: build, typecheck, lint, format clean; the retirement contract suite is 9/9; the runtime suite's failing set is byte-identical to the same suite at the merge base on this machine (environmental ripgrep/PTY/filesystem cases), and eval's is the Docker preflight. |
5 participants
Summary
Maka could sign in with a Claude Pro/Max subscription and send inference through it. Anthropic's Consumer Terms permit programmatic access to the consumer Claude services only through an API key or explicit permission, and this path had neither — it presented itself as Claude Code (borrowing that
client_id, its User-Agent, its beta header set, and anx-app: climarker) to get requests accepted. The account carrying that risk is the user's, not Maka's.This removes the capability rather than gating it further.
claude-subscriptionkeeps its registry entry, and the entry is markedretired— deliberately distinct from a provider that was never wired, since both end up with no Runtime adapter but only one of them used to work and needs to explain itself.Keeping it registered is a product choice, not a technical necessity. Unregistering is possible:
isRetiredGeminiCliConnectionis the precedent, filtering by rawproviderTypestring before canonical decode. It is avoided here because a user who signed in has a credential on this machine, and the connection is what makes that visible and deletable — unregister it and onboarding reportsneeds_connection, with the credential still on disk and nothing pointing at it. (decodeProviderTypedoes throw on unknown types andconnection-catalog-codec.tswould fail the whole document, so unregistering without the filtering precedent would cost the user every other connection — but that is a reason to use the precedent, not a reason the choice was forced.)Retirement is refused at each authority that could otherwise admit the connection, so no single revert makes it sendable again:
deriveProviderAuthContracthidden— this is what makes the storage layer refuse a model fetch or a connection testresolveExecutionConnectionprovider_retired, a first-class result, so a retained credential can no longer read asreadySessionCatalogCoordinatorisConnectionReadyprovider_retired, before a Desktop send is admittedbuildConnectionModelCatalogEntriesprovider_removed, so the pickers stop offering themresolveModelRuntimeOAUTH_LOGIN_PROVIDERS(Host wire)getAIModelLoading the catalog also releases a
defaultTargetthat points at a retained retired connection, so new Sessions stop defaulting to something that cannot run. Settings no longer shows a 默认 Badge on one either, since retirement is checked before both default states — a Badge with no control to move the default off it would be exactly the read-only label that slot exists to avoid.Settings explains the state instead of pointing at a sign-in that no longer exists, and stops offering "set as default" and "test connection". Onboarding gets its own
all_connections_retiredcopy — the generic blocked text tells the user to re-check credentials, sign-in status, and network, and for a retired provider all three lead nowhere. Deleting the connection is what clears the credential this machine still holds.The impersonation code goes with it: the cloaked request builder, the Claude token endpoint and its client identity, the cloaked model-fetch headers, and the subscription usage/quota path that needed that same identity to read.
RUNTIME_HOST_COMPATIBILITY_EPOCHmoves 25 → 26 (main reached 25 while this PR was in review, so the rebase re-took the next slot; the description tracks the actual diff). This PR changes the wire contract (removesclaude-subscriptionfromOAUTH_LOGIN_PROVIDERS, deletes theoauth.account.usage.fetchoperation), so it takes its own epoch rather than riding one raised elsewhere. An earlier revision of this description claimed a 22 → 23 bump that a rebase had turned into a no-op; that was wrong, and @Astro-Han and @hqhq1025 both caught it.Refs #3143
Verification
Built from clean (
npm run cleanfirst) — an incremental build hid a real error during this work, so every gate below was re-run after a full rebuild.npm run build,npm run typecheck— 0 errorslint,format:check,astryx:surface-inventory,check:stale,check:third-party-notices,check:windows-cargo-notices— passnpm test— the failing set is identical to the same suite on a cleanorigin/mainworktree on this machine (environment-related ripgrep / PTY / filesystem-worker cases). Diffed programmatically rather than eyeballed.Each enforcement point was mutation-checked: reverting it individually turns a test red.
runtimeAdapter: 'unavailable'retired: trueproviderAvailableretirement branchresolveExecutionConnectionrefusaldefaultTargetreleaseOAUTH_LOGIN_PROVIDERSisConnectionReadygateOne correction to an earlier revision of this table: the storage interactive-login allow-list row does not discriminate on its own — reverting only that line leaves the suite green, because the auth-contract gate independently produces the same refusal. That is defense in depth working as intended, but the row overstated its own power and @Astro-Han was right to flag it.
End-to-end, against a real
claude-subscriptionconnection already stored on this machine (packaged app, installed, launched) — see the before/after screenshots below.Not run: Windows and Linux packaging, and the Playwright E2E suite. The new retired UI states have no Storybook stories.
Breaking change
A user signed in through the Claude subscription can no longer send with it. The connection and its stored credential are left in place and readable so the state is explainable; an Anthropic API key connection is the replacement path. Clients built against an earlier epoch are rejected at the handshake by the bump above.
Follow-up
The
request_authorization_code/ paste-code presentation has no producer left once this lands, and both @M4n5ter and @hqhq1025 asked for its removal. It is the immediate next PR rather than part of this one; the exact file-by-file scope, what stays because the live device flows import it, and why the unused#exchangeCodefield is deliberately left in place here are in this comment.AI use
Select exactly one:
Tool(s) and scope: Claude Code — drafted the removal, the enforcement points and their tests, and ran the verification described above. Reviewed and submitted by the contributor of record.
Generated-by: Claude Codeis on the commit.Checklist
Does this PR entail a change in behavior?