Skip to content

fix(desktop): avoid Owner-only IPC requests from shared Session guests - #4858

Merged
M4n5ter merged 2 commits into
mainfrom
fix/guest-ipc-boundaries
Sep 5, 2026
Merged

fix(desktop): avoid Owner-only IPC requests from shared Session guests#4858
M4n5ter merged 2 commits into
mainfrom
fix/guest-ipc-boundaries

Conversation

@M4n5ter

@M4n5ter M4n5ter commented Sep 5, 2026

Copy link
Copy Markdown
Member
English

Fix

Shared Session guests were included in Owner-only onboarding and workspace-search requests. Because those handlers are never registered for Guests, each request waited through the 15-second reconnection window, producing repeated errors and delaying aggregate results. The transcript also requested Owner-only turn landmarks for shared Sessions.

  • Query only ready Owner hosts for onboarding aggregation and workspace search. Keep shared Sessions in the separate mount catalog.
  • Request full-history turn landmarks only for Owner Sessions; preserve Guest transcript paging and navigation over loaded content.
  • Seal each candidate's registration history separately from live handlers, so a connection closing before startup returns cannot erase its supported IPC surface. Reject unsupported channels immediately while retaining bounded recovery for supported channels during disconnects. Preserve uncertain-control reconciliation without replaying mutations.
  • Leave Guest grants and the wire protocol unchanged.

Validation

  • Desktop suite: 2,188 tests passed.
  • Main/preload builds and preload/renderer TypeScript checks passed.
  • Changed-file lint, app-shell hook checks, and ASF header checks passed.
  • Regression coverage exercises the production preload bundle with concurrent Owner/Guest identities, Guest catalog retention, unsupported IPC, Owner reconnect (including EOF after the final initialization response), unavailable reconciliation, and late Owner landmark responses after switching to Guest.
  • Windows-to-macOS live-device retest remains pending; automated validation ran under Linux/WSL.

AI assistance: OpenAI Codex.

中文

修复

共享会话 Guest 被错误纳入了仅限 Owner 的 onboarding 和工作区搜索请求。这些接口从未在 Guest 上注册,每次调用却会等待 15 秒重连窗口,造成反复报错及聚合结果延迟。历史导航也会对共享会话请求仅限 Owner 的完整导航标记。

  • onboarding 聚合和工作区搜索仅请求已就绪的 Owner;共享会话仍通过独立挂载目录保留在列表中。
  • 仅对 Owner 会话请求完整历史导航标记,保留 Guest 的历史分页和已加载内容导航。
  • 将每个 candidate 的注册记录与活跃 handler 分离并封存,避免初始化返回前断线清理错误地抹掉接口支持信息;不支持的调用立即拒绝,支持的接口仍按原有有界窗口恢复,不重放结果未确认的写操作。
  • 不扩大 Guest 权限,不修改通信协议。

验证

  • Desktop 全量 2,188 项测试通过。
  • main/preload 构建及 preload/renderer TypeScript 检查通过。
  • 改动文件 lint、app-shell hook 和 ASF 许可证头检查通过。
  • 回归测试覆盖真实 preload bundle 中 Owner/Guest 并存、共享目录保留、不支持的 IPC、Owner 重连(包括初始化最后一步响应后立即断线)、无法继续核对的写操作,以及切换 Guest 后迟到的 Owner 导航响应。
  • 尚未完成 Windows→macOS 真机复测;自动验证在 Linux/WSL 下执行。

AI 辅助:OpenAI Codex。

Avoid Owner-only onboarding, workspace search, and turn-landmark requests for shared Sessions. Distinguish unsupported IPC from handlers temporarily missing during reconnect.

Generated-by: OpenAI Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 5, 2026

@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.

Reviewed current head d8303a934b3fab0b1fbc6277b5a3f99bc5f5d293 (OPEN, MERGEABLE/BLOCKED awaiting human review). One P2 plus two P3s below, no P0/P1. The fix direction is right and tightly scoped; the P2 says it blocks guests on the fan-out path but misses the one fatal main path. All checks green on this head.

P2 — the fan-out path filters guests, but the onboarding main path does not, and main-path failure is fatal

loadDesktopOnboardingSnapshot (preload.ts:818) now filters to owner scopes via readyOwnerRuntimeHostScopes(), but defaultScope is placed first unconditionally with no owner filter — and results[0] failing throws the whole onboarding snapshot instead of degrading. Tracing defaultScope: activeRuntimeHostRef()runtimeHostManager.current()#current(this.#defaultProfileId), and setDefaultProfile is a bare assignment with no guest validation at the setDefault entry (runtime-host-boot.ts:603). The same file already has the ready-made predicate isSessionGuestProfile (runtime-host-desktop-manager.ts:1496), used in 4 other places to isolate guest behavior — just not on "can a guest become the default host". If a guest can be the default host, the 15 s wait this PR kills survives on the most important path — and worse, with the new "reject unsupported channels immediately", it changes from "slow, then partial results" to "throw immediately, no onboarding snapshot at all": faster, but degraded became hard failure. Fix: filter defaultScope by profileAccess === 'owner' (fall back to the local owner host for guests), or guard the setDefault entry with the existing predicate — the latter is one line and more radical. Graded P2 not P1 because I could not prove the UI actually offers "mount a shared session as the default host" (guests render through a separate mount directory and may never appear in the host picker) — but nothing in code prevents it, and this PR is precisely about tightening the guest/owner boundary. Someone who knows the desktop interaction should answer that.

