Repository navigation
test(managed-agent): pin untested contracts from the #12692 R2 review - #13348
Conversation
Twelve R2 review follow-ups on #12692 in the test surface, re-verified against current main (2c591ec) before writing each witness: - SessionEventHub evicts beyond CAPACITY (512) and reports whether the watermark still held the requested position, pinning the overflow signal the SSE fold relies on. - HarnessCoordinator fails the turn when a recovered runtime is not ready for continuation, fails terminally when the recovery moved between the witness read and the bind, and schedules a retry when the admission epoch moved before commit. - HarnessEventProjector substitutes the safe fallback code for an out-of-alphabet error code, and the secret-redaction sweep now holds both the key and the value of a sensitive attribute out of the leaked map. - QwenHostedHarnessConnector's unknown-outcome create falls back to load and propagates the full attachment (boot id, recovery handle, generate and event epoch); the load path propagates the same four fields; a non-conflict daemon error is rethrown without any fallback load. - ManagedSessionStore contract fixtures assert per-record session identity, record count by the records list, and a monotone sequence range with an event count that fits. - ManagedSessionStore integration holds the per-page restore byte budget: twelve dense two-line commits with a 1 MiB pad page flush the first page after nine transactions (revisions 1..9, hasMore=true) and finish on the second page (revisions 10..12, hasMore=false). - The MariaDB MySqlIT is rerunnable against a persistent database: the legacy fixture namespaces every fixed identity per run (upgrade/lifecycle/hooks tenants and sessions), skips the migration-bound assertions (V15/V17/V29 were applied once and cannot re-apply to a rerun's seeds), and re-seeds the V9 scope conflict source explicitly so the workspace idempotency probe is not migration-order dependent. The projection, private-store and lease-precision sub-exercises are equally namespaced so a second mvn failsafe run on the same database is green. - ManagedAgentServerIntegrationTest fixture harness is keyed by tenant for the runtime-recovery attachment, and the dispatchable turns it claims are cleaned up between sub-cases so the retries, the generation transfer and the epoch retraction stay deterministic against a shared H2. Verified on the managed-agent-server module with mvn verify after same-source reinstall of qwencode and runtime-broker (alpha-jar md5 fingerprints matched before and after): witness runs of the ManagedAgentMySqlIT against a persistent temp MariaDB passed twice in a row (19 tests, 0 failures, 0 errors each).
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Contract pins for the previously untested surfaces (projector redaction, out-of-alphabet error-code substitution, legacy seed backfill guards). Test-only, CI pending at review time.
Overlapping test additions: keep this branch's three recovery-fence witnesses and the out-of-alphabet fallback pin alongside main's retraction and deadline-expiry witnesses (both arms splice cleanly; test-compile is the gate for structure).
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
…java-tests # Conflicts: # packages/sdk-java/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/ManagedAgentServerIntegrationTest.java
- ManagedAgentMySqlIT: derive the first-pass gate from this run's own
Flyway target-15 migrate result instead of V15's item_id output, so a
V15 regression can no longer silence its own witnesses; correct the
rerun comment (consumer_progress is gated, agent_revision reads V13's
DEFAULT) and merge the adjacent gate blocks.
- ManagedAgentMySqlIT: derive the journal case-sensitivity probe from
the namespaced storeTenant so it stays a pure case variant.
- ManagedAgentMySqlIT: drop the inert per-connection
SET FOREIGN_KEY_CHECKS toggle; the child-first delete order is the
real invariant and is now stated as such.
- HarnessEventProjectorTest: assert secret absence on the serialized
data (any nesting depth), pin the raw code's absence from the
fallback-code event, and add the oversize/at-bound error-code arms
for SAFE_ERROR_CODE's {1,128} bound.
- ManagedSessionStoreContractFixtureTest: assert the transaction
summary rule the server actually enforces, branching on operation
(genesis zero rule vs range/record equality).
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #13348Base update (conflict resolution)Merged Findings addressed (8 of 11)
Deferred to the next round (batch bound ~8; replied on each thread, threads left open)
Verification
Mutation probes (each applied, measured red, then restored with
中文说明Autofix 本轮报告 — PR #13348基线更新（冲突解决）将 已处理的发现（11 条中的 8 条）
延后到下一轮（批量上限约 8 条；已在各线程回复，线程保持打开）
验证
变异探针（均已实际施加、实测变红、随后用
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。 🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…ermark operand (#13348) - ManagedAgentServerIntegrationTest: replace the per-call-site cleanupTurn helper with one @AfterEach sweep that settles every non-terminal Turn before the recovery scanner re-arms, covering the two retraction tests that never invoked the helper; the concurrent-submit finally whose re-await could mask the real failure goes away with it. - ManagedAgentMySqlIT: extend the warm-database delete list with managed_agent_operation and managed_workspace_create_command so it covers all seven RESTRICT children of managed_agent_session. - ManagedSessionStoreContractFixtureTest: drop the unreachable non-genesis else arm; pin the published genesis shape unconditionally. - QwenHostedHarnessConnectorTest: route the three new recovery tests through the existing connector(client) helper. - HarnessCoordinatorTest: add retriesTheTurnWhenTheRecoveredWatermarkRegresses — same epoch, lower admission watermark — the unique witness for the lastEventId guard disjunct (mutation probe: deleting the disjunct fails exactly this test). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #13348 review feedbackAll five inline findings from the latest review are addressed in commit Dispositions
Failed-check note
Verification
中文说明Autofix 本轮处理 — PR #13348 评审反馈最新一轮评审的 5 条行内发现全部在提交 处理结论
关于失败检查的说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 5 selected review thread(s). · 已关闭全部选中的 5 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…ts (#13348) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
…jection (#13348) The deterministic gate rejected 3a3b203 on npm run lint, but the 589 errors were all in untracked, git-ignored Maven javadoc output under packages/sdk-java/qwencode/target/reports/apidocs/ left in the runner workspace by a same-round mvn package/install of the qwencode module - not in anything the commit touched. The committed tree needs no change; this empty follow-up records the repair: the generated output was removed from the workspace and npm run build, npm run typecheck, and npm run lint all pass, with mvn -Dtest=HarnessCoordinatorTest test (33/33) and mvn checkstyle:check green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical-only review at head fd78979d81f5d0ad26e632540fdf5419b80808c0.
The one Critical ever filed is fixed at this head
Eight files, all under src/test/java — no production code. For a test-only PR the reportable defect class is a false green, and this PR had exactly one, R1-1, filed sev:"C" in rounds 1 and 2 and re-asserted at 82f9c2ad with the cited code byte-identical.
The charge was that legacySeedsBackfilled counted managed_agent_event rows with item_id IS NOT NULL, a column only V15's data migration writes for these seeds, so the flag meant "V15 succeeded" rather than "this database is a rerun". Break V15 and on a fresh CI database the count is 0, the gate reads false, and the four legacy-migration witnesses plus the V2-derived managed_agent_consumer_progress count all skip behind a plain if while the method still reports passed with failsafe skipped="0".
Verified in the source at this head rather than from the resolved flag: legacySeedsBackfilled no longer exists anywhere in ManagedAgentMySqlIT.java. The gate is now
boolean firstPass = Flyway.configure().dataSource(dataSource)
.locations("classpath:db/migration")
.target(MigrationVersion.fromVersion("15")).load().migrate()
.migrationsExecuted > 0;migrationsExecuted is Flyway's own count of what it ran during the staged v1→v15 step — a measurement of the database's prior state, taken before V15's data effects exist, and therefore independent of whether V15 is correct. The circularity is gone in the direction that matters: on a fresh database firstPass is true whatever V15 does, so LegacyEvents.assertBackfilled, LegacyLifecycleCommands.assertMigrated, LegacyHookRecords.assertBackfilled and the consumer_progress == 1 count all execute and a V15 regression fails loudly. On a genuine rerun the skip is correct rather than circular, since one-shot backfills cannot re-apply to that run's seeds, and the comment says so. The agent_revision V13-DEFAULT witness below the gate runs unconditionally.
Rounds 3 and 4 both report zero Criticals, and round 4 at this head filed two Suggestions.
Current scan — no provable Critical
I swept the added lines for the constructs that turn a new test into a silent no-op. There are zero occurrences of @Disabled, @EnabledIf*, @EnabledOnOs, assumeTrue, assumeThat, Assumptions. or ctx.skip in anything this diff adds — notably no JUnit assumption, so nothing here can report as skipped-instead-of-failed. The diff adds 11 @Test/@ParameterizedTest methods and 36 assertion lines.
Five added if statements exist and none of them gates an assertion away:
if (firstPass)andif (!firstPass)are the two arms of the gate above, and both do real work — the rerun arm re-seeds the idempotency conflict rather than returning early.if (revision == 1)/elseandif (revision >= 3)are arrange-phase request construction inside the commit lambda: genesis shape versus a ~1 MiB padded transaction, and when to attachpreviousCommitDigest. Both branches build a request, so no iteration silently does nothing.if (result.getResponse().getStatus() != 200)throwsIllegalStateExceptioncarrying the revision, the status and the first 240 characters of the body. A failed arrange-phase commit is a hard failure, which is the correct shape — the opposite of the R1-1 defect.
The blob-budget test those guards belong to commits one small genesis plus twelve transactions of roughly 1 MiB per record line and asserts the page is cut by the byte budget before the count limit, which is a real boundary rather than a restatement of the code.
On the standing CHANGES_REQUESTED
reviewDecision is CHANGES_REQUESTED, so it is worth being explicit about why that does not carry into this verdict. Both such reviews are against superseded heads — c1a61b2d31 at 10:55 and 82f9c2ad0b at 15:32 on 10-04. The same bot's two newest reviews of this PR are COMMENTED, at bd2e6e7b57 and at this head, and neither files a Critical. The Critical those requests rested on is the one I verified fixed above. The two unresolved threads are both [Suggestion].
CI
No failing, errored or timed-out check. For a test-only PR the lanes that execute the changed files are the ones that matter, and they ran rather than being skipped: Runtime Broker and Managed Agent MariaDB / Java 21 and Hosted process fault gates / MySQL 8.4 / Java 21 both pass, as do ubuntu-latest / Java 11, Java 17, Java 21, macos-latest / Java 21, windows-latest / Java 21, Real daemon E2E / Java 11, Test (ubuntu-latest, Node 22.x) and Lint & Static. I did not parse the job logs to confirm each new method by name, so I am relying on the suites being selected by those lanes rather than claiming per-case witnesses.
Note on the two open Suggestions
R4-1 is the one with teeth and is worth a follow-up: the rerun isolation uses a hand-maintained nine-table delete list that has already drifted once — it held 7 of the 9 names at the previous head and was missing two RESTRICT children of managed_agent_session. Its failure mode is loud rather than silent, an un-wrapped DELETE FROM managed_agent_session hitting MySQL error 1451 on a second run against the same database, so it cannot produce a false green; but the file's own dominant convention of namespacing the tenant per run needs no list at all, and a future migration adding a child table will break the rerun path until someone updates it. R1-11's residual is that the new watermark test verifies only store-side state and carries no harness-side verification, so a handler that drove the continuation twice would not be caught even though scheduleTurnRetry is called once. Both are coverage and maintainability, not correctness at this head.
No Critical found. Approving.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #13348This round was a same-run verification repair: the previous attempt had already implemented both round-4 findings in commit Feedback dispositions1.
|
|
Verdict: 中文摘要结论：findings（1 条非阻塞覆盖缺口） —— 核心主张全部实测成立，是否合并由维护者判断。
A/B baselines — identical production code, only the test files differ (A/A control: both arms must be green, both are):
Mutation A/B — single-point mutants of the unmodified production files, run against head tests and merge-base tests. Coverage mutants expected head-RED/base-GREEN, controls RED/RED. 11/13 coverage mutants killed at head, none killed at base (the two misses are adjudicated below); 6/6 final controls killed on both arms; no mutant regressed from killed-on-base to survived-on-head. Highlights: deleting the
MySqlIT rerunnability (headline claim) — two back-to-back failsafe runs against one persistent throwaway MySQL 8.4.7 datadir: both
Findings (both completeness notes, not defects):
One harness caveat, not a PR issue: when the unknown-outcome mock exception propagates (fallback deleted), JUnit's failure reporting NPEs on the mock and truncates the class at 10/22 — the red verdict was still caused by the removed fallback, and a second mutant (tuple mapping drops Not covered: statistical flake-rate A/B for Full harnesses ( |
…rsions (#13348) - ManagedAgentMySqlIT: replace the single firstPass flag with BackfillWitnesses derived from the versions Flyway had applied at the run's start. One shared flag reads a database left mid-window as a first pass (V2's consumer_progress count then fails on rows V2 never saw) or as a rerun (the V17 and V29 witnesses then silently skip over freshly seeded rows their migrations did migrate). The V9 re-seed now keys on V9's application too: at V14 firstPass is true, so the old keying skipped the re-seed and the idempotency_conflict probe failed. - BackfillWitnessesTest: pin the gate tuple against H2 databases staged at every boundary version (fresh, 1, 8, 9, 14, 15, 16, 17, 28, 29, head); reverting to the shared flag reds it. - QwenHostedHarnessConnectorTest: stub getSuppressed()/getStackTrace() on the mocked unknown-outcome exception so a regressed fallback reports the failing test's name instead of aborting the class report on a null array; capture the LoadHarnessSession and pin the passiveManagedRuntimeRecovery=false operand. - SessionEventHubTest: await one below the watermark so CAPACITY is pinned from above as well (a 513 ring now reds the witness). - HarnessEventProjectorTest: witness the tool_call allowlist, the sibling of the text-path redaction the PR already pinned (the raw update leak mutant now dies). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Round 6 — review feedback addressed (head
|
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Address-review round — PR #13348 (round 7)Feedback dispositions[rv:5415434423] @qwen-code-ci-bot — "Partially reviewed — gaps disclosed" (COMMENTED)
Inline comments: none. Issue-level comments: none. Failed checks: none. Base-branch conflict resolution (
|
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no action takenNo code changes were made in this round. WhyCritical-only mode is active for this PR. The deterministic brake has engaged: 5 change-producing rounds are complete in the current counting window, so this round's actionable sections exclude non-Critical feedback. The actionable sections of the prepared feedback were empty:
The one new item, a review by @qwen-code-ci-bot (link), is recorded in the Deferred non-Critical feedback section. That section is an audit record, not work: per the round rules, those items must not be addressed, have their threads resolved, or receive comment replies from this loop. They remain open for human follow-up, and a maintainer can comment Diff growth this window is within budget (source 186/400, test 0/400 net lines), so no growth audit was required, and What happens nextThe PR is left exactly as it was: working tree untouched, no commits added. A maintainer can act on the deferred non-Critical review directly, or re-arm the loop with 中文说明Autofix 本轮：未采取任何改动本轮未对代码做任何修改。 原因本 PR 已进入仅处理 Critical 的模式。 确定性刹车机制已触发:当前计数窗口内已完成 5 个产生改动的轮次,因此本轮的可执行区域已排除非 Critical 反馈。 准备好的反馈中,各可执行区域均为空:
唯一的新条目是 @qwen-code-ci-bot 的一条评审(链接),被记录在 Deferred non-Critical feedback(延后处理的非 Critical 反馈) 区域。该区域是审计记录,而不是待办工作:按照本轮规则,这些条目不得由本循环处理、不得解决其讨论串、也不得回复评论。它们保持开放状态,留待人工跟进;维护者如需让机器人处理这些条目,可评论 本窗口的 diff 增长在预算之内(源代码净增 186/400 行,测试净增 0/400 行),因此无需进行增长审计;且 后续PR 保持原样:工作区未被触碰,未新增任何提交。维护者可以直接处理被延后的非 Critical 评审,或使用 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Verdict: 中文摘要结论：
What changed since round 1 (
1. Mutation A/B across four arms (figure 1). From
2. Backfill gates on real MySQL 8.4.7 and MariaDB 10.11.18 (figure 2). I staged each database with the module's own Flyway migrations, including the Java V15/V29 migrations. The staged states were V1, 8, 9, 14, 15, 16, 17, 28, 29 and head, plus a fresh database and a genuine second run on the same database. Each cell then runs
3. Does the witness catch a broken backfill? (figure 3). I removed the V17 backfill (the
4. Suites, reruns and merge integrity (figure 4).
Notes for the merge decision (none blocking):
Not covered:
Environment: macOS arm64; Zulu JDK 21.0.12; Maven 3.9.16 with a private local repository per tree ( |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： AutoFix round — no action requiredOutcome: no code change this round. No commit was created and the working tree is untouched. What the feedback containedCritical-only mode is active for this PR (5 change-producing rounds complete), so only Critical findings, failed checks, and requested base-conflict resolution are actionable this round. The only new feedback since the last evaluation is one issue-level comment from @wenshao:
There are no inline comments, no failed checks, no still-red checks, and no requested conflict resolution ( Non-blocking notes from the report (acknowledged, not actionable by this bot)
ConclusionThe maintainer's independent verification passes the PR with no new findings and no blocking notes. With Critical-only mode engaged and zero actionable items, the correct outcome is no action; the merge decision rests with the maintainer. 中文说明AutoFix 本轮 —— 无需处理结论：本轮不改动代码。 未创建任何提交，工作树保持原样。 反馈内容本 PR 已进入仅处理 Critical 的模式（已完成 5 个产生改动的轮次），因此本轮只有 Critical 发现、失败的检查和明确要求处理的基线冲突才可执行。自上次评估以来唯一的新反馈是 @wenshao 的一条 issue 级评论：
本轮没有行内评论、没有失败的检查、没有持续飘红的检查，也没有要求处理冲突（ 报告中的非阻塞说明（已知悉，不属于本 bot 可执行范围）
结论维护者的独立验证已通过本 PR，无新发现、无阻塞说明。在 Critical-only 模式下没有任何可执行项，正确结果就是不采取行动；是否合并由维护者决定。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 57 passed · 2 failed · 59 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：57 通过 · 2 失败 · 59 总计 抖动门：不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR #13348 deep verification —
|
| # | Mutation to production code | Head | Base | Head tests red / class size |
|---|---|---|---|---|
| M1 | SessionEventHub.CAPACITY 512 → 513 |
KILLED | SURVIVED | 1 / 8 (base class had 7) |
| M2 | delete the admission.lastEventId() < attachment.lastEventId() disjunct |
KILLED | SURVIVED | 1 / 33 (base 29) |
| M3 | tool_call path returns the raw update map instead of safeToolData(...) |
KILLED | SURVIVED | 1 / 7 (base 4) |
| M4 | SAFE_ERROR_CODE alphabet check made vacuous |
KILLED | SURVIVED | 2 / 7 |
| M5 | SAFE_ERROR_CODE bound {1,128} → {1,129} |
KILLED | SURVIVED | 1 / 7 |
| M6 | recovered-runtime-not-ready guard disabled | KILLED | SURVIVED | 1 / 33 |
| M7 | bindRecoveredHarness generation-mismatch guard disabled |
KILLED | SURVIVED | 1 / 33 |
| M8 | 409-only create fallback widened to any DaemonHttpException |
KILLED | KILLED | 3 / 28 at head, 2 / 25 at base |
| M9 | catch (SessionCreationOutcomeUnknownException) fallback removed |
KILLED | SURVIVED | 1 error / 28 |
| M10 | Attachment drops lastEventId + eventEpoch on the create path |
KILLED | SURVIVED | 2 / 28 |
| M11 | per-page 8 MiB byte budget never cuts the page | KILLED | SURVIVED | 1 / 5 (base 4) |
| M12 | BackfillWitnesses reverted to one shared firstPass flag |
KILLED | target absent at base | 1 / 1 (class is new) |
Head 12/12 killed. Base 1/11 killed. Zero killed→survived regressions. The class-size deltas (7→8, 29→33, 4→7, 25→28, 4→5) independently confirm the witnesses are additions, and that each mutant ran against a suite that actually collected the mutated code.
The PR's own four mutation probes — all verified
The commit messages make four falsifiable mutation claims. Each was measured, not read:
| Claim (commit) | Measured |
|---|---|
| "a 513 ring now reds the witness" (55d45fc) | M1: exactly evictsBeyondCapacityAndSignalsOverflowBelowTheWatermark red, 1 of 8. Assertion is the intended one — Expecting value to be true but was false at SessionEventHubTest.java:84, the await(87) "one below the watermark" arm. Verified. |
| "deleting the disjunct fails exactly this test" (fd78979) | M2: exactly 1 of 33 red, retriesTheTurnWhenTheRecoveredWatermarkRegresses, on recordRecoveryAdmission — Never wanted here … But invoked here -> HarnessCoordinator.runClaimed(HarnessCoordinator.java:325) with arguments: […, epoch-new, epoch-new, 7]. The regressed watermark 7 is visible in the arguments, so the red is the behavioural mismatch (the regressed cursor being persisted), not a crash. Verified, including the word "exactly". See 03-m2-disjunct-kills-exactly-one-of-33.png. |
| "the raw update leak mutant now dies" (55d45fc) | M3: exactly 1 of 7 red, projectsToolCallsWithoutLeakingOtherUpdateFields, with the leak printed: Expecting actual: "{rawInput={path=/private/workspace}, secret=must-not-leak, …}" not to contain: "secret". Verified. |
| "reverting to the shared flag reds it" (55d45fc) | M12: gatesFollowTheVersionsPendingAtRunStart red with expected: BackfillWitnesses[consumerProgress=true, eventIdentity=true, pendingOperations=true, hookAdmissions=true, reseedCreationScope=false] but was: BackfillWitnesses[consumerProgress=false, …, reseedCreationScope=true]. Verified. |
Kill quality, honestly classified
Not every red is equally strong, and the difference matters:
- Intended-assertion kills (10): M1, M2, M3, M4, M5, M6, M7, M10, M11, M12 all fail on an AssertJ/Mockito assertion naming the expected-versus-actual value of the contract under test. M11 is a good example:
expected:<9> but was:<10>— the page cut at 9 transactions is exactly the 8 MiB budget. - Escaping-exception kill (1): M9's red is
Errors: 1, the mockedSessionCreationOutcomeUnknownExceptionescapingcreateOrLoadbecause the fallback catch was removed. This is not a failed assertion, but it does demonstrate the behaviour under test (fall back toload) did not happen. Counted as a kill with the mechanism stated. - Incidental-crash kill (1): M8's three reds are all
Expecting actual: java.lang.NullPointerException— with the fallback widened, the unstubbedclient.loadSession(...)returnsnullandattachments.put(key, null)throws insideConcurrentHashMap. The mutant is detected, but by a crash rather than by the intended.isSameAs(failure)mismatch. Worth knowing: M8 was already killed at base by two pre-existing tests (failedAttachmentReleasesTheAttachmentLock,failedFirstAttachmentKeepsQueuedCallersInASingleFlight), so the newrethrowsANonConflictDaemonErrorWithoutLoadingis a named witness for the 409-only guard, not its unique defence. That is redundant defence — correct as it stands, not a gap.
Positive control
The unmutated head run is green: 113 tests, 0 failures, 0 errors, 0 skipped across all 8 touched classes (01-baseline-green-113-tests.png). Combined with 12/12 kills, this rules out "the harness never ran the suite". Each mutant ran the whole class (not a single method), so a kill also proves the runner collected the mutated file, and the surviving base arm proves the harness can report green.
Corrections to the PR description
These are corrections to the description, not requests to change the code. In both cases the shipped code is the better of the two.
-
Redaction assertions (Reviewer Test Plan, "Projector codes & redaction"). The body says the existing redaction test "now also asserts
doesNotContainKey("secret")anddoesNotContainValue("must-not-leak")". Neither form appears anywhere inHarnessEventProjectorTest.java. What the test actually asserts (lines 33–34) isassertThat(event.data().toString()).doesNotContain("secret").doesNotContain("must-not-leak")— a substring check on the serialized map. That is stronger than the described form, because it holds at any nesting depth, which is precisely the reason commitbd2e6e7bgives for the change. The body was not updated when the assertion was strengthened. -
Turn cleanup mechanism (Reviewer Test Plan, "Integration flake"). The body says
ManagedAgentServerIntegrationTest"now deterministically cleans up claimed turns in a finally wrap". There is no per-call-sitefinallycleanup in the file. Cleanup is a single@AfterEach settleLeftoverTurnsAndRestoreHarness()that runs one SQLUPDATEsettling every non-terminal Turn. Commitfd78979din this same PR states it replaced the per-call-site helper with that sweep and that "the concurrent-submit finally whose re-await could mask the real failure goes away with it". The body describes a state this PR itself removed. -
A cited gate is vacuous for this diff. Commit
f093e176records "mvn checkstyle:checkgreen" as part of its repair evidence. That gate is real but blind here:includeTestSourceDirectoryis unset inmanaged-agent-server/pom.xml(and inqwencode/pom.xml), so the plugin's default offalseapplies and onlysrc/main/javais scanned. Measured with a planted-violation control (05-checkstyle-gate-blind-to-test-sources.png): the same tab character produces 3 violations + BUILD FAILURE in a main source file and 0 violations + BUILD SUCCESS in a test source file. For a PR whose entire diff is test code, that citation carries no information.I then answered the question the gate cannot, by overriding
<includeTestSourceDirectory>true</includeTestSourceDirectory>in the pom — itself validated with a planted-violation control (276 violations reported, so the override genuinely scans test sources). Result (06-checkstyle-ab-pr-adds-no-violations.png):arm test-source violations (whole module) in the PR's 10 files head 274 21 base ( HEAD^1)275 22 The PR introduces no new violations and removes one (
HarnessCoordinatorTest2 → 1). Both new files,BackfillWitnesses.javaandBackfillWitnessesTest.java, are clean at 0. All 21 remaining violations in PR-touched files are inherited, part of 274 across 46 test files whose largest offenders (HostedWorkspaceToolTurnIT28,O4MySqlGate22,HostedPublicWorkspaceIT21) this PR does not touch. So the disabled-test-scan configuration looks like deliberate accommodation of pre-existing debt, and this PR is not a reason to change it. Recording this because the author's evidence was vacuous, not because the code is dirty — it is clean.
Findings
Only the two description inaccuracies above (F1, F2 in assertions-detail.json), both low severity, both documentation-only. They are the reason the verdict is findings rather than merge-ready under the counting contract: a reviewer following the Reviewer Test Plan would look for two assertions that do not exist and for a finally block that was deliberately deleted.
No blocking finding. No behavioural defect. No survivor in the mutation matrix.
Two non-blocking observations, neither a defect:
- The
@AfterEachsweep is a blunt instrument by design. It force-fails every non-terminal Turn in the class's H2 database witherror_code = 'test_cleanup'. That is the right trade for cross-test isolation, but it means a future test that intends to leave a Turn non-terminal and assert on it from a later test would be silently broken by the sweep. Measured as harmless today: 28/28 green on 5 consecutive runs (07-fixture-mutant-and-determinism-x5.png). - The flake claim is confirmed only in the forward direction. I can confirm the class is now deterministic across 5 consecutive runs and across the full 976-test module run. I cannot confirm the pre-PR "~1-of-N flakes" rate, which would need the base test surface run many times under load; the mechanism (a shared single-shot
runtimeRecoveryfield consumed by whichever test claimed a dispatchable turn first, now a per-tenantConcurrentHashMapwithremove) is consistent with the described flake, and the base arm shows the old shape compiling and passing in isolation, which is what an order-dependent flake looks like.
Not covered
ManagedAgentMySqlITwas never executed. It is a failsafe IT requiring a real MariaDB (-Pmysql-integration -Dmysql.url=…). This container has nodocker, nomysql/mariadbdbinaries, and nosudo/apt(running as uid 1000node;/var/lib/apt/lists/lockis permission-denied). So the PR's central rerunnability claim — "run twice against the same persistent database; both runs go green (19/19)" — is unverified by execution. What I did verify instead: theBackfillWitnessesgate tuple it depends on, viaBackfillWitnessesTeston H2 staged at 11 boundary versions (fresh, 1, 8, 9, 14, 15, 16, 17, 28, 29, head), plus mutant M12 proving that gate is load-bearing; and theTests run: 19figure by arithmetic — 13@Testmethods plus a 4-value@ValueSourceplus a 2-value@ValueSource= 19, matching the claim exactly (a naivegrep -c "@Test"gives 14 because it also matches@TestMethodOrder). The namespacing-per-run rewrite and thelegacySeedsBackfilledconditionalization were read, not run.- Per-commit attribution. The checkout is shallow (
git rev-parse --is-shallow-repository→true) andgit rev-list HEAD^1..HEAD^2returns 1 commit while the snapshot lists 9. Only the aggregateHEAD^1..HEADdiff was verified. The four mutation-probe claims quoted above come from commit messages in the snapshot, which I tested against the aggregate head tree — I could not attribute any behaviour to an individual commit. - The
hosted-harness-mysql,o4-mysql-gatesandfault-gateslanes, andscripts/check-failsafe-reports.js(which requires failsafe reports that only exist after the ITs run). All need MySQL/MariaDB. - The Java 11 and Java 17 lanes of
sdk-java.yml. I installed only Temurin 21 (matchingmanaged-agent-server's<java.version>21</java.version>and the lane that actually runs this module). runtime-brokerandqwencodetest suites. Staged with-DskipTeststo make the module's test compile resolve; their tests are not affected by this PR (no production change) and were not run.- SpotBugs. Skipped on every invocation (
-Dspotbugs.skip=true) to stay inside budget. It analysessrc/main, which this PR does not touch. - No full-repo JS/TS gate was run (
npm run lint,npm run typecheck,npm run build). The diff contains no TypeScript; the tree was already built by the lane before this round started.
Methodology
Environment: the CI verify lane container (node:22-bookworm, x86_64, 64 cores, 240 GB RAM), working tree at the merge-ref checkout df63aed9c. The container ships no JDK and no Maven, so I bootstrapped both into $HOME/tools: Temurin JDK 21.0.12.1 from the Adoptium API, and Apache Maven 3.9.11 whose tarball SHA512 I checked against the value pinned in .github/workflows/sdk-java.yml (MAVEN_SHA512=bcfe4fe3…) — it matched. Sibling modules were staged with the workflow's own commands (qwencode then runtime-broker, -DskipTests … install), then managed-agent-server was test-compiled; all three stages exited 0 in ~50 s total, and ~/.m2 was empty beforehand so every dependency resolved from Maven Central.
Because this PR changes no production code, the head and base arms differ only in their test surface, so no dependency-tree confound applies and the usual monorepo symlink hazard does not arise — the Java modules resolve their siblings from ~/.m2 jars built from the same (unchanged) source on both arms. The base arm was produced by git checkout HEAD^1 -- <8 modified test files> plus deleting the 2 added files, with git checkout HEAD -- … restoring afterwards; both arms asserted a clean git status --porcelain -- packages/sdk-java before starting and after restoring. The round ends with git status --porcelain completely empty — the artifact directory is under tmp/, which .gitignore:134 excludes, so no residue of any mutant, planted violation, or temporary pom edit remains in the tree.
Mutants were applied as exact single-occurrence literal replacements (the harness aborts a mutant as ANCHOR-NOT-UNIQUE if the anchor does not match exactly once), each followed by a full mvn test -Dtest=<Class> run of the class containing the witness, so a kill also proves the runner collected the mutated file. Twelve mutants ran on the head arm and eleven on the base arm — M12's target, the new BackfillWitnesses.java, does not exist at HEAD^1, so the harness reported it TARGET-ABSENT-AT-BASE rather than fabricating an outcome. Raw per-mutant Maven output is in mutants/M*-<Class>.log and mutants/base-M*-<Class>.log, with machine-readable results in mutants/results.json, mutants/base-results.json and mutants/attribution.json. Harnesses: mutate.mjs (head arm), mutate-base.mjs (base arm plus the A/A control), probes.sh (checkstyle liveness, fixture mutant, determinism repeats), cs-test-scan.sh and cs-ab.sh (per-file checkstyle attribution, head vs base).
The one full-suite error at head — RuntimeBrokerDefaultOnTest.defaultCombinationBootsWithTheYmlDefaultsBound, 976 tests run with 0 failures and 1 error — was traced to java.nio.file.NoSuchFileException: /etc/machine-id, which the broker's durable-local-process mode requires; /etc/machine-id is confirmed absent in this container. Per the A/A rule I did not stop at "seems environmental": the same class was run on the base test surface and failed identically (exit=1, Errors: 1, same /etc/machine-id cause), and the class is byte-identical between arms (git diff --stat HEAD^1..HEAD -- '*RuntimeBrokerDefaultOnTest.java' is empty). Repo gates that did run: node scripts/check-flyway-migrations.js (45 migrations, all versions unique, exit 0) and mvn checkstyle:check (0 violations on main sources), both proven live by planted-violation controls before being cited.
One probe cell was superseded rather than counted: -Dcheckstyle.includeTestSourceDirectory=true on the command line silently had no effect — a tab planted in a test file still reported 0 violations. The planted-violation control caught this before the vacuous green could be cited, and the measurement was redone via a pom <configuration> override that was itself validated the same way. It is recorded in assertions-detail.json under harnessNote and excluded from the pass/fail tally because it is a Maven property-binding fact about this container, not a property of the PR.
Evidence images were rendered with scripts/verify-capture.mjs from the real command output; every number quoted above comes from a saved log in this artifact directory.
Flakiness gate log
verdict: n/a
summary: no runnable changed test files (0 out-of-scope file(s) noted in the log)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Reviewed head: bd13b6151591d8a94a784eca722e9d8debe38e57 (base main).
Approve. No historical blocking finding stands at this head, and the Critical-only scan found nothing blocking. This diff is test-only — all ten files are under src/test/java/ and no production source changes — so the one Critical class that applies is a false green: a witness that silently skips, silently narrows, or asserts something that cannot fail. That is what I looked for, and the changes move in the opposite direction.
Historical blocking findings
None to re-verify. All three review threads are resolved, there are no [Critical] inline comments and no review ledger, and reviewDecision is REVIEW_REQUIRED. The two dismissed reviews — yiliang114's LGTM and my own earlier pass at fd78979d — were dismissed by head movement and are not standing positions; this is a fresh pass over the current head, not a restatement of that one.
Critical-only scan: no false green
Nothing in the diff disables or conditions a test out. Across all 1,022 patch lines there is no added @Disabled, @EnabledIf, @DisabledIf, assumeTrue, @Tag or environment-variable gate. The only two occurrences of the word "skip" are in comments describing silent-skip behaviour the PR removes. Assertion density is real: 79 added assert*/verify( lines against ~759 added lines.
The migration witnesses are gated on the database's prior state, not on the code under test — which is what keeps a regression from switching off its own witness. BackfillWitnesses.appliedVersions(dataSource) reads Flyway's MigrationInfo.applied() before the run migrates anything, and forApplied(...) derives one boolean per witness: consumerProgress = !applied("2"), eventIdentity = !applied("15"), pendingOperations = !applied("17"), hookAdmissions = !applied("29"), reseedCreationScope = applied("9"). Each one-shot backfill applies only to rows present when it runs, so its witness is valid only when that specific migration was still pending at the run's start — and because the gate comes from the database rather than from the migration's outcome, a V15 regression cannot turn off the V15 witness. The helper's Javadoc names the exact trap this replaces: gating all of them on one shared flag reads a database left mid-window either as a first pass (so the V2-derived count fails) or as a rerun (so the V17 and V29 witnesses silently skip). The mixed polarity is deliberate and the fields are named for the witness rather than the migration, so the direction stays legible.
Two cross-test contamination paths are closed, and both were capable of making a test pass for the wrong reason. In ManagedAgentServerIntegrationTest, the @AfterEach now settles every Turn the test left non-terminal (ACCEPTED/RUNNING/CANCELLING → FAILED with error_code = 'test_cleanup', clearing dispatch_owner, dispatch_lease_until and retry_after) before re-arming the harness; the comment records why — a leftover Turn would be claimed once its dispatch lease expired and poison the counter-sensitive tests running later in the same shared H2 context. And FixtureHarness replaces its single global volatile HarnessRuntimeRecovery slot with a per-tenant map consumed by tenantRecoveries.remove(tenantId), so a test arms a recovery only its own tenant can consume and another test's background claim can never steal it. That is precisely the "second test to claim a dispatchable turn silently won" shape the PR set out to fix. Neither change weakens an assertion; the rest of that file's diff is assertion reflow with no semantic change.
The overflow witness cannot pass vacuously, and it pins the constant from both sides. evictsBeyondCapacityAndSignalsOverflowBelowTheWatermark publishes 600 events against CAPACITY = 512 and then makes three distinct observations: below the watermark await(0) must report overflowed == true with empty events (so a hub that stalls forever fails on the bounded 10 ms await, and one that returns nothing without the flag fails on the assertion); at the watermark await(88) must return exactly 89..600 via containsExactlyElementsOf, an ordered exact-sequence check rather than a size or contains check; and one below the watermark await(87) must also report overflowed == true, with the comment stating the purpose — "pins CAPACITY from above too, so a ring grown to 513 can no longer keep this green." An off-by-one in either direction therefore fails, which is the property that makes the witness non-vacuous rather than merely present.
No Critical found.
CI
Fully settled and green at this head: 24 pass, 27 skipped, 0 failed, 0 pending. The passing set includes the whole Java matrix (ubuntu Java 11/17/21, windows Java 21, macos Java 21) and both database-backed lanes — Runtime Broker and Managed Agent MariaDB / Java 21 and Hosted process fault gates / MySQL 8.4 / Java 21 — which are what execute the managed-agent-server witnesses, plus Flyway migration version uniqueness. Nothing to attribute to this PR.
Not scanned — disclosed, not asserted clean
Two honest limits on this approval. First, the MySQL IT's witnesses are conditional on the database's starting state by design — that conditionality is what makes the IT rerunnable at all — so against an already-migrated persistent database they skip. I did not prove by CI log-marker multiplicity that each individual witness executed in a lane rather than being correctly skipped as inapplicable. Second, I did not re-run the mutation witnesses the PR claims (each written against pre-change code and confirmed to fail or expose the gap first); I assessed the witnesses' assertion shapes for non-vacuity instead, which is what the capacity bracketing and the exact-sequence checks above evidence. I also did not read every one of the twelve witnesses line by line — the connector tuple-propagation, projector redaction and store-contract fixture cases were reviewed at the level of their added assertions rather than in full.
Scope note
Approval is bound to commit bd13b615. Since no production code changes, this approval concerns the test surface only: that the new witnesses pin what they claim, and that the two shared-state fixes remove contamination rather than masking it.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
No blocking findings at bd13b61 — approving.
This head has moved past two stale-head approvals (mine at c1a61b2, qqqys's at fd78979), so I re-reviewed the full base...head diff plus the three content commits after fd78979. The post-approval delta only strengthens the witnesses: 3a3b203 adds never scheduleTurnRetry to both recovery failure modes and exactly-once continueManagedRuntime / never-submit / never-failTurn guards on the recovery paths (closing the harness-side gap left by R1-11), and 55d45fc replaces the shared firstPass flag with per-migration BackfillWitnesses gates, pinned by an H2/Flyway boundary test at every boundary version. f093e17 is an empty re-verify commit.
Two things I checked specifically at this head:
- The witness gates are derived from the versions applied before this run migrates anything, so a broken backfill can't switch off its own witness; the V9 re-seed conditionalizes arrange only, and the
idempotency_conflictprobe stays unconditional. TheSessionEventHuboverflow witness pins CAPACITY from both sides (await(87)still reports overflow), so growing the ring to 513 turns it red instead of silently passing. - The main-merge reconciliation in the four overlapping test files is purely additive, and the contracts these tests pin still exist in the merged production code (hub
CAPACITY = 512+ overflow signaling; the connector'sSessionCreationOutcomeUnknownExceptionfallback at QwenHostedHarnessConnector.java:347).
CI is green on the test-bearing commit 55d45fc across all Java lanes, MariaDB, and the MySQL 8.4 fault gates; bd13b61 is a pure main-merge, so the full lanes did not re-run on the merge commit itself.
Non-blocking residual (R4-1): the rerun cleanup lists are now tenant-scoped and finally-wrapped in child-before-parent order, so a future child table fails loudly (FK 1451) instead of silently passing — but they still need a human touch when migrations add tables.
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： AutoFix round — no action required🤖 Reviewed the latest feedback — no changes needed. Round basis: head What was triaged this round
ConclusionNo Critical finding, no maintainer request, and no failing check is outstanding. The PR stands approved at the current head, so this round makes no code changes and posts no thread replies. 中文说明AutoFix 本轮——无需处理🤖 已审阅最新反馈——无需改动。 本轮依据： head 本轮分诊内容
结论没有未解决的 Critical 发现、维护者请求或失败的检查。PR 已在当前 head 获得批准,因此本轮不做任何代码改动,也不在讨论线程中发布回复。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
…-harness Absorbs fd4af70 (test: pin untested contracts from the #12692 R2 review, #13348): contract witnesses for the connector attachment propagation, the recovered-watermark drive guards, the store integration and the hub suites. Resolutions: - QwenHostedHarnessConnectorTest: this branch's load-timeout construction witness and main's three attachment-propagation tests coexist (distinct subjects, shared-tail splice). - HarnessCoordinatorTest: main's two new never-admission verifications name the old pre-R10 recordRecoveryAdmission shape; widened their matchers to the split-expectation signature (turn/session epochs are separate operands since R10-3) without touching their intent. Verified: HarnessCoordinatorTest 59, QwenHostedHarnessConnectorTest 38, ManagedAgentServerIntegrationTest 29; the test compile covers every auto-merged suite (Hub, Projector, Store, Backfill, MySQL).
|
Thanks — this one is easy to gate and worth gating carefully, because "tests only" is exactly the shape where a witness can pass whether or not the contract holds. Template looks good ✓ Problem: real and documented, not theoretical. These are the twelve test-surface follow-ups from the R2 post-merge review of #12692 (merged 2026-09-25), with the design-level deferrals tracked separately in #13302 (still open). I did not take the gap list on faith — I confirmed each named branch exists in production at this head: Direction: aligned. Our own review rules treat a missing test for changed behavior as a Suggestion rather than a blocker, which means those suggestions legitimately land after the merge — this is that follow-up. No production surface is touched, so there is no direction risk to weigh. Size: not applicable. All ten files sit under Approach: the scope is right — one witness per named branch, added to the existing test class for that component rather than into a new parallel harness. Two things I would question, neither blocking:
The one place I expected over-engineering was Risk: no elevated risk signals — nothing here matches the high-risk paths we track. One practical note: the branch is 31 commits behind Moving on to code review. 🔍 中文说明感谢贡献！这个 PR 属于「只加测试」的类型，恰恰是最需要认真把关的形态——见证用例可能在契约成立与否两种情况下都绿。 模板完整 ✓ 问题： 真实且有据可查，不是理论性加固。这是 #12692（已于 2026-09-25 合并）合并后 R2 评审提出的 12 条测试面跟进项，设计级延缓项由 #13302（仍 open）单独跟踪。我没有直接采信这份缺口清单——逐条核实了对应分支在本地 head 的生产代码中确实存在：coordinator 里的 方向： 对齐。我们自己的评审规则把「行为变更缺测试」定为 Suggestion 而非阻塞项，因此这类补充本来就应该在合并之后落地——本 PR 就是这个跟进。不涉及任何生产接口，没有方向性风险需要权衡。 规模： 不适用。10 个文件全部位于 方案： 范围合理——每条命名分支一条见证，加进该组件既有的测试类，而不是另起一套平行夹具。有两点想提出，均不阻塞：
我原以为会过度设计的地方是 风险： 无升级风险信号——未命中我们跟踪的任何高风险路径。一个实际提示：分支落后 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewBecause the branch is 31 commits behind Event hub overflow. The witness pins Projector. The 128/129 pair is exactly right against Connector recovery. Production is Per-page byte budget. I checked the arithmetic rather than trusting the expected counts. Paging breaks on MySqlIT rerunnability. Per-run UUID namespacing is the correct fix for fixture collisions, and deriving the gates from the versions applied before this run migrates anything — rather than from the migrated rows — avoids the self-disabling failure mode the comment describes, where a regression in one migration switches off its own witness. Two deletions verified:
Integration flake. Keying the fixture's recovery arm by tenant ( Two nits, both non-blocking: the formatting churn named in Stage 1, and the Test evidenceThis is an unattended CI run, so per the gate's rules I did not build or execute anything from this PR. The evidence below is the PR's own CI on the reviewed commit, fetched through the API: 24 non-skipped checks, all
What that green actually covers, and what it does not. The MariaDB lane runs Not verified: the load-bearing claim itself. A green suite proves these tests pass; it cannot prove they fail when the pinned contract breaks. That claim currently rests on the author's word — specifically the maintainer's own round-2 local report on this exact head, which describes mutation A/B kills (passive-recovery flag, Sandboxed verification would settle exactly that gap: Real-scenario testing: N/A — unattended CI run, and a test-only Java change with nothing user-visible to drive. 中文说明代码审查由于分支落后 集线器溢出。 该用例从两个方向钉住 投影器。 128/129 这一对与 连接器恢复。 生产代码是 单页字节预算。 我没有采信期望值，而是自己算了。分页在 MySqlIT 可重跑。 按运行 UUID 命名空间化是修复 fixture 碰撞的正确做法；门控取自本次运行开始迁移之前已应用的版本（而不是迁移后的行），避免了注释所说的自失效模式——某个迁移回归时会把自己的见证关掉。两处删除已核实：
集成抖动。 把夹具的 recovery arm 按租户分键（ 两个小问题，均不阻塞：Stage 1 提到的格式化杂音；以及预算测试里的 测试证据这是无人值守的 CI 运行，按门禁规则我没有构建或执行本 PR 的任何代码。下面的证据来自通过 API 取到的、针对被评审提交的 PR 自身 CI：24 项非 skipped 检查全部 （CI 表格见上方英文区，此处不重复。） 这份绿实际覆盖了什么、没覆盖什么。 MariaDB 通道对全新的 MariaDB 10.11.18 服务容器执行 未验证：承载性结论本身。 绿色的套件只证明这些测试通过，无法证明「被钉契约被破坏时它们会失败」。该结论目前依赖作者陈述——具体是维护者本人针对这个 head 的第 2 轮本地验证报告，其中描述了变异 A/B 击杀（passive-recovery 标志、 沙箱验证正好能补上这个缺口： 真实场景测试：N/A —— 无人值守 CI 运行，且为纯测试的 Java 改动，没有任何用户可见行为可驱动。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — every contract this PR pins checks out against production at the reviewed head and CI is fully green; the only things I would change are cosmetic, and the one gap that matters is inherent to CI rather than to this PR. My independent baseline before reading the diff was: one focused witness per R2 finding, added to the existing test class for that component; namespace the MySqlIT fixtures per run; derive the migration-bound gates from run-start state rather than a hand-maintained flag; and add deterministic cleanup for the shared-context flake. That is essentially what landed. I did not find a materially simpler path, and the one piece of machinery I was ready to challenge — The failure mode I was hunting for is the one that makes tests-only PRs dangerous: a witness that stays green whether or not the contract holds. I checked the three that are cheap to check by arithmetic rather than by trust — hub capacity from both sides, the 128/129 error-code boundary, and the 8 MiB page split — and all three are calibrated to go red on a one-unit change. The deletions worried me more than the additions, and both hold up: the removed Reservations, all non-blocking and none of them reasons to hold this up:
Would I curse whoever wrote this in six months? No — the comments explain why each witness is shaped the way it is, which is precisely the part that usually goes missing and the part that stops someone "simplifying" a deliberate boundary assertion later. Approving, pinned to the commit I reviewed. 中文说明信心：4/5 —— 本 PR 钉住的每一条契约都在被评审 head 的生产代码上核对通过，CI 全绿；我想改的地方都属外观层面，唯一实质缺口来自 CI 本身的性质，而非这个 PR。 读 diff 之前我的独立方案是：每条 R2 发现补一条聚焦见证，加进该组件既有的测试类；MySqlIT 夹具按运行做命名空间隔离；迁移相关门控取自运行起始状态而不是手工维护的标志；并为共享上下文抖动加上确定性清理。落地的基本就是这个方案。我没有找到明显更简的路径；而我原本准备质疑的那套机制—— 我要找的失效模式，正是让「只加测试」PR 危险的那种：契约成立与否都保持绿色的见证。我挑了三条能靠算术而非采信来核对的——集线器容量的双向钉定、128/129 错误码边界、8 MiB 分页切分——三条都在「改动一个单位」时会变红。删除比新增更让我警惕，而两处都站得住：删掉的 保留意见，均不阻塞，也不构成压住这个 PR 的理由：
六个月后我会骂写这段代码的人吗？不会——注释解释了每条见证为什么是这个形状，而这恰恰通常缺失的那部分，也是阻止后人把一个刻意的边界断言「简化」掉的那部分。予以批准，并钉定在我实际评审的那个提交上。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…e-budget test How: holdsRestorePagesInsideThePerPageByteBudget now builds each dense transaction from 210 well-formed message.delta lines (text at the reader's 4096-byte cap, ~1 MB per transaction) plus the commit marker, through a shared TurnEventLines composer, and asserts that revisions 1..9 fit the 8 MiB page budget while a tenth does not. The reader replay IT now also replays that delta shape. The class turns off MockMvc print-on-failure. Why: #13348 added the test with placeholder lines ({"subtype":"event_v1"} plus a padded commit_v1 line), and #13355 (merged later, never run against it) made the commit gate refuse any non-event line inside the event range, so every commit after genesis answered 409 and main's SDK Java lanes went red. The failure was masked as timeouts: Spring Boot printed the failed test's ~1.3 MB request body as one log line, which stalled the Actions log pipeline for ~45 min, so the Hosted Verify and O4 steps blew their ceilings and the MariaDB lane hit its job timeout. Test: ManagedSessionStoreIntegrationTest and ManagedExtensionRecordStoreTest green; full managed-agent-server surefire (1077) and checkstyle green; HostedCommittedEventLineReplayIT green through the real TS reader and red when the delta text exceeds 4096 bytes; dropping the page byte budget in ManagedSessionStore fails the test (10 rows instead of 9) with no log line over 400 chars.
…e-budget test (QwenLM#13551) How: holdsRestorePagesInsideThePerPageByteBudget now builds each dense transaction from 210 well-formed message.delta lines (text at the reader's 4096-byte cap, ~1 MB per transaction) plus the commit marker, through a shared TurnEventLines composer, and asserts that revisions 1..9 fit the 8 MiB page budget while a tenth does not. The reader replay IT now also replays that delta shape. The class turns off MockMvc print-on-failure. Why: QwenLM#13348 added the test with placeholder lines ({"subtype":"event_v1"} plus a padded commit_v1 line), and QwenLM#13355 (merged later, never run against it) made the commit gate refuse any non-event line inside the event range, so every commit after genesis answered 409 and main's SDK Java lanes went red. The failure was masked as timeouts: Spring Boot printed the failed test's ~1.3 MB request body as one log line, which stalled the Actions log pipeline for ~45 min, so the Hosted Verify and O4 steps blew their ceilings and the MariaDB lane hit its job timeout. Test: ManagedSessionStoreIntegrationTest and ManagedExtensionRecordStoreTest green; full managed-agent-server surefire (1077) and checkstyle green; HostedCommittedEventLineReplayIT green through the real TS reader and red when the delta text exceeds 4096 bytes; dropping the page byte budget in ManagedSessionStore fails the test (10 rows instead of 9) with no log line over 400 chars.
…wenLM#13174) * feat(managed-agent): adopt the next Hosted Harness generation (G3) A Hosted Session no longer dies with the Harness process generation that first served it (QwenLM#12952 G3, Steps 1+2). On a generation signal the control plane renegotiates once through the connector instead of failing every bound Session until its own restart; pending Turns re-attach through the takeover load as their retries come due. Takeover refusals split into a typed terminal decline (deterministic in the journal) and the existing retriable code; the takeover load is idempotent against a lost reply and gets its own timeout; negotiation requires the journal-contract feature token so a rolled-back Harness is refused once; a marked-but-never-admitted Turn withdraws its submission mark, backstopped by journal commandId idempotency. The failover runner gains a Harness-only restart arm on all three scenarios (harness_boot_id moves with no Java restart) and a SIGSTOP/SIGCONT frozen former-owner fencing arm, wired into hosted-harness-mysql together with the previously local-only basic mode. Design docs included in English and Chinese. * fix(managed-agent): tolerate the bootstrap 404 of a restarting Hosted Harness The serve delegating app answers every routed path with a bare pre-contract 404 while its runtime is still starting. The Harness-restart arms raced that window: a prompt submitted milliseconds after the new process bound the port could meet the bare 404, which the client read as a generation contract violation and ended the Turn as hosted_harness_protocol_error without any retry. A missing boot header on 404 now maps to a transient transport error so the coordinator retries through the window, and the failover runner waits on deep health (?deep=1) before driving turns into a restarted Harness. Terminal turn failures now also log their underlying exception. * fix(managed-agent): freeze only the Harness writer in the former-owner fencing arm SIGSTOPping both original owners left the original Spring's workspace binding unreclaimable: the reclaim proves death from /proc liveness, and a stopped JVM still reads as alive there, so the replacement's reconcile timed out (runtime_broker_reconcile_timeout) and the Turn never completed. The arm now stops only the original Harness (reclaiming its workspace binding requires the Spring to die anyway — the continuation mode's crashProcess), completes the Turn with a full replacement, then wakes the frozen writer and asserts the journal, transcript, and session binding are untouched. Design D7 updated in both languages with the discovered boundary: resource-level takeover of a surviving owner is structurally out of reach today, which is exactly what this issue's sequential-generation scope excludes. * ci: retrigger sdk-java workflow after event delivery stall * fix(managed-agent): pin the fencing proof on writer identity, not head revision The first merge-base run of the freeze arm failed its own gate: after waking the frozen Harness the journal head moved (rev 30 -> 45) while the writer generation, boot binding, transcript, and terminal count all held. The moved revisions were the replacement's own legal churn, not old-writer mutation, so the head-equality assertion conflated the two. The exit-check claim is identity: no transaction out of the old writer generation after it wakes. Count that straight off qwen_managed_session_journal_tx (which carries writer_generation), keep the binding/transcript/terminal assertions, and align design D7 in both languages. The error string now also reports the old-writer tx count. * fix(managed-agent): qualify the journal tx schema in the freeze-arm gate The identity-based old-writer count queried qwen_managed_session_journal_tx unqualified; the runner's MySQL connection has no default schema, so the arm died with ERROR 1046 instead of evaluating the fence. Qualify it as qwen_managed_agent.qwen_managed_session_journal_tx like every other query in this runner. No behavior change to the assertions themselves. * fix(managed-agent): harden takeover correctness from the R1 review - settle turn_settled checkpoints through the load route's own tail instead of a terminal decline, and keep transient store verdicts (missing_state/missing_checkpoint) retriable on both authorization reads - exempt only takeover-shaped 409s from the pre-admission retry budget; every other failure kind meets it - scope the 120s load-timeout to recovery loads so plain attaches under the CHM bin locks stay on 30s - adoptGeneration: move map churn out of the connector monitor (lock-order inversion), evict only stale-boot entries on equal-boot mismatches - answer a duplicate submit of an already-settled prompt with a 202 replay anchored at its input.accepted sequence - withdrawSubmissionAttempted: guard on the same status window as its peer CAS methods; complete capability mismatch terminally in both remaining coordinator layers - runner: freeze-arm pre-freeze liveness assertion, /health-proven wake non-vacuity, teardown SIGCONT, exact-string dispatch-generation bounds - CI: job ceiling 95 with corrected ceiling math; pin all seven failover arms in hosted-process-ci.test.js - sync both design-doc languages with the shipped behavior * fix(managed-agent): close the R2 review round on the replaceable-host-harness slice - withdraw + cancel interference (R2-1, Critical): a CANCELLING Turn whose never-admitted mark was withdrawn is fast-cancelled instead of being re-dispatched into a second live execution at the adopted generation - the generation-mismatch catch escapes the pre-admission budget only on a bound Session, like every sibling arm (R2-3) - bound the remote-supplied decline reason to the error_message column so the terminal write itself never throws (R2-5) - the last fail(...) in the catch ladder is failTerminally with the exception logged, like its siblings (R2-6); the withdrawal itself is logged too (R2-7) - freeze arm: the replacement Spring inherits the original port so the woken writer meets a live fencing control plane, not a dead socket (R2-2); the wake block asserts the head actually advanced past the frozen owner's writer generation, drops the wrong heartbeat claim (R1-8 follow-up), separates the two disk-audit meanings, and asserts the retained Harness home (R2-8) - the hosted /capabilities feature list derives from the registry instead of a hand-maintained literal (R2-9) - docs: D7 scoped to the journal half of the Q2 exit check, the binding half and the pre-contract-404 classification recorded in Boundaries (R2-4, R2-10) * docs(sdk-java): correct the load-timeout coverage comment in the client test * fix(managed-agent): reference the frozen Harness through the children registry in teardown The R1-4 teardown SIGCONT referenced the try-block-scoped `harness` binding from the finally block, so every freeze-arm run died with ReferenceError after all fencing assertions had already passed (CI run 37009843147). Reach the child through the children registry by its start() name instead. * docs(managed-agent): scope the adoption sentence to this slice's takeover reach * fix(managed-agent): close the R3 review round at its five shared roots - retract the turn_settled settle tail to a typed decline: the tail answered 200 with no recovery watermark (Java then failed the bind, orphaned a completed journal turn), fabricated state:'completed' on phase alone, ran mutating work under a passive load, and retained no snapshot for the lost-reply retry. D10 already owns rebind-and-keep-reading as the Step 3 row (R1-25-re, R3-2 .. R3-5) - the load route's restore guard applies the same durable-vs-transient verdict read, because the bundle collapses both into one blocked bit and the durable decline was unreachable from production (R1-26-re) - the pre-admission exemption keys on lease-shaped 409 codes only: configuration-shaped 409s (e.g. hosted_tool_profile_conflict) meet the budget, and the same 409 exempts nothing on an unbound Session (R1-1-re, R1-30-re, R3-13) - equal-boot adoption keeps pendingRecovery: the markers were minted by the same live client (R1-29-re) - lifecycle capability mismatch: revert completing-as-unconfirmed — that skipped the drain, flipped the session, and recorded a clean row with no FAILED vocabulary to tell it apart; the reason-loud retry is the honest end of the line until an operator realigns versions, and the README sentence says so. Action-response outbox keeps its honest FAILED-with-code terminal (R1-45-re, R3-5, R3-15) - test hardening: concurrency close-once now surfaces worker assertion failures via Future.get and pins that no real client build happened (R3-12); loadTimeout-vs-requestTimeout discriminated at the call site (R3-9); single-walk acceptedInputSequence (R3-18) - runner: 5-column execution queries pin runtime_session_id unchanged under --harness-only (R3-14); frozen-arm teardown looks the child up by a shared label constant and fails the run on a drift (R3-17) - CI: ceiling 105 covers the nine summed step ceilings (92) plus the uncapped setup steps (R3-6); the seven-arm pin also asserts each script's flags and the install step's existence (R3-16) - docs: both languages re-synced (D5 retraction, D9a code allow-list, D7 teardown, D8 runtime identity, honest validation digests, the four missing changes-table rows, README retry clause) * fix(managed-agent): replay takeover snapshots only to a same-shape request A passive load only asks broker.status(), so its snapshot keeps phase='await_runtime'; replaying it to a drive load handed the coordinator a checkpoint that isContinuationReady() can never accept, terminally failing an otherwise continuable Turn. Record the request shape with the snapshot and replay only on a mode match; the clear conditions stay promptId-only. Also make the adoption race test deterministic: a package-private createClient() seam lets the test inject the replacement instead of a real /capabilities call (proven flaky on CI), worker assertion failures now surface through Future.get, and the pendingRecovery fixture seeds its marker through the recovery path. * fix(managed-agent): close the self-audit round on the replaceable-harness slice Three rounds of undirected plus adversarial audit over the whole PR diff (31 files), with three parallel deep-dive passes over the Java, TypeScript and runner/docs surfaces. Every finding was re-verified against the tree before acting; five reported defects were rejected on evidence. Correctness: - the load route read the blocked-restore verdict AFTER managed.close(); sealing the journal makes every later store read fail as "writer is not active", which the authority erases into missing_state, so the durable branch this PR added could never fire and a corrupt checkpoint stayed in the retry-forever class. Read the verdict first, and pin the order with a witness that goes red when the read moves back (the local authority's seal does not disable reads, so only the ordering assertion catches it). - drop hosted_session_already_attached from the lease-bounded 409 set: the daemon removes an attachment only on an explicit detach or delete and this control plane never detaches, so a Spring restart against a surviving Harness retried a permanent refusal forever instead of ending the Turn. - adoptGeneration evicts by boot identity in both branches instead of clearing, so a marker minted by a concurrent rebuild can no longer be erased between two adjacent clears (which left a live attachment with a null recovery report), and markers follow their attachment. - the post-withdrawal fast-cancel reads the row again: a cancel landing after the claim is invisible in the claimed snapshot, and acting on it submitted a live execution only to cancel it below. - a durable verdict re-read after settling now declines as checkpoint_blocked, matching the identical pre-settle classification. - an unrecognized blocked reason throws instead of declining, so a reason core adds later cannot silently become a terminal Turn failure. - the operator-settable load-timeout is validated at connector construction; a bad value previously threw on every client() call and surfaced as an endless transient retry naming only the exception class. Tests: already_attached and lost-withdrawal-CAS witnesses; the fast-cancel witness now pins the re-read (cancel after claim); the concurrent-adoption test surfaces worker assertions through Future.get and injects the replacement through a package-private createClient() seam instead of standing up a live /capabilities call; the CI pin gains each arm's mode flag and a ceiling-sum invariant. Docs (both languages): D1 now describes the boot-identity eviction and lists what this slice really added to HostedHarnessClient; D3 no longer claims the bind-refusal path stops being reached (it is D4's trigger); D6 records the same-shape replay clause; D7's fence rationale no longer blames heartbeats (lease renewal touches writer_lease_until only); D9a names the two exempt codes and why already_attached is not one; the validation digest states which claims are pinned and which are not; the CI arithmetic reads 12 + 8x10 = 92; the stale workflow nit is dropped. * chore(ide): regenerate NOTICES for the merged dependency set (email channel) * fix(ide): replace the workspace-polluted NOTICES with the pristine regeneration The file committed in f7a960c was generated under a polluted module layout left by an aborted install: its resolution hoisted is-fullwidth-code-point@5.1.0 and produced 663 total dependencies, so CI's own regeneration (pristine: 662) mismatched and the up-to-date gate failed. Replace it with the pristine worktree's output, which the gate's own regeneration produces to a no-diff state. * fix(managed-agent): close the R4 review finds on the takeover and refusal paths - classify missing_checkpoint and a bare missing_state as durable verdicts so a permanently unrecoverable session declines instead of being retried past the lease window; only a missing_state carrying the erased store error's message stays retriable - map a fenced predecessor's managed_session_writer_conflict to the coded retriable 409 in the load route so the wait provably ends with the lease, and allowlist that code in LEASE_BOUNDED_409_CODES (dropping the inert hosted_prompt_recovery_required, which can only arrive post-mark where the exemption gate is already bypassed); null-guard the membership test against codeless error bodies - replay a takeover snapshot only to a request whose parsed store identity matches the attached session's, never to any takeover-shaped caller - fetch the harness client AFTER resolving the attachment at the six entry points, so an adoption completed during a create/load round trip cannot strand a call on the closed client - pin the reply watermark of the journal-replayed prompt admission, the construction-time loadTimeout guard, the marker-eviction half of adoptGeneration, and the withdrawal CAS ordering with witnesses, and widen the load-timeout test's durations so the capabilities negotiation keeps headroom on contended lanes - narrow the README's typed-decline sentence to the surviving row and generation binding, and mirror the exempt-code list in both design docs * fix(managed-agent): answer the round-5 waits without terminalizing them - a takeover now splits three ways: drivable answers recovered, a deterministically unrecoverable drive keeps its typed decline, and anything whose wait belongs to someone else (a requested approval, or the whole non-drivable space on a cancellation-only load) answers inapplicable so the route falls through to the plain attach — a user-cancelled Turn cancels instead of failing as managed_runtime_recovery_blocked, and an approval wait survives its generation rather than dying while the answer is still deliverable; the coordinator keeps the await_action guard as a retriable path even if a Harness still declines it - an unconfirmed cancellation re-drives with its snapshot restored: the cancel admission's rollback is now symmetric, so the retrying takeover load replays instead of being refused already-attached - an unrecognised blocked reason at the final re-read stays retriable on the drive side like the pre-settle read, instead of terminalizing on unresolved_after_settle; only durable verdicts may end a Turn - the hosted persona's curated tag set is declared once in the registry (hostedPersona descriptor flag) instead of being filtered by a third hand-maintained list at the route, pinned by the docs-contract test - the harness-only arms pin a surviving worker through the runtime session row's own liveness metric, not the insert-frozen id column - witnesses: declined reason on the wire, cancel-report replay after a lost answer, inapplicable shapes at the recovery kernel (both loads), the drive disjunct of the load-timeout predicate, the one rebuild per adoption race, and coordinator corners for cancelling/await_action * fix(ci): qualify the runtime-session heartbeat query with its schema The harness-only arms' liveness beat read qwen_runtime_session bare; mysql runs without a default database, so the Harness-restart in-flight arm died on ERROR 1046 with 'No database selected'. Every other probe in the runner already addresses qwen_managed_agent.<table> — this one now does too. * fix(managed-agent): close the round-6 shape and wedge findings - the pre-kernel no-tool refusal and the restore guard now honour the same shape rule the kernel does: a cancellation-only load answers inapplicable and attaches plain (or the baseline retriable refusal where no attach is possible) instead of terminalizing a Turn the user asked to cancel; a takeover that answers inapplicable no longer trips the bare-load refusal on its empty recovery - the lease-bounded exemption is keyed on the writer lease's own wire code only: hosted_turn_recovery_required covers arbitrary takeover failures, and exempting it wedged durable refusals in an unbounded pre-admission retry; a refusal carrying that code now meets the budget like every other body shape while managed_session_writer_conflict keeps its D9a window - witnesses: the no-tool plain-attach coincidence for cancel vs the typed model_start decline for drive, the durable restore verdict on the cancellation arm, and the budget-hit for a durable 409 on the bound path (the old takeover-409 premise rewritten, its writer- conflict twin already covered) * fix(managed-agent): settle a cancel first on a plain-attached replacement A real-stack round-4 wedge: cancelling a Turn whose Harness died mid model wedge (Work realized after the merge) failed as hosted_harness_generation_mismatch — the plain attach leaves the bind CAS unwinnable (boot-old != boot-new), the cancel was never issued, and everything queued behind the Turn then retry-forever: later prompts 409, a close 409, no public-API escape. runClaimed's recurrence without recovery now cancels before it binds: a CANCELLING Turn does not submit, and a bind exists for submissions — the next Turn's takeover adopt moves the generation when something actually continues. cancelledOnAttach suppresses the tail's later re-cancel. The R5 witness now also pins: bindHarness is never asked, withdrawSubmissionAttempted is never touched, and the Turn is neither recovery-blocked nor generation-mismatched. Design docs D3/D4 gain the bypass rule in both languages. * fix(managed-agent): close the inapplicable aftermath the round-8 review found - an inapplicable takeover must not latch session.blocked — a 200 answer that refuses every settlement route is a lie (R5-2's latch regression: nothing outside settleCancelledHookTurn clears it for a non-hooks session). The blocked latch now splits the same shape rule: only a genuinely unsettled takeover parks there, not an inapplicable answer. - a plain attach of a bound Session after adoption rebinds the Session row with its own CAS (owner + live lease + expected = the row's boot), since bindHarness provably refuses a turn that already posted its epoch — the Turn is neither generation-mismatched nor stranded (R8-1). - a Turn written turn_settled completes, so it is never stamped as managed_runtime_recovery_blocked: the kernel answers inapplicable on BOTH load shapes (the plain attach lets the daemon's own projection, not a declined verdict, write the terminal record), retaining the rebind-and-keep-reading rationale for Step 3 (R8-2). - the QwenLM#13388 pinning test's mock endpoint now advertises managed_session_journal_delta_v1 like every other Harness fixture of this branch — a merger's follow-up angry about the gate (D9c), not an upstream flake. - witnesses: turn_settled pinned as inapplicable on both shapes at the kernel; plainAttachAfterAdoptionRebindsBeforeItSubmits asserting the store CAS adoption; R5-2's corner witnessing 200 without recoveryRequired; coordinator 46/46, hosted client + pinning tests, TS session+recovery fully green. * fix(managed-agent): close the inapplicable aftermath the round-9 review found - the no-tool arm now refuses with the baseline retriable one again instead of minting a plain attach whose parked Turn nothing may resolve (R5-2'), and a takeover load without broker options shares that refusal; - the journal prompt replay answers 202 only when the request body's digest proves identity against the admission its input.accepted holds; carrying the same Id with different bytes is a real prompt_conflict, never a silent swap of watermark (R9-1); - a plain attach after adoption moves the Turn's stream epoch through recordRecoveryAdmission to the attach's epoch, on both the binding ladder and the cancel-on-attach arm — a stream opened with the claimed epoch would be rejected on the new generation as epoch mismatch (R8-1'). - witnesses: the no-tool cancel arm refusing retriably while drive declines with its typed reason, the digest mismatched replay timing out as hosted_prompt_conflict, and R8-1 streaming the adopted generation's epoch on both arms (plainAttachMoves* and cancelOnAttachStreams*) with recordRecoveryAdmission pinned. * deflake: carry QwenLM#13403's pinning fixture under the journal-delta gate + harden the 13328 mount-holder waitFor The merge of QwenLM#13403 turned the Java CI lanes red with DaemonProtocolException: Endpoint does not advertise managed_session_journal_delta_v1 on every burst caller: this branch's D9c capability gate (from QwenLM#13388's pinning work) rejects endpoints without that token, and QwenLM#13403's new /capabilities fixture did not advertise it. Same fixture-parity fix as QwenLM#13388's — the token is a fixture property, not production behavior; the deliberate negative stub (capabilitiesJsonWithoutJournalToken) stays as the gate's witness. The ubuntu Test lane red on the same head was the issue-13328 mount-holder vi.waitFor defaulting to its 1 s window against the admission pipeline on the loaded runner ("expected undefined to be defined", repeated across both retries). Give it an explicit 10 s timeout, assertions unchanged — the same deflake pattern as QwenLM#13411/QwenLM#13323/QwenLM#13430. * fix(managed-agent): close the R11 review round (inapplicable pays only what it can) R11 (qwen-code-ci-bot review, 5 threads, 3 new + 2 re-checks) landed on the shared consequence of R10's fixes: an inapplicable/plain-attach answer stays honest only where a settlement route can pay. - R11-1: the prompt route's session-scope guard now answers the SESSION-level wedge code (hosted_turn_recovery_required); the prompt-scoped code names only the requested prompt's own unsettled duplicate, so the duplicate-admission adopt branch can no longer be forged by a refusal about someone else's parked Turn — and the budget treats the session-level code as pre-admission whenever the Turn provably recorded no epoch, recording the wedge's own code on exhaustion (provenance-gated on an existing submission mark). - R11-2 / R10-1 re-check: both takeover arms compute the bare branch's settle projection inline (`settleProjectablePromptId`): approval waits keep the plain attach (resolve pays), turn_settled is settled by the load itself via the extracted `runSettleProjection`, and an unpayable park keeps the baseline retriable 409 instead of a healthy-looking 200. - R10-2 re-check: the kernel's passive inapplicable is narrowed to those two payable states; initial / durably blocked / model-start / unknown phase / wrong-checkpoint throw into the retriable refusal. - R11-3: createOrLoad's final attachment put restores the pendingRecovery marker for snapshot-carrying passive re-attaches, or the cached branch of a later recoverManagedRuntime answers "nothing parked" over a snapshot it holds. Witnesses: 245/245 CLI across the three hosted suites (2 new route witnesses + updated session-code assertion + 2 recovery-suite flips), QwenHostedHarnessConnectorTest 34/34 (marker witness), HarnessCoordinatorTest 52/52 (session-busy witness; R6 budget witnesses pin the load side as recorded). Docs updated in both languages. * fix(managed-agent): settle the no-tool cancellation takeover instead of refusing forever (Arm B) The round-6 real-stack wedge: a Turn with no Runtime work parked mid model round, its Harness generation dead, cancelled — the no-tool arm of the takeover branch answered the baseline retriable 409 on every cancellation-side redispatch, and with the Turn still carrying its submission mark, the retry budget never fired. Turn 2 stayed CANCELLING forever and every later Turn met the same wall. The fix keeps the wire honest and the settlement explicit: - LoadHarnessSession carries `cancellationTakeover`, set ONLY by recoverManagedRuntime's cancellation arm. A plain passive re-attach shares the passive wire shape and deliberately never sets it — nothing may mint a canned CANCELLED record for a wait whose owner could still exist. - With the signal present, the no-tool arm of the takeover branch settles the ownerless park into the daemon's journal itself (turn_result/cancelled, landing turn.settled); the writer fence on the very load is the safety boundary (it proves the producing generation can never write again). On write failure the arm keeps the baseline retriable 409. Without the signal the arm keeps its merge-baseline answers — retriable 409 on cancel, typed model_start decline on drive — so the collision guard holds by construction. Witnesses: `settles the cancellation of an ownerless parked no-tool Turn on the takeover load` (200 plain attach + durable settle tag + cancel reads settled-at-tail as 204 + next prompt admits; the revert flips the verdicts red) with the collision-guard passive-only rerun still at 409, plus wire-side `carriesTheCancellationTakeoverFlagOnlyOnTheCancellationLoad` asserting the flag travels only with the signal. Verification: TS hosted suites 259/259, qwencode 32/32, QwenHostedHarnessConnectorTest 34/34, HarnessCoordinatorTest 52/52, typecheck, eslint, prettier. Design doc updated in both languages. * fix(managed-agent): converge P1-1 (tool-config vs unpaid Runtime work) and P1-2 (stale approval copies) Round-7 re-review of 275f934 found two remaining wedge families inside the cancellation settlements this PR introduced. Both converge on durable proof, never on the stale checkpoint copy. P1-1: a takeover cancellation must separate "the Session is configured for tools" from "the Turn owes unpaid Runtime work". A files-profile session whose owner died BEFORE its first tool call carried no unsettled Runtime work (checkpointless or a bootstrap checkpoint naming no Turn) — and the kernel's different-Turn check threw on the bootstrap's null turnId, so the load answered the retriable 409 forever. The load route now hoists that split ahead of the kernel: with the explicit cancellationTakeover signal (and only with it — the collision guard stays), any ownerless park without unpaid Runtime work settles into the journal itself; work in flight follows the recovery-cancel report; the no-signal shape keeps its baseline refusal unchanged. P1-2: the durable action record — not the checkpoint's stale copy of the approval — decides an owner-died approval. When the record says the wait ended, the recovery kernel no longer maps it to the inapplicable plain attach: expired/cancelled is transient (route refusal), decided is advanced through its own durable gate ONLY on a drive load (a cancellation never crosses it), and every other shape (still requested, or undefined) keeps the plain attach a resolve route can pay. The cancel/expired arrive by the same separation (settle with the signal; decided is settleable with the signal too — a CANCELLING takeover does not resume approved work no matter which side the user took). Witnesses: `settles the cancellation of an ownerless parked Turn on the takeover load (profile=null|FILE_PROFILE)` parameterized exactly as the re-review demanded; three recovery-kernel witnesses for the cross-read (resolves-inapplicable only while requested; throws for ended-without-a- decision; drive declines checkpoint_blocked on an undrivable decided wait); route-level `settles a cancellation takeover whose durable approval already ended (state=cancelled|decided)`. Design doc updated in both languages. Suites: hosted-harness-session 222→(+10 new), hosted-runtime-recovery 36→(+3 new), issue-13328 33→, all green; typecheck and eslint clean. * chore(vscode-ide-companion): regenerate NOTICES for the merged dependency layout * fix(managed-agent): read owed Runtime work off the work itself and pay coded plain-cancel refusals with the cancellation takeover (R9) Round-9 real-stack measurement found the cancellation separation still wedging on two families: every cancelled Turn after the first, on both profiles, and every cancel row of P1-2 — where no load ever left the coordinator, because a plain-cancel retry can never settle a parked Turn and no redispatch was forthcoming. - The park gate now reads owed work off the work itself: a checkpoint naming an earlier Turn whose tool items all settled and were consumed owes nothing to a cancelled Turn that never reached a tool call; and a no-tool Session cannot owe Runtime work by definition, so its cancellation arm settles unconditionally again (the round-8 behavior the gate had displaced onto a blocked restore basis). - The admitted-cancel path turns a plain cancel's coded 409 hosted_turn_recovery_required into an immediate cancellation takeover load over the same attachment (recoverManagedCancellation), and the connector sends it even with a healthy cached attachment — the shortcut would swallow it and the load would never leave the process. A recovered Runtime park it reports is cancelled through its checkpoint admission; the already-running stream lands the settle either way. Witnesses: hosted-harness-session settles the second-Turn cancellation park on both profiles (the round-9 rig witness adopted); the coordinator drives the takeover load exactly on the coded refusal (plain park and recovered park) and on no other answer; the connector always sends the cancellation load over a cached attachment. Suites: hosted 229, HarnessCoordinatorTest 55, QwenHostedHarnessConnectorTest 35, ManagedAgentServerIntegrationTest 29. The decided-approval drive arm of P1-2 (rows where the user answers allow or the approval expires unanswered) remains open: it needs the plain-attached replacement to drive the parked Turn itself once the durable wait ends, and lands as its own slice. * fix(managed-agent): pay the cancellation signal identically on an attached re-answer (R9-2) A cancellation takeover load the coordinator forces onto a Session ALREADY attached in this daemon used to skip the entire settle separation: the attached branch called the passive kernel directly, which never advances a durable wait passively — so a park whose approval ended after the attach (the round-10 allow, then cancel repro) was thrown back as an unknown phase and refused retry-invariant, while the first-load branch settled the identical shape. Hoist the separation above the attachment split: the attached branch now reads the signal too and runs the same no-tool arm, owed-work gate and durable cross-read, settling through settleCancelledHarnessTurn into the journal the already-running stream lands. Witnesses parameterize both ended states: attach plain while the durable approval says requested, end the wait after the attach with the stale checkpoint copy untouched, then redrive with the cancel signal — 200 with the redriven-load settle log, and the plain cancel route reads settled-at-tail. Suites: hosted session 231, recovery 44. * fix(managed-agent): close the durable wait before the cancelled terminal lands (R9-3) settleCancelledHarnessTurn only wrote turn_result/cancelled into the journal, and the record sink's turn.settled only advances next-turn checkpoints at a model-start phase — so a cancelled Turn parked at an approval wait ended in the journal while its checkpoint stayed at await_action, and the Session's next prompt met `ManagedHarnessBlockedError: await_action is not a model-start phase` (the round-11 repro: old Turn completed cleanly, new Turn failed as hosted_turn_failed before the model was ever called, closing 1 P1). The cancelled settle now closes the ended wait first through the wait's own durable gate (resolveDurableWait → model_output_committed, a model-start family phase) whenever — and exactly when — the durable approval already ended, mirroring the cross-read the caller's gate performed. The decision remains the USER's and nothing resumes: the Turn dies immediately after. Runs of requested waits or of in-flight Runtime work keep the kernel's existing paths. Witness (both answers): requested approval attaches plain on the passive takeover, the record ends after the attach with the stale copy untouched, and the attached cancellation takeover settles — 200 with the redriven-load settle log, plain cancel reads 204, and the settle provably commits the model_output_committed boundary ahead of the terminal record. Suites: hosted session 231, recovery 44; typecheck, prettier, eslint clean. The unit suite cannot stage "approval-first park + literal owner death" today (DELETE refuses active Turns, and abort unwinds the wait cleanly), so the end-to-end relief-Turn acceptance stays with the real-stack probe; the annex-level witnesses cover the settlement payment and the wait-close commit. * chore(vscode-ide-companion): regenerate NOTICES from a clean-room install The previously committed file was generated in the long-lived worktree whose pnpm layout had drifted (is-fullwidth-code-point@5.1.0 + get-east-asian-width@1.5.0 + fdir/picomatch slot differences) even though pnpm-lock.yaml is byte-identical to main. Regenerated in a fresh worktree with a frozen-lockfile clean install: 662 deps, and the output matches the lint lane's renewed file byte for byte, which the polluted layout could not reproduce. * fix(managed-agent): pace forced takeover loads per Turn and make the cancelled settle idempotent (R9-4) Two round-10 verdict items: 1. The coded-409 escalation (R9-P1-2) ran on every ~500ms lease-renewal tick, so a Turn cancelled while its wait still owns a live durable record paid ~4 requests/s of daemon work forever (21 forced loads and 20 cancels in the verdict's captured window). cancelAdmittedTurn now paces the forced cancellation takeover load per Turn (5s minimum interval): the cheap plain-cancel retry keeps carrying the wait, and a wait that ends inside the interval still settles on the next paced attempt — far below the approval timeout's 45s budget. 2. A settle that already landed was not idempotent: the coordinator's next forced load inside the stream's landing window met `event id turn:<id> is already committed` (3/3). The cancelled settle now re-reads the journal's own turn.settled for this prompt before writing, so the redriven load is a re-answer, never a second turn_result racing the event-id CAS. Witnesses: three paced coded-refusals (t0 / t+500ms / t+5s) yield three plain cancels but two forced loads (HarnessCoordinatorTest 60); the round-9-2 witness pair now races a second identical load inside the landing window and asserts one cancelled record and no takeover_unavailable line (hosted suites 275). * test(managed-agent): commit authority-valid deltas in the restore byte-budget test How: holdsRestorePagesInsideThePerPageByteBudget now builds each dense transaction from 210 well-formed message.delta lines (text at the reader's 4096-byte cap, ~1 MB per transaction) plus the commit marker, through a shared TurnEventLines composer, and asserts that revisions 1..9 fit the 8 MiB page budget while a tenth does not. The reader replay IT now also replays that delta shape. The class turns off MockMvc print-on-failure. Why: QwenLM#13348 added the test with placeholder lines ({"subtype":"event_v1"} plus a padded commit_v1 line), and QwenLM#13355 (merged later, never run against it) made the commit gate refuse any non-event line inside the event range, so every commit after genesis answered 409 and main's SDK Java lanes went red. The failure was masked as timeouts: Spring Boot printed the failed test's ~1.3 MB request body as one log line, which stalled the Actions log pipeline for ~45 min, so the Hosted Verify and O4 steps blew their ceilings and the MariaDB lane hit its job timeout. Test: ManagedSessionStoreIntegrationTest and ManagedExtensionRecordStoreTest green; full managed-agent-server surefire (1077) and checkstyle green; HostedCommittedEventLineReplayIT green through the real TS reader and red when the delta text exceeds 4096 bytes; dropping the page byte budget in ManagedSessionStore fails the test (10 rows instead of 9) with no log line over 400 chars. * fix(managed-agent): propagate the settle's own wait-check fault instead of degrading it to a successful cancel (R9-5) The cancellation settle's authorization re-read was wrapped in `.catch(() => undefined)`, so a transient store fault surfacing between the caller's gate and the settle's own wait-check degraded into 'nothing to close' — the cancelled terminal landed over a wait the fault hid, and the Session's next prompt then found the same obsolete checkpoint that family wedges on (the round-11 fault injection at this exact boundary: authorization flips runnable → blocked/missing_state → runnable once, the settle still mints turn_result, and the run dies on `await_action is not a model-start phase`). A read failure is a retriable store fault, not 'nothing to close': it now propagates into the caller's try-catch, which keeps the baseline retriable refusal — the fault's own retry ladder owns the retry, and the replayed shape then settles identically. Witness mirrors the probe: the wait-gate reads fine, the settle's own check faults once — load refuses hosted_turn_recovery_required with no Cancelled record and an honest 409 on the plain cancel, then the same shape settles after the fault clears (settle log, the model_output_committed boundary, plain cancel 204). Suites 276/276 (hosted session 232, recovery 44), typecheck/prettier/eslint clean. * fix(managed-agent): refuse the cancelled settle on fault-shaped authorization verdicts too (R9-5') R9-5 propagated THROWN authorization faults into the caller's retriable refusal, but the authority never throws the same class of failure: the real path takes a retry-exhausted ManagedSessionStoreTransportError and converts it in place into `{status: 'blocked', reason: 'missing_state'}`. From there the cancelled settle still marched on — it skipped the wait advance and wrote the cancelled terminal anyway, spending the Session's next prompt on the obsolete `await_action` checkpoint the verdict hid (the round-11 verdict at `964b87a50a`, reproduced with the production error type: 200 on the takeover, the terminal lands, and the next turn dies `await_action is not a model-start phase`). Fault-shaped authorizations are never "nothing to close": `missing_state` (the TransportError conversion), `opaque_state` and `invalid_state` now all answer the same retriable refusal — the settle refuses rather than minting what it cannot prove. `missing_checkpoint` stays payable on purpose: it is the Arm B park's honest form (a no-tool Session with history provably has no checkpoint) and its settle is the already-verified behaviour. The witness covers both faces of the store fault (`it.each(['exception', 'missing_state'])`): same park, same stage, either a thrown fault or a folded `blocked/missing_state` — the load refuses with zero settle and an honest plain cancel 409, and the replay settles the moment the fault clears. Suites 277/277 (hosted session 233, recovery 44), typecheck/prettier/eslint clean; design docs note the boundary in both languages. * docs(design): split the R9-5' verdict paragraph into its own bullet The R9-5' text rode mid-bullet after R9-5's run-on, which Prettier then flattened to column zero — breaking the list structure the lint lane rejects outright. Both languages carry the split now, and the lint-facing check passes again.
|
Released in v0.25.1. |
What this PR does
Adds test witnesses for the twelve R2 review follow-ups on #12692 that were missing in the test surface. Every witness was first written against the pre-change code, confirmed to fail or to expose the gap, then edited to compile against the public surface whose contract it pins (no production code change in this PR). Each finding was re-verified against current main (
2c591ecc08) before the test was written.Why it's needed
These behaviors were shipped with the merged PR but lacked direct witnesses, so review could not read current-main behavior from tests:
SessionEventHubhad no overflow witness: a 600-event publish against capacity 512 plus a slow consumer was the only way to discover whetherawaitreported the dropped position or hung.HarnessCoordinatorhad no witness for the three failure modes the recovery flow can take: a recovered-but-not-ready runtime must fail the turn withmanaged_runtime_recovery_incomplete; a generation that moved between witness and bind must fail terminally withhosted_harness_recovery_generation_mismatch; an admission-epoch move triggers a retry instead of admission.HarnessEventProjectorhad no witness for the out-of-alphabet error code fallback, nor for the redaction rule excluding both the value and the key of a sensitive attribute.QwenHostedHarnessConnectorhad no witness thatUnknownCreateOutcomeExceptionactually falls back toloadand propagates the complete(bootId, recoveryHandle, harnessGeneration, eventEpoch)tuple, nor that a genuine load path carries the same four fields, nor that a 500 daemon error is rethrown untouched.ManagedSessionStorewere shape-only: nothing pinnedrecordCount == records.size(), per-record session identity, or(firstSequence..lastSequence).size + 1 ≥ eventCount.ManagedAgentMySqlITwas not rerunnable against a persistent database: reruns collided on the fixed-id legacy fixtures, and the migration-bound assertions (V15 event identity, V17 command-to-operation, V29 hook backfill) are by definition not re-applicable to a rerun's seeds.ManagedAgentServerIntegrationTesttreated the in-process recovery attachment as a global single-shot; the second test to claim a dispatchable turn out of the shared H2 silently won, leaving earlier sub-cases to flake.Reviewer Test Plan
How to verify
evictsBeyondCapacityAndSignalsOverflowBelowTheWatermarkpublishes 600 events againstCAPACITY = 512; the slow consumer'sawait(0)reports overflowed;await(88)returns the contiguous 89..600.failsTurnWhenARecoveredRuntimeIsNotReadyForContinuation(verifiesfailTurnwithmanaged_runtime_recovery_incomplete, never continue/submit),failsTerminallyWhenTheRecoveredGenerationMoved(bind false, codehosted_harness_recovery_generation_mismatch),retriesTheTurnWhenTheRecoveryWatermarkMoved(epoch-move →scheduleTurnRetry, neverrecordRecoveryAdmission).substitutesTheSafeFallbackCodeForAnOutOfAlphabetErrorCodepinshosted_harness_error; the existing redaction test now also assertsdoesNotContainKey("secret")anddoesNotContainValue("must-not-leak").unknownCreateOutcomeFallsBackToLoadAndPropagatesTheFullAttachment,loadPathAlsoPropagatesTheFullAttachment,rethrowsANonConflictDaemonErrorWithoutLoading(each uses the new exception seam).ManagedSessionStoreContractFixtureTestpins record count, per-record session identity, and a sequence range consistent with event count.holdsRestorePagesInsideThePerPageByteBudgetwrites a genesis then twelve 1 MiB-padded commits in two-line canonical form; page 1 returns transactions 1..9 withhasMore=true; page 2 (afterRevision=9) returns 10..12 andhasMore=false. The test goes red against any of the 8 MiB, 1 MiB, record-count, or firstSequence rules being violated.mvn failsafe:integration-test -Dit.test='ManagedAgentMySqlIT' -Dmysql.url=...twice against the same persistent database; both runs go green (19/19). Sources of rerun collision:mysql-upgrade/mysql-lifecycle/mysql-hooksnamespaces per run; migration-bound assertions are guarded bylegacySeedsBackfilled; the workspace idempotency probe re-seeds the V9 scope row explicitly.ManagedAgentServerIntegrationTestpreviously had ~1-of-N flakes after broad runs; it now deterministically cleans up claimed turns in a finally wrap.Evidence (Before & After)
N/A in the production sense — the new tests pin existing contracts; on the pre-change code, no production test or witness existed for the listed behaviors.
Tested on
mvn verifyonpackages/sdk-java/managed-agent-server(JDK 21, H2) after same-source reinstall ofqwencode-sdkandqwen-managed-runtime-broker(alpha jar~/.m2md5 fingerprints verified equal pre- and post-run). The MySqlIT witness ran against a throwaway MariaDB 11.3 datadir on 127.0.0.1:33307; bothTests run: 19, Failures: 0, Errors: 0invocations passed back-to-back.Environment (optional)
JDK 21 (
~/Install/jdk21); Maven 3.9.11; MariaDB 11.3.2 only for the MySqlIT witness runs (the unit/integration suite is H2).Risk & Scope
Linked Issues
Follow-up to the post-merge R2 review on #12692. Design-level deferrals tracked by #13302.
中文说明
本 PR 内容
为 #12692 合并后 R2 评审提出的 12 条「测试面不足」跟进项补见证用例。每条都先在改动前代码上写出并确认失败/暴露缺口,再调到贴合被钉住的公共契约——本 PR 不改任何生产代码。每条 finding 在落笔前都对照当前 main(
2c591ecc08)重新核实过。动机
这些行为随合并 PR 上线,但缺少直接的回归见证,导致评审无法从测试中读出当前 main 的真实行为:
SessionEventHub没有溢出见证:600 条事件怼到CAPACITY=512配上慢消费者,只能跑一遍才知道await是返回 overflow 还是挂起。HarnessCoordinator的三条恢复失败路径均无见证:已恢复但未 ready 的 runtime 必须以managed_runtime_recovery_incomplete失败 turn;见证读到 bind 之间代际变化必须以hosted_harness_recovery_generation_mismatch终止;admission-epoch 移动必须改成 scheduleTurnRetry 而不是 admission。HarnessEventProjector没有「字母表外错误码兜底为hosted_harness_error」的见证;敏感 attributes 的 key 与 value 都不可进泄露容器,此前只测了一半。QwenHostedHarnessConnector没有见证UnknownCreateOutcomeException会真的回退去 load 并把(bootId, recoveryHandle, harnessGeneration, eventEpoch)四元组完整透传到 Attachment;load 路径同样未钉;500 类 daemon 错误必须原样重抛,不能误触 fallback。ManagedSessionStore的 contract fixture 只测形状,没有钉recordCount == records.size()、逐条 record 的 session identity、(first..last).size + 1 ≥ eventCount。ManagedAgentMySqlIT在持久 db 上不可重跑:重跑会撞固定 id 的 Legacy fixture;且 V15/V17/V29 这三条数据迁移物理上不可能被重放到「同一份种子」上。ManagedAgentServerIntegrationTest的 fixture 把「runtime recovery attachment」当全局一次性值用,共享 H2 上后续的 dispatchable-turn 子断言会静默漂移。评审者验证计划
如何验证
evictsBeyondCapacityAndSignalsOverflowBelowTheWatermark按 512 容量推 600 条;慢消费者await(0)报告 overflow、await(88)返回连续 89..600。failTurn的 code 与 never-continue、bind 失败的 terminate code、epoch-move 的 retry-not-admission。substitutesTheSafeFallbackCodeForAnOutOfAlphabetErrorCode钉hosted_harness_error;现有 redaction 用例追加doesNotContainKey/Value断言。unknownCreateOutcomeFallsBackToLoadAndPropagatesTheFullAttachment/loadPathAlsoPropagatesTheFullAttachment/rethrowsANonConflictDaemonErrorWithoutLoading。holdsRestorePagesInsideThePerPageByteBudget写 genesis + 12 条 1 MiB pad 双行 commit;page1 拿事务 1..9hasMore=true,page2(after=9)拿 10..12hasMore=false。任何 8 MiB / 1 MiB / recordCount / firstSequence 规则破坏都会让此用例红。failsafe:integration-test -Dit.test='ManagedAgentMySqlIT',两轮 19/19 全绿。ManagedAgentServerIntegrationTest通过 finally 包裹清理 claimed turn,fresh-namespaced tenant recovery map,跨子用例不再漂移。前后对比
生产行为无差异 —— 本 PR 全是测试与见证,没有生产代码改动。
测试平台
macOS 已测(JDK 21,H2):
packages/sdk-java/managed-agent-server执行mvn verify通过;qwencode-sdk与qwen-managed-runtime-broker在装到~/.m2后, jar md5 在测试前后一致(防跨 worktree 竞写)。MySqlIT 双跑 witness 使用了临时 11.3 MariaDB(127.0.0.1:33307),两轮Tests run: 19, Failures: 0, Errors: 0。风险与范围
legacySeedsBackfilled,作为固定 cost 换来了 namespace-per-run 的真实重跑能力。fresh db 的首跑做完整 backfill 断言;重跑把同一份断言退化为「种子不冲突 + store 后续操作仍正常」。关联 Issue
#12692 合并后 R2 评审的跟进。设计级延缓项由 #13302 跟踪。