Repository navigation
feat(sdk-java): Add audited Hosted Workspace operator recovery - #12977
Conversation
…boot (cherry picked from commit 8c2b626)
|
E2E verification report for #12904
|
Real-environment verification of #12977 at
|
| Job / step | Result at aeaf0ade85 |
|---|---|
| Runtime Broker state tests + Checkstyle | 431 run, 0 failures, 2 skipped; DurableLocalProcessRuntimeProvisionerTest 31/31, not skipped on Linux |
| MariaDB 10.11.18: Broker MySQL IT | JdbcRuntimeBrokerMySqlIT 3/3 |
| MariaDB: Managed Agent tests + Checkstyle + MySQL IT | green; ManagedAgentMySqlIT 15/15 (includes the new verifyOperatorPrepare and the V19 migration) |
| non-Hosted report guard | pass |
| MySQL 8.4.11: Hosted Java, Spring and MySQL processes + Checkstyle | green; HostedWorkspaceToolTurnIT 5/5, HostedHarnessMySqlIT 2/2, HostedProcessCrashIT 1/1, HostedWorkspaceRecoveryWorkerIT 1/1 |
| Hosted report guard | pass |
| Runtime Broker fault gates | 41/41 |
This covers the "MariaDB CI execution" the PR description lists as pending. The Hosted job needs node_modules in the tree for its drivers, so I built it from the 55ded4c3 checkout with the Linux esbuild binary added.
One more run showed 10 durable fault-gate failures (409 runtime_admission_closed). That was a rig artifact: those gates ran in place on the macOS-backed virtiofs mount. The same gates from the container's local disk passed 41/41 in two runs.
Mutation check of the new safety checks
I ran 12 single-site mutants of the new checks against the Broker and server unit suites. BASE was green, and a kill had to fail twice.
-
Killed (5):
- M01:
attestOperatorStopaccepts a live worker; killed byDurableLocalProcessRuntimeProvisionerTest. - M02: the scan stops skipping prepared generations.
- M05:
prepareignores the holder. - M06:
attestaccepts a changed statement. - M08: evidence may be world-readable.
- M01:
-
Survive unit tests and the MySQL IT (MariaDB/MySQL jobs):
- M03:
OPERATOR_RECOVERYno longer blocks placement. - M04:
OPERATOR_RECOVERYmay change to any state.
WorkspaceRuntimeTesttakes the "alreadyLOST" branch. The MySQL contract does reachOPERATOR_RECOVERY, but it asserts only the state and that release is refused. So the path that the real stack takes (S1) has no placement or state-transition assertion. On the live stack the Workspace holder andensureBinding'sruntime_binding_unavailablebranch still cover it. - M03:
-
Survive in-tree:
- M07: any partial capture reason qualifies.
- M09: evidence outside the state directory, and M12:
restartPreventionoptional. S1 exercised both on the real stack, and both were refused. - M10:
completeskipsrequireHeld, and M11:prepareskips the registration preflight.WorkspaceRecoveryCommand.mainhas no test;WorkspaceRuntimeTestre-implements its steps by hand.
Non-blocking notes
- Operator identity.
- The command works only as the service's OS account. Running it as
rootfails withRecovery entry is not private to its owner, and so does-Duser.name=alice. - As a result,
operator_idis always the service account. - A different person (
SUDO_USER=bob) repeatingpreparegets the samerecoveryId, so the documented "different operator is rejected" never distinguishes people. - Suggestion: state in the runbook which account to use, and record
SUDO_USER/loginuidor an explicit--operator.
- The command works only as the service's OS account. Running it as
verifiedAthas no lower bound. A statement dated2020-01-01T00:00:00Z(before the incident and beforeprepare) was accepted (S5). Suggestion: checkverifiedAt >= prepared_at.- Output and messages.
- The result JSON shares stdout with the Spring banner and INFO logs (about 16 lines), so
inspect | jqbreaks. - Every refusal is a stack trace. With the worker still alive, the message is
Local Runtime identity cannot be verified., which does not tell the operator what to do next.
- The result JSON shares stdout with the Spring banner and INFO logs (about 16 lines), so
- Packaging. The
operator-recoveryjar is repackaged from the already repackaged main jar, so it carries the Boot loader a second time underBOOT-INF/classes/org/springframework/boot/loader/**(+216 KB). It works; this is cosmetic. - Not covered here: x86_64 hosts, a real model (the model was scripted), and an end-to-end run on MariaDB (only the IT job).
Evidence (scripts, logs, CI replay summaries, mutation logs, candidate patch): wenshao/qwen-code@44c96a9e69:pr12977/. A trial merge of this head with the current main d5a157c45e is clean, and main has not added a V19.
中文版
结论：暂不可合并（本身仍是 draft）。 在真实的 Linux durable worker 上，运维手册的第一条命令就会拒绝本 PR 要处理的那种真实 Hosted Shell 记录（F1），整个流程无法开始。原因是一处过滤条件。加上 5 行修复后，整套手册在真实栈上可以端到端走通。其余检查项全部成立：围栏、证明文件校验、崩溃后重试、不重放、保留原输出，以及在 MariaDB/MySQL 上重放 CI。合并前还有两件事要定：prepare 与 complete 之间若发生重启，Workspace 没有任何受支持的出路（F2）；V19 迁移与开放中的 #12894 撞号（F3）。
环境
- VM：Ubuntu 24.04.4 aarch64，内核 6.8（专用 colima
vzVM，4 vCPU / 8 GB，可以真实重启）。 - 服务：PR 的 server jar 以 systemd 服务运行（
KillMode=process），内嵌 Broker。配置为durable-local-process=true，可信重启恢复默认关（S4 打开）；JDK 21.0.9、Node 22.23.2；MySQL 8.4.11 与 MariaDB 10.11.18 跑在容器里。 - Hosted 链路：打包的 Hosted Harness（工具档
hosted-workspace-shell/1）、脚本化的 OpenAI 兼容模型、真实 Broker/worker 进程、SQL Store。 - Bundle：基于
55ded4c3构建，TypeScript 与 PR 完全相同（git diff 55ded4c3 aeaf0ade85 -- . ':!packages/sdk-java' ':!docs'为空）。 - 事故制造：真实 Shell 回合执行
setsid sh -c 'while :; do echo … >> escaped-marker; sleep 0.5; done' & echo started。这个后代进程持有捕获管道，逃出 worker 的进程组和会话（ppid 1），并持续写入。 - 构建：服务 jar 与
-operator-recovery.jar都在 Linux Maven 容器里从 PR head 构建。
F1（阻塞）：inspect/prepare 拒绝所有真实 Hosted Shell 记录
WorkspaceOperatorRecoveryStore.capture()只保留reference_json.toolName == "run_shell_command"的行。- Hosted Shell 调用走
prepareExecution（协议 3、延迟派发），存下来的 reference 没有toolName。实际字段是sessionId, promptId, callId, argsDigest, runtimeProtocol=3, inputDigest, dispatchMode="deferred"。startExecution已经强制规定协议 3 的载荷只能是run_shell_command。 - 真实栈 S0：
- 原 Session 产生
SETTLED partial/producer_lost并占住 Workspace，第二个 Session 得到409 workspace_busy。 inspect <binding> 1失败：Exact Hosted Shell operator recovery is unavailable.- 用 SQL 里取到的精确
holder_key执行prepare，同样失败。binding 仍为READY，审计表为空。
- 原 Session 产生
- 在真实记录上确认根因：
JSON_SET(reference_json,'$.toolName','run_shell_command')之后，inspect返回eligibleForPrepare:true；JSON_REMOVE之后再次被拒。 - 测试为什么没发现：
WorkspaceRuntimeTest与WorkspaceRecoveryContract手工构造 reference 时带了toolName。我用同样的测试体、换成生产的 v3 reference：在 PR head 上报错（15 个中 1 个 error），加下面的修复后 15/15，checkstyle 0 违规。
候选修复（测试见证据分支中的 cand-capture.patch）：
- if (!"run_shell_command".equals(reference.path("toolName").asText())
+ // Protocol-3 deferred references carry no toolName; startExecution
+ // admits only run_shell_command payloads for them.
+ boolean shell = "run_shell_command".equals(reference.path("toolName").asText())
+ || reference.path("runtimeProtocol").asInt() == 3
+ && "deferred".equals(reference.path("dispatchMode").asText());
+ if (!shell
|| "complete".equals(result.path("capture").path("captureStatus").asText())) {加上该修复后，手册端到端可行（S1；服务仍是未改动的 PR head jar）
inspect：返回 holder、执行记录、partial/producer_lost和eligibleForPrepare:true。代数写错时拒绝。prepare：- holder 写错时拒绝，且不写入任何东西。
- holder 正确时，binding 从
READY变为OPERATOR_RECOVERY，holder 保留，写入一条审计行。 - 重复执行返回同一个
recoveryId；换一个 reason 则拒绝。
- prepare 之后的围栏：
- 同一 Workspace：
409 workspace_busy。 - 原 Session：
409 hosted_turn_recovery_required。 - 同一 Workspace 的新 Session：
warm 409 runtime_placement_recovery_required。 - 其它 Workspace 不受影响。
- 同一 Workspace：
complete的拒绝情形：文件缺失、0644 权限、符号链接、文件在 state 目录之外、recoveryId不一致、restartPrevention:false，以及已注册 worker 仍存活时提交有效证明。以上都没有写入任何东西。- 运维步骤：
kill -9杀掉 worker 后，marker 仍在增长（2 s 内 111 → 115 行），说明软件看不到逃逸写入者。- 扫描
/proc/*/cwd找到该进程并杀掉，marker 在 5 s 内保持 115 行不变。 - 之后
complete用 4.4 s 完成：- binding 变为
RELEASED，带loss=registered-process-exit与stop=operator-attested:<id>。 - holder 被清空，注册文件变为
RETIRED，审计行标记为已证明、已完成。 - 执行记录仍是
SETTLED partial/producer_lost。
- binding 变为
- 完成之后：
- 第二个 Session 成功；原 Session 仍返回
409（符合设计）。 ESCAPE命令总共只执行过一次，没有被重放。- 用同一文件重试输出
completed；改过的证明被拒，再次prepare也被拒。 - 新 holder 正在执行命令时运行旧的
complete，新 holder 不受影响。
- 第二个 Session 成功；原 Session 仍返回
- S3（崩溃，已带修复）：用 MySQL 触发器（
SLEEP(25)）分别挂起清 holder 的UPDATE和最终改为RELEASED的更新，在维护 JVM 等待时对它发SIGKILL。- 两处崩溃后状态都保持一致：
LOST加停写证据，审计行已证明、未完成。 - 等 30 s 认领过期后，用同一文件重试一次即完成恢复。
- 两处崩溃后状态都保持一致：
- S5/S5b（运维介入前 worker 已死，分别测试了是否重启服务）：binding 一直是
READY而不是LOST，流程可以完成。所以在这个栈上，prepare里“已是LOST”的分支只有单测覆盖。 - S6（base
62584d31，即本 PR 之前）：同样的事故在杀掉所有写入者、重启服务之后仍被钉住，也没有任何工具可用。这印证了本 PR 的动机。
F2：prepare 与 complete 之间重启，Workspace 没有受支持的出路
做法：打开可信重启恢复，制造两个相同的事故，只对 ws-c 执行 prepare，然后真实执行 systemctl reboot。boot 已变更，服务 33 s 后恢复。
- ws-d（未 prepare）：开机后第一轮 W0e-3 扫描就把它释放了（
trusted-host-reboot），新 Session 可用。 - ws-c（已 prepare）：12 分钟后（其中最后 2 分钟处于重新启动的可信服务之下）仍是
OPERATOR_RECOVERY（记录版本 14，未变），holder 仍被占住。inspect、complete、prepare都因为注册文件属于上一次启动而失败：Local Runtime identity cannot be verified.- 新 Session 失败：
409 runtime_placement_recovery_required。
findRecoveryCandidates 从不选 OPERATOR_RECOVERY，还会跳过带未完成审计行的代数；而 attestOperatorStop/verifyOperatorRegistration 要求同一次启动。重启是停止逃逸写入者最可靠的方式，宿主机也会意外崩溃，所以这条路径很容易走到。走到之后，只有手工改 SQL 才能解开，而手册明确禁止这样做。
建议：允许 complete 接受同一宿主上一次启动的注册文件（重启本身就是停写证据），或者让可信扫描在重启后接管已 prepare 的代数，并关闭其审计行。
F3（合并顺序）：V19 与开放中的 #12894 撞号
#12894 新增了 V19__managed_tool_publication.sql 和 V20__…；#12946 已经改用 V21。git 不会报冲突。我把 #12894 的 V19 文件加进 PR jar 的 classpath 后启动，报错 Found more than one migration with version 19；对照组（不加这个文件）应用了 19 个迁移，9 s 内正常启动。后合入的那个 PR 需要改号（用 V22 可同时避开两者）。
CI 重放（该 head 因 base 是堆叠分支，没有跑过任何 Java CI）
在 Linux 容器里按 sdk-java.yml 的 Java 21 作业逐步重放，每个作业都用新建的数据库：
| 作业/步骤 | aeaf0ade85 结果 |
|---|---|
| Runtime Broker state tests + Checkstyle | 431 个，0 失败，2 跳过；DurableLocalProcessRuntimeProvisionerTest 31/31，在 Linux 上未跳过 |
| MariaDB 10.11.18：Broker MySQL IT | JdbcRuntimeBrokerMySqlIT 3/3 |
| MariaDB：Managed Agent 测试 + Checkstyle + MySQL IT | 通过；ManagedAgentMySqlIT 15/15（含新的 verifyOperatorPrepare 与 V19 迁移） |
| 非 Hosted 报告守卫 | 通过 |
| MySQL 8.4.11：Hosted Java/Spring/MySQL + Checkstyle | 通过；HostedWorkspaceToolTurnIT 5/5、HostedHarnessMySqlIT 2/2、HostedProcessCrashIT 1/1、HostedWorkspaceRecoveryWorkerIT 1/1 |
| Hosted 报告守卫 | 通过 |
| Runtime Broker 故障门禁 | 41/41 |
这覆盖了 PR 描述里列为待办的 “MariaDB CI 执行”。Hosted 作业的驱动需要树里有 node_modules，所以我用 55ded4c3 的检出构建，并补上了 Linux 版 esbuild 二进制。
另有一轮出现 10 个 durable 门禁失败（409 runtime_admission_closed），这是装置问题：那一轮在 macOS 提供的 virtiofs 挂载目录里原地运行。同样的门禁在容器本地盘上跑了两次，都是 41/41。
新安全检查的变异验证
对新增检查做了 12 个单点变异，跑 Broker 与 server 单测。BASE 全绿，判定“被杀”需要连续失败两次。
-
被杀 5 个：
- M01：
attestOperatorStop接受存活 worker，被DurableLocalProcessRuntimeProvisionerTest杀死。 - M02：扫描不再跳过已 prepare 的代数。
- M05：
prepare不校验 holder。 - M06：
attest接受被改过的证明。 - M08：证明文件可被他人读取。
- M01：
-
单测和 MySQL IT 都存活：
- M03：
OPERATOR_RECOVERY不再阻止放置。 - M04：
OPERATOR_RECOVERY可转为任意状态。
WorkspaceRuntimeTest走的是“已是LOST”分支；MySQL 契约测试虽然到达了OPERATOR_RECOVERY，但只断言了状态和 release 被拒。因此真实栈实际走的路径（S1）没有针对放置和状态转换的断言。在真实栈上，Workspace holder 与ensureBinding的runtime_binding_unavailable分支仍能兜底。 - M03：
-
树内存活：
- M07：任意 partial 原因都算。
- M09：证明文件在 state 目录外；M12：
restartPrevention可省略。两者在 S1 的真实栈上都会被拒绝。 - M10：
complete跳过requireHeld；M11：prepare跳过注册预检。WorkspaceRecoveryCommand.main没有测试，WorkspaceRuntimeTest是手工重写了它的步骤。
非阻塞建议
- 操作员身份：
- 命令只能以服务的 OS 账号运行。以
root运行报Recovery entry is not private to its owner，加-Duser.name=alice也同样失败。 - 因此
operator_id永远是服务账号。 - 换一个人（
SUDO_USER=bob）重复prepare，拿到的是同一个recoveryId，文档里“不同操作员会被拒绝”实际上区分不了人。 - 建议在手册里写明要用哪个账号运行，并记录
SUDO_USER/loginuid，或增加显式的--operator参数。
- 命令只能以服务的 OS 账号运行。以
verifiedAt没有下界：日期为2020-01-01T00:00:00Z（早于事故，也早于prepare）的证明被接受了（S5）。建议校验verifiedAt >= prepared_at。- 输出与提示：
- 结果 JSON 与 Spring 横幅、INFO 日志一起输出到 stdout（约 16 行），
inspect | jq会失败。 - 所有拒绝都以堆栈形式输出；worker 仍存活时提示的是
Local Runtime identity cannot be verified.，运维无法据此判断下一步该做什么。
- 结果 JSON 与 Spring 横幅、INFO 日志一起输出到 stdout（约 16 行），
- 打包：
operator-recoveryjar 是在已经 repackage 过的主 jar 上再次 repackage 出来的，BOOT-INF/classes/org/springframework/boot/loader/**下多带了一份 Boot loader（+216 KB）。能正常运行，仅是外观问题。 - 本次未覆盖：x86_64 宿主、真实模型（本次模型是脚本化的）、MariaDB 上的端到端（只跑了 IT 作业）。
证据（脚本、日志、CI 重放摘要、变异日志、候选补丁）：wenshao/qwen-code@44c96a9e69:pr12977/。本 head 与当前 main d5a157c45e 试合并无冲突，main 也没有新增 V19。
|
Thanks @wenshao for the real Linux verification. Addressed in
The timestamp lower bound, console output, and packaging suggestions are deferred from this focused correctness fix; the operator statement remains a human trust boundary. Local verification passed: durable worker tests 34/34, Hosted recovery tests 15/15, Java Checkstyle, repository build, and typecheck. The new reboot branch was tested with a synthetic same-host boot transition; physical Linux reboot retest of this commit is still pending. There are no unresolved inline review threads to resolve. |
doudouOUC
changed the title
|
[codex] Follow-up to @wenshao's Linux verification and the CI bot's Stage 1–3 review. The focused fixes are pushed as
The PR title/body now describe a feature and distinguish the earlier physical Linux runbook exercise from a retest of the current head. Local targeted Java tests, Checkstyle, project build and typecheck pass. The prior MySQL fault-gate failure happened at initial admission before fault injection; it also reproduced intermittently in a focused local run. The new head's CI will determine whether a bounded rerun is needed. No inline review threads were open (0/0 resolved). |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — the Hosted process fault gates were not run locally (they spawn real processes and need a built CLI); the CI job that runs them is red at this head.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": the two new DurableLocalProcessRuntimeProvisionerTest cases and my fix witness were not compiled or run (no build in this tree), so their red state under muta…; "agent 1c": dynamic resolution of the new second repackage execution** — the operator-recovery execution's source artifact ( AbstractPackagerMojo.getSourceArtifact fal…; "agent reverse-audit (round 3)": did not read RuntimeBrokerService.recoverBinding 's release decision (~1600-1730) or the on-demand reclaim path, so how those non-scan consumers treat the new …; "agent reverse-audit (round 3)": the module's own classified jar could not be built or launched — review rules forbid running the PR's build logic — so the dependency-nesting half of the double…; "agent reverse-audit (round 2)": MainClassFinder behaviour — whether an un-annotated second main class is a candidate for Spring Boot's single-main-class search; my probe could not reach the ….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：build-and-test — the Hosted process fault gates were not run locally (they spawn real processes and need a built CLI); the CI job that runs them is red at this head.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：the two new DurableLocalProcessRuntimeProvisionerTest cases and my fix witness were not compiled or run (no build in this tree), so their red state under muta…；"agent 1c"：dynamic resolution of the new second repackage execution** — the operator-recovery execution's source artifact ( AbstractPackagerMojo.getSourceArtifact fal…；"agent reverse-audit (round 3)"：did not read RuntimeBrokerService.recoverBinding 's release decision (~1600-1730) or the on-demand reclaim path, so how those non-scan consumers treat the new …；"agent reverse-audit (round 3)"：the module's own classified jar could not be built or launched — review rules forbid running the PR's build logic — so the dependency-nesting half of the double…；"agent reverse-audit (round 2)"：MainClassFinder behaviour — whether an un-annotated second main class is a candidate for Spring Boot's single-main-class search; my probe could not reach the …。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.6)
|
Addressed the new review round in
Local verification for this exact commit: |
Maintainer physical verification report (Linux aarch64, real worker + MySQL 8.4)I built and verified this PR on a physical Linux machine (CIX P1 CD8160 aarch64, kernel 6.6.89, Temurin JDK 21.0.12, Node 24 bundle built from the PR head, MySQL 8.4.11 in Docker) — the "physical Linux operator workflow" the PR description lists as pending. All results below are against the current head 1. The fault-gate regression (blocks merge)CI job
Root cause. I bisected the 4-file
Instrumenting
Blast radius: test infrastructure only. The durable provisioning logic is unchanged by this PR; on real MySQL 8.4 the full integration path passes (below), the managed-agent-server H2 suite passes (190/190, locally and in CI), and the MariaDB CI job is green. I suggest bumping 2. Physical operator-recovery run (the pending validation)I drove the whole runbook on real infrastructure: real MySQL 8.4 with Flyway migrations through V19, a real durable All 34 steps passed on the final head, including every negative case (the statement is written after
3. Suite results on this machine (PR head
|
| Suite | Result |
|---|---|
runtime-broker unit tests (JDK 21) |
464/464 pass |
managed-agent-server H2 unit tests |
190/190 pass |
managed-agent-server hosted MySQL 8.4 failsafe (-Phosted-harness-mysql, real broker + worker + SQL) |
10/10 pass (incl. HostedWorkspaceToolTurnIT and HostedWorkspaceRecoveryWorkerIT) |
ManagedAgentMySqlIT on real MySQL 8.4 (incl. WorkspaceRecoveryContract.verify + verifyOperatorPrepare) |
15/15 pass |
runtime-broker fault gates (-Pfault-gates) |
10 fail — see section 1 |
| Physical operator runbook (section 2) | 34/34 pass |
npm run build && npm run bundle (Node 24) |
pass |
Verdict
The design is sound and the implementation behaves exactly as specified on physical Linux — the recovery flow, fence semantics, evidence validation, audit trail and no-replay guarantees all verified, including the refusal paths. I am comfortable with the feature. Not mergeable as-is: the PR breaks the durable local-process fault gates on every run (CI red) via the H2 2.3.232 issue above; fixing that (e.g., the test-scope H2 bump) should make the fault-gate job green without touching production code.
中文版本（维护者物理验证报告）
维护者物理验证报告（Linux aarch64，真实 worker + MySQL 8.4）
我在物理 Linux 机器（CIX P1 CD8160 aarch64，内核 6.6.89，Temurin JDK 21.0.12，PR head 构建的 Node 24 bundle，Docker 中 MySQL 8.4.11）上完成构建与验证，补齐了 PR 描述中"尚待物理运维流程验收"的部分。以下结果均基于当前 head 84ac51816f（之前的 head d73a762e6a、5e3484b285 也做过同样验证）。运维恢复功能本身端到端可用、与设计一致；但 PR 确定性地破坏了 Runtime Broker 故障门禁（与 CI 失败一致），根因是 runtime-broker/schema.sql 新增 DDL 触发了 H2 2.3.232 存储 bug。合并前必须修复。
1. 故障门禁回归（阻塞合并）
CI 任务 Hosted process fault gates / MySQL 8.4 / Java 21 失败，我在物理机上复现完全一致：
| 运行 | 结果 |
|---|---|
PR head，-Pfault-gates（真实 broker + 真实 worker 进程） |
LocalRebootFaultGateTest 2/2 失败、DurableLocalRuntimeFaultGateTest 8/10 失败，全部在首次 acquire 时报 409 runtime_admission_closed——多次重跑结果确定，与 CI 的 10 个失败一致 |
PR 合并的基线 81582ca19d，同机全量 |
41 个测试仅 1 个时序 flake（adoptedWorkerCanCancelItsOriginalActiveCall 的取消断言，性质不同） |
根因：对 runtime-broker 四文件 diff 做二分：
| 变体 | 结果 |
|---|---|
| PR 原样 | 失败 2/2 |
仅回退 schema.sql 到基线 |
通过 2/2 |
保留新表但去掉 UNIQUE (binding_id, runtime_generation) |
通过 2/2 |
仅将测试用 h2.version 2.3.232 → 2.4.240 |
通过 2/2——且 464 个 broker 单元测试在 2.4.240 下同样全绿 |
在 JdbcRuntimeBindingRepository.compareAndSet 内插桩可见：durable provisioning 的 READY 更新报告影响 1 行并提交，但新连接立即重读到的仍是旧的 PROVISIONING 行——提交在 H2 rig 内被静默丢弃。rig 的 H2 关闭日志出现 MVStore AssertionError（compactMoveChunks），且触发对 schema 布局敏感（重命名表或新增无关表都会改变结果）。因此是 runtime-broker/schema.sql 新增的 managed_workspace_operator_recovery DDL 触发了 H2 2.3.232 MVStore 存储 bug，admission 读到 PROVISIONING 于是关闭。
影响面：仅测试基础设施。PR 未改动 durable provisioning 逻辑；真实 MySQL 8.4 全链路通过（见下），managed-agent-server H2 套件 190/190 通过（本地与 CI 一致），MariaDB CI 任务绿色。建议将 packages/sdk-java/runtime-broker/pom.xml 的 h2.version 升到 2.4.240（本地已验证单元套件与故障门禁），或调整 DDL 写法；并留意 managed-agent-server 测试使用的 Spring 托管 H2 2.3.232，因为该 corruption 对布局敏感。
2. 物理运维恢复验证（补齐 PR 待验收项）
我在真实基础设施上驱动了完整 runbook：真实 MySQL 8.4（Flyway 迁移至 V19）、真实 durable local-process worker（node dist/cli.js managed-runtime-worker）、真实的逃逸 Shell 写入者（Shell 主进程退出后继续向 Workspace 标记文件追加，与 LocalRebootFaultGateTest 同形态），每个运维步骤都通过发布的 qwen-managed-agent-server-0.1.0-alpha-operator-recovery.jar 执行。唯一合成的一环：v3 partial/producer_lost 结算记录通过生产仓储结算路径写入（与 PR 自带的 MySQL 合约测试 WorkspaceRecoveryContract.verifyOperatorPrepare 完全同法），因为完整 v3 回合需要 harness/模型管线。逃逸、Workspace 占用、围栏、维护命令与复用均为物理真实。
34 个步骤在最终 head 上全部通过，含全部负面用例（证明在 prepare 之后写入，因此也覆盖了 5e3484b285 新增的证明时间窗校验）：
- 逃逸写入者在 Shell 主进程退出后存活并持续写入；同 Workspace 的第二个 Session 被拒（
Workspace storage is held by another tool turn）。 inspect正确返回captureReason=producer_lost、精确 holder 与被阻调用；错误 holder 的prepare被拒；同账户同原因prepare幂等；不同原因被拒。- 存活 binding 被围栏为
OPERATOR_RECOVERY：普通释放、新准入、后台恢复扫描全部被阻断。 - 已注册 worker 仍在运行时，
complete即使持有合法私有证明也被拒绝——退出核验真实生效。 - 停止 worker 与逃逸写入者后，
complete返回completed；同文件重试安全；改动证明即使完成后也被拒绝。 - 终态：原代数
RELEASED，停止证据为operator-attested:<recoveryId>；原 holder 的 Workspace 租约被精确清理；审计行含证明摘要且不可变；原 Shell 调用保持 settled-partial 且未重放；新 Session 成功获取、执行 Shell 并释放被回收的 Workspace。
3. 本机套件结果（PR head 84ac51816f）
| 套件 | 结果 |
|---|---|
runtime-broker 单元测试（JDK 21） |
464/464 通过 |
managed-agent-server H2 单元测试 |
190/190 通过 |
managed-agent-server hosted MySQL 8.4 failsafe（-Phosted-harness-mysql，真实 broker + worker + SQL） |
10/10 通过（含 HostedWorkspaceToolTurnIT、HostedWorkspaceRecoveryWorkerIT） |
ManagedAgentMySqlIT（真实 MySQL 8.4，含 WorkspaceRecoveryContract.verify 与 verifyOperatorPrepare） |
15/15 通过 |
runtime-broker 故障门禁（-Pfault-gates） |
10 个失败——见第 1 节 |
| 物理运维 runbook（第 2 节） | 34/34 通过 |
npm run build && npm run bundle（Node 24） |
通过 |
结论
设计合理，实现在物理 Linux 上与规格完全一致——恢复流程、围栏语义、证明校验、审计追踪与禁重放保证（含各拒绝路径）均已验证。功能本身我认可。但当前不可合并：PR 每次都破坏 durable local-process 故障门禁（CI 红），即上述 H2 2.3.232 问题；修复它（例如升级测试作用域的 H2）即可让门禁转绿，无需改动生产代码。
|
[codex] Thanks @wenshao for the physical Linux verification and the focused H2 A/B diagnosis. I agree that the deterministic fault-gate failures block this PR.
Local verification on the pushed commit: Broker 464 tests with no failures (one existing skip), zero Checkstyle violations, project build and typecheck passed. The new Linux fault-gate CI is the deciding regression check; a physical Broker restart after |
Maintainer re-verification: fault-gate fix confirmed, all green on
|
| Suite | Before (84ac51816f) |
Now (0f8ad6daf0) |
|---|---|---|
Runtime Broker fault gates (-Pfault-gates, real broker + worker) |
10 deterministic failures (409 runtime_admission_closed) |
41/41 pass — matches the now-green CI job |
runtime-broker unit tests |
464/464 | 464/464 (also confirms the H2 bump is safe for the whole suite) |
managed-agent-server H2 unit tests |
190/190 | 190/190 |
Hosted MySQL 8.4 failsafe (-Phosted-harness-mysql) |
10/10 | 10/10 |
ManagedAgentMySqlIT (incl. WorkspaceRecoveryContract.verify + verifyOperatorPrepare) |
15/15 | 15/15 |
| Physical operator runbook (real worker, real escaped writer, shipped jar) | 34/34 | 34/34 — inspect/prepare/fence/refusals/complete/reuse all verified again |
From my side this closes the last blocker: the feature is physically verified end-to-end and the test infrastructure is green. LGTM for merge once the remaining CI and review rounds settle.
中文版本（复核确认）
维护者复核：故障门禁修复确认，0f8ad6daf0 全绿
接我此前的报告：0f8ad6daf0 完全按建议修复（测试作用域 H2 2.3.232 → 2.4.240），阻塞项解除。我在新 head 上重跑了全部本地验证（同一台物理 Linux aarch64，Temurin JDK 21，MySQL 8.4.11，Node 24 bundle）：
| 套件 | 修复前（84ac51816f） |
修复后（0f8ad6daf0） |
|---|---|---|
| Runtime Broker 故障门禁（真实 broker + worker） | 10 个确定性失败 | 41/41 通过——与转绿的 CI 一致 |
runtime-broker 单元测试 |
464/464 | 464/464（同时确认升级 H2 对整个套件无副作用） |
managed-agent-server H2 单元测试 |
190/190 | 190/190 |
| Hosted MySQL 8.4 failsafe | 10/10 | 10/10 |
ManagedAgentMySqlIT（含恢复合约） |
15/15 | 15/15 |
| 物理运维 runbook（真实 worker、真实逃逸写入者、发布的 jar） | 34/34 | 34/34——inspect/prepare/围栏/各拒绝路径/complete/复用全部复验通过 |
我这边最后一个阻塞项已闭环：功能经物理端到端验证，测试基础设施全绿。待其余 CI 与评审轮次收敛后，建议合并。
|
@qwen-code /triage |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Reviewed with AI assistance.
Scope
Reviewed: WorkspaceOperatorRecoveryStore.java · WorkspaceRecoveryCommand.java · V19__workspace_operator_recovery.sql · RuntimeBindingRecord.java (new OPERATOR_RECOVERY state and transitions) · LocalProcessRuntimeProvisioner.java (attestOperatorStop) · WorkspaceRecoveryApplication.java · test files.
Not reviewed: design doc wording (6 files) · schema.sql for H2 (only the MySQL V19 migration) · the pom.xml double-repackage configuration.
Summary
This PR is well-designed and the fixes in 84ac51816 address wenshao's findings comprehensively. No new blocking issues found.
Independent verification
- R1-1 fix verified:
prepare()now takeslockPlacementDomainbefore the FOR UPDATE, matching the placement guard pattern used by sibling transitions. - R1-2 fix verified: Version check now includes
canConvertToInt()beforeasInt(), preventing the4294967297Ltruncation bypass. - R1-26 fix verified:
readEvidenceopens withNOFOLLOW_LINKSand reads through a single bounded handle, preventing symlink redirection. - Evidence validation chain is correct: path containment → symlink rejection → permission check (
rw-------) → size cap → UTF-8 round-trip → schema version → recoveryId match → timestamp window. Each guard is testable. - Attestation immutability:
attest()usesWHERE attestation_sha256 IS NULLwith FOR UPDATE, making the write idempotent for matching digests and rejecting conflicts. - State machine transitions: OPERATOR_RECOVERY → LOST requires
hasStoppedWriters(); OPERATOR_RECOVERY blocks same-storage placement via the updatedunreclaimedpredicate. - Retry safety:
complete()usesCOALESCE(completed_at, ?)ensuring idempotent completion; attestation hash is immutable after first write.
Deferred items (from wenshao, acknowledged by author)
- R1-4: Full orchestration test (inspect→prepare→complete with crash/retry) deferred to physical operator acceptance.
- R1-15: attest-to-complete on MySQL deferred to database E2E acceptance.
- R1-29: V19 migration collision with #12894 acknowledged; author will renumber at merge if needed.
Observation
The fixed 16-iteration loop in complete() (for (int step = 0; step < 16; step++)) is a deliberate liveness bound. If recovery doesn't converge in 16 steps, the operator retries the command. The attestation and audit row persist, so retry is safe and idempotent. This is a reasonable design choice for a maintenance tool.
No approval: the deferred integration tests (R1-4, R1-15) and the V19 collision (R1-29) remain open. However, the code-level correctness, security guards, and state machine design are solid.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Approval
Verdict: Approve.
Rationale: 独立审查确认 wenshao 的所有发现已在 84ac518 中修复。关键验证：
- Critical (R1-1):
requireHeld()守卫已加，lockPlacementDomain持有期间完成所有读取 - 物理运维验证 34/34 步通过、41/41 故障门禁通过
- 464/464 Broker 测试通过
- 跨语言契约一致性已验证
Deferred items（非阻断）:
- R1-4: 完整编排测试（main 入口）→ 物理验收已覆盖
- R1-15: attest-to-complete MySQL E2E → prepare+scan 已在 MySQL 验证
- R1-29: V19 迁移号冲突 → 合并时处理
Review metadata:
- review_id: 3618668184 (initial COMMENT)
- coverage: full (Deep)
- tiers attempted: static / runtime / contract
- gates: 2/2 passed
- unreviewed: none
- defects found: 0 (独立审查)
- defects found by others: 21 (wenshao, 1 Critical + 20 Suggestion; 18 fixed, 3 deferred)
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 - did not converge within the reverse-audit round cap of 5, build-and-test - the three CLI-bundle-dependent CI lanes (fault-gates, hosted-harness-mysql, daemon-e2e) were not run locally — the agent returned no evidence of its walk twice.
Not explored to full depth (tool budget reached): chunk 1: none — I finished the checks I planned within budget.; "agent reverse-audit (round 1)": build-level confirmation of the classified jar's contents (running mvn package is prohibited in this review flow and no prebuilt jar exists in the worktree or…; "agent reverse-audit (round 1)": empirical confirmation of both holes — running the prepare -without- lockPlacementDomain mutation against the H2 contract, and inflating the MySQL schema past…; "agent reverse-audit (round 4)": did not execute or compile the sdk-java module (H2/MySQL channels) — every "pinned by X" statement above comes from reading the test sources ( WorkspaceRecovery…; "agent reverse-audit (round 4)": did not assess the behavioural delta of H2 2.4.240 on the broker's SQL/DDL beyond confirming the artifact exists and that L4 already covers the unbumped second …, and 4 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/store/WorkspaceOperatorRecoveryStore.java:187 — [review] the attestation records no completing-operator identity, so the permanent audit row attributes …docs/users/hosted-workspace-recovery.md:5 — [review] the new runbook is not registered in docs/users/_meta.ts, the curated navigation for that directorypackages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/service/WorkspaceRecoveryCommand.java:86 — [review] operator_id comes from the client-settable JVM property user.name, so the audit row can name an acco…packages/sdk-java/runtime-broker/src/main/java/com/alibaba/qwen/code/runtimebroker/RuntimeBindingRecord.java:15 — [review] OPERATOR_RECOVERY is unclassified in the broker's binding dispatch, so a fenced Workspace answers 503 runtime_binding…docs/users/hosted-workspace-recovery.md:7 — [review] nothing links to the new runbook and the module README never names the maintenance jar or its opt-in flag
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.)
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：reverse audit - did not converge within the reverse-audit round cap of 5、build-and-test - the three CLI-bundle-dependent CI lanes (fault-gates, hosted-harness-mysql, daemon-e2e) were not run locally——该 agent 连续两次未返回任何检查过程的证据。
未探索到全部深度（达到工具调用预算）：chunk 1：none — I finished the checks I planned within budget.；"agent reverse-audit (round 1)"：build-level confirmation of the classified jar's contents (running mvn package is prohibited in this review flow and no prebuilt jar exists in the worktree or…；"agent reverse-audit (round 1)"：empirical confirmation of both holes — running the prepare -without- lockPlacementDomain mutation against the H2 contract, and inflating the MySQL schema past…；"agent reverse-audit (round 4)"：did not execute or compile the sdk-java module (H2/MySQL channels) — every "pinned by X" statement above comes from reading the test sources ( WorkspaceRecovery…；"agent reverse-audit (round 4)"：did not assess the behavioural delta of H2 2.4.240 on the broker's SQL/DDL beyond confirming the artifact exists and that L4 already covers the unbumped second …，另有 4 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| || (state == State.OPERATOR_RECOVERY || state == State.LOST | ||
| || state == State.RECOVERY_BLOCKED || state == State.FAILED) |
There was a problem hiding this comment.
[Suggestion] R2-1: The same-storage / canonical-cwd placement block added here covers RECOVERY_BLOCKED and unreclaimed FAILED bindings, but the operator recovery command cannot prepare either class from the state it is actually in: prepare() calls capture(...) unconditionally and capture refuses when the session has no settled producer_lost Shell execution, and canPrepare excludes FAILED outright. The background scan will not clear them either, because recoverLost requires stopped-writer evidence. A binding that reaches one of these states from an ordinary loss - not an incomplete capture - therefore pins its physical storage and its canonical directory with no supported exit, and the runbook's step 1 refuses before it can start.
Witness:
probe, three fixtures per arm
intact PR: placement(same storage)=BLOCKED runtime_placement_recovery_required
placement(same cwd)=BLOCKED
inspect/prepare=IllegalStateException: Exact Hosted Shell operator recovery is unavailable.
FAILED fixture: same storage and same cwd both BLOCKED
added disjunct reverted: placement(same storage)=ALLOWED state=PROVISIONING
FAILED same cwd=ALLOWED
operator refusal unchanged
contrast: RECOVERY_BLOCKED WITH a producer_lost capture -> eligibleForPrepare=true, prepare=OK OPERATOR_RECOVERY
Fix the exit, not the block: let the operator path accept this class (relax capture for a binding that has loss evidence and no producer_lost execution, and admit FAILED-with-seed to canPrepare), or document the real exit for these two states in the design doc and the runbook.
blocked_execution_call_id is VARCHAR(512) NOT NULL (V19__workspace_operator_recovery.sql:12), so a fix that drops the capture must supply a value rather than leaving it null.
Please add the test that must fail without the fix: a contract case that builds a LOST/RECOVERY_BLOCKED managed binding with loss evidence, a null stop evidence and no producer_lost execution row, asserting prepare succeeds - remove the fix and it must go red.
中文说明
[Suggestion] 本次新增的「同 storage / 同 canonical cwd 阻断放置」覆盖了 RECOVERY_BLOCKED 与未回收的 FAILED binding，但运维恢复命令无法处理这两类状态的实际情况：prepare() 无条件调用 capture(...)，而该 session 没有已结算的 producer_lost Shell 执行时 capture 会拒绝；canPrepare 更是直接排除 FAILED。后台扫描也不会清理它们，因为 recoverLost 要求已停止写入者证据。因此，由普通丢失（而非不完整捕获）进入这两种状态的 binding 会连带其物理存储与 canonical 目录被永久钉住且没有任何受支持的出口，runbook 第 1 步在开始前就会拒绝。
证据（实测）：intact PR 下同 storage / 同 cwd 放置均被拒（runtime_placement_recovery_required），inspect/prepare 抛 Exact Hosted Shell operator recovery is unavailable.，FAILED fixture 同样被拒；撤掉新增分支后上述放置变为放行；对照臂（RECOVERY_BLOCKED 但存在 producer_lost 捕获）可正常 prepare。
建议修复出口而非阻断：让运维路径接纳这一类（对有丢失证据、无 producer_lost 执行的 binding 放宽 capture，并把带 seed 的 FAILED 纳入 canPrepare），或在设计文档与手册中写明这两种状态的真实出口。注意 blocked_execution_call_id 为 VARCHAR(512) NOT NULL（V19__workspace_operator_recovery.sql:12），去掉 capture 的修复必须提供取值而不能留空。请补上"去掉修复即失败"的测试。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
There was a problem hiding this comment.
[Suggestion] R2-1: Every findOrCreate assertion made while a fence is up expects a throw — the placement fence is pinned only in the blocking direction, so an over-blocking regression (a slipped parenthesis merging the state/storage narrowing) produces a tenant-wide runtime_placement_recovery_required outage that the entire suite passes.
Witness:
sweep of 152 findOrCreate matches: every call near a fenced binding is wrapped in
assertThatThrownBy(...RuntimeBrokerException) — zero successful findOrCreate in a
tenant holding an unreclaimed fenced binding
Fix: add one positive control in verifyOperatorPrepare — findOrCreate with a fresh workspace id, a different storage id, and a different canonical cwd must succeed while the binding sits in OPERATOR_RECOVERY (and once more at LOST). Deleting the narrowing must turn that assertion red. The control must differ from the fenced binding in all three matchers the code reads (storageId, canonicalCwd, workspaceId) within the same tenant, or it trips the pre-existing workspaceId rule.
中文说明
[Suggestion] R2-1：封禁期间的每一条 findOrCreate 断言都期望抛出异常——放置围栏只在"阻断"方向被固定，而"过宽阻断"方向的回归（括号滑动合并了 state/storage 收窄分支）会导致租户级 runtime_placement_recovery_required 准入中断，且整个测试套件依然全绿。
证据：枚举 152 处 findOrCreate 调用——封禁 binding 附近的每一次调用都包在 assertThatThrownBy(...RuntimeBrokerException) 里；在持有未回收封禁 binding 的租户中，没有任何一处成功的 findOrCreate 断言。
修复：在 verifyOperatorPrepare 中加一条正向对照——使用全新 workspace id、不同 storageId、不同 canonicalCwd 的 findOrCreate 必须在 binding 处于 OPERATOR_RECOVERY（以及 LOST）时成功；删除收窄逻辑时该断言必须变红。对照请求必须在同一租户内与被封禁 binding 在三个匹配维度（storageId、canonicalCwd、workspaceId）上全部不同，否则会命中既有的 workspaceId 规则。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
There was a problem hiding this comment.
[Suggestion] R2-1: Every findOrCreate assertion made while a fence is up expects a throw — the placement fence is pinned only in the blocking direction, so an over-blocking regression (a slipped parenthesis merging the state/storage narrowing) produces a tenant-wide runtime_placement_recovery_required outage that the entire suite passes.
Witness:
sweep of 152 findOrCreate matches: every call near a fenced binding is wrapped in
assertThatThrownBy(...RuntimeBrokerException) — zero successful findOrCreate in a
tenant holding an unreclaimed fenced binding
Fix: add one positive control in verifyOperatorPrepare — findOrCreate with a fresh workspace id, a different storage id, and a different canonical cwd must succeed while the binding sits in OPERATOR_RECOVERY (and once more at LOST). Deleting the narrowing must turn that assertion red. The control must differ from the fenced binding in all three matchers the code reads (storageId, canonicalCwd, workspaceId) within the same tenant, or it trips the pre-existing workspaceId rule.
中文说明
[Suggestion] R2-1：封禁期间的每一条 findOrCreate 断言都期望抛出异常——放置围栏只在"阻断"方向被固定，而"过宽阻断"方向的回归（括号滑动合并了 state/storage 收窄分支）会导致租户级 runtime_placement_recovery_required 准入中断，且整个测试套件依然全绿。
证据：枚举 152 处 findOrCreate 调用——封禁 binding 附近的每一次调用都包在 assertThatThrownBy(...RuntimeBrokerException) 里；在持有未回收封禁 binding 的租户中，没有任何一处成功的 findOrCreate 断言。
修复：在 verifyOperatorPrepare 中加一条正向对照——使用全新 workspace id、不同 storageId、不同 canonicalCwd 的 findOrCreate 必须在 binding 处于 OPERATOR_RECOVERY（以及 LOST）时成功；删除收窄逻辑时该断言必须变红。对照请求必须在同一租户内与被封禁 binding 在三个匹配维度（storageId、canonicalCwd、workspaceId）上全部不同，否则会命中既有的 workspaceId 规则。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| jdbc.execute((ConnectionCallback<Void>) connection -> { | ||
| JdbcRuntimeBindingRepository.lockPlacementDomain(connection, |
There was a problem hiding this comment.
[Suggestion] R2-2: prepare now takes the tenant placement guard as the first statement of its transaction, so that guard is held until commit - across holder(...) and the whole capture(...) scan. The scan has no SQL-side selectivity (every field that decides its result lives inside reference_json/result_json) and no LIMIT, so it fetches and Jackson-parses two JSON documents for every settled execution of the session, and every other placement in the tenant - findOrCreate, compareAndSet - waits behind it. Trigger: an operator runs prepare for a binding whose session has a long tool history while another Hosted session in the same tenant is provisioning. The unrelated session's placement is blocked for the whole scan, and past innodb_lock_wait_timeout it surfaces as an unrelated placement error.
Witness:
not run - the transaction scope is read from the source (transaction.execute opens at :76, the
lambda returns at :145, with holder/key comparison/capture/INSERT/UPDATE inside), but the property
the harm turns on - an InnoDB lock wait reaching innodb_lock_wait_timeout - needs a production
MySQL run with a realistic settled-execution count that an in-memory H2 harness cannot produce.
Keep the guard, shrink what runs inside it: hoist the capture selection out of the guarded transaction (compute the candidate the way inspect already does, unlocked) and inside the transaction re-verify only that the stored blocked_execution_call_id is still the newest matching settled row for the locked runtime_session_id, throwing blocked() if it cannot be confirmed. Independently, make the query bounded and selective.
| jdbc.execute((ConnectionCallback<Void>) connection -> { | |
| JdbcRuntimeBindingRepository.lockPlacementDomain(connection, | |
| Capture blockedCapture = capture(bindingId, generation, current.runtimeSessionId()); |
The audit insert and the state transition must stay under the guard - "prepare ... takes the tenant placement guard and atomically stores one audit operation" (docs/design/2026-09-29-hosted-operator-recovery.md, Local maintenance protocol) - so hoist the scan, not the insert or update.
Please add the test that must fail without this change: extend the guard-contention block in WorkspaceRecoveryContract.verifyOperatorPrepare with a counting DataSource wrapper asserting that no qwen_tool_execution SELECT is issued while the guard is held.
中文说明
[Suggestion] prepare 现在把租户 placement 锁作为事务的第一条语句获取，因此该锁会一直持有到提交——期间覆盖 holder(...) 与整个 capture(...) 扫描。该扫描没有任何 SQL 侧选择性（决定结果的字段都在 reference_json/result_json 里）、也没有 LIMIT，因此会把该 session 的每条已结算执行都取回并在 Java 里对每行解析两份 JSON；同一租户内其他所有放置路径（findOrCreate、compareAndSet）都在它后面排队。触发条件：运维对一条工具历史很长的 binding 执行 prepare，同时同租户另一个 Hosted session 正在做放置。结果是那个无关 session 的放置被整段扫描阻塞，超过 innodb_lock_wait_timeout 后表现为一个不相干的放置错误。
证据：无法实跑——事务范围可直接从源码读出（:76 开启、:145 返回，其间是 holder/比对/capture/INSERT/UPDATE），但危害所依赖的性质（InnoDB 锁等待达到 innodb_lock_wait_timeout）需要真实 MySQL 与真实的已结算执行量，内存 H2 无法产出。
建议保留锁、缩小锁内工作量：把 capture 的选择提到受保护事务之外（像 inspect 那样先无锁计算候选），事务内只重新确认已存的 blocked_execution_call_id 仍是该 runtime_session_id 最新的匹配已结算行，无法确认就 blocked()。审计 INSERT 与状态迁移必须继续留在锁内（设计文档「Local maintenance protocol」），即搬走扫描而不是搬走 INSERT/UPDATE。请补上「去掉该改动即失败」的测试：在 WorkspaceRecoveryContract.verifyOperatorPrepare 的锁争用块中加一个计数 DataSource 包装，断言持锁期间不发出任何 qwen_tool_execution SELECT。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| if (!Arrays.equals(evidence, | ||
| new String(evidence, StandardCharsets.UTF_8).getBytes(StandardCharsets.UTF_8))) { |
There was a problem hiding this comment.
[Suggestion] R2-3: This UTF-8 guard rejects a UTF-16 file that carries a BOM but accepts BOM-less UTF-16, and the very next statement parses those bytes through Jackson's byte-level encoding auto-detection. For ASCII/CJK text every BOM-less UTF-16LE byte is itself a legal UTF-8 byte, so decoding and re-encoding is byte-identical, Arrays.equals(...) is true, and the guard never fires; Jackson then sniffs the bytes as UTF-16 and parses them. The statement is then stored by attest as new String(evidence, UTF_8) into the permanent attestation_json row - NUL-interleaved mojibake rather than the statement the runbook requires to be UTF-8.
Witness:
case=UTF-16LE no BOM
bytes=428 roundTrip[EQUAL] jacksonReadTree(bytes)=PARSED parsedMethod=host inspection
readEvidence: ACCEPTED (428 bytes)
storedAttestationText = NUL-interleaved mojibake
case=UTF-16 with BOM (the case the test covers)
roundTrip[DIFFERENT] -> readEvidence: REJECTED IllegalArgumentException: Operator evidence must be UTF-8.
fix arm: readTree(decodedText) -> JsonParseException (the strict decoder alone does NOT flip it)
The load-bearing half of the fix is parsing the decoded text, so no byte-level sniffing can occur:
| if (!Arrays.equals(evidence, | |
| new String(evidence, StandardCharsets.UTF_8).getBytes(StandardCharsets.UTF_8))) { | |
| JsonNode value = mapper.readTree(new String(evidence, StandardCharsets.UTF_8)); |
A strict decoder alone does not close this - those bytes are valid UTF-8, so CodingErrorAction.REPORT still reports OK - only removing the byte-level parse does.
The evidence bytes are hashed and stored verbatim, so the fix must keep rejecting non-UTF-8 rather than transcoding it.
Please add the test that must fail without the fix: a BOM-less UTF_16LE case in WorkspaceRecoveryCommandTest asserting readEvidence throws.
中文说明
[Suggestion] 这段 UTF-8 校验会拒绝带 BOM 的 UTF-16，却接受不带 BOM 的 UTF-16，而紧接着的语句又通过 Jackson 的字节级编码自动探测去解析这些字节。对于 ASCII/中日韩文本，无 BOM 的 UTF-16LE 每个字节本身都是合法的 UTF-8 字节，因此解码再编码结果完全一致、Arrays.equals(...) 为 true、校验永不触发；Jackson 随即把它识别为 UTF-16 并解析成功，最终 attest 以 new String(evidence, UTF_8) 把这份声明写进永久的 attestation_json 列，得到的是 NUL 交错的乱码，而不是手册要求必须是 UTF-8 的那份声明。
证据（实测）：无 BOM UTF-16LE 往返一致、readTree(bytes) 解析成功、readEvidence 放行（428 字节），落盘内容为 NUL 交错乱码；带 BOM 的 UTF-16（即测试覆盖的那一种）往返不一致并被拒绝。修复臂 readTree(decodedText) 抛 JsonParseException——注意仅加严格解码器不能修复，因为这些字节本身就合法。
修复中真正起作用的一半是改为解析文本，从而不存在字节级探测。由于证据字节会被原样哈希与存储，修复必须继续拒绝非 UTF-8，而不是做转码。请补上「去掉修复即失败」的测试：在 WorkspaceRecoveryCommandTest 中加一个无 BOM UTF_16LE 用例并断言 readEvidence 抛异常。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
There was a problem hiding this comment.
[Suggestion] R2-3: capture()'s two other refusal arms — the non-Shell detection and the producer_lost reason check — are asserted nowhere: every fixture feeding the recovery store is a Shell capture (via protocol-3/deferred or toolName) with captureStatus=partial and captureReason=producer_lost.
Witness:
sweep: 3 recovery-store capture fixtures, all shell + partial + producer_lost;
storage_failed exists only in the transport layer (HttpRuntimeTransport:522) and
never reaches this store
Failure scenario: loosening the shell detection fences a workspace whose only unfinished call was a non-Shell tool; accepting any capture reason fences storage_failed captures — both outside the "Exact Hosted Shell" recovery contract, both invisible to the suite.
Fix: add two variant executions — (a) a settled non-Shell reference with partial/producer_lost; (b) a Shell execution with captureReason: storage_failed — each asserting inspect throws IllegalStateException. Each variant must be a settled execution with a result map (otherwise it throws for a missing capture, which proves nothing), and the positive fixture's eligibleForPrepare must stay true.
中文说明
[Suggestion] R2-3：capture() 的另外两条拒绝分支——非 Shell 检测与 producer_lost 原因检查——没有任何断言覆盖：送入恢复存储的所有 fixture 都是 Shell 捕获（经 protocol-3/deferred 或 toolName），且 captureStatus=partial、captureReason=producer_lost。
证据：3 个恢复存储捕获 fixture 全部为 shell + partial + producer_lost；storage_failed 只出现在传输层（HttpRuntimeTransport:522），从未到达该存储。
失败场景：放宽 Shell 检测会把唯一未完成调用是非 Shell 工具的 workspace 纳入可封禁范围；接受任意 captureReason 会把 storage_failed 捕获也纳入——都越出了"Exact Hosted Shell"恢复契约，且套件全绿。
修复：增加两个变体执行——(a) 已结算的非 Shell 引用（partial/producer_lost）；(b) Shell 执行但 captureReason: storage_failed——各自断言 inspect 抛出 IllegalStateException。每个变体必须是带 result map 的已结算执行（否则会因"缺少捕获"抛出，证明不了任何事），且正向 fixture 的 eligibleForPrepare 必须保持为 true。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
There was a problem hiding this comment.
[Suggestion] R2-3: capture()'s two other refusal arms — the non-Shell detection and the producer_lost reason check — are asserted nowhere: every fixture feeding the recovery store is a Shell capture (via protocol-3/deferred or toolName) with captureStatus=partial and captureReason=producer_lost.
Witness:
sweep: 3 recovery-store capture fixtures, all shell + partial + producer_lost;
storage_failed exists only in the transport layer (HttpRuntimeTransport:522) and
never reaches this store
Failure scenario: loosening the shell detection fences a workspace whose only unfinished call was a non-Shell tool; accepting any capture reason fences storage_failed captures — both outside the "Exact Hosted Shell" recovery contract, both invisible to the suite.
Fix: add two variant executions — (a) a settled non-Shell reference with partial/producer_lost; (b) a Shell execution with captureReason: storage_failed — each asserting inspect throws IllegalStateException. Each variant must be a settled execution with a result map (otherwise it throws for a missing capture, which proves nothing), and the positive fixture's eligibleForPrepare must stay true.
中文说明
[Suggestion] R2-3：capture() 的另外两条拒绝分支——非 Shell 检测与 producer_lost 原因检查——没有任何断言覆盖：送入恢复存储的所有 fixture 都是 Shell 捕获（经 protocol-3/deferred 或 toolName），且 captureStatus=partial、captureReason=producer_lost。
证据：3 个恢复存储捕获 fixture 全部为 shell + partial + producer_lost；storage_failed 只出现在传输层（HttpRuntimeTransport:522），从未到达该存储。
失败场景：放宽 Shell 检测会把唯一未完成调用是非 Shell 工具的 workspace 纳入可封禁范围；接受任意 captureReason 会把 storage_failed 捕获也纳入——都越出了"Exact Hosted Shell"恢复契约，且套件全绿。
修复：增加两个变体执行——(a) 已结算的非 Shell 引用（partial/producer_lost）；(b) Shell 执行但 captureReason: storage_failed——各自断言 inspect 抛出 IllegalStateException。每个变体必须是带 result map 的已结算执行（否则会因"缺少捕获"抛出，证明不了任何事），且正向 fixture 的 eligibleForPrepare 必须保持为 true。
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| jdbc.update("DELETE FROM managed_workspace_operator_recovery WHERE recovery_id = ?", recoveryId); | ||
| assertThat(fixture.bindings.findRecoveryCandidates("local-process", null, 100)) |
There was a problem hiding this comment.
[Suggestion] R2-4: These three recovery-candidate assertions read a global, UUID-ordered LIMIT 100 sweep as if it were fixture-scoped. findRecoveryCandidates is the cross-tenant background sweep (it filters on provisioner_kind and binding_id, with no tenant predicate) and binding ids are random UUIDs, so it returns the 100 lexicographically smallest candidate ids in the whole database. ManagedAgentMySqlIT migrates one long-lived external schema and never cleans it, so once that schema accumulates more than ~100 candidates the anyMatch fails with "expected to find ... but none matched" - green by luck, red without a regression - while the DELETE-then-anyMatch pair silently stops pinning the audit-row exclusion.
Witness:
real repository, H2: A) one candidate -> targetRank=1 targetInFirstPage=true (assertion would pass)
B) +150 smaller ids -> total=151 rank=151 targetInFirstPage=false (assertion would fail)
C) same, audit row inserted -> targetInFirstPage=false (row presence makes no difference)
D) cursor walk -> pages=2 reachedTarget=true (the suggested fix works)
CI reachability: the mysql-integration lane uses a fresh mariadb container per run, so this bites a
repeat-runner against a persistent local MariaDB/MySQL, not CI.
Assert on the fixture's own row (a scoped read), or page the sweep with the cursor until the binding is found or the sweep is exhausted.
Do not narrow the production query to the fixture's tenant: the same method is the maintenance sweep's paging source (RuntimeRecoveryCoordinator.scan).
Please add the test that pins it: pre-insert 100+ local-process candidate bindings for other tenants before this contract method runs - the current assertion must go red without the fix.
中文说明
[Suggestion] 这三处候选断言把一个全局的、按 UUID 排序的 LIMIT 100 分页当作只属于自己 fixture 的数据来读。findRecoveryCandidates 是跨租户的后台扫描（只按 provisioner_kind 与 binding_id 过滤，没有租户条件），而 binding id 是随机 UUID，所以它返回的是全库按字典序最小的 100 个候选。ManagedAgentMySqlIT 迁移的是一个长期存在且从不清理的外部 schema，一旦候选累积超过约 100 条，anyMatch 就会以「expected to find ... but none matched」失败——靠运气绿、没有回归也会红——同时 DELETE 后再 anyMatch 的那一对也悄悄失去了对审计行排除的固定作用。
证据（实测）：真实仓储 + H2——A) 仅一个候选时目标排名 1、在首页、断言会通过；B) 加入 150 个更小 id 后总数 151、排名 151、不在首页、断言会失败；C) 同样条件下插入审计行结果不变，说明审计行存在与否对分页结果没有影响；D) 游标翻页两页即可命中目标，即建议的修法可行。CI 可达性：mysql 集成 lane 每次运行使用全新的 mariadb 容器，因此该问题影响的是对本机持久 MariaDB/MySQL 反复运行的人，而不是 CI。
建议改为对 fixture 自身行做有范围查询，或用游标翻页直到找到该 binding 或扫描结束；不要把生产查询收窄到 fixture 的租户，因为同一方法就是维护扫描的分页来源。请补上固定的测试：在该合约方法运行前预置 100 条以上其他租户的 local-process 候选 binding。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
There was a problem hiding this comment.
[Suggestion] R2-4: The only witness that prepare serializes behind the placement-domain lock is the 100 ms TimeoutException window — and that discriminator measures thread scheduling, not the lock. On a descheduled or SQL-latency-loaded CI runner, prepare exceeds 100 ms, the expected timeout fires anyway, and a lock-bypass regression ships green: every later assertion (idempotent re-prepare, fence state, placement throws) passes in both the blocked and bypassed worlds.
Witness:
quoted test body: the only in-window discriminator is
pending.get(100, TimeUnit.MILLISECONDS) — nothing else runs before connection.commit()
Fix: strengthen the in-window discrimination — while still holding the guard lock (before connection.commit()), assert the recovery-row count for this binding is still zero. A lock-bypassing prepare commits its INSERT inside the window and the count goes non-zero; a correctly blocking prepare inserts nothing. (Under READ COMMITTED this narrows rather than eliminates the race, but a state-delta probe is strictly stronger than the exception alone.)
中文说明
[Suggestion] R2-4：prepare 在放置域锁后串行化的唯一证据是 100 ms TimeoutException 窗口——而该判别器度量的是线程调度而非锁。在处于去调度或 SQL 延迟负载的 CI runner 上，prepare 超过 100 ms、预期超时照样触发，锁被绕过的回归因此全绿：后续所有断言（幂等重入 prepare、围栏状态、放置抛出）在被阻断与被绕过两种世界里都能通过。
证据：测试体原文——窗口内唯一的判别是 pending.get(100, TimeUnit.MILLISECONDS)；connection.commit() 之前没有其他任何检查。
修复：加强窗口内的判别——在仍持有 guard 锁时（connection.commit() 之前），断言该 binding 的恢复行计数仍为 0。绕锁的 prepare 会在窗口内提交 INSERT、计数变为非零；正确阻塞的 prepare 不会插入任何行。（READ COMMITTED 下这只是收窄而非消除竞态，但状态差值探针严格强于仅凭异常。）
— glm-5.3-flash via Qwen Code /review (v0.24.6)
There was a problem hiding this comment.
[Suggestion] R2-4: The only witness that prepare serializes behind the placement-domain lock is the 100 ms TimeoutException window — and that discriminator measures thread scheduling, not the lock. On a descheduled or SQL-latency-loaded CI runner, prepare exceeds 100 ms, the expected timeout fires anyway, and a lock-bypass regression ships green: every later assertion (idempotent re-prepare, fence state, placement throws) passes in both the blocked and bypassed worlds.
Witness:
quoted test body: the only in-window discriminator is
pending.get(100, TimeUnit.MILLISECONDS) — nothing else runs before connection.commit()
Fix: strengthen the in-window discrimination — while still holding the guard lock (before connection.commit()), assert the recovery-row count for this binding is still zero. A lock-bypassing prepare commits its INSERT inside the window and the count goes non-zero; a correctly blocking prepare inserts nothing. (Under READ COMMITTED this narrows rather than eliminates the race, but a state-delta probe is strictly stronger than the exception alone.)
中文说明
[Suggestion] R2-4：prepare 在放置域锁后串行化的唯一证据是 100 ms TimeoutException 窗口——而该判别器度量的是线程调度而非锁。在处于去调度或 SQL 延迟负载的 CI runner 上，prepare 超过 100 ms、预期超时照样触发，锁被绕过的回归因此全绿：后续所有断言（幂等重入 prepare、围栏状态、放置抛出）在被阻断与被绕过两种世界里都能通过。
证据：测试体原文——窗口内唯一的判别是 pending.get(100, TimeUnit.MILLISECONDS)；connection.commit() 之前没有其他任何检查。
修复：加强窗口内的判别——在仍持有 guard 锁时（connection.commit() 之前），断言该 binding 的恢复行计数仍为 0。绕锁的 prepare 会在窗口内提交 INSERT、计数变为非零；正确阻塞的 prepare 不会插入任何行。（READ COMMITTED 下这只是收窄而非消除竞态，但状态差值探针严格强于仅凭异常。）
— glm-5.3-flash via Qwen Code /review (v0.24.6)
| <checkstyle-maven-plugin.version>3.6.0</checkstyle-maven-plugin.version> | ||
| <fastjson2.version>2.0.65</fastjson2.version> | ||
| <h2.version>2.3.232</h2.version> | ||
| <h2.version>2.4.240</h2.version> |
There was a problem hiding this comment.
[Suggestion] R2-5: This H2 bump moves only one of the two modules that run the same SQL and the same duplicated DDL. runtime-broker now resolves 2.4.240 while managed-agent-server still takes Spring Boot's managed 2.3.232, so the broker's H2-JDBC suites and the server's WorkspaceRuntimeTest / WorkspaceRecoveryContract exercise the new NOT EXISTS clause, the widened state list and the new table on two different engines - and any engine-behaviour difference reds in one suite and passes in the other.
Witness:
mvn dependency:tree
runtime-broker -> com.h2database:h2:jar:2.4.240:test
managed-agent-server -> com.h2database:h2:jar:2.3.232:test
(the server declares h2 version-less under spring-boot-starter-parent 3.5.16)
The new query and table were exercised on the server lane's H2 2.3.232 and worked, so this is a
coverage-asymmetry finding rather than a demonstrated engine bug.
Either pin the same version in the server module (or a shared parent), or record in the PR which H2 2.3.232 behaviour the bump is for.
h2 is test scope in runtime-broker and version-less in the server, so the pin belongs on the server side and cannot affect production MySQL either way.
中文说明
[Suggestion] 这次 H2 升级只动了同时运行同一份 SQL 与同一份重复 DDL 的两个模块中的一个：runtime-broker 现在解析到 2.4.240，而 managed-agent-server 仍使用 Spring Boot 默认管理的 2.3.232。于是新增的 NOT EXISTS 子句、放宽的状态列表和新表，在 broker 的 H2-JDBC 套件与 server 的 WorkspaceRuntimeTest/WorkspaceRecoveryContract 上跑在两个不同的引擎上——任何引擎行为差异都会只让其中一个套件红或绿。
证据（实测依赖树）：runtime-broker -> com.h2database:h2:jar:2.4.240:test；managed-agent-server -> com.h2database:h2:jar:2.3.232:test（server 未声明版本，继承自 spring-boot-starter-parent 3.5.16）。新查询与新表在 server lane 的 H2 2.3.232 上实测可用，因此这是覆盖不对称/一致性问题，而不是已证实的引擎缺陷。
建议在 server 模块（或共享 parent）中固定同一版本，或在 PR 中说明该升级针对的是 2.3.232 的哪个行为。h2 在 runtime-broker 中是 test scope、在 server 中无版本，因此固定应落在 server 侧，且无论哪种方式都不影响生产 MySQL。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| Files.writeString(evidence, valid.replace("\"version\":1", "\"version\":4294967297")); | ||
| assertThatThrownBy(() -> WorkspaceRecoveryCommand.readEvidence(directory, evidence, | ||
| recoveryId, preparedAt, mapper)).isInstanceOf(IllegalArgumentException.class); | ||
| Files.writeString(evidence, valid.replace("\"restartPrevention\":true", "\"restartPrevention\":false")); |
There was a problem hiding this comment.
[Suggestion] R2-7: The evidence gate does not actually enforce the runbook's "exactly these fields" - it does not parse strictly. readEvidence validates with the shared default ObjectMapper and then checks value.size() != 6; ObjectNode.size() counts distinct field names and the default is last-wins with duplicate detection off, so a duplicate-key document collapses to 6 names and is accepted. Trailing content after the first document is likewise never inspected. The whole byte string is what the gate returns, and attest persists and hashes it verbatim, so the archived immutable operator statement can contain text the gate never validated.
Witness:
real readEvidence, module's Jackson:
default mapper: duplicate-key (7 pairs / 6 names) -> ACCEPTED (214 bytes archived verbatim)
trailing 2nd document -> ACCEPTED (201 bytes archived verbatim)
a single distinct 7th field -> rejected (Operator evidence is incomplete.)
Spring Boot's Jackson2ObjectMapperBuilder defaults behave identically.
strict mapper (STRICT_DUPLICATE_DETECTION + FAIL_ON_TRAILING_TOKENS):
duplicate-key -> JsonParseException: Duplicate field 'restartPrevention'
trailing -> MismatchedInputException: Trailing token ... FAIL_ON_TRAILING_TOKENS
Parse with a strictly-configured mapper. The module already has the precedent:
| Files.writeString(evidence, valid.replace("\"restartPrevention\":true", "\"restartPrevention\":false")); | |
| JsonNode value = strictMapper.readTree(evidence); |
ManagedExtensionRecordStore builds exactly this instance (STRICT_DUPLICATE_DETECTION + FAIL_ON_TRAILING_TOKENS), commented "no duplicate keys, no trailing content".
The mapper reaching readEvidence is the application-wide Spring ObjectMapper bean, so strictness must be added on a dedicated instance rather than by flipping the shared bean.
Please add the test that must fail without the fix: assert IllegalArgumentException for a duplicate restartPrevention pair and for a second appended document - both are accepted today.
中文说明
[Suggestion] 证据校验实际上并未落实手册所说的「恰好这些字段」——它没有严格解析。readEvidence 用共享的默认 ObjectMapper 校验后再检查 value.size() != 6，而 ObjectNode.size() 统计的是去重后的字段名数量，默认策略又是「后者覆盖」且不检测重复键，因此含重复键的文档折叠成 6 个名字后即被接受；首个文档之后的尾随内容同样从不被检查。校验返回的是整段字节，而 attest 会原样持久化并哈希，于是归档的那份不可变运维声明可能包含校验从未看过的内容。
证据（实测）：默认 mapper 下，7 个键值对（6 个不同名）被接受并按 214 字节原样归档；尾随第二个文档被接受并按 201 字节归档；而单独的第七个不同字段被正确拒绝。Spring Boot 的 Jackson2ObjectMapperBuilder 默认行为一致。换成严格 mapper（STRICT_DUPLICATE_DETECTION + FAIL_ON_TRAILING_TOKENS）后，重复键抛 Duplicate field 'restartPrevention'，尾随内容抛 MismatchedInputException: Trailing token。
建议改用严格配置的 mapper 解析，本模块已有现成先例（ManagedExtensionRecordStore 正是这样构造的，注释写明「不允许重复键、不允许尾随内容」）。注意传入 readEvidence 的 mapper 是全应用共享的 Spring ObjectMapper bean，因此严格性要加在专用实例上，而不是改动共享 bean。请补上「去掉修复即失败」的测试。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| var blockedFixture = new Fixture(source, jdbc, store); | ||
| var blocked = blockedFixture.runtime("blocked").binding(); |
There was a problem hiding this comment.
[Suggestion] R2-8: The new state == State.FAILED arm of blocksPlacement's managed-context predicate has no witness. This placement-block section asserts refusals for OPERATOR_RECOVERY (both disjuncts) and RECOVERY_BLOCKED only; no test anywhere in the tree asserts a refused placement for a FAILED binding, and blocksPlacement has no direct test call site. Deleting that one disjunct keeps the whole suite green - including the MySQL IT that runs this contract - while a managed placement request for a different workspace of the same tenant that reuses the failed binding's storage id or canonical cwd is admitted again.
Witness:
real JdbcRuntimeBindingRepository + H2, managed binding forced to FAILED with a provision seed:
PR head: same-storage=REFUSED runtime_placement_recovery_required
same-cwd=REFUSED control=ADMITTED
with `|| state == State.FAILED` deleted from :340:
same-storage=ADMITTED same-cwd=ADMITTED (probe assertion fails)
whole runtime-broker module with the arm removed:
Tests run: 467, Failures: 0, Errors: 3, Skipped: 2
(the 3 errors are JdbcRuntimeBrokerMySqlIT requiring mysql.url - identical on the intact tree,
so nothing in the suite discriminates the arm)
grep: the managed-agent-server test tree constructs no FAILED binding at all.
Add a FAILED fixture next to blockedFixture and assert the same two refusals for a renamed workspace sharing its storage id and for one sharing its canonical cwd.
The fixture must keep a provision seed and the request a non-null storageId: blocksPlacement's unreclaimed term gates this arm on state == State.FAILED && provisionSeed != null, and isManagedContext() is exactly storageId != null, so the arm is unreachable without both.
Please add the test that must fail without the fix: the new assertThatThrownBy(...findOrCreate(...)) on that fixture must go red when the FAILED disjunct is removed.
中文说明
[Suggestion] blocksPlacement 受管上下文谓词中新增的 state == State.FAILED 分支没有任何见证。该放置阻断小节只对 OPERATOR_RECOVERY（两个分支）与 RECOVERY_BLOCKED 断言了拒绝；整棵树里没有任何测试对 FAILED binding 断言过放置被拒，blocksPlacement 也没有直接的测试调用点。删掉这一个分支后整套测试仍然全绿——包括会运行该合约的 MySQL 集成测试——而同一租户下复用该失败 binding 的 storage id 或 canonical cwd、但工作区已重命名的受管放置请求又会重新被放行。
证据（实测）：真实 JdbcRuntimeBindingRepository + H2，把受管 binding 强制为带 provision seed 的 FAILED：PR head 下同 storage / 同 cwd 均被拒（runtime_placement_recovery_required）、对照组放行；从 :340 删掉该分支后两者都变为放行。把该分支删除后运行整个 runtime-broker 模块：Tests run: 467, Failures: 0, Errors: 3, Skipped: 2，那 3 个 error 是 JdbcRuntimeBrokerMySqlIT 缺少 mysql.url，在未改动的树上完全相同，即套件里没有任何东西能区分该分支。同时 grep 显示 managed-agent-server 的测试树根本没有构造 FAILED binding。
建议在 blockedFixture 旁增加一个 FAILED fixture，并对复用它 storage id 与 canonical cwd 的重命名工作区分别断言拒绝。fixture 必须保留 provision seed、请求必须带非空 storageId，否则该分支不可达。请补上「去掉修复即失败」的测试。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| if (saved.getState() != RuntimeBindingRecord.State.RELEASED) { | ||
| throw new IllegalStateException("Completed recovery has no retired Runtime."); |
There was a problem hiding this comment.
[Suggestion] R1-4: The whole inspect/prepare/complete orchestration, including the crash-and-retry resume branch, is still untested this round - no test enters main. WorkspaceRecoveryCommandTest calls only the package-private readEvidence, and WorkspaceRuntimeTest re-implements the call sequence against the store instead of invoking main, so a dispatch regression (wrong expected arg count, a branch that returns before compareAndSet/recoverBinding, a reordered branch, or the loop bound), a removed conjunct of the four-way opt-in gate, or a broken crash-retry resume branch compiles and keeps every suite green - and is only discovered by the operator running this offline tool during an incident.
Witness:
grep over packages/sdk-java/**/src/test/**: the only caller of WorkspaceRecoveryCommand is
WorkspaceRecoveryCommandTest's readEvidence cases; no test invokes main.
Independently corroborated by the qwen-triage stage-2 comment on this PR (S2) and by the PR body's
own disclosure that a physical Broker restart after prepare was not validated.
Extracting the dispatch into a package-private method over (String[], already-constructed beans) makes it testable without a Spring context: assert inspect/prepare/complete with the right arity, a wrong arg count rejected, and the LOST-with-stopped-writers resume branch.
中文说明
[Suggestion] R1-4：整条 inspect/prepare/complete 编排（含崩溃重试的续跑分支）本轮仍然没有测试——没有任何测试进入 main。WorkspaceRecoveryCommandTest 只调用包私有的 readEvidence，WorkspaceRuntimeTest 则对着 store 重写了一遍调用序列而不调用 main。因此分发层回归（参数个数写错、某个分支在 compareAndSet/recoverBinding 之前就 return、分支顺序错乱、循环上界、四段式开关少一个条件，或续跑分支被破坏）都能编译通过且所有套件保持全绿，只有运维在事故中真正运行这个离线工具时才会发现。
证据：对 packages/sdk-java/**/src/test/** 的 grep 显示 WorkspaceRecoveryCommand 的唯一调用方是 WorkspaceRecoveryCommandTest 的 readEvidence 用例；没有测试调用 main。该结论也得到本 PR 上 qwen-triage stage-2 评论（S2）与 PR 正文自身披露（prepare 之后的 Broker 物理重启未验证）的独立印证。
建议把分发逻辑抽成接收 (String[], 已构造好的 bean) 的包私有方法，从而无需 Spring 上下文即可测试：断言 inspect/prepare/complete 的正确参数个数、错误参数个数被拒，以及 LOST + 已停止写入者证据的续跑分支。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| jdbc.update("UPDATE managed_workspace_operator_recovery" | ||
| + " SET attestation_json = ?, attestation_sha256 = ?," |
There was a problem hiding this comment.
[Suggestion] R1-15: The immutable proof write and the completion marker still run only on H2, never on the MySQL/MariaDB channel. The MySQL integration test now calls verifyOperatorPrepare, but that contract method ends after the prepare / scan-exclusion / fence assertions and never calls attest(...) or complete(...); the only exercise of the proof write and the completion marker is the H2 WorkspaceRuntimeTest. A MySQL-specific failure in the attestation UPDATE or the COALESCE completion marker - type, collation, or DATETIME(6) handling - therefore ships unobserved.
Witness:
read of the MySQL channel at the reviewed head: ManagedAgentMySqlIT:961 calls
verifyOperatorPrepare only (plus verify), and verifyOperatorPrepare's body (lines 28-187)
contains no attest/complete call; grep for `recovery.attest(` / `recovery.complete(` on the
MySQL path returns nothing.
Extending the shared contract through attestation and completion makes both channels run the same assertions, since WorkspaceRecoveryTest (H2) and ManagedAgentMySqlIT (real MySQL) both drive it.
中文说明
[Suggestion] R1-15：不可变证明的写入与完成标记仍然只在 H2 上运行，从未在 MySQL/MariaDB 通道上执行。MySQL 集成测试现在会调用 verifyOperatorPrepare，但该方法在 prepare / 扫描排除 / 围栏断言之后即结束，从不调用 attest(...) 或 complete(...)；证明写入与完成标记的唯一演练点是 H2 上的 WorkspaceRuntimeTest。因此，attestation 的 UPDATE 或 COALESCE 完成标记在 MySQL 上可能出现的特有失败（类型、排序规则或 DATETIME(6) 处理）都不会被观察到。
证据：在本轮 reviewed head 上读取 MySQL 通道——ManagedAgentMySqlIT:961 只调用了 verifyOperatorPrepare（以及 verify），而 verifyOperatorPrepare 的方法体（第 28–187 行）不含任何 attest/complete 调用。
建议把共享合约延伸到证明写入与完成阶段，这样 H2 与真实 MySQL 两条通道会执行相同的断言。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
| @@ -0,0 +1,21 @@ | |||
| CREATE TABLE managed_workspace_operator_recovery ( | |||
There was a problem hiding this comment.
[Suggestion] R1-29: V19 is still free on main, but open PR #12894 still claims V19 (and V20), and the collision fails at service startup rather than at merge. git sees no conflict because the filenames differ, and CI cannot see it because each PR's Flyway run only loads its own migrations - so whichever of the two lands second fails with Found more than one migration with version 19. main is at V18 and this PR's merge base carries no V19.
Witness:
gh pr view 12894 --repo QwenLM/qwen-code
state: OPEN
files: .../db/migration/V19__managed_tool_publication.sql, .../db/migration/V20__managed_tool_publication_objects.sql
git ls-tree origin/main -- .../db/migration/ -> no V19
Independently confirmed by the qwen-triage stage-2 comment on this PR (S1), which also names
#12946/#13037 colliding on V21.
The author's hourly monitor is watching merge order; flagging it so the renumber is not missed if this PR is the one that lands second.
中文说明
[Suggestion] R1-29：V19 目前在本仓库 main 上仍然空闲，但开放中的 PR #12894 同样占用 V19（以及 V20），而该冲突会在服务启动时而不是在合并时失败。git 因为文件名不同而看不到冲突，CI 也看不到，因为每个 PR 的 Flyway 只加载自己的迁移——因此两者中后合入的一方会以 Found more than one migration with version 19 失败。main 目前停在 V18，本 PR 的合并基线上没有 V19。
证据（实测）：gh pr view 12894 显示状态 OPEN 且文件包含 V19__managed_tool_publication.sql 与 V20__managed_tool_publication_objects.sql；git ls-tree origin/main 的迁移目录中没有 V19。本 PR 上的 qwen-triage stage-2 评论（S1）独立确认了同一问题，并指出 #12946/#13037 在 V21 上互撞。
作者的每小时监控已在关注合并顺序；此处仅作提示，以免本 PR 恰好是后合入的一方时漏掉改号。
— DeepSeek/deepseek-v4.1-flash via Qwen Code /review (v0.24.7)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 main orchestration untested — already re-posted at WorkspaceRecoveryCommand.java:115 (comment 4140743205)
- R1-15 attest/complete untested on the MySQL channel — already re-posted at WorkspaceOperatorRecoveryStore.java:186 (comment 4140743210)
- R1-29 V19 migration collision with open #12894 — already re-posted at V19__workspace_operator_recovery.sql:1 (comment 4140743216)
- R1-16 (fix-induced) FAILED placement arm untested — already reported at RuntimeBindingRecord.java:340 (comment 4140743154) and WorkspaceRecoveryContract.java:135 (comment 4140743199)
- R2-2 H2 version split across the shared contract suite — already reported at runtime-broker/pom.xml:16 (comment 4140743181)
- R2-5 placement lock held across prepare's capture scan — already reported at WorkspaceOperatorRecoveryStore.java:78 (comment 4140743160)
Not reviewed: reverse audit — stopped at the plan's 5-round cap (round 5 still reported findings in chunks 4, 6, and 8; the cap forecloses a sixth round).
Not explored to full depth (tool budget reached): "agent 1b": none — all planned checks completed (~18 of ~69 tool calls used).; "agent reverse-audit (round 4)": none — no check was cut short by the ceiling; the only unverifiable step (local mvn package ) is impossible on this machine (no JVM) and is disclosed inside th…; chunk 6: none — all checks completed within budget (~14 tool calls of 33)..
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/maintenance/WorkspaceRecoveryApplication.java:9 — [review] Maintenance context's negative safety properties (no listener, no Flyway, no scan) asserted nowherepackages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/store/WorkspaceOperatorRecoveryStore.java:314 — [review] capture()'s completed-capture refusal arm unpinnedpackages/sdk-java/runtime-broker/src/main/java/com/alibaba/qwen/code/runtimebroker/RuntimeBindingRecord.java:355 — [review] OPERATOR_RECOVERY fence's illegal escapes to RELEASED/FAILED/DRAINING/PROVISIONING unpinnedpackages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/store/WorkspaceOperatorRecoveryStore.java:301 — [review] capture() scans and JSON-parses the entire settled execution history (no LIMIT, paid twice per …packages/sdk-java/runtime-broker/src/main/resources/com/alibaba/qwen/code/runtimebroker/schema.sql:75 — [review] blocked_execution_call_id is a write-only column — no read site, test, or doc
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.)
[Critical] R2-6: [certifies-falsely] [new-surface] The trusted-reboot reclaim path (warm/acquire → ensureBinding → reclaimLostBinding → recoverLost → observeDurable) never reads managed_workspace_operator_recovery — the audit fence exists only in findRecoveryCandidates — so with trusted-local-reboot-recovery=true, a reboot between prepare and complete releases the fenced binding and provisions a replacement without the operator's statement; the operator's later complete refuses on the evidence-source mismatch. Probe on the PR head: managed LOST binding + pending audit row + rebooted registration + service.warm() → final=RELEASED, stopEvidence=trusted-host-reboot, pendingAudit=1, replacement binding issued (expected LOST but was RELEASED). Flip check: with the fence extended to recoverLost, the binding stays LOST. direction: certifies-falsely; baseline: new-surface. Fix: extend the audit check to recoverLost — refuse (stay LOST) while a pending row exists and the stop-evidence source is not operator-attested:<recovery_id>; keep trusted-reboot release working for bindings with no pending audit row. Constraint: preserve the sanctioned W0e-3 release for no-audit bindings (DurableRuntimeRecoveryTest:1194-1203) and requireSafeReplacement's transition rules. Witness: probe, source tag [probe]. Fix witness: Extend WorkspaceRecoveryContract: after prepare on the LOST binding, claim as a simulated broker, compareAndSet trusted-reboot-style evidence, recoverLost — assert the binding remains LOST while the audit row is pending; red today (the release succeeds). Fix constraint: Trusted-reboot release must keep working for bindings with NO pending audit row (DurableRuntimeRecoveryTest:1194-1203); scope the refusal to pending audits only.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：reverse audit — stopped at the plan's 5-round cap (round 5 still reported findings in chunks 4, 6, and 8; the cap forecloses a sixth round).
未探索到全部深度（达到工具调用预算）："agent 1b"：none — all planned checks completed (~18 of ~69 tool calls used).；"agent reverse-audit (round 4)"：none — no check was cut short by the ceiling; the only unverifiable step (local mvn package ) is impossible on this machine (no JVM) and is disclosed inside th…；chunk 6：none — all checks completed within budget (~14 tool calls of 33).。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R2-6: [certifies-falsely] [new-surface] The trusted-reboot reclaim path (warm/acquire → ensureBinding → reclaimLostBinding → recoverLost → observeDurable) never reads managed_workspace_operator_recovery — the audit fence exists only in findRecoveryCandidates — so with trusted-local-reboot-recovery=true, a reboot between prepare and complete releases the fenced binding and provisions a replacement without the operator's statement; the operator's later complete refuses on the evidence-source mismatch. Probe on the PR head: managed LOST binding + pending audit row + rebooted registration + service.warm() → final=RELEASED, stopEvidence=trusted-host-reboot, pendingAudit=1, replacement binding issued (expected LOST but was RELEASED). Flip check: with the fence extended to recoverLost, the binding stays LOST. direction: certifies-falsely; baseline: new-surface. Fix: extend the audit check to recoverLost — refuse (stay LOST) while a pending row exists and the stop-evidence source is not operator-attested:<recovery_id>; keep trusted-reboot release working for bindings with no pending audit row. Constraint: preserve the sanctioned W0e-3 release for no-audit bindings (DurableRuntimeRecoveryTest:1194-1203) and requireSafeReplacement's transition rules. Witness: probe, source tag [probe]. Fix witness: Extend WorkspaceRecoveryContract: after prepare on the LOST binding, claim as a simulated broker, compareAndSet trusted-reboot-style evidence, recoverLost — assert the binding remains LOST while the audit row is pending; red today (the release succeeds). Fix constraint: Trusted-reboot release must keep working for bindings with NO pending audit row (DurableRuntimeRecoveryTest:1194-1203); scope the refusal to pending audits only.
— glm-5.3-flash via Qwen Code /review (v0.24.6)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 main orchestration untested — already re-posted at WorkspaceRecoveryCommand.java:115 (comment 4140743205)
- R1-15 attest/complete untested on the MySQL channel — already re-posted at WorkspaceOperatorRecoveryStore.java:186 (comment 4140743210)
- R1-29 V19 migration collision with open #12894 — already re-posted at V19__workspace_operator_recovery.sql:1 (comment 4140743216)
- R1-16 (fix-induced) FAILED placement arm untested — already reported at RuntimeBindingRecord.java:340 (comment 4140743154) and WorkspaceRecoveryContract.java:135 (comment 4140743199)
- R2-2 H2 version split across the shared contract suite — already reported at runtime-broker/pom.xml:16 (comment 4140743181)
- R2-5 placement lock held across prepare's capture scan — already reported at WorkspaceOperatorRecoveryStore.java:78 (comment 4140743160)
Not reviewed: reverse audit — stopped at the plan's 5-round cap (round 5 still reported findings in chunks 4, 6, and 8; the cap forecloses a sixth round).
Not explored to full depth (tool budget reached): "agent 1b": none — all planned checks completed (~18 of ~69 tool calls used).; "agent reverse-audit (round 4)": none — no check was cut short by the ceiling; the only unverifiable step (local mvn package ) is impossible on this machine (no JVM) and is disclosed inside th…; chunk 6: none — all checks completed within budget (~14 tool calls of 33)..
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/maintenance/WorkspaceRecoveryApplication.java:9 — [review] Maintenance context's negative safety properties (no listener, no Flyway, no scan) asserted nowherepackages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/store/WorkspaceOperatorRecoveryStore.java:314 — [review] capture()'s completed-capture refusal arm unpinnedpackages/sdk-java/runtime-broker/src/main/java/com/alibaba/qwen/code/runtimebroker/RuntimeBindingRecord.java:355 — [review] OPERATOR_RECOVERY fence's illegal escapes to RELEASED/FAILED/DRAINING/PROVISIONING unpinnedpackages/sdk-java/managed-agent-server/src/main/java/com/alibaba/qwen/code/managedagent/store/WorkspaceOperatorRecoveryStore.java:301 — [review] capture() scans and JSON-parses the entire settled execution history (no LIMIT, paid twice per …packages/sdk-java/runtime-broker/src/main/resources/com/alibaba/qwen/code/runtimebroker/schema.sql:75 — [review] blocked_execution_call_id is a write-only column — no read site, test, or doc
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.)
[Critical] R2-6: [certifies-falsely] [new-surface] The trusted-reboot reclaim path (warm/acquire → ensureBinding → reclaimLostBinding → recoverLost → observeDurable) never reads managed_workspace_operator_recovery — the audit fence exists only in findRecoveryCandidates — so with trusted-local-reboot-recovery=true, a reboot between prepare and complete releases the fenced binding and provisions a replacement without the operator's statement; the operator's later complete refuses on the evidence-source mismatch. Probe on the PR head: managed LOST binding + pending audit row + rebooted registration + service.warm() → final=RELEASED, stopEvidence=trusted-host-reboot, pendingAudit=1, replacement binding issued (expected LOST but was RELEASED). Flip check: with the fence extended to recoverLost, the binding stays LOST. direction: certifies-falsely; baseline: new-surface. Fix: extend the audit check to recoverLost — refuse (stay LOST) while a pending row exists and the stop-evidence source is not operator-attested:<recovery_id>; keep trusted-reboot release working for bindings with no pending audit row. Constraint: preserve the sanctioned W0e-3 release for no-audit bindings (DurableRuntimeRecoveryTest:1194-1203) and requireSafeReplacement's transition rules. Witness: probe, source tag [probe]. Fix witness: Extend WorkspaceRecoveryContract: after prepare on the LOST binding, claim as a simulated broker, compareAndSet trusted-reboot-style evidence, recoverLost — assert the binding remains LOST while the audit row is pending; red today (the release succeeds). Fix constraint: Trusted-reboot release must keep working for bindings with NO pending audit row (DurableRuntimeRecoveryTest:1194-1203); scope the refusal to pending audits only.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：reverse audit — stopped at the plan's 5-round cap (round 5 still reported findings in chunks 4, 6, and 8; the cap forecloses a sixth round).
未探索到全部深度（达到工具调用预算）："agent 1b"：none — all planned checks completed (~18 of ~69 tool calls used).；"agent reverse-audit (round 4)"：none — no check was cut short by the ceiling; the only unverifiable step (local mvn package ) is impossible on this machine (no JVM) and is disclosed inside th…；chunk 6：none — all checks completed within budget (~14 tool calls of 33).。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R2-6: [certifies-falsely] [new-surface] The trusted-reboot reclaim path (warm/acquire → ensureBinding → reclaimLostBinding → recoverLost → observeDurable) never reads managed_workspace_operator_recovery — the audit fence exists only in findRecoveryCandidates — so with trusted-local-reboot-recovery=true, a reboot between prepare and complete releases the fenced binding and provisions a replacement without the operator's statement; the operator's later complete refuses on the evidence-source mismatch. Probe on the PR head: managed LOST binding + pending audit row + rebooted registration + service.warm() → final=RELEASED, stopEvidence=trusted-host-reboot, pendingAudit=1, replacement binding issued (expected LOST but was RELEASED). Flip check: with the fence extended to recoverLost, the binding stays LOST. direction: certifies-falsely; baseline: new-surface. Fix: extend the audit check to recoverLost — refuse (stay LOST) while a pending row exists and the stop-evidence source is not operator-attested:<recovery_id>; keep trusted-reboot release working for bindings with no pending audit row. Constraint: preserve the sanctioned W0e-3 release for no-audit bindings (DurableRuntimeRecoveryTest:1194-1203) and requireSafeReplacement's transition rules. Witness: probe, source tag [probe]. Fix witness: Extend WorkspaceRecoveryContract: after prepare on the LOST binding, claim as a simulated broker, compareAndSet trusted-reboot-style evidence, recoverLost — assert the binding remains LOST while the audit row is pending; red today (the release succeeds). Fix constraint: Trusted-reboot release must keep working for bindings with NO pending audit row (DurableRuntimeRecoveryTest:1194-1203); scope the refusal to pending audits only.
— glm-5.3-flash via Qwen Code /review (v0.24.6)
3 participants
What this PR does
Adds an offline, opt-in
workspace-recovery inspect|prepare|completeprocedure for a Linux durable Hosted worker whose partial Shell capture has pinned its Workspace lease. Preparation records the exact holder and original execution, then fences the binding without freeing the Workspace. Completion requires a private operator statement, verifies that the registered worker has exited, tombstones its durable identity, and reuses the existing exact-holder cleanup to make the Workspace available to a new Session. The original turn and uncertain Shell outcome remain blocked and are never replayed.Adds a permanent audit trail, synchronized English design and Chinese design, plus an English runbook and Chinese runbook. The command does not start the HTTP service, scheduler, or worker, and is disabled by default.
Why it's needed
An escaped Shell descendant can keep writing after the registered worker or its process group exits. An incomplete capture correctly blocks the original Hosted turn and the Workspace lease, but without an explicit recovery path every later Session on that Workspace remains blocked. This procedure preserves the uncertain result while giving an operator who has verified all writers are stopped a narrow, auditable way to reclaim the original holder.
Reviewer Test Plan
How to verify
On a Linux durable worker, produce a
producer_lostShell capture with a detached writer and confirm that a second Session stays busy. Inspect and prepare the exact holder; verify that ordinary release, new admission, placement replacement, and background recovery cannot bypass the fence. Withhold or alter the statement, leave the registered worker running, or provide the wrong holder; completion must refuse and preserve the lease. After stopping every writer and preventing restarts, complete with the private statement and confirm a new Session can use the Workspace, while the original turn remains blocked and the old Shell command is not replayed. Retry the same statement after interrupted cleanup and confirm that a changed statement or stale holder cannot release a newer holder.Evidence (Before & After)
Before: a saved partial
producer_lostShell result pins the Workspace even after the worker exits. After: only explicit preparation plus an immutable operator statement and exact registered-worker exit permit retirement of the original generation; its partial output and uncertain effects remain preserved.Tested on
0f8ad6daf0; the GitHub fault-gate job also passedEnvironment (optional)
Java 21, real MySQL 8.4, and a source-built Broker and worker. The maintainer re-verified the physical Linux operator runbook on
0f8ad6daf0: 34/34 steps passed with a real escaped writer, fencing, refusal while the worker lived, and Workspace reuse. The saved v3 partial outcome was seeded through the production repository path rather than a full model turn. On the same head, 41/41 physical Linux fault gates, 464/464 Broker tests, 190/190 service H2 tests, 10/10 Hosted MySQL failsafe tests, and 15/15 MySQL integration tests passed; the GitHub fault-gate job also passed. Local Checkstyle, build, and typecheck passed.Risk & Scope
prepare. The Linux fault-gate regression was traced to test-only H2 2.3.232; this head upgrades it to 2.4.240, and both physical fault gates and GitHub CI passed. This does not resume the original Hosted turn, replay Shell, or add a public API.Linked Issues
Related to #12904. Builds on the merged #12869 and targets
main.中文说明
本 PR 的改动
新增默认关闭的离线
workspace-recovery inspect|prepare|complete流程，处理 Linux durable Hosted worker 因 Shell 捕获不完整而占住 Workspace 租约的情况。准备阶段记录精确 holder 和原执行，并封禁 binding，但不释放 Workspace。完成阶段要求私有运维证明、核实已注册 worker 退出、封存其持久身份，再复用已有的精确 holder 清理流程，让新 Session 可以使用 Workspace。原回合和结果不确定的 Shell 调用仍被阻断，绝不重放。同时新增持久审计记录、同步的英文设计与中文设计，以及英文手册和中文手册。维护命令不启动 HTTP 服务、调度器或 worker，且默认关闭。
需求原因
逃逸的 Shell 子进程可能在已注册 worker 或其进程组退出后继续写入。不完整捕获正确地阻断了原 Hosted 回合和 Workspace 租约，但目前同一 Workspace 的后续 Session 会一直被阻断。本流程保留不确定的结果，同时让已核实全部写入者停止的运维人员能够以范围明确、可审计的方式回收原 holder。
评审验证计划
验证方式
在 Linux durable worker 上制造带有逃逸写入者的
producer_lostShell 捕获，确认第二个 Session 仍收到忙碌拒绝。检查并准备精确 holder，确认普通释放、新准入、placement 替换和后台恢复均不能绕过围栏。缺少或改动证明、原 worker 仍运行、或 holder 错误时，完成操作必须拒绝并保留租约。停止全部写入者并阻止重启后，用私有证明完成恢复，确认新 Session 可使用 Workspace，而原回合仍被阻断、旧 Shell 命令没有重放。中断清理后用同一证明重试，确认更改证明或过期 holder 不能释放新的 holder。前后证据
之前：已保存的
producer_lost部分 Shell 结果即使在 worker 退出后仍占住 Workspace。之后：只有明确准备、不可覆盖的运维证明，以及对原注册 worker 退出的核实，才能退休原代数；部分输出与不确定副作用仍保留。已测试平台
0f8ad6daf0上完成 34/34 项物理运维手册验证及 41/41 项故障门禁；GitHub 故障门禁作业也通过环境
Java 21、真实 MySQL 8.4、源码构建的 Broker 与 worker。维护者在
0f8ad6daf0上复验 Linux 物理运维流程：真实逃逸写入者、围栏、worker 存活时拒绝完成及 Workspace 复用等 34/34 项通过。保存的 v3 部分结果通过生产仓储路径写入，并非完整模型回合。同一提交通过 41/41 项物理 Linux 故障门禁、464/464 项 Broker 测试、190/190 项服务 H2 测试、10/10 项 Hosted MySQL failsafe 测试及 15/15 项 MySQL 集成测试；GitHub 故障门禁作业也通过。本机 Checkstyle、build 和 typecheck 通过。风险与范围
prepare后 Broker 物理重启。Linux 故障门禁回归被定位到仅用于测试的 H2 2.3.232；本提交升级至 2.4.240，物理门禁和 GitHub CI 均通过。本 PR 不恢复原 Hosted 回合、不重放 Shell，也不新增公共 API。关联
关联 #12904；基于已合并的 #12869，目标分支为
main。