You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
follow-up(managed-hooks): decide idle ownership and address deferred review diagnostics #13133
Explicitly deferred findings from #13129's real-stack review: #13129 (comment) . The author requested Critical-only changes after review round five. The current PR fixes recoverable owner leaks, cancelled pre-model prompt recovery and the permanent receipt-cap policy failure; the following design/diagnostic work remains separate.
Decide the Workspace lease lifetime for an attached but idle Hook Session. It currently retains ownership until detach/delete, as MCP does, and blocks another Session's tool acquisition. Releasing between operations needs a recoverable owner/receipt lifecycle and shared Hook/MCP/tool ownership rules. Unknown physical effects must retain their recovery barrier; releasing them is not a safe availability fix.
Design acknowledged receipt reclamation for the 4096-operation Runtime limit. The immediate fix blocks permanently exhausted execution, including async and PermissionRequest paths; it does not extend worker lifetime capacity. Eviction without durable acknowledgement can permit side-effect replay.
F5: preserve PreToolUse permissionDecisionReason in the model-visible denial. The current fallback can say "No reason provided" while the denial itself is enforced.
Return a conflict status for operationId/input conflicts rather than generic 503, and use a Hook-specific busy diagnostic rather than hosted_mcp_operation_active.
Document that a surviving background command descendant keeps a delegated cgroup nonempty and can turn a successful parent exit into a timeout.
Track idle Runtime worker reaping and retained memory separately from Hook correctness; the review reports this predates this PR.
Compatibility limit: old versions used random Hook owner IDs. Existing execution records allow those owners to be discovered and released. If the old process died after acquiring for catalog registration but before writing any execution record, there is no durable owner identity in the Session journal; that legacy leak needs operator recovery through the original Broker owner. New activation-derived owner identities close that window for subsequent executions.
Remaining review items from PR 13129
These items are retained under the author's post-round-five Critical-only cutoff; they are not claimed fixed by the SessionStart cancellation correction. Individual thread links below identify the exact findings.
Complete administrative recovery of other blocked model/tool continuations only with durable continuation settlement. PR feat(managed-agent): implement durable Hosted Hooks (H2) #13129 now additionally fixes a fresh refused PreToolUse child explicitly cancelled with not_started_proven in the first committed tool-call batch, requiring ended model attempts, exact original prompt/call proof, no tool intent/receipt and no pending Hook/approval/history. It persists missing matching refusals before cancelled settlement, preserving existing responses on retry/load. Genuine unknown effects, already executed batches and other unproven continuations stay fenced; never simply clear the recovery bit (R1-6: feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment); fixed by c5a4cd2).
Define an explicit typed not-started proof for generic Runtime error-only replies and transport-level conflicts. Current recognized missing-handler/isolation replies support safe explicit cancellation; other incomplete replies conservatively retain recovery state. Do not infer no effects from a conflict response or a missing original receipt (R1-7: feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).
Unknown physical outcomes, owner loss and failed drain intentionally block release (R1-1/R1-3/R1-27). Any future administrative abandonment operation needs a separate ownership/replay contract; removing the current barrier is not an accepted fix. The history-cloning and quadratic record-scan findings (R1-8/R1-28/R1-20) belong to #13132.
Compatibility with deployed unmerged H2 builds: an existing SessionDelete occurrence whose immutable input omitted deleted_session_id keeps that original input. Retrying DELETE after a failed close can conflict with the corrected new input; do not mutate that saved plan or re-execute its handler. Detach and original-owner reconciliation remain available. Any migration of these unpublished lifecycle records needs an explicit compatibility policy.
Round-3 scope update for R1-7: a real HTTP runner-construction failure before dispatch was reproduced and is now addressed in PR #13129 by returning a settled failure result under the saved fail policy. This supersedes the earlier blanket reachability assessment. The remaining item concerns incomplete transport/error-only replies without equivalent no-dispatch proof; it is not a claim that all settled errors remain broken.
Round-5 HTTP timeout and unknown recovery follow-ups
Source: PR #13129 round-5 real-stack review at d5de4f1813. The reviewer confirms delayed-response user cancellation now recovers at all four Hook stages on macOS/MySQL and Linux/MySQL, with no new defect found in that increment. The items below are recorded under the author's post-round-five Critical-only cutoff; they are not changes requested for the current PR and are not claimed fixed.
Decide a managed HTTP default or deployment-wide timeout maximum before GA. Current HTTP default is 600 seconds; cancellation of an already dispatched HTTP Hook waits for the real response or its existing deadline. Preserve the response as completion evidence, pre-dispatch cancellation, immediate Runtime shutdown, and native compatibility when choosing a shorter bound. A shorter timeout alone does not recover unknown work.
Design an operator resolve/abandon-unknown workflow, explicitly covering slow or never-returning HTTP endpoints both with and without user cancellation. A timeout can close the local transport while remote effects continue, so it cannot establish no effects or authorize a replay. Current behavior retains the original Hook owner and blocks the Session/Workspace; the reviewer observed next-prompt 409, detach 503 and another Session without Hooks failing its tool turn. Reconciliation must verify the original receipt; abandonment needs a durable ownership/replay fence and an explicit operator decision. Do not implement availability recovery by merely clearing a recovery bit or expiring the owner lease.
Document per-type manifest timeout units and evaluate per-type validation ranges. HTTP/prompt use seconds; function uses milliseconds; command uses seconds except values ≥ 1000 retain legacy-millisecond meaning. The current shared numeric cap is 600,000, not a common unit or a managed HTTP maximum duration. An HTTP timeout: 10000 means 10,000 seconds (about 2.8 hours). Include explicit unit examples and align managed limits with the Hosted observation lifetime: Hosted currently polls running Hook executions with a 660-second deadline, so a longer Runtime request can remain live after Harness has recorded recovery-required. Assess both boundaries; do not imply the public cancel always waits the entire larger configured HTTP timeout.
The reported never-reply cases used a 15-second HTTP timeout on fresh Workspaces; the five-second response control completed. The contaminated rerun on already-blocked Workspaces was explicitly excluded by the reviewer. Keep this separate from the fixed user-cancel case that receives a complete original response.
Explicit H2 decision for unknown availability (review 5379504700)
Source: #13129 (review) . H2 accepts the following recovery/availability limitation, rather than declaring these findings fixed or removing the original-owner/replay barrier:
If a SessionEnd or SessionDelete outcome is genuinely unknown, DELETE returns 503 and the Session remains attached; detach also requires Hook settlement. The original Workspace owner remains held, which can block tools in another Session in that Workspace indefinitely. Repeated DELETE observes the original occurrence; a real-stack three-attempt probe saw one HTTP request and one hook-execute, not a repeated effect. A different Workspace Runtime remained usable.
The Runtime worker is tenant/Workspace scoped. Unknown entries retain holds and occupy its 16-operation admission quota for that worker's lifetime. Sixteen unresolved entries can exhaust admission; the separate 4096 immutable-receipt lifetime cap also counts settled entries. H2 has no acknowledgement/eviction or administrative abandon route. Restart, timeout, cancel or deleting an in-memory entry does not prove effects stopped and is not an approved recovery procedure.
Safe operator reconciliation or abandonment must preserve the original receipt identity and install a durable ownership/replay fence before allowing new execution. Acknowledged receipt reclamation and this operator route remain required follow-up design decisions before GA. This explicit H2 acceptance is not a bounded recovery SLA and does not imply reviewer approval or thread resolution.
The separate confirmed prompt-cancel defect is corrected by the current PR: after a committed assistant call, cancellation before the first Hook plan or between batch calls saves a cancelled occurrence and a refusal for each call, rather than leaving unmatched calls and permanent recovery-required. Real unknowns, failed persistence and other unproven continuations remain fenced. Review 5379995629 also confirmed a distinct fresh, proven-unstarted PreToolUse refusal; c5a4cd2 now settles its explicitly cancelled first batch durably instead of deferring that confirmed Critical. This does not add an unknown-abandonment route.
PR #13129 commit 251117c3d5 also fixes native InstructionsLoaded refused and explicitly cancelled before any main model/tool/non-user work, matching the original durable prompt identity. It splits a legal first tool-call batch's refusal results by complete-record UTF-8 byte size within the 64 KiB limit, preserving committed prefixes, call identities and parent links across retry/reload before a unique cancelled settlement. These are the two confirmed deterministic blockers from #13129 (review) ; they are fixed in the PR rather than deferred as genuine unknown.
Other unproven continuations, a single unsplittable oversized response, unknown effects, original-owner uncertainty and receipt reclamation remain subject to the existing barriers and follow-up decisions above. No unknown-abandonment route is added. The new five-case cross-process verification uses actual Hosted dispatcher/Runtime and HTTP resource size guard with Config/Broker transport fixtures and local JSONL/resources; it does not establish Java/H2 or SQL ACK evidence. The previous local ACK fixture had an invalid original-method capture, so its claimed post-write proof is withdrawn; corrected tests now assert an actually committed prefix before the application-level exception.
Source: PR #13129 round-3 self-review at 4dbdd48 (posted 2026-10-01T22:56Z). Recorded under the same post-round-five Critical-only cutoff; none of these is claimed fixed by this PR. Verified still standing at 4dbdd48.
R3-11/R3-43: Shape-check a PreToolUse hookSpecificOutput.updatedInput identically at every read site. The executor requires a non-array object and runs the original args on an array, while completeHookResults (:491-495) and sequentialInput (hosted-hook-session.ts:98, :741, :1368) adopt the array for after-hooks, the next hook in a sequential chain and its durable managed-hook-input record, so hooks are shown an input the tool never ran.
R3-13: Preserve a managed PermissionRequest deny message in the model-visible refusal; only decision.behavior is recorded, so every refusal reads "PermissionRequest Hook denied the call."
R3-23: A MessageDisplay hook's suppressOutput on a turn's final round makes the turn return parts: []; the hosted caller persists and later replays that empty assistant message into provider requests (hosted-harness-model.ts:497).
R3-46: Distinguish a transient hook-status control-read failure (transport error/5xx/restart) from an answered outcome_unknown. Today one failed internal read is treated as terminal evidence: recovery_blocked/outcome_unknown is durably committed, polling stops, prompts 409 and DELETE 503, though the Runtime still runs the operation. Keep polling within the deadline (same for the cancel path); only a definitive reconciled answer may commit unknown.
R3-48: Build the managed PostToolBatch payload in the documented serialized shape (response_parts, result_display, error, error_type, execution_status, content_length, plus tool_call_id) instead of the raw Runtime envelope (executionStatus/responseParts/outputOmitted/summary), so hooks written against the documented contract work on hosted sessions too.
R3-49: Pass through retryable workspace acquisition failures (409 workspace_busy/workspace_unavailable) out of the pending-file-history recovery acquisition; the current catch wraps them in HostedToolRecoveryRequiredError, defeating the caller's explicit retryable passthrough (hosted-workspace-tool-turn.ts:380-382).
R3-64: Preserve the cgroup launcher's spawn error (stderr/error field) so an unspawnable hook command reports spawn <shell> ENOENT rather than a bare failed hook with no diagnostic.
R3-68: On the untracked (CLI) HTTP hook path, lift the response-body read out of the malformed-JSON bare catch {}: a hook-timeout abort during the body read is currently swallowed and reported as outcome: 'success' with empty output (httpHookRunner.ts:389-401).
R3-104: Handle LlmEventType.LoopDetected on the hosted model path: fire the managed StopFailure hook with error: loop_detected and end the turn with the loop warning instead of throwing "unsupported continuation" (core's own producer cannot cover it because the managed dispatcher reports no StopFailure subscription on the hosted path).
R3-128: Make the MAX_OPERATIONS capacity refusal replayable (record it like the quota refusal, or answer status/cancel for it distinctly). Today the refusal returns before operations.set, so the same operationId reads settled/blocking at dispatch and outcome_unknown on any later control read, indistinguishable from "never answered".
Source: PR #13129 round-4 review thread findings (posted before merge, 2026-10-02). Post-merge bookkeeping found the two Criticals below were never recorded here and remained unfixed at merge; both are addressed in #13243 (pending merge). The four test-coverage Suggestions are deferred under the same post-round-five Critical-only cutoff and are not claimed fixed.
Round-3 follow-up from PR #13243: post-evaluation rejection still certifies not_started_proven
Source: PR #13243 review round 3 (finding R3-2). The runtime maps every non-abort, non-evaluation-timeout failure of the handler-module step to managed_hook_handler_unavailable, and the host's executionUnavailable() turns that code into the durable proof execution: 'not_started_proven'. But the same throw arm is reached by two different situations:
a module that failed before any top-level statement ran (missing file, syntax error, throw as the first statement) — the proof is accurate;
a module that evaluated in full, running its top-level side effects, and was rejected only afterwards by the shape / handlerRevision guard — the proof is false.
The second case is an ordinary rollout race: a handler module deployed one revision behind its manifest pin completes its top-level await, writes into the workspace and registers module state, and only then fails the revision check. The Harness records not_started_proven, hasHolds() is already false, and releaseEarlierOwners' settled test releases the owner — so a later rewind or file-history bind is admitted over mutations the durable record asserts never happened. This contradicts the record contract's own precondition (docs/design/2026-09-27-managed-extension-record-contract.md: "A dispatch that the Runtime refuses before any side effect proves nothing started"), which defines the proof by no side effect, not by pre-dispatch timing.
Give the post-evaluation shape/revision rejection its own code, outside executionUnavailable(), so it fences as outcome_unknown instead of certifying non-execution. The test that must go red without it: a case beside fails fast when the handler module itself rejects in packages/cli/src/serve/managed-hook-runtime.test.ts whose fixture writes a marker at top level before throwing, asserting the receipt does not certify non-execution and that the hold stays.
The behaviour is pre-existing (at PR #13243's merge base the same catch wrapped the import, the shape check and the revision guard alike), so it was documented rather than changed there: docs/design/2026-09-30-managed-hooks-runtime.md now states that a module rejected after evaluating in full is not covered by that proof, in both language versions.
Round-4 addition to the item above (PR #13243 review round 4, finding R3-2 fix-induced): the same single code also covers a third case, which is the most dangerous one — a trusted handler module that runs top-level side effects (appendFileSync, opening a socket, registering module state) and only then throws, for example on a failed config parse, a missing dependency or a guard clause. managed-hook-runtime.ts has one try around both the module import (which rejects with the module's raw error) and the shape/handlerRevision guard, and its catch rethrows only the abort and evaluation-timeout cases, so all three shapes settle as managed_hook_handler_unavailable and the Harness records not_started_proven for a hook whose top-level write did happen. Nothing records it as unknown, so a later rewind or file-history bind is admitted over mutations the durable record asserts never happened.
When splitting the post-evaluation rejection into its own code, cover this middle case too — a distinct code, or an evaluationStarted marker on the operation view, added to executionUnavailable()'s negative set. The pin: a case beside fails fast when the handler module itself rejects in packages/cli/src/serve/managed-hook-runtime.test.ts whose fixture writes a marker at top level before throwing, asserting the receipt does not certify non-execution and that the hold stays.
docs/design/2026-09-30-managed-hooks-runtime.md and its Chinese twin now state the precondition as the code actually implements it (one code covering rejection before, partway through, and after top-level execution, with the proof not covering effects that already ran), so the documentation no longer over-claims while the code split is pending.
Source: PR #13243 review round 5 (3b0f29cbc5 era, posted 2026-10-04T10:09Z). The round's one Critical (R4-1 fix-induced: the per-turn retry made acquire() fallible on an already-acquired Session, so a stale earlier owner's untolerated release refusal failed a healthy live turn) is fixed in that commit. The six Suggestions below are deferred under the author's post-round-five Critical-only cutoff; none is claimed fixed.
R5-1: names a hold-fenced current-owner release instead of leaking the refusal (hosted-hook-session.test.ts) exercises the close() path that writes an operator-facing stderr line but installs no vi.spyOn(stdio, 'writeStderrLineSafe'), unlike its siblings. The production line escapes to the runner's real stderr on every CI run of that file, and the close-path log's format is unasserted.
R5-2: holdFencedRelease() infers "this owner still holds unfinished work" from two codes that do not carry that meaning. managed_runtime_provider_operation_failed is the worker route's else-branch for every error that is not protocol/preparation/unavailable, and managed_runtime_identity_conflict is ManagedToolConflictError's default code with at least three throw sites on the release path, only one of which is a genuine hold fence. An unrelated refusal is therefore booked as a retained hold and fenced permanently (silently absorbed, retried forever). A durable fix needs a dedicated code, which requires amending HttpRuntimeTransport.providerFailure's closed switch in packages/sdk-java in the same change.
R5-3: nothing exercises a fenced earlier owner whose later retry succeeds, so fencedOwners.delete(id) on the success path — the only thing that lets close() and DELETE complete again — is unpinned. Every new test keeps the fence throwing unconditionally and none closes after a successful retry.
R5-4: the retrying single-flight guard has no test. The existing shares one acquisition between parallel callers case constructs a fresh session, so both callers take the acquiring ??= branch and nothing reaches retryFencedOwners(). Inlining the retry loop would let two parallel Hooks on an already-acquired Session each run releaseOwner(id) for the same fenced owner.
R5-5: the per-turn retry is awaited on the hook-dispatch critical path with no attempt cap, no backoff and no log dedup, for a fence that by construction may never clear. A permanently fenced owner costs one blocking broker round trip (capped at 30 s by the broker's own AbortSignal.timeout) plus one byte-identical stderr line on every hook dispatch for the daemon's life. Needs a bounded/deferred retry policy (and interacts with the acknowledged-reclamation design already tracked above).
R2-1 (remainder): the admission-slot test writes the budget as the literal 16 while production compares against the exported MANAGED_HOOK_MAX_RUNNING; import the constant so the case tracks a future change to the budget instead of silently testing the wrong number.
Also recorded from the same round's verification report (qwen-triage:verify, verdict findings — 61/61 scripted assertions passed, "nothing here blocks the merge"): F4 the residual evaluation bound is 600 s in the worst case (max(timeout, 500 ms) with the manifest's timeout <= 600_000) and the design doc does not state it; F5 the accepted-tradeoff list names one fenced operation while the same permanently-true hasHolds() also fences workspace deactivation and file-history rewind; F6 one raw-path dynamic import remains elsewhere in the repo (outside this PR's scope).
Source: PR #13243 review round 8 (posted 2026-10-05T07:10Z at head 930b3092d7). The round's only posted finding was Critical R7-1 — the abandonment fence had no terminal transition, so a cancel that merely raced a healthy cold module import tombstoned the Hook Session for the worker's lifetime — fixed in be10a118a1. The two items below were recorded by the review under its convergence posture and not posted inline; they are deferred under the same post-round-five Critical-only cutoff and are not claimed fixed.
D8-1: the PR description's claim that a user-requested cancel never becomes a blocked turn is falsified by the mid-evaluation abandonment guard, in both design-doc language versions and in the description's Test Plan bullet. A cancel that lands while a trusted function handler's module is still evaluating is fenced recovery_blocked / outcome_unknown until the evaluation definitively ends and the receipt republishes; the correct claim is "never becomes a permanently blocked turn", with the transient fence named. (packages/cli/src/serve/managed-hook-runtime.ts around the abandonment guard.)
D8-2 — verified at be10a118a1 and does not stand; recorded so nobody chases it. The claim was that nothing exercises close() concurrently with an in-flight abandoned evaluation, and that a never-settling evaluation would wedge close(). Both halves are false. fences a stuck module evaluation cancelled mid-evaluation without wedging close uses a module whose top-level await new Promise(() => {}) never settles, cancels it, asserts the managed_hook_module_evaluation_abandoned receipt and hasHolds() === true, and only then calls close(), which resolves. Since hasHolds() is view.state !== 'settled' || moduleEvaluationPending === true and the view is already settled at that point, the true value can only come from moduleEvaluationPending, which is cleared solely in the import's settle handlers — so the evaluation is provably still in flight when close() runs. And close() awaits entry.done, the execute() promise, which returns as soon as the abandonment receipt is published without waiting for the import, so a never-settling evaluation cannot wedge it. The wedge exists only when there is no bound at all: removing it makes afterEach's close() hang (mutation-verified in round 1).
Also carried from the same round's advisory: the review reports the diff has grown 4.4x since it first measured it (79 → 351 source diff lines), that its reverse audit stopped at its round cap without converging, and that findings keep returning to the same two files — it recommends a human decide whether the shape of the change is still right, or split the independent cluster into its own pull request, before further rounds. That scope decision is with the maintainer; the branch also carries two autofix-authored commits (c5909d530c, 930b3092d7) that are not part of the original two-finding scope.
Explicitly deferred findings from #13129's real-stack review: #13129 (comment) . The author requested Critical-only changes after review round five. The current PR fixes recoverable owner leaks, cancelled pre-model prompt recovery and the permanent receipt-cap policy failure; the following design/diagnostic work remains separate.
Compatibility limit: old versions used random Hook owner IDs. Existing execution records allow those owners to be discovered and released. If the old process died after acquiring for catalog registration but before writing any execution record, there is no durable owner identity in the Session journal; that legacy leak needs operator recovery through the original Broker owner. New activation-derived owner identities close that window for subsequent executions.
Remaining review items from PR 13129
These items are retained under the author's post-round-five Critical-only cutoff; they are not claimed fixed by the SessionStart cancellation correction. Individual thread links below identify the exact findings.
Unknown physical outcomes, owner loss and failed drain intentionally block release (R1-1/R1-3/R1-27). Any future administrative abandonment operation needs a separate ownership/replay contract; removing the current barrier is not an accepted fix. The history-cloning and quadratic record-scan findings (R1-8/R1-28/R1-20) belong to #13132.
Round-3 scope update for R1-7: a real HTTP runner-construction failure before dispatch was reproduced and is now addressed in PR #13129 by returning a settled failure result under the saved fail policy. This supersedes the earlier blanket reachability assessment. The remaining item concerns incomplete transport/error-only replies without equivalent no-dispatch proof; it is not a claim that all settled errors remain broken.
Round-5 HTTP timeout and unknown recovery follow-ups
Source: PR #13129 round-5 real-stack review at d5de4f1813. The reviewer confirms delayed-response user cancellation now recovers at all four Hook stages on macOS/MySQL and Linux/MySQL, with no new defect found in that increment. The items below are recorded under the author's post-round-five Critical-only cutoff; they are not changes requested for the current PR and are not claimed fixed.
timeout: 10000means 10,000 seconds (about 2.8 hours). Include explicit unit examples and align managed limits with the Hosted observation lifetime: Hosted currently polls running Hook executions with a 660-second deadline, so a longer Runtime request can remain live after Harness has recorded recovery-required. Assess both boundaries; do not imply the public cancel always waits the entire larger configured HTTP timeout.The reported never-reply cases used a 15-second HTTP timeout on fresh Workspaces; the five-second response control completed. The contaminated rerun on already-blocked Workspaces was explicitly excluded by the reviewer. Keep this separate from the fixed user-cancel case that receives a complete original response.
中文版:第五轮 HTTP 超时后续项
第五轮在 d5de4f1 上确认 HTTP 取消修复在 macOS/MySQL、Linux/MySQL 的四个阶段均通过,本次增量未发现新缺陷。以下三项按第五轮后只处理 Critical 的范围要求记录在此,不作为当前 PR 的追加修改,也不宣称已解决:
审查者的始终不回复场景使用全新 Workspace 和15秒 HTTP 超时,5秒正常响应对照成功;已阻塞 Workspace 上的污染重跑未作为证据。上述边界与能收到完整原响应的已修复取消场景分别记录。
Explicit H2 decision for unknown availability (review 5379504700)
Source: #13129 (review) . H2 accepts the following recovery/availability limitation, rather than declaring these findings fixed or removing the original-owner/replay barrier:
The separate confirmed prompt-cancel defect is corrected by the current PR: after a committed assistant call, cancellation before the first Hook plan or between batch calls saves a cancelled occurrence and a refusal for each call, rather than leaving unmatched calls and permanent recovery-required. Real unknowns, failed persistence and other unproven continuations remain fenced. Review 5379995629 also confirmed a distinct fresh, proven-unstarted PreToolUse refusal; c5a4cd2 now settles its explicitly cancelled first batch durably instead of deferring that confirmed Critical. This does not add an unknown-abandonment route.
Proven-unstarted recovery update (review 5381095301)
PR #13129 commit
251117c3d5also fixes native InstructionsLoaded refused and explicitly cancelled before any main model/tool/non-user work, matching the original durable prompt identity. It splits a legal first tool-call batch's refusal results by complete-record UTF-8 byte size within the 64 KiB limit, preserving committed prefixes, call identities and parent links across retry/reload before a unique cancelled settlement. These are the two confirmed deterministic blockers from #13129 (review) ; they are fixed in the PR rather than deferred as genuine unknown.Other unproven continuations, a single unsplittable oversized response, unknown effects, original-owner uncertainty and receipt reclamation remain subject to the existing barriers and follow-up decisions above. No unknown-abandonment route is added. The new five-case cross-process verification uses actual Hosted dispatcher/Runtime and HTTP resource size guard with Config/Broker transport fixtures and local JSONL/resources; it does not establish Java/H2 or SQL ACK evidence. The previous local ACK fixture had an invalid original-method capture, so its claimed post-write proof is withdrawn; corrected tests now assert an actually committed prefix before the application-level exception.
本轮
251117c3d5另修复两条具有无执行证明的确定性阻塞:模型前原生 InstructionsLoaded 被拒绝后显式取消,按原持久化 prompt 身份结算;首个合法工具批次的拒绝响应按完整记录 UTF-8 大小分批,在 64 KiB 内保存,重试/加载保留已提交前缀、原调用身份及 parent 链,全部保存后才生成唯一 cancelled 终态。其他未证明 continuation、单条不可分且超限的响应、unknown、原 owner 不确定性和回执回收仍保留上述屏障及后续决策,不新增 unknown 放弃接口。五组跨进程验证使用真实 Hosted dispatcher/Runtime 和 HTTP 大小门禁、受控 Config/Broker transport 及本地 JSONL/resources,不作为 Java/H2 或 SQL ACK 证据;旧本地 ACK 夹具的写入后证明已撤回,修正后的测试先断言实际提交前缀,再注入应用层异常。Round-3 (R3) deferred Suggestions
Source: PR #13129 round-3 self-review at 4dbdd48 (posted 2026-10-01T22:56Z). Recorded under the same post-round-five Critical-only cutoff; none of these is claimed fixed by this PR. Verified still standing at 4dbdd48.
hookSpecificOutput.updatedInputidentically at every read site. The executor requires a non-array object and runs the original args on an array, whilecompleteHookResults(:491-495) andsequentialInput(hosted-hook-session.ts:98, :741, :1368) adopt the array for after-hooks, the next hook in a sequential chain and its durable managed-hook-input record, so hooks are shown an input the tool never ran.messagein the model-visible refusal; onlydecision.behavioris recorded, so every refusal reads "PermissionRequest Hook denied the call."suppressOutputon a turn's final round makes the turn returnparts: []; the hosted caller persists and later replays that empty assistant message into provider requests (hosted-harness-model.ts:497).hook-statuscontrol-read failure (transport error/5xx/restart) from an answeredoutcome_unknown. Today one failed internal read is treated as terminal evidence: recovery_blocked/outcome_unknown is durably committed, polling stops, prompts 409 and DELETE 503, though the Runtime still runs the operation. Keep polling within the deadline (same for the cancel path); only a definitive reconciled answer may commit unknown.response_parts,result_display,error,error_type,execution_status,content_length, plustool_call_id) instead of the raw Runtime envelope (executionStatus/responseParts/outputOmitted/summary), so hooks written against the documented contract work on hosted sessions too.409 workspace_busy/workspace_unavailable) out of the pending-file-history recovery acquisition; the current catch wraps them inHostedToolRecoveryRequiredError, defeating the caller's explicit retryable passthrough (hosted-workspace-tool-turn.ts:380-382).spawn <shell> ENOENTrather than a bare failed hook with no diagnostic.catch {}: a hook-timeout abort during the body read is currently swallowed and reported asoutcome: 'success'with empty output (httpHookRunner.ts:389-401).LlmEventType.LoopDetectedon the hosted model path: fire the managed StopFailure hook witherror: loop_detectedand end the turn with the loop warning instead of throwing "unsupported continuation" (core's own producer cannot cover it because the managed dispatcher reports no StopFailure subscription on the hosted path).MAX_OPERATIONScapacity refusal replayable (record it like the quota refusal, or answer status/cancel for it distinctly). Today the refusal returns beforeoperations.set, so the same operationId readssettled/blocking at dispatch andoutcome_unknownon any later control read, indistinguishable from "never answered".Duplicates of items already listed above: R3-12 (= F5, PreToolUse permissionDecisionReason), R3-24 (= Hosted Stop continuation cap), R3-140 (= R1-22, StopFailure error classification).
Round-4 (R4) review items
Source: PR #13129 round-4 review thread findings (posted before merge, 2026-10-02). Post-merge bookkeeping found the two Criticals below were never recorded here and remained unfixed at merge; both are addressed in #13243 (pending merge). The four test-coverage Suggestions are deferred under the same post-round-five Critical-only cutoff and are not claimed fixed.
await import(modulePath)), which throwsERR_UNSUPPORTED_ESM_URL_SCHEMEon Windows and would redden the schedule-onlytest_windowslane on main; the fix imports throughpathToFileURLlike the production sibling (feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).await import()had no timeout and ignored the operation abort signal, so a handler module whose top-level await never settles wedged the operation, held one of 16 admission slots and hung runtimeclose(); the fix races the import against the manifest function timeout and the abort signal (feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).executions[0].runtimeSessionIdinstead ofpromptId) is unpinned by any test — reverting it keeps all recovery tests green (feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).managed-tool-argsshell-owner branch (definitionhookCatalog/mcpServersdecline check andowners.add(promptId)) lacks direct coverage (feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).originalRuntimeBroker(harnessSessionIdcomparison andowners.sizeguard) lacks direct coverage (feat(managed-agent): implement durable Hosted Hooks (H2) #13129 (comment)).Round-3 follow-up from PR #13243: post-evaluation rejection still certifies not_started_proven
Source: PR #13243 review round 3 (finding R3-2). The runtime maps every non-abort, non-evaluation-timeout failure of the handler-module step to
managed_hook_handler_unavailable, and the host'sexecutionUnavailable()turns that code into the durable proofexecution: 'not_started_proven'. But the same throw arm is reached by two different situations:handlerRevisionguard — the proof is false.The second case is an ordinary rollout race: a handler module deployed one revision behind its manifest pin completes its top-level
await, writes into the workspace and registers module state, and only then fails the revision check. The Harness recordsnot_started_proven,hasHolds()is already false, andreleaseEarlierOwners' settled test releases the owner — so a later rewind or file-history bind is admitted over mutations the durable record asserts never happened. This contradicts the record contract's own precondition (docs/design/2026-09-27-managed-extension-record-contract.md: "A dispatch that the Runtime refuses before any side effect proves nothing started"), which defines the proof by no side effect, not by pre-dispatch timing.executionUnavailable(), so it fences asoutcome_unknowninstead of certifying non-execution. The test that must go red without it: a case besidefails fast when the handler module itself rejectsinpackages/cli/src/serve/managed-hook-runtime.test.tswhose fixture writes a marker at top level before throwing, asserting the receipt does not certify non-execution and that the hold stays.The behaviour is pre-existing (at PR #13243's merge base the same catch wrapped the import, the shape check and the revision guard alike), so it was documented rather than changed there:
docs/design/2026-09-30-managed-hooks-runtime.mdnow states that a module rejected after evaluating in full is not covered by that proof, in both language versions.Round-4 addition to the item above (PR #13243 review round 4, finding R3-2 fix-induced): the same single code also covers a third case, which is the most dangerous one — a trusted handler module that runs top-level side effects (
appendFileSync, opening a socket, registering module state) and only then throws, for example on a failed config parse, a missing dependency or a guard clause.managed-hook-runtime.tshas onetryaround both the module import (which rejects with the module's raw error) and the shape/handlerRevisionguard, and its catch rethrows only the abort and evaluation-timeout cases, so all three shapes settle asmanaged_hook_handler_unavailableand the Harness recordsnot_started_provenfor a hook whose top-level write did happen. Nothing records it as unknown, so a later rewind or file-history bind is admitted over mutations the durable record asserts never happened.evaluationStartedmarker on the operation view, added toexecutionUnavailable()'s negative set. The pin: a case besidefails fast when the handler module itself rejectsinpackages/cli/src/serve/managed-hook-runtime.test.tswhose fixture writes a marker at top level before throwing, asserting the receipt does not certify non-execution and that the hold stays.docs/design/2026-09-30-managed-hooks-runtime.mdand its Chinese twin now state the precondition as the code actually implements it (one code covering rejection before, partway through, and after top-level execution, with the proof not covering effects that already ran), so the documentation no longer over-claims while the code split is pending.Round-5 deferred Suggestions from PR #13243
Source: PR #13243 review round 5 (
3b0f29cbc5era, posted 2026-10-04T10:09Z). The round's one Critical (R4-1 fix-induced: the per-turn retry madeacquire()fallible on an already-acquired Session, so a stale earlier owner's untolerated release refusal failed a healthy live turn) is fixed in that commit. The six Suggestions below are deferred under the author's post-round-five Critical-only cutoff; none is claimed fixed.names a hold-fenced current-owner release instead of leaking the refusal(hosted-hook-session.test.ts) exercises theclose()path that writes an operator-facing stderr line but installs novi.spyOn(stdio, 'writeStderrLineSafe'), unlike its siblings. The production line escapes to the runner's real stderr on every CI run of that file, and the close-path log's format is unasserted.holdFencedRelease()infers "this owner still holds unfinished work" from two codes that do not carry that meaning.managed_runtime_provider_operation_failedis the worker route's else-branch for every error that is not protocol/preparation/unavailable, andmanaged_runtime_identity_conflictisManagedToolConflictError's default code with at least three throw sites on the release path, only one of which is a genuine hold fence. An unrelated refusal is therefore booked as a retained hold and fenced permanently (silently absorbed, retried forever). A durable fix needs a dedicated code, which requires amendingHttpRuntimeTransport.providerFailure's closed switch inpackages/sdk-javain the same change.fencedOwners.delete(id)on the success path — the only thing that letsclose()and DELETE complete again — is unpinned. Every new test keeps the fence throwing unconditionally and none closes after a successful retry.retryingsingle-flight guard has no test. The existingshares one acquisition between parallel callerscase constructs a fresh session, so both callers take theacquiring ??=branch and nothing reachesretryFencedOwners(). Inlining the retry loop would let two parallel Hooks on an already-acquired Session each runreleaseOwner(id)for the same fenced owner.AbortSignal.timeout) plus one byte-identical stderr line on every hook dispatch for the daemon's life. Needs a bounded/deferred retry policy (and interacts with the acknowledged-reclamation design already tracked above).16while production compares against the exportedMANAGED_HOOK_MAX_RUNNING; import the constant so the case tracks a future change to the budget instead of silently testing the wrong number.Also recorded from the same round's verification report (
qwen-triage:verify, verdictfindings— 61/61 scripted assertions passed, "nothing here blocks the merge"): F4 the residual evaluation bound is 600 s in the worst case (max(timeout, 500 ms)with the manifest'stimeout <= 600_000) and the design doc does not state it; F5 the accepted-tradeoff list names one fenced operation while the same permanently-truehasHolds()also fences workspace deactivation and file-history rewind; F6 one raw-path dynamic import remains elsewhere in the repo (outside this PR's scope).Round-8 deferred items from PR #13243
Source: PR #13243 review round 8 (posted 2026-10-05T07:10Z at head
930b3092d7). The round's only posted finding was Critical R7-1 — the abandonment fence had no terminal transition, so a cancel that merely raced a healthy cold module import tombstoned the Hook Session for the worker's lifetime — fixed inbe10a118a1. The two items below were recorded by the review under its convergence posture and not posted inline; they are deferred under the same post-round-five Critical-only cutoff and are not claimed fixed.recovery_blocked/outcome_unknownuntil the evaluation definitively ends and the receipt republishes; the correct claim is "never becomes a permanently blocked turn", with the transient fence named. (packages/cli/src/serve/managed-hook-runtime.tsaround the abandonment guard.)be10a118a1and does not stand; recorded so nobody chases it. The claim was that nothing exercisesclose()concurrently with an in-flight abandoned evaluation, and that a never-settling evaluation would wedgeclose(). Both halves are false.fences a stuck module evaluation cancelled mid-evaluation without wedging closeuses a module whose top-levelawait new Promise(() => {})never settles, cancels it, asserts themanaged_hook_module_evaluation_abandonedreceipt andhasHolds() === true, and only then callsclose(), which resolves. SincehasHolds()isview.state !== 'settled' || moduleEvaluationPending === trueand the view is already settled at that point, the true value can only come frommoduleEvaluationPending, which is cleared solely in the import's settle handlers — so the evaluation is provably still in flight whenclose()runs. Andclose()awaitsentry.done, theexecute()promise, which returns as soon as the abandonment receipt is published without waiting for the import, so a never-settling evaluation cannot wedge it. The wedge exists only when there is no bound at all: removing it makesafterEach'sclose()hang (mutation-verified in round 1).Also carried from the same round's advisory: the review reports the diff has grown 4.4x since it first measured it (79 → 351 source diff lines), that its reverse audit stopped at its round cap without converging, and that findings keep returning to the same two files — it recommends a human decide whether the shape of the change is still right, or split the independent cluster into its own pull request, before further rounds. That scope decision is with the maintainer; the branch also carries two autofix-authored commits (
c5909d530c,930b3092d7) that are not part of the original two-finding scope.