Skip to content

feat(runtime-host): bind OAuth login to Connection entities - #3924

Merged
me2seeks merged 4 commits into
apache:mainfrom
me2seeks:feat/3852-oauth-connection-entity
Aug 31, 2026
Merged

feat(runtime-host): bind OAuth login to Connection entities#3924
me2seeks merged 4 commits into
apache:mainfrom
me2seeks:feat/3852-oauth-connection-entity

Conversation

@me2seeks

@me2seeks me2seeks commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • bind every interactive OAuth login to one explicit Connection target: create(providerType) or existing(connectionId)
  • allocate and recover OAuth Connection identity, credential publication, and authenticated receipts in Runtime Host/Storage
  • migrate Desktop add/re-login/status/refresh/logout paths to exact Connection identity and fail closed on mismatch or ambiguity
  • replace silent attempt supersede with operation_conflict and advance the Runtime Host compatibility epoch from 53 to 54

Stack

This is the OAuth entity-binding workstream of #3852 and is stacked after #3882. Review the final commit c88dd63b1 for this PR's isolated change.

Closes #3888.

Validation

  • npm --workspace @maka/core run typecheck
  • npm --workspace @maka/storage run typecheck
  • npm --workspace @maka/runtime-host run typecheck
  • npx tsc -p apps/desktop/tsconfig.main.json --noEmit
  • Storage full suite: 995 tests, 977 passed, 18 skipped
  • Runtime Host OAuth/protocol focused suite: 73 passed
  • Desktop OAuth/Connections focused suite: 17 passed
  • node scripts/protocol-epoch-check.mjs --base f1fa3f9c7 --head HEAD (53 -> 54)
  • git diff --check f1fa3f9c7..HEAD

@me2seeks

Copy link
Copy Markdown
Contributor Author

Addressed the current hosted format:check failure on head 04dd9c1b7 by applying Biome formatting to the two reported Storage files. The follow-up commit changes only line wrapping; two independent reviews confirmed no identity, authority, argument, or control-flow change.

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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 27, 2026
@jackwener
jackwener requested a review from Astro-Han August 28, 2026 04:15
@me2seeks
me2seeks force-pushed the feat/3852-oauth-connection-entity branch from 04dd9c1 to 01a325e Compare August 29, 2026 16:25

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 负责。

@me2seeks
me2seeks force-pushed the feat/3852-oauth-connection-entity branch 2 times, most recently from 23cae6d to 4fbdfe5 Compare August 30, 2026 14:13

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread apps/desktop/src/main/runtime-host-oauth-ipc-main.ts

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread apps/desktop/src/main/__tests__/runtime-host-oauth-ipc-main.test.ts Outdated
Comment thread packages/storage/src/__tests__/runtime-policy-stores.test.ts
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
@me2seeks
me2seeks force-pushed the feat/3852-oauth-connection-entity branch from 4c6698b to 2df8f7d Compare August 31, 2026 02:27

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@me2seeks
me2seeks merged commit 28bdbc6 into apache:main Aug 31, 2026
2 checks passed
saltand pushed a commit to saltand/maka-agent that referenced this pull request Aug 31, 2026
)

* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OAuth] Bind interactive login to explicit Connection entities

3 participants