P3 — completeRegistration snapshots once; the "registration is complete" premise lives only in a comment

The new method (runtime-host-reconnecting-ipc-main.ts:147) freezes the epoch's registered channel set; any channel registered later on that epoch is permanently judged unsupported. The ordering currently holds (target created as an argument at :1083, completeRegistration called after ready at :1103) — but it is convention only. If someone later makes a channel lazily registered, it will be permanently rejected with RuntimeHostHandlerUnsupportedError, which reads like "peer doesn't support it" and will mislead debugging toward registration timing. Suggest making the invariant code: throw if a new channel is #handled on an epoch after completeRegistration.

P3 — the optimization does not apply during the reconnect window, which is exactly when it matters

deactivate(epoch) deletes the snapshot and completeRegistration only runs on the manager's ready branch, so between activate and the next ready, supported is undefined and the new fast-reject is skipped — falling back to old timeout behavior. Safe degradation (don't reject what you don't know), and the right call — but the PR description's "immediately reject unsupported channels" does not hold during reconnect, precisely when the 15 s wait is most likely to trigger. Worth one line in the description.

Checked and found sound

ownerActiveId change is correct and consistent with the identity guard (value can only be activeId or undefined; guest passes undefined, and refreshTranscriptTurnLandmarks actively clears the landmark index rather than keeping a stale one — more important than saving one request). The owner-filtered scope list cannot silently narrow on cold start (recordRuntimeHostIdentity writes metadata per identity and prunes non-authoritative keys before returning). The reconciling early-return preserves existing semantics for the new error type. The new error class is separate from RuntimeHostHandlerUnavailableError, so callers can handle the two cases distinctly.

What I could not judge

Whether the UI path to set a guest mount as default host exists was not verified; that answer belongs to someone who knows the desktop interaction.


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.

简体中文

本条结论全部来自 @Opus-Qronos-AstroHan 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移、以及 exact-head 的门禁状态。当前 head 是 d8303a9,未关闭。方向对范围克制,一条 P2:扇出挡了访客但主路径没挡,失败还是致命的,另有两条小的时序与范围提醒。修好再合。

@jackwener jackwener 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.

Approved at exact head d8303a934b3fab0b1fbc6277b5a3f99bc5f5d293 at the explicit direction of M4n5ter. No technical review was performed as part of this action; M4n5ter requested the approval and accepts responsibility for subsequent handling.


Review notice: This approval was submitted by an automated review agent operated by jackwener and is published at the direction of M4n5ter, who requested this action and is the human accountable for it.

…rdown

Seal the candidate-owned registration set independently of live handlers so EOF during initialization cannot turn supported Owner requests into unsupported calls. Cover startup-close recovery with real candidates.

Generated-by: OpenAI Codex
@M4n5ter

M4n5ter commented Sep 5, 2026

Copy link
Copy Markdown
Member Author
English

Addressed in 08cf52906. Disposition of the review on d8303a934:

Confirmed and fixed: EOF immediately after the final initialization response could tear down live handlers before the manager took its support snapshot. The base recovered the pending Owner read; the previous PR head incorrectly returned RuntimeHostHandlerUnsupportedError. Support now comes from the candidate-owned registration history, independently of handler cleanup. A regression using real candidates verifies recovery through a replacement.

Validation: 2,188 Desktop tests passed, including the new startup-close regression; main build, preload/renderer type checks, changed-file lint, and ASF header checks passed. Live Windows-to-macOS retest remains pending.

AI assistance: OpenAI Codex.

中文

已更新至 08cf52906

  • Guest 默认 Host:不采纳。Guest 被 profile 接入规则拒绝、只保留为独立会话挂载;启动时清理旧 Guest profile 并归一化默认项,切换默认项也要求已启用的 profile。主 onboarding 失败在原代码中就会抛错,并非本 PR 新增的行为。
  • 晚注册永久拒绝:原结论不成立,存在的 handler 优先于支持表检查。此次根因修复同时将 candidate 注册记录封存,明确拒绝完成后的追加注册;替代 candidate 仍可正常注册。
  • 正常重连清空支持表:不成立。重连保留同一活跃 epoch,不调用 deactivate。
  • 确认并修复了初始化末尾 EOF 的竞态:接口支持信息改为来自 candidate 自身的注册记录,不再从可能已经被清理的活跃 handler 反推。真实 candidate 回归测试验证请求可以等到替代连接恢复。

Desktop 全量 2,188 项测试及上述静态检查通过;Windows→macOS 真机复测仍待完成。

@M4n5ter
M4n5ter merged commit efa3ea7 into main Sep 5, 2026
1 check passed
@M4n5ter
M4n5ter deleted the fix/guest-ipc-boundaries branch September 5, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants