fix(desktop): avoid Owner-only IPC requests from shared Session guests - #4858
Conversation
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
Astro-Han
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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
EnglishAddressed in
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 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. 中文已更新至
Desktop 全量 2,188 项测试及上述静态检查通过;Windows→macOS 真机复测仍待完成。 |
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.
Validation
AI assistance: OpenAI Codex.
中文
修复
共享会话 Guest 被错误纳入了仅限 Owner 的 onboarding 和工作区搜索请求。这些接口从未在 Guest 上注册,每次调用却会等待 15 秒重连窗口,造成反复报错及聚合结果延迟。历史导航也会对共享会话请求仅限 Owner 的完整导航标记。
验证
AI 辅助:OpenAI Codex。