feat(runtime-host): bind OAuth login to Connection entities - #3924
Conversation
bab2ae4 to
c88dd63
Compare
c88dd63 to
04dd9c1
Compare
|
Addressed the current hosted The OAuth entity-binding commit is patch-equivalent across the final restack onto #3882. Validation: Storage 995 tests passed with 18 platform skips, Storage typecheck, Biome check, protocol epoch guard (54), and diff-check. Generated-by: Codex |
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found no blocking issues.
Binds each interactive OAuth login to an explicit target (create(providerType) or existing(connectionId)) with persistent oauth-login-receipts (monotonic order, attemptId dedup, target↔identity checks), turning silent supersede into operation_conflict; correct identity matching, fail-closed conflict, legacy adoption CAS retry, and onboarding ticket fixing TOCTOU; hosted test+windows_recovery SUCCESS.
No P0-P3.
简体中文
该头无阻断。Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
04dd9c1 to
01a325e
Compare
M4n5ter
left a comment
There was a problem hiding this comment.
I found no P0-P3 source issues on exact head 01a325eff179de48ac0b4e371f5c64fe1f7f6138, but I cannot approve this head while its required test check is red.
The first run failed three WorkHub E2E cases; I reran the failed job on the unchanged head and it again failed the two reconstruction cases: WorkHub rebuilds delegated execution feedback after navigating away and back and WorkHub defers destructive correction until linked delegation exists. The PR does not modify WorkHub and the frozen base's hosted test is green, so I am not attributing these failures to this patch without a causal base/head comparison. The gate can be lifted by making the exact-head check green or by demonstrating and resolving an inherited failure with repeatable base/head evidence.
On the reviewed change itself, the flow keeps one immutable Connection identity from the closed create(providerType) / existing(connectionId) target through Host admission, Storage allocation, credential/catalog commit, the durable receipt, and Desktop completion. A new Connection remains invisible until authentication commits; exact re-login and account actions preserve the selected ID; ambiguous aggregate mutations fail closed. The durable intent recovers vault/catalog/receipt boundaries before orphan cleanup.
Validation outside the failing WorkHub E2E gate included the complete Core, Storage, and Runtime Host suites; focused Desktop OAuth tests; affected builds; protocol epoch guard 68 -> 69; diff check; and a byte-identical current-main merge tree.
Automated review notice: This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for restacking this onto the current Connection foundation and narrowing the PR to the OAuth entity-binding slice. The underlying problem is real: OAuth login must target an explicit create(providerType) or existing(connectionId) intent rather than selecting a Connection by provider.
The overall architecture is sound. Runtime Host owns the active OAuth lifecycle, Storage remains the single durable Connection and credential authority, and Desktop only carries the user’s exact target. The receipt and journal mechanisms have distinct recovery responsibilities and do not create a second Connection authority.
I found one P2 concurrency issue:
When OAuth request A has been admitted by the Host but is still waiting for browser presentation, request B calls RuntimeHostOAuthPresentation.expect(). The current implementation rejects and replaces A’s pending presentation before B reaches Host admission. The Host correctly rejects B with operation_conflict, but A then observes its rejected presentation while startedOnHost=true and calls cancelOAuthLogin(), cancelling the original active login.
This is a reachable concurrent Desktop path and contradicts the intended “a conflicting second start must not cancel the active attempt” behavior. The impact is a recoverable interrupted login rather than identity corruption, so I am classifying it as P2 rather than P1.
The smallest fix is to make presentation admission non-destructive: if a presentation is already pending, reject the new expectation without replacing the existing one. Please add a regression where request B arrives after A is active on the Host but before A’s browser presentation completes, and assert that A is never cancelled.
The exact-head test check is currently red on two WorkHub reconstruction E2E assertions. They do not appear related to OAuth, but the required exact-head check still needs to be green before merge.
Review analysis was assisted by Codex and independent @reviewer agents. Astro-Han verified the exact head, the concurrent production path, the severity and recoverability, and the current CI evidence, and owns this review.
中文对照
谢谢你把这个 PR 重新整理到当前 Connection 基础上,并收敛为 OAuth entity binding 这一条完整切片。底层问题是真实的:OAuth 登录必须明确指向 create(providerType) 或 existing(connectionId),不能继续按 provider 猜测某条 Connection。
整体架构是正确的。Runtime Host 持有活跃 OAuth 生命周期,Storage 仍然是 Connection 和 credential 的唯一持久化 authority,Desktop 只传递用户选择的精确目标。receipt 与 journal 分别承担恢复职责,没有形成第二套 Connection authority。
当前有一个 P2 并发问题:
当 OAuth 请求 A 已被 Host 接受、但仍在等待浏览器展示时,请求 B 会调用 RuntimeHostOAuthPresentation.expect()。当前实现会先拒绝并替换 A 的 pending presentation,然后 B 才进入 Host admission。Host 会正确地以 operation_conflict 拒绝 B,但 A 随后发现 presentation 被拒绝,并且 startedOnHost=true,于是调用 cancelOAuthLogin(),反而取消了原本活跃的登录。
这是可达的 Desktop 并发路径,也违背了“第二个冲突请求不能取消活跃 attempt”的目标。后果是一次可恢复的登录中断,而不是身份损坏,因此定为 P2,不升 P1。
最小修复是让 presentation admission 不具破坏性:已有 pending presentation 时,只拒绝新 expectation,不替换旧 expectation。请补一个回归测试:B 在 A 已被 Host 接受、但 A 的浏览器展示尚未完成时到达,并确认 A 从未被取消。
当前 exact-head test 还因为两条 WorkHub reconstruction E2E 断言失败而红。它们看起来与 OAuth 无关,但合并前 required exact-head check 仍必须恢复绿色。
本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、并发生产路径、问题的分级与可恢复性以及当前 CI 证据,并对本次 Review 负责。
23cae6d to
4fbdfe5
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
I found one concurrency issue in the Desktop OAuth presentation bridge. The Host/storage ownership and connection-identity design otherwise look coherent on this exact head. The required test check is still running, and this PR still needs an independent human committer review.
中文摘要
发现 1 个并发路径 P2:第二次登录请求会先替换 Desktop 侧已有 presentation expectation,随后即使 Host 正确拒绝第二次请求,也会反过来取消第一条已经被 Host 接受的登录。其余 Host/storage/Connection identity 边界在当前 exact head 上未发现新的 P0/P1。
AI-assisted review disclosure: Codex coordinated independent reviewer lanes; Astro-Han independently checked the exact head, production path, and severity, and owns this review.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving OAuth enrollment identity and recovery into durable Runtime Host-owned contracts. I reviewed exact head 4c6698bb across the Host, Storage, Desktop IPC, and two-client paths; the solution and distinct trust-boundary coverage look sound, and the exact-head checks pass, so I’m approving it.
I left two non-blocking P3 suggestions to reduce test debt without dropping semantic coverage: consolidate repeated OAuth client fixtures, and prefer production-owner fault injection over hand-built persistence states. Please push back if the explicit setup is preserving a contract I missed.
AI-assisted review: Codex inspected the exact-head diff, production path, recovery boundaries, and tests; I reviewed the findings and approval decision.
Make interactive OAuth enrollment create or reauthenticate one exact Connection, recover credential and catalog publication durably, and keep Desktop account actions entity-scoped. Generated-by: Codex
4c6698b to
2df8f7d
Compare
M4n5ter
left a comment
There was a problem hiding this comment.
I reviewed exact head 2df8f7db7059d1d6c95555555f035bc574d01cbb and found no P0-P3 issues.
The OAuth presentation reservation remains non-destructive: a conflicting second request cannot replace or cancel the first request after the Host has admitted it. The new fail-closed test fixture removes duplicated client scaffolding while recording every undeclared effect before throwing, so best-effort production error handling cannot conceal an unexpected call. The focused OAuth IPC suite passes 7/7.
The exact-head hosted checks are terminal green, all three review threads are resolved, compatibility epoch 78 is newer than current main's 77, and the current-main merge tree is clean with no changed-file overlap.
Review notice: This review was prepared by an automated review agent operated by M4n5ter and is published at the direction of me2seeks, who has read these findings and is the human accountable for them.
) * feat(runtime-host): bind OAuth login to Connection entities Make interactive OAuth enrollment create or reauthenticate one exact Connection, recover credential and catalog publication durably, and keep Desktop account actions entity-scoped. Generated-by: Codex * style(storage): format OAuth enrollment paths * fix(desktop): preserve active OAuth presentation * test(desktop): consolidate OAuth IPC fixtures
Summary
create(providerType)orexisting(connectionId)operation_conflictand advance the Runtime Host compatibility epoch from 53 to 54Stack
This is the OAuth entity-binding workstream of #3852 and is stacked after #3882. Review the final commit
c88dd63b1for this PR's isolated change.Closes #3888.
Validation
npm --workspace @maka/core run typechecknpm --workspace @maka/storage run typechecknpm --workspace @maka/runtime-host run typechecknpx tsc -p apps/desktop/tsconfig.main.json --noEmitnode scripts/protocol-epoch-check.mjs --base f1fa3f9c7 --head HEAD(53 -> 54)git diff --check f1fa3f9c7..HEAD