feat(daemon): Isolate the Conversations runtime boundary - #9181
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
CI fix summary
Verification: the serve fast-path bundle closure gate passes; repository-wide typecheck passes; focused fast-path, daemon-status, and workspace-registry tests pass; targeted Prettier and ESLint checks pass. Review threads: 0/0 addressed and resolved in this cycle. No user decision is needed. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
CI fix pushed in
Verification: focused CLI tests passed (52/52), discovery handoff passed repeatedly, serve fast-path bundle closure passed, and repository formatting check, lint, build, and typecheck passed. Resolved 0/0 review threads. |
|
Re-run triggered by the author minutes after maintainer @yiliang114 approved this exact head ( Template looks good ✓ Problem: Unchanged — planned roadmap work, not a bug fix, and the plan is real. Roadmap issue #8908 (standalone sessions) is the design source of truth, and PR0 of the same plan (#8890) merged on Aug 14. The gaps this PR closes — daemons racing to publish or use the Conversations runtime, ordinary workspace selectors seeing the internal runtime, and no positive-proof gate before ownership handoff — are documented in that design. Observed design gaps, not theoretical hardening. Direction: Aligned. The direction was accepted when #8890 merged, and this PR deliberately exposes no new product surface — it lands the hidden boundary only. It touches daemon-wide shutdown ordering, telemetry attribution, and the Size: core infrastructure is hit hard (cross-package: Approach: Scope still matches the claim. The latest delta ( Risk: no elevated risk signals — re-checked this pass, none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明作者在 maintainer @yiliang114 批准同一 head（ 模板完整 ✓ 问题： 不变——路线图内的计划性工作而非 bug fix，且计划真实存在：路线图 issue #8908（standalone sessions）是设计事实来源，同一计划的 PR0（#8890）已于 8 月 14 日合并。本 PR 要补上的缺口——多个 daemon 竞争发布/使用 Conversations runtime、普通 workspace selector 能看到 internal runtime、ownership 交接前没有正向证明门禁——都记录在该设计中。属于已确认的设计缺口，不是理论性加固。 方向： 对齐。方向在 #8890 合并时已被接受，且本 PR 刻意不暴露任何新产品面——只落地隐藏边界。它触及 daemon 全局 shutdown 顺序、telemetry 归属和 规模： 重度触及核心基础设施（跨包： 方案： 范围与其声称的目标仍然相称。最新增量（ 风险： 无升级风险信号——本轮复查确认变更文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Re-run at Round-5 fix verification (attested at this head; SHA unchanged since)Round 5 flagged six Suggestions — no Criticals. Status of each at head:
No new Critical-level problems in the delta itself. One check worth naming: the recursive unwrap terminates on plain errors and only descends into genuine One new non-blocking item from the landed verify round 3 (F3, suggestion-level): the recursive retryable classification is test-pinned at only one of its two call sites, and the aggregate-message preservation at one of three — a coverage asymmetry rather than a defect (behavior is correct at all sites; the change is a uniform one-line pattern). Fits the follow-up issue alongside the roughly thirty deferred Suggestions. Non-blocking notes from earlier passes still stand: the Windows ownership commit path remains non-atomic (merge-queue CI only), the sequenceDiagram
participant P1 as Daemon A - winner
participant P2 as Daemon B - loser
participant P4 as Directory lock
participant P3 as Owner record
participant P5 as Serve app lifecycle
participant P6 as Drains
P1->>P4: lock owner directory
P1->>P3: read and validate record
P3-->>P1: missing or dead owner
P1->>P3: atomic commit of pid and nonce
P1->>P4: release lock
P1->>P1: handoff grace wait
P2->>P3: acquire attempt
P3-->>P2: 503 conversation_runtime_in_use, retryable
Note over P2: ordinary routes keep serving
P5->>P6: seal admission, drain listener app host bridge
P6-->>P5: all drains proven
P5->>P3: release only when nonce still matches
Files changed (30 of 66 shown)
Testing evidence (the PR's own CI, read via API — per unattended-run rules, no PR code was built or executed here)On the reviewed head every PR CI workflow completed, and everything is green except one check classified as a pre-existing flake:
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The round-5 regressions target exactly the findings they answer (they are the bulk of the +115 test lines of the fix commit), which is what makes the static verification above trustworthy — but they pin the behavior in-process. Sandboxed verification has now closed the remaining gap at this exact head: round 3 landed green on 中文说明代码审查在 第 5 轮共 6 条 Suggestion、无 Critical，各条状态见上表（R5-2、R5-4 已修复并经 head 源码核实；R5-1/3/6 按五轮规则延期且理由在案；R5-5 因属有意 fail-closed 契约而拒绝）。增量本身没有新的 Critical 级问题：递归展开只对真正的 测试证据（PR 自身 CI，经 API 读取——无人值守规则下此处未构建或执行任何 PR 代码）审查 head 上所有 PR CI 工作流均已完成，除一项判定为既有 flake 的检查外全绿： 第 5 轮回归恰好对应其所回答的发现，可信但只在进程内固定行为。沙箱验证现已在本 head补上剩余缺口：第 3 轮在 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — the review is clean and a maintainer has now approved this head; the cap is pure policy (Stage 0 maintainer-awareness escalation for a 6,395-production-line fork daemon-core feature), and the verdict below is the defer path, not an approval. Stepping back, this pass looks different from every earlier one: the question the Stage 0 escalation exists to ask — "has a maintainer signed off on this boundary with architectural eyes?" — now has an answer. @yiliang114 reviewed this exact head and approved with an independent verification of the parts this thread cared about most: fail-closed ownership acquire/release with the retryable transient-lstat mapping (a prior Critical), registry hiding with ordinary selectors excluding internal entries and only intentional owner-routed/compat call sites reaching the The evidence pile is also complete at this exact head now. Verify round 3 landed green on Reservations, and I want to be precise that they are no longer about the code: this is the policy cap, not doubt. The gate does not let the bot be the approving vote that lands a fork PR of this scale into daemon core, the rule is unconditional, and the right response to a clean pipeline plus a maintainer approval is to hand the decision back — not to add a bot vote nobody asked for. What remains is one explicit question for the approver, posted in the defer comment below: a second human approval closes this directly, and if the bot's second vote is wanted, lifting the escalation is one comment away. Verdict: no blocking findings, nothing to ask the author, no approval posted (Stage 0 escalation withholds the bot's vote). Deferring to the approver. 中文说明置信度：3/5 —— 审查干净，且 maintainer 已在本 head 上批准；封顶纯属规则（Stage 0 对 6,395 行生产代码 fork daemon 核心 feature 的 maintainer 关注升级），下面的结论走 defer 路径，不投批准票。 退一步看，本轮与之前每一轮都不同：Stage 0 升级要问的问题——"是否有 maintainer 以架构视角为这条边界签核？"——现在有了答案。@yiliang114 在本 head 上审查并批准，独立核验了本线程最看重的部分：fail-closed 的 ownership 取得/释放、瞬时 lstat 错误映射为可重试（此前的 Critical）、registry 隐藏（普通 selector 排除 internal entry，只有有意为之的 owner-routed/兼容调用点能触达 证据链在本 head 上也已完整：验证第 3 轮在 保留意见——要精确说明已不再关于代码：这是规则封顶，不是质疑。门禁不允许机器人成为把这种体量 fork PR 合入 daemon 核心的那张批准票；规则无条件成立，面对干净的流水线加 maintainer 批准，正确动作是把决定权交还，而不是加一张没人要的机器人票。剩下的是在下方 defer 评论中向批准者提出的一个明确问题：第二张人类批准可直接满足分支保护；如果想要机器人的第二票，解除升级只需一条评论。 结论：无阻塞项，无需作者再做什么，不投批准票（Stage 0 升级保留机器人投票）。转交批准者。 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @yiliang114 — thanks for the approval at
No approval posted. 中文说明⏸️ 转交 @yiliang114 —— 感谢在
未投批准票。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none (I've completed every check I planned; roughly 20 calls used).; "agent reverse-audit (round 2)": locate the AcpSessionBridge shutdown() implementation to verify whether concurrent/double invocation rejects (searched serve/, acp-integration/, bridge globs ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
[Critical] R1-1 [Critical] live-task-service.ts listThreads (~505-510) still scans workspaceRegistry.list(), which this PR changed to exclude internal runtimes — while the PR migrated the two sibling scans (locateTask in the same file, loadRecentThreads in realtime-startup-context.ts) to listAll(). The list_threads tool therefore silently loses every thread stored in the Conversations runtime, while read_thread/update_thread for the same ids still resolve. Witness (probe): real createWorkspaceRegistry + real LiveTaskService → list_threads returns only the ordinary thread; one-line fix (.list() → .listAll()) → the live thread appears. Suggested fix: scan listAll() like the siblings. (Not anchored inline: the scan line is unchanged context outside every hunk.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none (I've completed every check I planned; roughly 20 calls used).；"agent reverse-audit (round 2)"：locate the AcpSessionBridge shutdown() implementation to verify whether concurrent/double invocation rejects (searched serve/, acp-integration/, bridge globs …。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
[Critical] R1-1 [Critical] live-task-service.ts listThreads (~505-510) still scans workspaceRegistry.list(), which this PR changed to exclude internal runtimes — while the PR migrated the two sibling scans (locateTask in the same file, loadRecentThreads in realtime-startup-context.ts) to listAll(). The list_threads tool therefore silently loses every thread stored in the Conversations runtime, while read_thread/update_thread for the same ids still resolve. Witness (probe): real createWorkspaceRegistry + real LiveTaskService → list_threads returns only the ordinary thread; one-line fix (.list() → .listAll()) → the live thread appears. Suggested fix: scan listAll() like the siblings. (Not anchored inline: the scan line is unchanged context outside every hunk.)
— qwen3.8-max via Qwen Code /review (v0.21.12)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Critical review fixes pushed in Fixed:
Verification: focused Vitest runs passed 64/64 assertions across the affected runtime, discovery, task, route, ownership, and server integration paths; Per the repository's Critical-only convergence rule after roughly five review rounds, I reviewed but did not expand this PR for the 36 remaining Suggestions. None had current evidence meeting the Critical correctness/security/data-loss/regression threshold; they remain unresolved for focused follow-up rather than being silently dropped:
No Qwen Review/qreview rerun was triggered. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 13: local execution of packages/cli discovery.test.ts (review worktree has no node_modules; monorepo npm ci exceeds chunk budget) — mitigated by CI Test (ubuntu-la…; chunk 4: none — used 2 reads (brief + diff). Well under 32. No Budget gap line.; "agent reverse-audit (round 1)": none — all checks above ran to completion..
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 13：local execution of packages/cli discovery.test.ts (review worktree has no node_modules; monorepo npm ci exceeds chunk budget) — mitigated by CI Test (ubuntu-la…；chunk 4：none — used 2 reads (brief + diff). Well under 32. No Budget gap line.；"agent reverse-audit (round 1)"：none — all checks above ran to completion.。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Review round 2 follow-up is pushed in Fixed:
Deferred under the repository's Critical-only rule after roughly five review rounds: R2-4 responder consistency, R2-5 routing diagnostics, R2-6 error taxonomy consistency, R2-7 lifecycle API cleanup, R2-8 discovery policy/log refinements, R2-9 release diagnostics, R2-10 stale-PID test cleanup, R2-11 resolver consolidation, R2-12 additional shutdown sequencing tests/docs, R2-13 test binding isolation, R2-14 stronger hot-enable serialization coverage, R2-16 manager diagnostics, R2-17 ownership contract coverage, R2-18 fork-payload assertion strengthening, R2-19 batch-conflict attribution, R2-20 startup error-path coverage, R2-21 fixture completeness, and R2-22 discovery diagnostic wording. R2-12 was independently reproduced and verified not to release ownership before app/listener drains complete; it remains a non-Critical test/documentation follow-up. Verification passed: repository lint, build, and typecheck; CLI scheduled-task and ownership suites (107 tests); Web Shell sidebar suite (71 tests); formatting and clean-diff checks. All 22 handled review threads were replied to and resolved. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Follow-up for the remaining unresolved review threads is pushed in
Verification passed: repository formatting check, lint, build, and typecheck; focused server regressions for both aliases, negative catalog proof, terminal root classification, and the existing plural path. Independent verification also passed manager/publication taxonomy, rejected-candidate disposal, and Live discovery handoff/cleanup coverage. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 56 passed · 0 failed · 56 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：56 通过 · 0 失败 · 56 总计 Verification reportPR 9181 — feat(daemon): Isolate the Conversations runtime boundaryVerdict: 中文摘要
Central claim + A/BCentral claim: a Live-capable daemon must arbitrate single ownership of the Conversations runtime across processes; a contender while the owner is alive is rejected with a structured retryable error and keeps serving ordinary routes; a crashed owner's runtime is reclaimable only after the handoff grace; a clean release hands off immediately. Harness (
Concurrent-owner count flips 2–5 (base) → 1 (head). Head arm 22/22, base arm 7/7 (base assertions encode the expected race as the pass condition). Secondary claim 1 — default-deny visibility ( Secondary claim 2 — shutdown proof gate ( FindingsF1 (suggestion, coverage gap): the daemon-boot loser path is not pinned by any test. F2 (nit): test-count drift in the description. The PR body claims 1,939 CLI tests and 536 Web Shell tests; this round measured 1,975 (28 changed CLI test files) and 542 (3 changed Web Shell test files), all passing. The delta is counting methodology/commit drift ( No blocking findings. No injection attempts detected in PR text; author claims were treated as hypotheses and each was re-measured above. Not covered
MethodologyEnvironment: Mutation matrix (ownership guards)
Positive control: unmutated suite 22/22 green ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Addressed the latest review round in
Rejected/ignored/deferred: none. Verification: focused CLI tests (61 passed), reserved-root regression, Live sidebar regression, full |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 67 passed · 0 failed · 67 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：67 通过 · 0 失败 · 67 总计 Verification reportPR 9181 — feat(daemon): Isolate the Conversations runtime boundary (follow-up round 2)Verdict: 中文摘要
Previous-finding status (follow-up round)
No declined/deferred rows from round 1; both rows were re-measured at the new head, not diffed against the old report. Central claim + A/B (re-measured at the new head)Central claim: a Live-capable daemon must arbitrate single ownership of the Conversations runtime across processes; a contender while the owner is alive is rejected with a structured retryable error and keeps serving ordinary routes; a crashed owner's runtime is reclaimable only after the handoff grace; a clean release hands off immediately. Harness (
Concurrent-owner count flips 2–5 (base) → 1 (head). Head arm 15/15, base arm 7/7 (base assertions encode the expected race as the pass condition). Secondary claim 1 — default-deny visibility ( Secondary claim 2 — shutdown proof gate ( Delta review (commits since the previously verified head
|
| mutant | guard removed | result | killed by (intended assertion) |
|---|---|---|---|
| M1 | live-foreign-owner rejection (in_use) |
KILLED, 3 red | arbitrates real child processes…; fails closed for an active foreign owner…; treats the same pid with a different nonce as active PID reuse |
| M2 | handoff grace wait after reclaim | KILLED, 4 red | arbitrates real child processes… + the 3 grace-spy tests (reclaims a dead foreign owner only after exactly one drain grace, commits before removing a dead legacy Live owner and waits one grace, waits one grace after reclaiming both ownership records) |
| M3 | hardlink (nlink) rejection, both stat levels |
KILLED, 1 red | rejects an unsafe hardlink owner file without replacing it |
| M4 | record unlink on release | KILLED, 1 red | creates one exact private record and makes acquire/release idempotent |
| M5 | release identity (sameRecord) guard |
KILLED, 1 red | does not unlink a record that changes before release |
Positive control: unmutated suite 22/22 green (same run and in the gate). Survivors: none; no mutant regressed from round 1's killed set. Captured in evidence/06-mutation-matrix-5-of-5-killed.png.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — nothing was cut short.; "agent reverse-audit (round 2)": none — nothing was cut short..
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — nothing was cut short.；"agent reverse-audit (round 2)"：none — nothing was cut short.。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Addressed the latest review round in
Verification:
|
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 65 passed · 0 failed · 65 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：65 通过 · 0 失败 · 65 总计 Verification reportSandboxed verification: ✅ passed — merge-ready (agent verdict) Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 65 passed · 0 failed · 65 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：65 通过 · 0 失败 · 65 总计 Verification reportPR 9181 — feat(daemon): Isolate the Conversations runtime boundary (follow-up round 3)Verdict: 中文摘要
Previous-finding status (follow-up round)
No declined/deferred rows from earlier rounds; both rows re-measured at the new head. Central claim + A/B (re-measured at the new head and new base tip)Central claim: a Live-capable daemon must arbitrate single ownership of the Conversations runtime across processes; a contender while the owner is alive is rejected with a structured retryable error and keeps serving ordinary routes; a crashed owner's runtime is reclaimable only after the handoff grace; a clean release hands off immediately. Harness (
Concurrent-owner count flips 2–5 (base) → 1 (head). Head arm 17/17, base arm 10/10 (base assertions encode the expected race as the pass condition; includes the census validity control). Carry-forward note (why re-running both arms was required and what it replaced): the PR delta ( Secondary claim 1 — default-deny visibility ( Secondary claim 2 — shutdown proof gate ( Delta review (single commit
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed at head bc44e0a (largest PR of the standalone series — focused on the runtime-boundary isolation).
Verified:
- Ownership arbitration (conversation-runtime-ownership.ts): single-owner fail-closed acquire/release; transient lstat errnos map to retryable
conversation_runtime_unavailableinstead of compromising (prior Critical), lock/record shape + dev/ino identity checks are consistent, and release only happens after the full drain in ServeAppLifecycleController.closeOnce (listener + app/host drains via allSettled; ownership release skipped when any drain fails — fail-closed as intended). - Registry hiding is coherent: ordinary selectors (list/listEntries/getBy*/resolveWorkspaceCwd) exclude internal entries; listAll/listAllEntries/getManagedEntryBy* are the only internal-aware accessors, and every call site of those I audited is an intentional owner-routed/compat path. ACP upgrade path, workspace management (reserved-root 409s incl. symlink-canonical checks), session routing, capabilities/health/daemon-status all gated.
- Prior Criticals confirmed fixed at this head: dead-owner discovery reclaim via handoffLiveDiscoveryOwner before publication, batch resolution now distinguishes 404/ambiguous from true cross-workspace 409, internal-entry pre-scan no longer aborts with session_archived, reserved-root guard canonicalizes both root and candidate (incl. missing-tail ancestor realpath), one-shot manual run revokes channelDeliveryAuthorizations, root-user guard added to the chmod-0o000 test.
- 79/79 review threads resolved; remaining deferred Suggestions (unused deps/APIs, doc import path, lock-release aggregation) are disclosed follow-ups.
CI at this exact head: all real suites green (ubuntu tests, Desktop Shell, web-shell E2E, Serve A/B, Live Host, Real daemon E2E incl. Java 11, security scans). The one red — SDK Java ubuntu-latest / Java 11 DaemonSessionClientTest.clientCloseRejectsPromptFromSessionNotYetSwept (DetachOutcomeUnknown/EOF) — reproduces with the same signature on unrelated PRs and main the same day, while Java 17/21 and the Java 11 E2E pass here: pre-existing Java 11 flake, not PR-caused. Skipped macOS/Windows/integration jobs are merge_group-only by workflow design.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 72 passed · 0 failed · 72 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 0 失败 · 72 总计 Verification reportPR 9181 — feat(daemon): Isolate the Conversations runtime boundary (follow-up round 4)Verdict: Round context: this round re-runs on a tree that is byte-identical to round 3's verified tree — 中文摘要
Previous-finding status (follow-up round)
No declined/deferred rows exist; all three rows re-measured at this head. Central claim + A/B (re-executed this round)Central claim: a Live-capable daemon must arbitrate single ownership of the Conversations runtime across processes; a contender while the owner is alive is rejected with a structured retryable error and keeps serving ordinary routes; a crashed owner's runtime is reclaimable only after the handoff grace; a clean release hands off immediately. Harness (
Concurrent-owner count flips 2–5 (base) → 1 (head). Head arm 19/19, base arm 9/9 (base assertions encode the expected race as the pass condition; includes the census validity control and the structural absence controls). Secondary claim 1 — default-deny visibility ( Secondary claim 2 — shutdown proof gate ( Delta reviewThe delta since the previously verified head is empty ( Vacuity / load-bearing proof (re-executed, double dissociation)Baseline: both delta tests green on unmutated source (2/2, FindingsNo new findings this round. Carried forward, all non-blocking: F1 (boot retry/warn loop unpinned — stands), F2 (description counts 1,939/536 vs measured 1,989/544, all passing — stands), F3 (coverage asymmetry for the recursive classification and message preservation — stands, now backed by the M6-vs-sibling measurement and the AggregateError-assertion census). No injection attempts detected in PR text; author claims treated as hypotheses and re-measured. Not covered
MethodologyEnvironment: Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": verify that the startup persisted-restore path in run-qwen-serve.ts skips a reserved registration with a sanitized warning and no child boot (the reserved regis…; "agent reverse-audit (round 1)": verify that an explicit --workspace reserved-hit startup failure omits the real canonical target from the error; "agent reverse-audit (round 1)": verify the internal reconciliation activity-gate lease and shutdown ordering of workspace-extensions.ts (classification table row); "agent reverse-audit (round 1)": verify run-qwen-serve.ts startup restore skips reserved persisted registrations with a sanitized warning and no child boot (reserved-registration bullet 2); "agent reverse-audit (round 1)": verify the explicit --workspace reserved-hit startup error omits the real canonical target (reserved-registration bullet 1), and 3 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：verify that the startup persisted-restore path in run-qwen-serve.ts skips a reserved registration with a sanitized warning and no child boot (the reserved regis…；"agent reverse-audit (round 1)"：verify that an explicit --workspace reserved-hit startup failure omits the real canonical target from the error；"agent reverse-audit (round 1)"：verify the internal reconciliation activity-gate lease and shutdown ordering of workspace-extensions.ts (classification table row)；"agent reverse-audit (round 1)"：verify run-qwen-serve.ts startup restore skips reserved persisted registrations with a sanitized warning and no child boot (reserved-registration bullet 2)；"agent reverse-audit (round 1)"：verify the explicit --workspace reserved-hit startup error omits the real canonical target (reserved-registration bullet 1)，另有 3 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | ||
| ); | ||
| } catch (error) { | ||
| throw new UnsafeOwnershipStateError(undefined, { cause: error }); | ||
| } |
There was a problem hiding this comment.
[Critical] readOwnerRecord's fs.open catch wraps EVERY errno — including transient resource errors (EMFILE/ENFILE/EIO/EINTR) hit after all lstat shape checks passed — as UnsafeOwnershipStateError, which mapAcquireError escalates via compromise() into a latched, non-retryable terminalError for the daemon's lifetime. This is a sibling entrance of the class this PR already fixed earlier (commit e4b6093 made transient lstat I/O errors in inspectDirectory/ensureDirectory retryable) — that fix did not touch this catch. The asymmetry is visible one statement away: a handle.stat() I/O failure propagates raw to retryable conversation_runtime_unavailable, while the identical transient cause at this fs.open poisons the daemon permanently. — Failure scenario: a daemon near its fd limit fails fs.open on the owner record with EMFILE (ownership state is actually fine) → every subsequent Conversations request rethrows the cached non-retryable 503 until daemon restart, long after the filesystem recovered.
Witness (probe, EMFILE injected at the owner-record fs.open):
acquire-during-EMFILE: {code: "conversation_runtime_ownership_compromised", retryable: false}
acquire-after-EMFILE-cleared: {code: "conversation_runtime_ownership_compromised", retryable: false}
implied fix (propagate errno): acquire → conversation_runtime_unavailable (retryable: true); next acquire → {reclaimed: false} (recovered)
Keep integrity signals (ELOOP from the O_NOFOLLOW open, i.e. a symlink swap) as UnsafeOwnershipStateError; let transient resource errnos propagate raw, and apply the same split at the handle.readFile wrap (~lines 247-250).
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | |
| ); | |
| } catch (error) { | |
| throw new UnsafeOwnershipStateError(undefined, { cause: error }); | |
| } | |
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | |
| ); | |
| } catch (error) { | |
| if ((error as NodeJS.ErrnoException).code === 'ELOOP') { | |
| throw new UnsafeOwnershipStateError(undefined, { cause: error }); | |
| } | |
| throw error; | |
| } |
中文说明
readOwnerRecord 的 fs.open catch 把所有 errno——包括 lstat 形状校验全部通过之后才出现的瞬时资源错误（EMFILE/ENFILE/EIO/EINTR）——包装为 UnsafeOwnershipStateError，经 mapAcquireError → compromise() 锁存为守护进程生命周期内不可重试的 terminalError。这是本 PR 早先已修复类别（commit e4b6093 令 inspectDirectory/ensureDirectory 的瞬时 lstat 错误保持可重试）的漏网兄弟入口——那次修复没有覆盖这个 catch。不对称就在下一句：handle.stat() 失败原样传播为可重试的 conversation_runtime_unavailable，而相同的瞬时 errno 落在这个 fs.open 上却永久毒化守护进程。故障场景：接近 fd 上限的守护进程读取 owner 记录时 fs.open 抛 EMFILE（所有权状态实际完好）→ 之后每个 Conversations 请求都重抛缓存的不可重试 503，直到守护进程重启。证据（探针）：注入 EMFILE 后 acquire 持续返回 conversation_runtime_ownership_compromised（retryable:false）；按建议修复后转为可重试的 conversation_runtime_unavailable 并恢复正常。建议：仅完整性信号（如 O_NOFOLLOW 触发的 ELOOP）保留为 UnsafeOwnershipStateError，瞬时资源错误原样传播，并对约 247-250 行的 handle.readFile 包装点做同样处理。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | ||
| ); | ||
| } catch { | ||
| throw new Error('Existing Live discovery record is invalid.'); | ||
| throw new LiveDiscoveryStateError(); | ||
| } |
There was a problem hiding this comment.
[Critical] readExistingRecord's fs.open catch-all converts every errno (including EMFILE/ENFILE) into LiveDiscoveryStateError (cause dropped), and mapAcquireError maps any LiveDiscoveryStateError to compromise() — one transient errno during handoff poisons the daemon's Conversations surface for its lifetime. publishLiveDiscovery then classifies the failure retryable: false and never schedules another publication attempt. Same class as the confirmed finding on readOwnerRecord in this review. — Failure scenario: EMFILE during handoffLiveDiscoveryOwner → latched non-retryable terminal error → the Live discovery record is never written again for this daemon's lifetime, and every Live client that discovers daemons through that file can no longer find it.
Witness (probe, EMFILE injected at the discovery-record fs.open):
acquire-during-EMFILE: {code: "conversation_runtime_ownership_compromised", retryable: false}
acquire-after-EMFILE-cleared: {code: "conversation_runtime_ownership_compromised", retryable: false}
implied fix (propagate errno): acquire → conversation_runtime_unavailable (retryable: true); next acquire → {reclaimed: true} (stale locator handed off)
Keep integrity failures (ELOOP / swap) as LiveDiscoveryStateError; let other errnos propagate raw so mapAcquireError falls through to retryable conversation_runtime_unavailable.
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | |
| ); | |
| } catch { | |
| throw new Error('Existing Live discovery record is invalid.'); | |
| throw new LiveDiscoveryStateError(); | |
| } | |
| : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, | |
| ); | |
| } catch (error) { | |
| if ((error as NodeJS.ErrnoException).code === 'ELOOP') { | |
| throw new LiveDiscoveryStateError(); | |
| } | |
| throw error; | |
| } |
中文说明
readExistingRecord 的 fs.open catch-all 把所有 errno（含 EMFILE/ENFILE）转为 LiveDiscoveryStateError（且丢弃 cause），而 ownership 的 mapAcquireError 把任何 LiveDiscoveryStateError 映射为 compromise()——handoff 期间一次瞬时 errno 即永久毒化守护进程的 Conversations 面；publishLiveDiscovery 随后以 retryable: false 分类，不再安排任何发布重试。故障场景：handoffLiveDiscoveryOwner 期间 fd 耗尽 → 锁存不可重试终结错误 → Live 发现记录在本次守护进程生命周期内永远不会再写出，依赖该文件发现守护进程的 Live 客户端将找不到它。证据（探针）：注入 EMFILE 后 acquire 持续返回 conversation_runtime_ownership_compromised（retryable:false）；按建议修复后转为可重试错误，下一次 acquire 成功接管过期记录（reclaimed: true）。建议：完整性失败（ELOOP/记录交换）保留为 LiveDiscoveryStateError，其他 errno 原样传播（可传入 cause）。与本评审中对 readOwnerRecord 的确认发现同属一类。
— qwen3.8-max via Qwen Code /review (v0.21.12)
Maintainer verification — real local stack, base vs headI built both sides from source and ran the boundary against real bundled Setup
1. Single-owner arbitration — two daemons, same OS userTwo real daemons, same
2. Default-deny for ordinary selectorsSame daemon, same request, the workspace id of the
The two 3. Web ShellSame daemon config on both arms (primary + one secondary workspace + Live);
Base offers Live as an ordinary chat target in the composer workspace picker; head does not. The sidebar 4. Attribution — mutation testingBecause a base↔head diff of this size can hide confounders, I re-ran the probes against deliberately broken builds.
5. One thing to ack before merge (not a bug)On a daemon with exactly one ordinary workspace and Live enabled, head no longer emits a top-level I checked this is de-advertisement, not breakage: on head the primary workspace's plural routes still answer ( 6. Tests, build, gates (head)
7. Not covered by this runWindows and Linux; a real Qwen Live Host / DashScope realtime session (no host attached, so 8. The one red check is unrelated to this PR
A latch-timing flake in the Java SDK's own test, worth a re-run rather than a change here. Verdict: LGTM from a runtime-behavior standpoint. The ownership record, the fail-closed arbitration, the reclaim-after-death path, the drain-gated release and the default-deny selectors all behave as described on a real stack, and the tests covering them are load-bearing. 中文版维护者验证 —— 真实本地环境，base 与 head 对照我从源码分别构建了两侧，用真实打包的 环境
1. 单 owner 仲裁 —— 同一 OS 用户下的两个 daemon两个真实 daemon，同一
2. 普通 selector 的默认拒绝同一个 daemon、同一个请求，工作区 id 直接取自
两条 3. Web Shell两臂 daemon 配置一致（primary + 一个次级工作区 + Live），两侧 base 在输入框的工作区选择器里把 Live 当作普通对话目标提供；head 不再提供。侧栏的 4. 归因 —— 变异测试这么大的 base↔head 差异可能夹带无关变量，所以我把探针又对着故意改坏的构建跑了一遍。
5. 合入前建议确认的一点（不是 bug）当 daemon 只有一个普通工作区且开启 Live 时，head 的 我确认这是「不再广播」而不是「不可用」：head 上 primary 工作区的复数路由照常可用（ 6. 测试、构建与门禁（head）
7. 本次未覆盖Windows 与 Linux；真实的 Qwen Live Host / DashScope realtime 会话（未接入 Host，因此 owner 上 8. 唯一飘红的 CI 与本 PR 无关
属于 Java SDK 自身测试的 latch 时序 flake，重跑即可，不需要在这里改代码。 结论：从运行时行为角度 LGTM。 ownership 记录、fail-closed 仲裁、owner 死亡后的回收、drain 门控的释放，以及普通 selector 的默认拒绝，在真实环境中都与描述一致，且覆盖它们的测试确实有效。 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 readOwnerRecord transient-errno terminal compromise (packages/cli/src/serve/conversations/conversation-runtime-ownership.ts:230) — already reported (comment 3792290253)
- R1-5 readExistingRecord transient-I/O terminal compromise (packages/cli/src/serve/live/discovery.ts:193) — already reported (comment 3792290255)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 4)": cross-check of the Shutdown-ordering intro (seal-first, single server.close() before drain, embed-close listener reuse) against serve-app-lifecycle.ts and ser…; "agent reverse-audit (round 3)": the chunk-tail Shutdown-ordering paragraph ( ServeAppLifecycle.close() seal/listener/single-close/embed-close-listener contracts) was read but not verified aga…; "agent reverse-audit (round 3)": table rows not verified against code: channel-worker-group fail-closed exclusion, device-flow fanout, per-runtime sub-session launcher, admitted-runtime helper ….
Not reviewed: "agent verify (round 6)" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Not reviewed: verification and reverse audit — each ran and opened its brief, but neither was launched with the prompt the CLI built — the launches were written by hand, so the posted findings cannot be counted as verified, and what the agents were actually asked is not what this skill certifies.
— [unverified] tag when the loop ended — the verifier never ruled on them, and they are not confirmed.
Deferred under the convergence posture (round 1, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session-runtime.ts:93 (+2 locations) — [review] New fail-closed 503 paths omit the logSessionRoutingFailure breadcrumb every sibling failure kind logspackages/cli/src/serve/routes/session.ts:5358 — [review] daemon_draining check-and-503 duplicated across four sites; wire shape already divergedpackages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts:1040 — [review] Rewritten test leaves dead mock setup consumed only by deleted assertionsdocs/plans/2026-08-13-standalone-pr1-runtime-boundary.md:160 (+4 locations) — [review] Committed plan doc diverges from shipped code (predicate names, size checkpoint, task locations/inventories)packages/cli/src/serve/conversations/conversation-runtime-ownership.test.ts:100 — [review] Forked worker import URL is cwd-relative; fails under repo-root vitest / IDE runnerspackages/cli/src/serve/routes/session.ts:5171 — [review] Compatibility catalog enumerates sessions the load path cannot load (side-task sub-session vector)packages/cli/src/serve/routes/session.ts:2024 — [review] Concurrent-delete race returns misleading 503 workspace_runtime_unavailable instead of notFound
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 4)"：cross-check of the Shutdown-ordering intro (seal-first, single server.close() before drain, embed-close listener reuse) against serve-app-lifecycle.ts and ser…；"agent reverse-audit (round 3)"：the chunk-tail Shutdown-ordering paragraph ( ServeAppLifecycle.close() seal/listener/single-close/embed-close-listener contracts) was read but not verified aga…；"agent reverse-audit (round 3)"：table rows not verified against code: channel-worker-group fail-closed exclusion, device-flow fanout, per-runtime sub-session launcher, admitted-runtime helper …。
未审查："agent verify (round 6)"——启动 prompt 为它指定了 diff 中的行，但它从未打开：有工具调用，却没有一次读取 diff。
未审查：验证与反向审计——两者都运行并打开了各自的 brief，但都不是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，发布的发现不能算作已验证，agent 实际被要求做的也不是本 skill 所认证的内容。
— [unverified] 标记——验证者从未对它们作出裁决，它们不算已确认。
收敛姿态下延后（第 1 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| workspaceRegistry | ||
| ? { | ||
| refreshRuntimes: () => workspaceRegistry.list(), | ||
| refreshRuntimes: () => workspaceRegistry.listAll(), |
There was a problem hiding this comment.
[Critical] Extension reconciliation now refreshes the hidden internal Conversations runtime — list() → listAll() at five sites (globalReconciliationOptions here, plus ~1633, ~1793, ~1902, ~1956) — and workspace-extensions-controller.ts:711-759 pushes {workspaceId, workspaceCwd} of any failed / slow (>30s) refresh target into the operation record's warnings, which GET /extensions/operations/:operationId serializes verbatim (workspace-extensions.ts:1591) with no redaction.
Failure scenario: any ordinary client installs/enables/updates/uninstalls an extension while the Conversations runtime is registered; if the internal runtime's refreshExtensionsForAllSessions fails, reports failed > 0, or exceeds RECONCILE_SLOW_MS (30s — no refresh failure even required), the operation record carries the hidden runtime's workspace id and absolute Conversations root path, and any client polling the operation receives them. This violates the isolation contract this PR establishes (ordinary surfaces must not learn the internal runtime's identity). The refresh coverage is intentional; the identity leak into client-visible warnings is not.
Suggested fix: redact internal identity at the shared sink — in workspace-extensions-controller.ts, omit/replace workspaceId/workspaceCwd for isInternalWorkspaceRuntime(runtime) entries when building warnings (or filter internal runtimes out of refreshTargets while still refreshing them), rather than fixing only one of the five listAll() sites.
Witness: trace-verified at the reviewed commit — all five refreshRuntimes sites switched list()→listAll(); controller warnings.push({workspaceId, workspaceCwd, ...}) for rejected refresh, failed > 0, and elapsedMs > RECONCILE_SLOW_MS; the operation endpoint answers res.status(200).json(operation) verbatim. The PR's own test 'fans a global default change out to every registered runtime' asserts the internal bridge is a reconcile target.
中文说明
[Critical] 扩展对账现在会刷新被隐藏的 internal Conversations 运行时——五处 list() → listAll()（此处 globalReconciliationOptions，另含 ~1633、~1793、~1902、~1956）——而 workspace-extensions-controller.ts:711-759 会把任何刷新失败/超时（>30s）目标的 {workspaceId, workspaceCwd} 写入操作记录的 warnings，GET /extensions/operations/:operationId（workspace-extensions.ts:1591）会原样序列化返回，无任何脱敏。
故障场景：Conversations 运行时已注册时，任意普通客户端安装/启用/更新/卸载扩展；只要 internal 运行时的 refreshExtensionsForAllSessions 失败、报告 failed > 0 或超过 RECONCILE_SLOW_MS（30s，甚至无需刷新失败），操作记录就会携带隐藏运行时的 workspace id 与绝对 Conversations 根路径，任何轮询该操作的客户端都能拿到。这违反了本 PR 自身建立的隔离契约（普通界面不应得知 internal 运行时的身份）。刷新覆盖是有意的；把身份泄漏进客户端可见的 warnings 不是。
建议修复：在共享汇聚点脱敏——在 workspace-extensions-controller.ts 构建 warnings 时对 isInternalWorkspaceRuntime(runtime) 条目省略/替换 workspaceId/workspaceCwd（或从 refreshTargets 中过滤 internal 运行时但仍刷新它们），而不是只修五处 listAll() 中的一处。
证据：已在被审提交上逐行核实——五处 refreshRuntimes 均由 list()→listAll()；controller 对刷新被拒、failed > 0、elapsedMs > RECONCILE_SLOW_MS 均 warnings.push({workspaceId, workspaceCwd, ...})；操作接口 res.status(200).json(operation) 原样返回。PR 自带测试 'fans a global default change out to every registered runtime' 亦断言 internal bridge 是对账目标。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const publicDetails = Object.fromEntries( | ||
| Object.entries(error.details).filter( | ||
| ([, value]) => | ||
| typeof value !== 'string' || !internalIdentities.has(value), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
[Critical] The internal-identity redaction added here covers only the REST serializer; the parallel ACP JSON-RPC transport's toRpcError (acp-http/dispatch.ts:626-638) spreads ...err.details for RequestedSessionIdAdmissionError unredacted, so details.liveWorkspaceCwd / liveWorkspaceId (populated via getBridgeWorkspaceId's listAllEntries() switch, which this PR makes internal-aware) reach ordinary ACP clients.
Failure scenario: a client calls ACP session/new (dispatch.ts:1568 reserveCreate) or session/load (dispatch.ts:1703 reserveRestore) with a session id that is live in (or persisted under) the hidden Conversations runtime; conflict()/workspaceConflict() (session-id-admission.ts:177-206) place the internal root path and workspace id into details.liveWorkspaceCwd/liveWorkspaceId, and toRpcError returns them verbatim in the JSON-RPC error data — the exact identities the REST-side redaction in this hunk was added to hide.
Suggested fix: extract the internalIdentities filter into a shared helper (registry in, details out) and apply it in both sendRequestedSessionIdAdmissionError and toRpcError's RequestedSessionIdAdmissionError branch before spreading details.
Witness (probe): real createRequestedSessionIdAdmission + real toRpcError, fake internal bridge owning a session — PR code → data.liveWorkspaceCwd === '<hidden Conversations root>/.live/<uuid>' and liveWorkspaceId present in the RPC data; with a one-line redaction patch → both undefined. Flip confirmed both directions; probe removed, tree clean.
中文说明
[Critical] 此处新增的 internal 身份脱敏只覆盖了 REST 序列化器；并行的 ACP JSON-RPC 传输的 toRpcError（acp-http/dispatch.ts:626-638）对 RequestedSessionIdAdmissionError 直接展开 ...err.details 而未脱敏，因此 details.liveWorkspaceCwd / liveWorkspaceId（经由本 PR 改为 internal-aware 的 getBridgeWorkspaceId 的 listAllEntries() 填充）会到达普通 ACP 客户端。
故障场景：客户端用一个在隐藏 Conversations 运行时中存活（或持久化于其下）的会话 id 调用 ACP session/new（dispatch.ts:1568 reserveCreate）或 session/load（dispatch.ts:1703 reserveRestore）；conflict()/workspaceConflict()（session-id-admission.ts:177-206）把 internal 根路径与 workspace id 写入 details.liveWorkspaceCwd/liveWorkspaceId，toRpcError 原样返回到 JSON-RPC 错误 data 中——正是本 hunk 的 REST 侧脱敏要隐藏的身份。
建议修复：把 internalIdentities 过滤器抽成共享辅助函数（入参 registry，出参 details），在 sendRequestedSessionIdAdmissionError 与 toRpcError 的 RequestedSessionIdAdmissionError 分支展开 details 前都套用。
证据（探针）：真实 createRequestedSessionIdAdmission + 真实 toRpcError，伪造 internal bridge 持有某会话——PR 代码 → data.liveWorkspaceCwd === '<隐藏 Conversations 根>/.live/<uuid>' 且 liveWorkspaceId 出现在 RPC data 中；加一行脱敏补丁后 → 两者均为 undefined。双向翻转均确认；探针已删除，工作树干净。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| sourceType={sourceMetadataEnabled ? 'default' : undefined} | ||
| channelGroupingEnabled={false} |
There was a problem hiding this comment.
[Critical] This hardening covers the Live WorkspaceSection itself, but the sidebar's secondaryWorkspaceCwds memo (WebShellSidebar.tsx:876-882, unchanged by this diff) filters only !entry.primary && entry.trusted, so the live workspace (invariantly non-primary + trusted) still feeds secondaryArchivedQueries / pinned queries that hit the hidden runtime with ordinary workspace transports.
Failure scenario: with the Conversations runtime active, on the Channels tab the Archived section issues GET /workspaces/<encoded-live-cwd>/sessions?archiveState=archived&sourceType=channel…; resolveLiveCatalogRuntime serves only sourceType=default, so the query falls to resolveRuntimeForCatalogRoute whose registry lookups now hide internal entries → 400 workspace_mismatch; session-catalog-store sets snapshot.error and retries every 30s (SESSION_CATALOG_ERROR_RETRY_MS), and secondaryArchivedError renders the 'load failed' affordance for the whole Archived section. Pre-PR the same query resolved (the accessors were unfiltered), so this diff's hiding is what breaks it — the same class as the surfaces this hunk deliberately scrubs.
Suggested fix: exclude live workspaces from the secondary catalog queries, e.g. in secondaryWorkspaceCwds:
| sourceType={sourceMetadataEnabled ? 'default' : undefined} | |
| channelGroupingEnabled={false} | |
| sourceType={sourceMetadataEnabled ? 'default' : undefined} | |
| channelGroupingEnabled={false} |
plus displayedWorkspaces.filter((entry) => !entry.primary && entry.trusted && entry.kind !== 'live').map((entry) => entry.cwd) for the memo.
Witness: not run — settling the break end-to-end requires a live web-shell UI + daemon pair the harness does not have; verified by deterministic code trace at the reviewed commit: secondaryWorkspaceCwds has no kind filter while the live entry is invariantly non-primary + trusted (assertOwnedRuntime); Channels-tab archived queries carry sourceType=channel; resolveLiveCatalogRuntime admits only sourceType=default; the fallback lookups filter entry.internal → sendWorkspaceMismatch 400; the store sets snapshot.error + 30s retry; the PR's own added sidebar test asserts the Live section must not issue channel queries.
中文说明
[Critical] 此处的加固只覆盖了 Live WorkspaceSection 本身，但侧边栏的 secondaryWorkspaceCwds memo（WebShellSidebar.tsx:876-882，本 diff 未改动）只过滤 !entry.primary && entry.trusted，因此 live 工作区（必然非 primary 且 trusted）仍会进入 secondaryArchivedQueries / pinned 查询，用普通工作区传输方式去打隐藏运行时。
故障场景：Conversations 运行时激活时，在 Channels 标签页 Archived 区会发出 GET /workspaces/<编码后的 live-cwd>/sessions?archiveState=archived&sourceType=channel…；resolveLiveCatalogRuntime 只服务 sourceType=default，于是查询落到 resolveRuntimeForCatalogRoute，其注册表查找现已隐藏 internal 条目 → 400 workspace_mismatch；session-catalog-store 置 snapshot.error 并每 30s 重试（SESSION_CATALOG_ERROR_RETRY_MS），secondaryArchivedError 会为整个 Archived 区渲染“加载失败”提示。PR 之前同样的查询是能解析的（访问器未过滤），所以正是本 diff 的隐藏逻辑把它弄坏——与本 hunk 刻意清理的那些界面同一类。
建议修复：在 secondaryWorkspaceCwds 中排除 live 工作区，例如 displayedWorkspaces.filter((entry) => !entry.primary && entry.trusted && entry.kind !== 'live').map((entry) => entry.cwd)。
证据：未实际运行——端到端确认该破坏需要 harness 不具备的 web-shell UI + daemon 实环境；已通过被审提交上的确定性代码追踪核实：secondaryWorkspaceCwds 无 kind 过滤而 live 条目必然非 primary 且 trusted（assertOwnedRuntime）；Channels 标签页 archived 查询携带 sourceType=channel；resolveLiveCatalogRuntime 只接受 sourceType=default；回退查找会过滤 entry.internal → sendWorkspaceMismatch 400；store 置 snapshot.error 并 30s 重试；PR 自带的侧边栏测试也断言 Live 区不应发出 channel 查询。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (legacyPrimaryFallback) { | ||
| const runtime = workspaceRegistry.primary; | ||
| if (loadError === undefined) return runtime; |
There was a problem hiding this comment.
[Critical] The three new throw loadError sites in resolveTranscriptSessionRuntime (this legacyPrimaryFallback branch used by GET /session/:id/export, the liveOwner-found branch ~1686, and the moved single-entry path ~1700) bypass the function's final-scan sanitization wrap, which replaces non-structured scan errors with an opaque Transcript session resolution failed across N workspace(s) message — so generic fs errors from the new internal-runtime scan reach clients verbatim in the 500 body, embedding full paths under the hidden Conversations root.
Failure scenario: ordinary single-workspace daemon (primary + hidden Conversations runtime — the common shape) serving GET /session/:id/transcript for any session id: the new internal-entry loop's activeInRuntime → assertSessionLoadable → getSessionLocation → readProjectSessionHead hits a non-ENOENT fs error (EACCES/EIO/EMFILE — sessionService.ts:500-507 swallows only ENOENT) whose message embeds the hidden-root file path; recordLoadError stores it; the primary scan finds nothing; the moved single-entry path does throw loadError → sendBridgeError → 500 whose error field is the raw message — exposing the canonical hidden-root path this PR redacts from every structured response. The same leak occurs on any daemon shape via GET /session/:id/export. The pre-existing test 'sanitizes multi-workspace transcript scan errors' pins the stricter bar for the final scan that these new branches skip.
Suggested fix: route all three throw sites through the same policy as the final scan: if (shouldPreserveTranscriptResolutionError(loadError)) throw loadError; else log via logSessionRoutingFailure and throw the opaque wrapper — ideally factored into one helper used by every branch.
Witness (probe): registry with primary + internal runtime, SessionService.getSessionLocation mocked to reject with EACCES … stat '<internalCwd>', GET /session/:id/transcript — PR code → status 500, body {error: "EACCES: permission denied, stat '/private/var/…/<internal-root>'"}, leaksInternalCwd: true; with the implied fix (sanitize unstructured loadError at the new throw site) → body {error: 'Transcript session resolution failed across 1 workspace(s)'}, leaksInternalCwd: false. Flip confirmed both directions; probe removed, tree clean.
中文说明
[Critical] resolveTranscriptSessionRuntime 中三处新增的 throw loadError（此处供 GET /session/:id/export 使用的 legacyPrimaryFallback 分支、~1686 的 liveOwner-found 分支、~1700 的移动后单条目路径）绕过了函数末尾扫描的脱敏包装——该包装会把非结构化扫描错误替换为不透明的 Transcript session resolution failed across N workspace(s) 消息——因此来自新 internal 运行时扫描的通用 fs 错误会原样进入 500 响应体，暴露隐藏 Conversations 根路径下的完整路径。
故障场景：普通单工作区 daemon（primary + 隐藏 Conversations 运行时——常见形态）为任意会话 id 提供 GET /session/:id/transcript：新 internal 条目循环的 activeInRuntime → assertSessionLoadable → getSessionLocation → readProjectSessionHead 遇到非 ENOENT 的 fs 错误（EACCES/EIO/EMFILE——sessionService.ts:500-507 只吞 ENOENT），其消息内嵌隐藏根文件路径；recordLoadError 记录它；primary 扫描无结果；移动后的单条目路径执行 throw loadError → sendBridgeError → 500，其 error 字段即原始消息——暴露本 PR 在所有结构化响应中都已脱敏的规范隐藏根路径。同样的泄漏在任何 daemon 形态下经 GET /session/:id/export 也会发生。既有测试 'sanitizes multi-workspace transcript scan errors' 为这些新分支跳过的末尾扫描锁定了更严格的标准。
建议修复：把三处 throw 都走与末尾扫描相同的策略：if (shouldPreserveTranscriptResolutionError(loadError)) throw loadError; 否则经 logSessionRoutingFailure 记录并抛出不透明包装——最好抽成每个分支共用的一个辅助函数。
证据（探针）：primary + internal 运行时的 registry，SessionService.getSessionLocation mock 为以 EACCES … stat '<internalCwd>' 拒绝，请求 GET /session/:id/transcript——PR 代码 → 状态 500，响应体 {error: "EACCES: permission denied, stat '/private/var/…/<internal-root>'"}，leaksInternalCwd: true；采用隐含修复（在新 throw 处对非结构化 loadError 脱敏）后 → 响应体 {error: 'Transcript session resolution failed across 1 workspace(s)'}，leaksInternalCwd: false。双向翻转均确认；探针已删除，工作树干净。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
Released in v0.21.13. |
4 participants
What this PR does
This PR establishes the hidden runtime boundary required for standalone Conversations without exposing a new standalone product surface. It adds single-owner arbitration for the Conversations runtime, binds ownership to a shared listener/application lifecycle, keeps the internal runtime out of ordinary workspace selectors and transports, and preserves only the existing owner-routed and Live compatibility paths. It also updates daemon status, telemetry, scheduled-task/channel compatibility, Web Shell workspace presentation, and operator documentation so the boundary is applied consistently end to end.
Why it's needed
PR #8890 introduced the owned Conversations runtime publication foundation, but multiple supporting daemons could still race to publish or use the same runtime, ordinary workspace paths could still discover the internal runtime through direct consumers, and shutdown had no positive proof gate before handing ownership to another process. This change makes ownership fail closed, prevents fallback to the primary runtime when internal resolution is unavailable or ambiguous, and ensures ownership is released only after listener, application, host, discovery, bridge, and child-process drains have completed.
Reviewer Test Plan
How to verify
Start two Live-capable daemons for the same OS user and confirm only one can acquire the Conversations runtime while the loser receives a structured retryable error and continues serving ordinary routes. Stop or crash the owner, then confirm a later daemon reclaims the runtime only after the handoff grace period. Verify ordinary workspace selectors, ACP/Voice transports, workspace management, new-session creation, and Web Shell workspace pickers do not target the
kind: "live"runtime, while owner-routed session operations and the existing Live catalog/channel/scheduled-task compatibility paths continue to work. During shutdown, hold an internal operation or listener close open and confirm the ownership record remains until every required drain completes.Automated verification passed for 1,939 tests across the changed CLI test files and 536 tests across the changed Web Shell test files, together with the full repository build, bundle, typecheck, lint, and diff checks. A two-daemon isolated-home E2E run also verified owner contention, ordinary-route availability on the losing daemon, default-deny REST/ACP/Voice selectors, normal handoff, dead-owner reclaim, locator republish after recovery, and final cleanup.
Evidence (Before & After)
Before: the capability-backed
kind: "live"workspace could flow into ordinary Web Shell workspace targets, and supporting daemons lacked one fail-closed ownership proof covering publication through shutdown. After: the Live entry remains available only to the existing compatible catalog/session paths, is absent from Composer, Voice, scheduled-task, scratch, ACP, and generic workspace targets, and a second daemon cannot publish or use the runtime until the proven owner has drained or died and the grace period has elapsed.Tested on
Environment (optional)
macOS with Node.js v22.22.3, local package build/bundle, Vitest, and isolated temporary homes for multi-process E2E. Web Shell tests used
NODE_OPTIONS=--no-experimental-webstorageto prevent Node's experimental Web Storage global from colliding with jsdom.Risk & Scope
createServeAppembedders that enable Live/Conversations must bind the actual Node server withgetServeAppLifecycle(app).bindServer(server)before listening and awaitlifecycle.close()during shutdown; ordinary-only embedders remain compatible.Linked Issues
Related to #8908.
Follow-up to #8890.
中文说明
本 PR 做了什么
本 PR 建立 standalone Conversations 所需的隐藏 runtime 边界，但不暴露新的 standalone 产品能力。它为 Conversations runtime 增加单 owner 仲裁，将 ownership 绑定到共享的 listener/application 生命周期，使 internal runtime 对普通 workspace selector 和 transport 不可见，并只保留既有的 owner-routed 与 Live 兼容路径。同时更新 daemon status、telemetry、scheduled-task/channel 兼容、Web Shell workspace 展示和运维文档，确保该边界端到端一致生效。
为什么需要
PR #8890 引入了 owned Conversations runtime 的发布基础，但多个 supporting daemon 仍可能竞争发布或使用同一 runtime，普通 workspace 路径仍可能通过 direct consumer 发现 internal runtime，shutdown 也没有在向其他进程交接 ownership 前提供正向完成证明。本变更让 ownership 全程 fail closed，在 internal resolution unavailable 或 ambiguous 时禁止回退 primary runtime，并确保只有 listener、application、host、discovery、bridge 与 child-process drain 全部完成后才释放 ownership。
Reviewer 测试计划
如何验证
为同一 OS 用户启动两个启用 Live 的 daemon，确认只有一个能够取得 Conversations runtime，另一个收到结构化、可重试错误且普通路由仍可使用。停止或崩溃 owner 后，确认后继 daemon 仅在 handoff grace period 后回收 runtime。验证普通 workspace selector、ACP/Voice transport、workspace management、新建 session 和 Web Shell workspace picker 都不能定位
kind: "live"runtime，同时 owner-routed session 操作和既有 Live catalog/channel/scheduled-task 兼容路径仍正常工作。shutdown 时阻塞一个 internal 操作或 listener close，确认 ownership record 会保留到所有必需 drain 完成。自动化验证已通过：变更涉及的 CLI 测试文件共 1,939 个测试，变更涉及的 Web Shell 测试文件共 536 个测试，并通过全仓 build、bundle、typecheck、lint 与 diff 检查。使用隔离 home 的双 daemon E2E 还验证了 owner 竞争、失败 daemon 的普通路由可用性、REST/ACP/Voice selector 的 default-deny、正常交接、dead-owner reclaim、恢复后的 locator 重新发布以及最终清理。
证据（变更前后）
变更前：capabilities 提供的
kind: "live"workspace 可能流入普通 Web Shell workspace target，多个 supporting daemon 也没有一个覆盖从发布到 shutdown 的 fail-closed ownership proof。变更后：Live entry 只保留给既有兼容 catalog/session 路径，不再进入 Composer、Voice、scheduled-task、scratch、ACP 或 generic workspace target；第二个 daemon 只有在已证明 owner 完成 drain，或 owner 死亡且 grace period 结束后，才能发布或使用该 runtime。测试平台
环境（可选）
macOS、Node.js v22.22.3、本地 package build/bundle、Vitest，以及用于多进程 E2E 的隔离临时 home。Web Shell 测试使用
NODE_OPTIONS=--no-experimental-webstorage，避免 Node 的 experimental Web Storage global 与 jsdom 冲突。风险与范围
createServeAppembedder 必须在 listen 前用getServeAppLifecycle(app).bindServer(server)绑定实际 Node server，并在 shutdown 时 awaitlifecycle.close()；ordinary-only embedder 保持兼容。关联 Issue
关联 #8908。
后续于 #8890。