Repository navigation
feat(managed-agent): ask for Hosted tool approvals (D6a) - #13071
Conversation
The Hosted Harness now asks before a tool call that the Session's approval mode does not pre-approve, waits durably, and runs or refuses the call once its Action is decided, expires or is cancelled. - Core: commitDurableWait binds the Turn an approval starts, as commitAwaitRuntimeBatch already does, and the Session authority reports when its writes have stopped. - Hosted Harness: yolo, default and auto-edit modes are pinned at creation and reported on create and load; calls are asked about one at a time after the assistant message; refusals keep the model's order; a Turn stops asking after its first expiry or a cancellation; a waiting call also notices stopped writes; a private route records the trusted decision. - A bilingual design note covers D6a and the Java slice D6b; the D1 note records the slice. Part of #12867
Linux real-stack verification (maintainer rig)I ran this PR (
Linux 6.6.89-cix, Node v24.14.0. Harness, logs and screenshots: branch Real-stack scenarios — 84/84 checks pass
Linux unit tests (reviewer test plan) — all pass
The flaky Screenshots
More evidence (per-phase terminal captures)
Verdict: no Linux-specific issue found — from the real-stack perspective this is good to merge. The table in the PR description can flip 🐧 Linux to ✅. Two rig notes (not product defects): a SIGKILLed Harness holds the Store writer lease until 中文验证报告（折叠）Linux 真实环境验证（维护者 rig)在本 PR 提交
系统：Linux 6.6.89-cix,Node v24.14.0。Harness、日志与截图见分支 真实场景 —— 84/84 项全部通过
Linux 单元测试（评审测试计划）—— 全部通过
PR 描述中提到的不稳定 结论：未发现 Linux 特有问题 —— 从真实栈角度建议合并。 PR 描述表格中的 🐧 Linux 可以翻为 ✅。 两条 rig 备注（非产品缺陷）：被 SIGKILL 的 Harness 会持有 Store writer 租约直到 🤖 Generated with Claude Code |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — the loop ran rounds 1 through 5 and ended at the plan's 5-round cap with round 5 still reporting findings (chunks 5 and 10 retired early on their two-consecutive-dry certificates), so convergence was not demonstrated.
Not reviewed: build-and-test — the test-efficacy probe graded nothing (all five probe files inconclusive and its control never ran) and the net-new-vs-pre-existing attribution could not be measured locally (the base rerun timed out), so the PR's own mutation numbers were not independently reproduced.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": §7's D6a test list was spot-checked against the added test names (the mode-pinning, resolve-across-reload, cancel-releases-Workspace and next-poll items), not e…; "agent reverse-audit (round 5)": nothing cut short (the chunk's 389 diff lines were received untruncated and read end to end); the conclusions are static-reading ones — no test or probe was run…; "agent reverse-audit (round 1)": mutation run — the worktree has no built workspace deps ( packages/core/dist absent, no vitest binary under packages/cli/node_modules/.bin ), so "delete hos….
Test Plan (not a blocker): src/managed-runtime/managed-harness-factory.test.ts — no such file or directory; src/managed-runtime/managed-session-authority.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查（原文为英文）：reverse audit — the loop ran rounds 1 through 5 and ended at the plan's 5-round cap with round 5 still reporting findings (chunks 5 and 10 retired early on their two-consecutive-dry certificates), so convergence was not demonstrated.
未审查（原文为英文）：build-and-test — the test-efficacy probe graded nothing (all five probe files inconclusive and its control never ran) and the net-new-vs-pre-existing attribution could not be measured locally (the base rerun timed out), so the PR's own mutation numbers were not independently reproduced.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：§7's D6a test list was spot-checked against the added test names (the mode-pinning, resolve-across-reload, cancel-releases-Workspace and next-poll items), not e…；"agent reverse-audit (round 5)"：nothing cut short (the chunk's 389 diff lines were received untruncated and read end to end); the conclusions are static-reading ones — no test or probe was run…；"agent reverse-audit (round 1)"：mutation run — the worktree has no built workspace deps ( packages/core/dist absent, no vitest binary under packages/cli/node_modules/.bin ), so "delete hos…。
Test Plan（非阻断）：src/managed-runtime/managed-harness-factory.test.ts — no such file or directory; src/managed-runtime/managed-session-authority.test.ts — no such file or directory。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.6)
- Recognise allow under the policy revision the Action's options recorded, not the current constant. - Answer an Action from its record whenever one landed: before the write, after the decision is published and when a write conflicts, so a same decision that won a race answers like a replay and a different one gets action_already_resolved. - Ask the Session's writability inside the authority's serial section through a new optional resolveAction guard, so a decision is not written after the Session blocks while the write waits its turn. - Treat a failed expiry write like a failed decision write: 409 hosted_turn_recovery_required and a woken Turn when the Session can no longer write, 503 only when a retry can succeed. - Tests for each review suggestion and race, and a 10 s wait limit for the Session-level approval tests. - Docs: the server README lists the Actions note, and the Hosted Workspace tool-turn note carries an approval update banner. Part of #12867
|
Thanks for the triage and the review. Follow-up in 706d402:
The audits of this follow-up also closed two narrow failure-path races. A decision can no longer be written after the Session blocks while the write waits in the authority's queue: Also updated the Tested on table: 🐧 Linux is ✅ per the real-stack verification above (84/84 scenario checks and the unit suites on Linux). |
…aces) Co-Authored-By: Claude Code <[email protected]>
Linux real-stack re-verification — review-fix head
|
| Phase | Result | Notes for this head |
|---|---|---|
| A — pinning & validation | 17/17 | unchanged behavior re-confirmed |
| B — allow / deny / replay | 22/22 | unchanged behavior re-confirmed |
| C — expiry | 12/12 | unchanged behavior re-confirmed |
| D — cancel | 12/12 | unchanged behavior re-confirmed |
| E — pre-approved tools | 10/10 | unchanged behavior re-confirmed |
| F — Harness restart | 4/4 | unchanged behavior re-confirmed |
| G — journal failure | 7/7 | resolve → 409 hosted_turn_recovery_required, session blocked in 53 ms |
| H — answer races (new) | 12/12 | see below |
Phase H targets the review fixes directly:
- H1: 8 concurrent identical
allowanswers → all 8 answer200with identical bodies (a same decision that loses the write race now answers like a replay), exactly one durable decision in the journal, the call ran exactly once. - H2: 4
allowvs 4denyconcurrently → one side wins (200), the losing side uniformly gets409 action_already_resolved, exactly one durable decision, and the file was written iffallowwon. (In this rundenywon — all 4 denies answered200.) - H3: a wrong
inputRevisionin the middle of the same race →400 invalid_action_response, good answers unaffected. - H4: journal down across the expiry → the Turn's failed expiry write blocks the Session and the late answer gets
409 hosted_turn_recovery_required— never503(the old code threw here).
Linux unit tests at 706d402aeb — all pass
| Suite | Result |
|---|---|
packages/core — vitest run src/managed-runtime (27 files) |
1,732 passed |
packages/cli — vitest run src/serve/hosted (9 files) |
191 passed (182 + the 9 new review-fix tests) |
packages/cli — tsc --noEmit |
clean |
| ESLint + Prettier on all changed files | clean |
Race-phase evidence and per-phase captures
Evidence (screenshots, full logs, harness incl. the new h-race.ts) updated on branch assets-pr13071.
Verdict: the review fixes behave as designed on the real stack; still no Linux-specific issue — good to merge.
中文复验报告（折叠）
Linux 真实环境复验 —— 评审修复提交 706d402aeb
承接此前的验证评论:PR 推进到 706d402aeb("address D6a review on Hosted approvals")，我在该提交重新构建打包产物、重置 rig 数据库并完整重跑了真实栈，另外新增 Phase H 专门针对评审修复改动的语义——对同一个 Action 的并发回答竞态。
验证栈不变：MySQL 8.4 ← Spring Session Store + 内嵌 Runtime Broker,PR worktree 打包的 Harness，假 OpenAI 模型；持久状态通过直接读取 MySQL 中已提交的 journal/资源字节独立验证。Linux 6.6.89-cix,Node v24.14.0。
真实场景 —— 96/96 项全部通过
A(17/17)、B(22/22)、C(12/12)、D(12/12)、E(10/10)、F(4/4)、G(7/7)全部复验通过，H 竞态（新增）12/12:
- H1: 8 个并发相同
allow回答 → 8 个全部200且响应体一致（输掉写入竞态的相同决定现在按重放回答）,journal 中只有一条决定，调用只执行一次。 - H2: 4 个
allow对 4 个deny并发 → 一方获胜（200)，失败方一致得到409 action_already_resolved，只有一条持久决定，文件是否写入与allow是否获胜一致（本轮deny获胜，4 个 deny 全部200)。 - H3: 同一竞态中混入错误的
inputRevision→400 invalid_action_response，正常回答不受影响。 - H4: journal 在过期前后宕掉 → Turn 自己的过期写入失败使 Session 阻塞，迟到的回答得到
409 hosted_turn_recovery_required——不再是503（旧代码在此抛异常）。
Linux 单元测试（706d402aeb)—— 全部通过
core src/managed-runtime(27 个文件）1,732 通过;cli src/serve/hosted(9 个文件）191 通过(182 + 9 个评审修复新增）;tsc --noEmit、ESLint、Prettier 均干净。
证据（截图、完整日志、含新 h-race.ts 的 harness）已更新到 assets-pr13071 分支。
结论：评审修复在真实栈上行为符合设计；仍无 Linux 特有问题 —— 建议合并。
🤖 Generated with Claude Code
|
@qwen-code /triage |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Verdict: APPROVE — reviewed at head 706d402aeb0a53c94b1c8b5db50eebe35971da8e.
Critical-only scan found no merge-blocking defect. All five production files were read in full — the 358-line approval module at head and the complete patch of each of the four modified ones — plus the four edited doc/README hunks and the new design doc's status and scope claims.
No blocker on record
No CHANGES_REQUESTED was ever filed. The single review round recorded nine findings, all severity S, and the author replied to each at this head. So there is no historical blocking issue to adjudicate and the verdict rests on the current scan.
The approval gate fails closed at every branch I could reach
This adds a permission gate to a tool-execution path, so the question is whether any branch lets a call run without a decision. I traced each one:
- Mode parsing refuses rather than defaults.
parseHostedApprovalSettingsaccepts onlyyolo,defaultandauto-edit;plan,autoand anything unknown returnundefined, and the create route answers400 invalid_hosted_approvalwhen a tool profile is present and parsing failed. An invalid mode cannot silently degrade to yolo.mode === undefinedmapping toyolois the pre-existing behaviour, not a new default. - A malformed saved definition cannot load.
readHostedApprovalDefinitionreturnsundefinedwhen exactly one ofapprovalMode/approvalTimeoutMsis present, and the load path turns that into409 hosted_tool_profile_conflictwith the session closed. A definition carrying neither — an older Harness — resolves toyolo, so existing sessions still load. - Pre-approval is an allow-list, not a deny-list.
PREAPPROVED_TOOLSenumeratesread_filefordefaultandread_file/write_file/editforauto-edit, sorun_shell_commandis asked about in both, and a tool added to a profile later is asked about until someone decides otherwise. That is the safe direction. - Refused calls are neither reserved nor executed. The binding loop skips any ordinal with a refusal before
broker.prepare, and the execution loop commits the refusal as atool_resultandcontinues beforebroker.execute. When every call is refused, the turn commits the refusals, clearsuncertain, and returns without touching the Broker.reserved.get(index)!is only reached for an ordinal the binding loop populated, since both loops test the samerefusals[index] === undefinedcondition — and a throw anywhere in between lands in the catch that cancels every reserved execution. - Every unexpected path refuses.
ask()runs inside thetrywhose catch raisesHostedToolRecoveryRequiredError, so a non-decided/expiredstate, a vanished Action or a failed publish all end in recovery with nothing executed. Expiry setsunanswered, which refuses the remaining calls in the turn without asking; an aborted signal maps every unanswered entry to the cancelled refusal. All four refusal literals produce atool_result, so no function call is ever left without a response.
A decision cannot be forged, and the approved bytes are the executed bytes
resolveHostedAction requires a body with exactly three keys, an optionId that is one of the two the server offered, an inputRevision equal to the recorded Action's, and a policyRevision equal to the one in the server-published options resource — not a value the client asserts. The digest is then computed from the server-side existing.inputRevision and options.policyRevision, and hostedActionAllowed recomputes it for 'allow' against the Action's own inputRevision and the caller's current policy revision. Any drift in option, input revision or policy revision yields a different digest and reads as not allowed, so every mismatch fails closed rather than open. Repeating the same decision returns 200 with the same option; a different one returns 409 action_already_resolved.
The TOCTOU that would matter most is also closed: approve() publishes managed-tool-input once per asked call, stores the ref in inputRefs, and hands that same ref to the durable wait as both invocationRef and routeRef; the binding loop then reuses inputRefs.get(ordinal) instead of publishing again. The responder therefore sees, and the Broker dispatches, one identical durable object.
Blocked and unwritable sessions record nothing
writable() is !isBlocked() && !authority.writesStopped, evaluated before each of the three write attempts, and the new optional admit callback on resolveAction is invoked inside runSerial immediately before the commit — so the precondition is re-checked after the write has queued rather than before, and a refusal writes nothing. writesStopped is a read-only exposure of the existing writeFailure. When a write cannot happen the route answers 409 hosted_turn_recovery_required, where no retry can succeed, and wakes the waiting turn so it stops immediately instead of at the expiry; a failure that recorded nothing rethrows and becomes 503 action_resolution_failed, which is safe to retry, with both promise handlers attached so no rejection goes unhandled.
The core-module changes are additive and behaviour-preserving when unused
commitDurableWait's new turn parameter rejects an approval that would change the current unfinished turn or continue a prior activation, and binds a turn-starting approval to the live activation; with turn omitted, previous = runnable and the path is byte-identical to before, so existing callers are unaffected. resolveAction's admit is likewise optional and unchecked when absent.
The new resolve route sits behind the same identity(req, sessions) gate as its sibling session routes and answers 404 hosted_session_not_found without it, and the design doc labels it a private route — the public Action operations and the Java server half are marked planned in that doc and (Java routes planned) in the README, so nothing over-claims a public capability. The pre-existing tool-turn design doc gains a dated banner scoping its "does not implement interactive approvals" clause to the earlier slice rather than leaving it to contradict the code, and both bilingual pairs moved together.
CI
Green at this head: Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), Serve A/B, web-shell E2E Smoke, TUI parity snapshots, OpenTUI no-flicker gate, Hosted process fault gates / MySQL 8.4 / Java 21, Runtime Broker and Managed Agent MariaDB / Java 21, Real daemon E2E / Java 11 and the full Java matrix all pass. review-pr is pending, which this channel does not treat as a gate.
Coverage disclosure
I did not read the five test files (~1,900 lines) or the two .zh-CN.md companions, and I read the new 299-line design doc's status and scope sections rather than all of it. My verdict is therefore about the production surface, which I read completely; the nine open Suggestions concern test pinning and doc cross-references and do not gate approval under this channel's rules. Two are worth landing on their own merits — the one noting that nothing pins the per-turn scope of unanswered, and the one noting the default approval timeout is never asserted as a parse result — because both guard behaviour this review relied on.
|
One point on the Workspace-hold bound in §8, at head The stated bound holds only when nobody answers. Approval runs after
So a Turn whose owner keeps answering near the deadline can hold the Workspace for roughly (calls asked in the Turn) × timeout, not one timeout. Options:
Not blocking. The first option alone would make the risk accurate. Two smaller notes:
I've put the client-facing contract questions for D6b on #12867, since they mostly constrain the public Action. 中文说明关于 §8 中 Workspace 占用上界的一点意见，基于 head 文中给出的上界只在无人应答时成立。 审批发生在
因此一个 owner 总在临近截止时作答的 Turn，占用 Workspace 的时间可达约 （该 Turn 中被询问的调用数）× 超时，而不是一个超时。可选做法：
不阻塞合入。仅第 1 项就能让风险描述准确。 两个小点：
面向客户端的 D6b 契约问题我发在了 #12867，因为它们主要约束公开的 Action。 |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent test-matrix": none — the full diff walk, source verification, and test runs completed within budget..
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not reviewed: reverse audit — its prompt was built, but no agent was launched with it — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries, and cannot be certified.
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent test-matrix"：none — the full diff walk, source verification, and test runs completed within budget.。
未审查：反向审计——评审时间预算不足，未能开始第 2 轮。
未审查：反向审计——它的 prompt 已构建，但没有 agent 用它启动——负责搜寻评审其余部分遗漏问题的这道工序，即便运行过，也缺失了 brief 承载的方法，无法作证。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| [English](2026-09-27-managed-agent-api-contract.md) | [简体中文](2026-09-27-managed-agent-api-contract.zh-CN.md) | ||
|
|
||
| Status: D1 implemented; D2 implemented in [Session query](2026-09-27-managed-agent-session-query.md); D3 implemented in [Event replay](2026-09-27-managed-agent-event-replay.md); the lifecycle work implemented as D4 of [#12867](https://github.com/QwenLM/qwen-code/issues/12867) in [Durable lifecycle](2026-09-28-managed-agent-durable-lifecycle.md); D5 implemented in [Turn queries](2026-09-28-managed-agent-turn-queries.md) | ||
| Status: D1 implemented; D2 implemented in [Session query](2026-09-27-managed-agent-session-query.md); D3 implemented in [Event replay](2026-09-27-managed-agent-event-replay.md); the lifecycle work implemented as D4 of [#12867](https://github.com/QwenLM/qwen-code/issues/12867) in [Durable lifecycle](2026-09-28-managed-agent-durable-lifecycle.md); D5 implemented in [Turn queries](2026-09-28-managed-agent-turn-queries.md); D6 designed in [Actions](2026-09-30-managed-agent-actions.md), with its Hosted Harness part (D6a) implemented |
There was a problem hiding this comment.
[Suggestion] R1-1: The design record's D-series roster is extended here, but the server's own copy of that roster still stops at D5. Still stands at the reviewed commit (dcec43b) — packages/sdk-java/managed-agent-server/README.md (lines 44-53) still ends at Turn queries, so the D6 note whose §6 specifies the Java side's next work remains unreachable from the server README.
Witness:
sweep: both rosters parsed out of their files and diffed — roster \ README = ["2026-09-30-managed-agent-actions.md"]; dead README links = []
Fix: add one entry in the server README in the same bilingual-pair form as its neighbours: Actions (D6): English | 简体中文. The original comment below carries the full fix and fix witness.
中文说明
建议 R1-1：设计记录里的 D 系列名册在本文档更新了，但服务端自己的那份名册仍停在 D5。
在本轮审查的提交（dcec43b9）上仍然成立——packages/sdk-java/managed-agent-server/README.md（第 44-53 行）仍止于 Turn queries，规定 Java 侧下一步工作的 D6 说明依然无法从服务端 README 进入。
修复：在服务端 README 中按邻近条目的双语对照格式补一条 Actions (D6) 的索引。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
|
|
||
| [English](2026-09-30-managed-agent-actions.md) | [简体中文](2026-09-30-managed-agent-actions.zh-CN.md) | ||
|
|
||
| Status: D6a (Hosted Harness) implemented; D6b (Java server) designed and lands |
There was a problem hiding this comment.
[Suggestion] R1-2: This note declares D6a implemented, but the earlier note it Builds on still records interactive approvals as not implemented. Still stands at the reviewed commit (dcec43b) — docs/design/2026-09-27-hosted-workspace-tool-turn.md:19 and its zh-CN mirror still read "This slice does not implement interactive approvals…", which this note's now-implemented D6a supersedes.
Witness:
witness: not run — no run capability can discriminate a design-prose claim; deciding evidence is the read at HEAD of hosted-workspace-tool-turn.md:19 and its zh-CN mirror against this note's status line, plus commit 48107945ea's own added banner.
Fix: add a dated banner to the earlier doc in both languages (as D4 did in 4810794) pointing at the Actions note. The original comment below carries the full fix and fix witness.
中文说明
建议 R1-2：本说明声明 D6a 已实现，但它在 Builds on 里引用的更早说明仍写着未实现交互式审批。
在本轮审查的提交（dcec43b9）上仍然成立——docs/design/2026-09-27-hosted-workspace-tool-turn.md:19 及其中文镜像仍写着 “This slice does not implement interactive approvals…”，而本说明的 D6a 已实现并取代了该句。
修复：按 D4 阶段（48107945ea）的做法，在该文档的两种语言版本中加带日期的说明，指向 Actions 说明。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| await headers(supertest(server).post(`/session/${SESSION_ID}/title`)) | ||
| .set('X-Qwen-Client-Id', clientId) | ||
| .send({ title: 'renamed' }) | ||
| .expect(503); |
There was a problem hiding this comment.
[Suggestion] R1-3: This test never observes its own premise: every assertion holds identically whether or not the Session's writes were actually stopped. Still stands at the reviewed commit (dcec43b) — no second /title call is driven, so the decided-before-writable ordering still has no positive witness.
Witness:
probe (round 1): a mutant that stops assigning authority.writeFailure leaves the unmodified test green (1 passed, 30 skipped); adding the premise observation makes the same mutant fail inside the test (expected 503, got 200).
Fix: drive the /title failure twice so only a genuinely stopped Session can reject the second write, and assert the premise before the answers. The original comment below carries the full fix and fix witness.
中文说明
建议 R1-3：该测试从未验证它自己的前提：无论写入是否真的停止，每条断言都照样成立。
在本轮审查的提交（dcec43b9）上仍然成立——仍未驱动第二次 /title 失败，「先 decided 后 writable」的顺序依然缺少正向见证。
修复：连续触发两次 /title 失败，使只有真正停止写入的 Session 才会拒绝第二次写入，并在回答前断言该前提。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| return undefined; | ||
| return { | ||
| mode: parsed, | ||
| timeoutMs: (timeoutMs as number | undefined) ?? HOSTED_APPROVAL_TIMEOUT_MS, |
There was a problem hiding this comment.
[Suggestion] R1-4: The documented default approval timeout is never asserted as a parse result — only the bare constant and the yolo branch are pinned. Still stands at the reviewed commit (dcec43b) — no assertion names 600000 as a parse result, so a changed fallback still silently changes how long an attended Turn waits.
Witness:
probe (round 1): with the fallback mutated to ?? 60_000, both affected suites stay green (37 passed); the suggested definition-echo assertion under that same mutant fails (- "approvalTimeoutMs": 600000, + 60000).
Fix: assert the definition echo for the existing timeout-less create, through the exported constant (hosted-tool-approval.test.ts:22 already owns the value). The original comment below carries the full fix and fix witness.
中文说明
建议 R1-4：文档承诺的默认审批超时从未被当作解析结果断言——只有常量本身和 yolo 分支被钉住。
在本轮审查的提交（dcec43b9）上仍然成立——没有任何断言把 600000 作为解析结果固定下来，改动回退值仍会悄悄改变等待中的 Turn 的等待时长。
修复：对现有的不含超时的创建请求断言其定义回显，并使用导出的常量（hosted-tool-approval.test.ts:22 已拥有该值）。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| }, WAIT_POLL_MS); | ||
| poll.unref(); | ||
| // An answer may have landed before the waiter was registered. | ||
| if (isFinal() || signal.aborted) resolve(); |
There was a problem hiding this comment.
[Suggestion] R1-5: Only the isFinal() half of the waiter's pre-registration check is tested; the abort half has no witness. Still stands at the reviewed commit (dcec43b) — no test hands wait() an already-aborted signal, so deleting || signal.aborted stays invisible to all three suites.
Witness:
probe (round 1): deleting || signal.aborted left the three approval suites green over 3 consecutive runs (99 passed each), exactly as the intact tree is.
Fix: add a case in hosted-tool-approval.test.ts that hands wait() an already-aborted signal and asserts the promise settles without advancing the clock. The original comment below carries the full fix and fix witness.
中文说明
建议 R1-5：waiter 注册前检查只测了 isFinal() 那一半，abort 那一半没有见证。
在本轮审查的提交（dcec43b9）上仍然成立——没有任何测试把已 abort 的 signal 交给 wait()，删掉 || signal.aborted 三个套件仍然全绿。
修复：在 hosted-tool-approval.test.ts 中增加一个用例：把已 abort 的 signal 交给 wait()，断言 promise 在不推进时钟的情况下定局。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| expect((await checkpoint()).continuation.phase).toBe('results_ready'); | ||
| await expect( | ||
| resolveHostedAction(session, waiters, requestId, answer('allow')), | ||
| ).resolves.toMatchObject({ status: 200 }); |
There was a problem hiding this comment.
[Suggestion] R1-6: The same-decision replay is asserted as a status code only, and no test ever replays a deny. Still stands at the reviewed commit (dcec43b) — the replay body's optionId is still never read by any assertion.
Witness:
probe (round 1): with the replay branch mutated to a hardcoded optionId 'allow', 93/93 still pass (the body is never read); adding the proposed deny-replay assertion under that same mutation fails (- optionId "deny" / + optionId "allow").
Fix: assert the replay body where the status is asserted today, and cover the deny direction. The original comment below carries the full fix and fix witness.
中文说明
建议 R1-6：相同决定的重复提交只断言了状态码，而且没有任何测试重复提交 deny。
在本轮审查的提交（dcec43b9）上仍然成立——重复分支返回体中的 optionId 依然没有被任何断言读取。
修复：在现有断言状态码处同时断言返回体，并补上 deny 方向的用例。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| ).resolves.toMatchObject({ status: 200 }); | ||
| await expect( | ||
| resolveHostedAction(session, waiters, decided, answer('deny'), () => true), | ||
| ).resolves.toEqual({ status: 409, code: 'action_already_resolved' }); |
There was a problem hiding this comment.
[Suggestion] R1-7: The blocked-Session clause is asserted for only two of its three shapes: no test answers an ended Action on a recovery-blocked Session. Still stands at the reviewed commit (dcec43b) — the ended-code-before-writable-gate branch still has no witness, so moving the writable gate above the ended check stays green.
Witness:
probe (round 1): moving the writable gate above the ended answer leaves all three suites green (99/99); the proposed test passes on the intact tree with 409 action_cancelled and fails under that mutation with code hosted_turn_recovery_required.
Fix: add the third shape: resolveHostedAction against an ended Action with the blocked predicate returning true, expecting the ended code (design §5.4 lines 185-187). The original comment below carries the full fix and fix witness.
中文说明
建议 R1-7：blocked Session 条款只断言了三种形态中的两种：没有测试在恢复阻塞的 Session 上回答已结束的 Action。
在本轮审查的提交（dcec43b9）上仍然成立——「先于可写性门返回结束代码」这条分支依然没有见证，把可写性门移到 ended 检查之前套件仍然全绿。
修复：补第三种形态的用例：以 blocked 谓词对已结束的 Action 调用 resolveHostedAction，断言返回结束代码（设计 §5.4 第 185-187 行）。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| denied: 'The Session owner denied this tool call, so it was not run.', | ||
| expired: | ||
| 'Nobody answered the approval request before it expired, so this tool call was not run.', | ||
| cancelled: 'The turn was cancelled before this tool call ran.', |
There was a problem hiding this comment.
[Suggestion] R1-8: The cancelled approval refusal text is the only one of the four APPROVAL_REFUSALS messages never asserted by any test. Still stands at the reviewed commit (dcec43b) — the literal still occurs only at this line, and this round's audit independently re-derived the same gap at the same line.
Witness:
probe (round 1): mutating the literal to the sibling denied text left both affected suites green (93 passed); a grep shows the string occurs only at this line.
Fix: pin the text in one cancellation-path test (extend the toolResults() helper to surface each part's response.error). The original comment below carries the full fix and fix witness.
中文说明
建议 R1-8：cancelled 拒绝文案是四条 APPROVAL_REFUSALS 文案中唯一没有测试断言的一条。
在本轮审查的提交（dcec43b9）上仍然成立——该字面量仍只出现在这一行，本轮审计在同行独立重新发现了同一缺口。
修复：在某个取消路径的测试中钉住该文案（扩展 toolResults() 辅助函数以暴露每个 part 的 response.error）。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| private publisher?: HostedShellPublisher; | ||
| private bindingGeneration?: string; | ||
| // Once an approval expires nobody is answering, so the Turn asks no more. | ||
| private unanswered = false; |
There was a problem hiding this comment.
[Suggestion] R1-9: Nothing pins the per-Turn scope of unanswered: after one Turn's approval expires unanswered, a new prompt on the same Session must ask again. Still stands at the reviewed commit (dcec43b) — the field still lives on the per-prompt turn object and no test pins that boundary.
Witness:
probe (round 1, two-sided): a session-scoped mutant (a WeakMap keyed by the session) passes the whole existing suite (99 passed); a probe driving expire-then-new-prompt flips — mutant fails (expected length 2, got 1), intact passes.
Fix: add a Turn-boundary case: first prompt expires unanswered, the next one must ask again (a fresh approval, not the refusal). The original comment below carries the full fix and fix witness.
中文说明
建议 R1-9：没有任何测试钉住 unanswered 的每 Turn 作用域：某个 Turn 的审批无人回答而过期后，同一 Session 的新 prompt 必须重新询问。
在本轮审查的提交（dcec43b9）上仍然成立——该字段仍在按 prompt 构造的 turn 对象上，且没有测试钉住这一边界。
修复：增加 Turn 边界用例：第一个 prompt 过期无人回答后，下一个 prompt 必须重新发起询问（新的审批，而不是拒绝文案）。（完整修复与验收标准见下方原始评论）。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| expect(loaded.status).toBe(200); | ||
| expect(loaded.body.approvalMode).toBe('default'); |
There was a problem hiding this comment.
[Suggestion] R2-1: No test exercises the load path's fail-closed recovery guard for the mid-approval crash shape this PR introduces. Every reload in the suite happens on a fully settled session, so the restore.recoveryStatus !== 'ok' || hasUnsettledInput(session) 409 hosted_turn_recovery_required branch of open() has no JS witness. The Java process-crash ITs pin only the pre-existing mid-execution crash shape, and their driver sends no approvalMode — it runs yolo and never requests an approval. The mid-approval shape (session lost while an Action is requested) is exercised by no test anywhere, while the design's restart story rests on this guard (Actions doc §4 Non-goals, §5.5). If hasUnsettledInput(session) is dropped from the guard, the suite stays green and the load attaches a session whose previous turn never settles — further prompts can be admitted on top of the dangling requested action.
Witness:
probe: intact tree — crash state built on disk (await_action checkpoint, action requested), load answers 409 hosted_turn_recovery_required (1 passed); with the hasUnsettledInput disjunct dropped, the same input answers 200 (approvalMode 'default') and the existing suite passes 31/31.
Suggested fix: one test beside the existing reload — reuse waitingSession(), DELETE the session while the approval is pending, then POST /session/:id/load expecting 409 with code hosted_turn_recovery_required (a follow-up status call 404s).
Acceptance criterion: dropping hasUnsettledInput(session) from the guard at hosted-harness-session.ts must turn that test red. The settled-reload half already pinned at these lines (expect(loaded.status).toBe(200)) must keep passing.
中文说明
建议 R2-1：本 PR 引入的「审批等待中崩溃」形态没有任何测试覆盖加载路径的 fail-closed 恢复守卫。套件里的每次 reload 都发生在完全结算的会话上，因此 open() 中 restore.recoveryStatus !== 'ok' || hasUnsettledInput(session) 返回 409 hosted_turn_recovery_required 的分支在 JS 侧没有见证。Java 进程崩溃 IT 只覆盖了既有的执行中崩溃形态（其驱动不发送 approvalMode，跑的是 yolo，从不请求审批）；而「Action 仍为 requested 时会话丢失」这一新形态没有任何测试覆盖，设计的重启语义却正依赖这个守卫（Actions 文档 §4 非目标、§5.5）。若从守卫中删去 hasUnsettledInput(session)，套件保持全绿，加载会附上一个上一 Turn 永不结算的会话——后续 prompt 可以叠加在悬挂的 requested Action 之上。
Witness：完整树在磁盘上构造 await_action 崩溃状态后 load 返回 409；删除该析取分支后同一输入返回 200 且现有套件 31/31 通过。
修复建议：在现有 reload 旁加一个用例——复用 waitingSession()，在审批等待中 DELETE 会话，再 POST /session/:id/load，断言 409 且 code 为 hosted_turn_recovery_required。
验收标准：从 hosted-harness-session.ts 的守卫中删去 hasUnsettledInput(session) 必须让该用例变红；这几行已钉住的结算 reload 断言必须继续通过。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
4 participants
What this PR does
This is slice D6a of #12867. The Hosted Harness now asks for approval before a tool call that the Session's approval mode does not pre-approve, waits durably for the answer, and then runs or refuses the call. A new private route records the trusted decision. The bilingual design note covers this slice and the Java slice D6b that will serve Actions publicly.
approvalModeJava already sends when it creates a Hosted Session, and the create request gainsapprovalTimeoutMs(default 10 minutes, 1 second to 24 hours).defaultpre-approvesread_file;auto-editalso pre-approveswrite_fileandedit;yolopre-approves everything, as today. Each mode lists what it pre-approves, so a tool added to a profile later is asked about. For a tool profile,plan,autoor a bad timeout answer400 invalid_hosted_approval. The mode is saved in the Session definition (ayoloSession saves nothing, so its definition bytes do not change); a load uses the saved mode, and a saved mode the Harness cannot read, or a partial one, fails the load closed. Create and load report the pinned mode asapprovalModefor a Session with a tool profile, which D6b uses to tell a Harness that supports approvals from an older one.hosted-tool-approval/1,allow/deny, creation and expiry times), opens apermissionAction throughcommitDurableWaitand waits. An allowed call joins the Runtime batch as before; a denied, expired or cancelled one gets a refusal result, and the model continues. Results are committed in the model's order. Refused-only rounds dispatch nothing.POST /session/:id/actions/:requestId/resolverecords the decision. It uses the other Session routes' client identity, token and protocol checks. It answers404 action_not_found,400 invalid_action_response,409 action_expired,409 action_cancelledor409 action_already_resolved, and otherwise commits the decision as deterministic bytes and answers200. A repeated decision answers the same result. An answer after the expiry time expires the Action even before the timer fires. A recovery-blocked Session still answers what it has recorded but writes nothing (409 hosted_turn_recovery_required). A failure before the write answers503; a failed journal write, which stops all later writes, wakes the waiting Turn so it blocks the Session at once, and answers409 hosted_turn_recovery_required.commitDurableWaitaccepts the Turn that an approval starts and binds it to the activation, ascommitAwaitRuntimeBatchalready does. Without this, an approval in a Turn's first round left the next Runtime batch refusing to change the unfinished Turn. The Session authority reportswritesStoppedafter an append failed, andresolveActionaccepts an optional guard that it asks inside its serial section, so a decision cannot be written after the Session blocks while the write waits its turn.yolo, so no deployed Session asks until D6b can answer Actions.Why it's needed
#12867 defines D6's exit check: an approval that the Harness requested can be answered through either public surface and the Turn continues, a replayed response returns the original result, and a responder without the right gets
403. The core already had the durable pieces (requestToolAction,resolveAction,commitDurableWait), but nothing used them: the Hosted Harness never asked, and a tool turn ran every call underpreapproved-workspace-tools/1. The decisions on the issue (Q1, Q4 and a single arbiter) put the Harness side in D6; this PR does that part so D6b can add the public routes, the owner check and the projection on top of it.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/managed-runtime/managed-harness-factory.test.ts src/managed-runtime/managed-session-authority.test.tsandcd packages/cli && npx vitest run src/serve/hosted(the paths are relative to each package).approvalMode: 'default'and have the model callwrite_file. An Action is requested and nothing is prepared.allow: the call runs, the Turn completes, and a replay answers200. Withdenythe model gets a refusal and the next Turn still asks and runs.Evidence (Before & After)
N/A (no UI change; Java does not enable asking modes yet).
Local results on macOS:
unref()on the expiry timer, two immediate wake-ups that the one-second check backs up, a load-time parse whose result is unused, and a restoreddecidedcheck that the only caller already makes.read ECONNRESET(or a cleanupENOTEMPTY). The same failure shows up onmain(1 of 7 runs) as on this branch (2 of 6).Tested on
Environment (optional)
Node 24; unit and route tests with the repository's fake model and local Session stores.
Risk & Scope
409 hosted_turn_recovery_required.approvalTimeoutMssetting and the check of the reportedapprovalMode.planandautomodes for tool turns.yoloSessions and Sessions without a tool profile keep their definition bytes and flow. Create and load responses gainapprovalModefor tool Sessions, which the Java client ignores.Design note: English · 简体中文. Both versions are complete and synchronized.
Linked Issues
Part of #12867.
中文说明
这个 PR 做了什么
这是 #12867 的 D6a 切片。Hosted Harness 现在会在 Session 的审批模式不预批准的工具调用之前请求审批，持久地等待回答，然后执行或拒绝该调用。新的私有路由记录可信的决定。双语设计说明涵盖本切片以及将公开提供 Action 的 Java 切片 D6b。
approvalMode，创建请求新增approvalTimeoutMs（默认 10 分钟，范围 1 秒到 24 小时）。default预批准read_file；auto-edit另外预批准write_file与edit；yolo与现在一样预批准所有调用。每种模式列出的是它预批准的工具，因此之后加入配置的工具会被询问。对于工具配置，plan、auto或不合法的超时返回400 invalid_hosted_approval。模式保存在 Session 定义中（yoloSession 不保存任何内容，因此其定义字节不变）；加载使用保存的模式，Harness 读不懂的或只保存了一半的模式会让加载失败关闭。对于带工具配置的 Session，创建与加载会以approvalMode报告已固定的模式，D6b 用它区分支持审批的 Harness 与旧版本。hosted-tool-approval/1、allow/deny、创建与过期时间），通过commitDurableWait开启一个permissionAction 并等待。被允许的调用与以前一样加入 Runtime 批次；被拒绝、过期或取消的调用得到拒绝结果，模型继续。结果按模型给出的顺序提交。只有拒绝的轮次不派发任何调用。POST /session/:id/actions/:requestId/resolve记录决定。 它使用与其他 Session 路由相同的客户端身份、令牌与协议检查。它返回404 action_not_found、400 invalid_action_response、409 action_expired、409 action_cancelled或409 action_already_resolved，否则以确定性的字节提交决定并返回200。重复同一个决定返回同样的结果。在过期时间之后到达的回答会让 Action 过期，即使计时器还没有触发。处于恢复阻塞的 Session 仍回答已记录的内容，但不写入任何东西（409 hosted_turn_recovery_required）。写入之前的失败返回503；journal 写入失败会停止之后的所有写入，路由会唤醒等待中的 Turn，让它立即阻塞 Session，并返回409 hosted_turn_recovery_required。commitDurableWait接受审批所开启的 Turn，并把它绑定到 activation，与commitAwaitRuntimeBatch已有的做法一致。没有这一点时，Turn 第一轮中的审批会让之后的 Runtime 批次拒绝更改未完成的 Turn。Session authority 在追加失败后报告writesStopped；resolveAction接受一个可选的守卫，在其串行区内调用，因此写入排队期间 Session 被阻塞后，决定不会再被写入。yolo以外的任何模式同时开启，因此在 D6b 能够回答 Action 之前，已部署的 Session 都不会询问。为什么需要
#12867 规定的 D6 验收条件是：Harness 发起的审批可以通过任一公开入口回答，Turn 随之继续；重放的回答返回原结果；无权的回答者得到
403。核心层已经具备持久化所需的部件（requestToolAction、resolveAction、commitDurableWait），但没有任何代码使用它们：Hosted Harness 从不询问，工具回合在preapproved-workspace-tools/1下执行每个调用。Issue 上的决定（Q1、Q4 与单一仲裁者）把 Harness 侧放在 D6 中；本 PR 完成这一部分，D6b 可以在此之上加入公开路由、owner 检查与投影。评审测试计划
如何验证
cd packages/core && npx vitest run src/managed-runtime/managed-harness-factory.test.ts src/managed-runtime/managed-session-authority.test.ts与cd packages/cli && npx vitest run src/serve/hosted（路径相对于各自的包）。approvalMode: 'default'创建工具 Session，让模型调用write_file。会请求一个 Action，且不会准备任何调用。allow回答：调用执行，Turn 完成，重放返回200。以deny回答时模型得到拒绝结果，下一个 Turn 仍会询问并执行。证据（前后对比）
不适用（无 UI 变化；Java 尚未开启会询问的模式）。
macOS 本地结果：
unref()、两处有每秒检查兜底的立即唤醒、一个结果未被使用的加载期解析，以及一个唯一调用方已经做过的decided检查（已按审计建议恢复）。read ECONNRESET（或清理时的ENOTEMPTY）失败。同样的失败在main上也会出现（7 次中 1 次），本分支为 6 次中 2 次。测试平台
环境（可选）
Node 24；使用仓库自带的假模型与本地 Session 存储进行单元与路由测试。
风险与范围
409 hosted_turn_recovery_required。approvalTimeoutMs设置，以及对报告的approvalMode的检查。plan与auto模式。yoloSession 与没有工具配置的 Session 保持定义字节与流程不变。创建与加载的回答对工具 Session 新增approvalMode，Java 客户端会忽略它。设计说明：English · 简体中文。两个版本完整且同步。
关联 Issue
属于 #12867。