test(ci): stage on-disk session state in the serve A/B - #9444
Conversation
The serve A/B drives every scenario against a freshly started, empty daemon, so the entire session-admission surface — case resolution, transcript integrity, active/archive conflicts, reserved sources — is unreachable and a PR that rewrites it diffs as "no response changes". #9341 is the worked example: the posted A/B reported no change across 4 scenarios while the same build pair, driven with transcripts on disk, answers differently on six requests. Scenarios can now stage transcripts before their request and capture a reduced projection of the response, and the HTTP status is recorded on every capture so a status-only difference is visible. Six session-admission scenarios use that: a healthy restore, the legacy uppercase spelling, case-only twins, an unreadable transcript, one id in both the active and the archive directory, and creation carrying a source type. The staged fixtures depend on the on-disk project layout, which the harness mirrors rather than imports. If that mirror ever drifts the transcripts land nowhere and every staged scenario would quietly answer 404 on both arms, so the healthy restore doubles as a canary that fails the drive instead of publishing a reassuring all-clear.
|
@qwen-code /takeover |
|
Re-run on Template looks good ✓ Problem: observed, not theoretical. #9341 is the worked example — its A/B reported "no response changes" while the PR rewrote exactly the session-admission surface, because every probe hit a freshly started daemon with an empty home. #9443 tracks the gap, and the description's before/after shows the same build pair answering differently on six requests once transcripts are staged on disk. Direction: aligned. This widens the repo's own serve A/B harness rather than adding a parallel mechanism, and it attacks a known blind spot in the automated verification this repo invests in. CHANGELOG is not relevant for CI tooling. Size: Stage 0 not applicable — nothing under core paths; everything lives in Approach: the scope still feels right, and everything that landed since the last triage stays inside the stated goal. The R6 round fixed a real critical: the unreserved-source witness was sending Risk: no elevated risk signals — none of the changed files match the revert-correlated paths. Moving on to code review. 🔍 中文说明在 模板完整 ✓ 问题:已观测到,不是理论性的。#9341 就是实例——它的 A/B 报告"无任何响应变化",而该 PR 重写的恰恰是 session 准入表面,因为所有探测都打在一个刚启动、home 为空的 daemon 上。#9443 记录了这一缺口,描述中的 before/after 显示:一旦把 transcript 摆到磁盘上,同一构建对在六个请求上给出不同答案。 方向:对齐。这扩展了仓库自己的 serve A/B harness,而不是引入平行机制,直指仓库在自动化验证上投入的一个已知盲区。CHANGELOG 与 CI 工具无关。 规模:Stage 0 不适用——未触及核心路径,所有改动都在 方案:范围依然合理,且上一轮 triage 之后落地的所有改动都没有超出既定目标。R6 轮修复了一个真正的 critical:unreserved-source witness 原来发送 风险:无升级风险信号——改动文件均未命中与 revert 相关的路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run on The substantive harness was deep-reviewed at The R6 critical fix is correct. The old witness sent The R5 hardening checks out too. The rest is byte-identical to the diff verified at Non-blocking, carried over from the /review rounds' own deferral ledger: the per-drive Test evidenceUnattended CI run — no PR code was built or executed during this review; the evidence below is the PR's own CI on
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Skipped checks are the usual conditional legs (macOS/Windows test profiles, fork-only jobs), not failures. All The deeper behavioural evidence already sits in this thread: the sandboxed verification of Not verified here: the cross-PR demonstration against #9341's literal build pair — the description's table is the author's local run; the sandboxed verify substituted a mechanism-equivalent pair, and the in-flight verify run covers the current head. Real-scenario tmux testing does not apply on unattended CI runs, and this PR changes no user-visible product surface — the live-behaviour signal is the Serve A/B job itself, green on this head. 中文说明代码审查:在 harness 本体已在 R6 关键修复正确。旧 witness 发送 R5 加固同样核实无误。 其余部分与 非阻断、沿用 /review 各轮自己的延后清单:每次驱动的 测试证据:无人值守 CI 运行——审查过程未构建或执行任何 PR 代码,以上是 更深的行为证据已在本帖: 未在此验证:针对 #9341 字面构建对的跨 PR 演示——描述中的表格是作者本地运行结果;沙箱验证以机制等价的构建对替代,进行中的 verify 运行覆盖当前 head。真实场景 tmux 测试不适用于无人值守 CI,且本 PR 不改任何用户可见的产品表面——活行为信号就是 Serve A/B 任务本身,它在本 head 上为绿。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean re-run on Stepping back: this round came down to one question — did Against my independent proposal the PR still matches point for point and exceeds it; every edit earns its place, and seven review rounds plus two autofix rounds have each landed real fixes rather than churn. The remaining nits are the disclosed deferrals — the per-drive temp home is never cleaned up, and a diff-based witness can only pin "not reserved" structurally — neither worth holding this for. The description's "across 10 scenario(s)" quote is also stale (it is 12 now); cosmetic only. CI has fully settled on 中文说明置信度:5/5——在 退一步看:这一轮归结为一个问题—— 与我的独立方案相比,PR 依然逐点一致且有所超出;每处改动都有存在理由,七轮评审加两轮 autofix 每轮落地的都是真实修复而非 churn。剩余的小瑕疵是已披露的延后项——每次驱动的临时 home 从不清理,且 diff 型 witness 只能在结构上钉住"非保留"——都不足以成为扣住本 PR 的理由。描述中引用的 "across 10 scenario(s)" 也已过时(现在是 12 个),仅属表面。 CI 已在 — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| project: admissionOnly, | ||
| expectStatus: 200, |
There was a problem hiding this comment.
[Suggestion] The canary's hard expectStatus throw aborts the whole drive on mismatch and cannot distinguish 'the fixture/on-disk layout drifted' (its stated purpose) from 'this PR intentionally changed the restore admission decision' — exactly the PR class this diff newly triggers (sessionService.ts/jsonl-utils.ts added to paths:): the head drive step fails (no continue-on-error) and the diff/comment step never runs, so the author gets a red check with no admission table; the base arm is driven by the head script too, so for a layout-changing PR the base canary fails because the base build genuinely differs, and the throw discards that signal; the throw branch also has no test coverage (the test only pins that the canary declares expectStatus: 200).
Failure scenario: A PR renaming/moving the chats/archive layout or tightening transcript validation so the committed fixture no longer loads → canary non-200 → drive aborts → no A/B diff published → a red job with no actionable output instead of the admission table showing the change (e.g. _status 200 → 404), for precisely the class of PRs the harness exists to illuminate.
Suggested fix: On canary mismatch, still write the capture (the projected {_status, code} is exactly the evidence) and surface the deviation through the diff/comment (a 'canary deviated' note plus the admission table) instead of aborting before the diff step; and extract the check into an exported helper tested on both branches.
中文说明
问题:canary 的硬性 expectStatus 抛错会在不匹配时中止整个驱动,且无法区分「fixture/磁盘布局漂移」(其设计目的)与「本 PR 有意改变了 restore 准入结论」—— 后者恰是本 diff 新触发的那类 PR(sessionService.ts/jsonl-utils.ts 被加入 paths:):head drive 步骤失败(无 continue-on-error),diff/comment 步骤根本不会运行,作者只得到一个红色对勾而没有准入对照表;base arm 也由 head 脚本驱动,因此对 layout 变更 PR,base canary 失败恰恰因为 base 构建真的不同,而抛错把这个信号丢弃了;抛错分支也没有测试覆盖(测试只钉住 canary 声明了 expectStatus: 200)。
失败场景:一个重命名/移动 chats/archive 布局、或收紧 transcript 校验使已提交 fixture 无法加载的 PR → canary 非 200 → 驱动中止 → 不发布任何 A/B diff → 只剩下红色任务与零可操作输出,而不是展示变化(如 _status 200 → 404)的准入对照表 —— 恰是该 harness 存在意义所在的那类 PR。
修复建议:canary 不匹配时仍写入采集(投影后的 {_status, code} 恰是证据),并通过 diff/comment 暴露偏差(「canary 偏差」说明 + 准入对照表),而不是在 diff 步骤前中止;同时把该检查抽成导出 helper 并对两个分支加测试。
— qwen3.8-max via Qwen Code /review (v0.21.13)
| - 'packages/core/src/services/sessionService.ts' | ||
| - 'packages/core/src/utils/jsonl-utils.ts' |
There was a problem hiding this comment.
[Suggestion] The hand-enumerated trigger paths list cannot be completed against the admission surface this PR now covers — verified omissions recur every audit round and the dependency chain's closure has no principled stop: packages/core/src/utils/paths.ts (sanitizeCwd, which chatsDirFor mirrors), packages/core/src/config/storage.ts (getProjectDir/PROJECT_DIR_NAME shape hardcoded by chatsDirFor), packages/core/src/utils/transcript-records.ts (prepareTranscriptRecords/validateTranscriptRecord on the restore chain), packages/acp-bridge/src/session-source.ts (parseSessionSource deciding the POST /session source admission session-create-source-type probes), and packages/acp-bridge/src/workspacePaths.ts (canonicalizeWorkspace deciding the directory staged fixtures are written into); a sixth candidate with weaker scenario harm is packages/cli/src/config/session-id.ts (normalizeSessionIdForLookup — the scenarios send lowercased ids and findSessionIdIgnoringCase in the listed sessionService.ts absorbs case resolution).
Failure scenario: A PR touching only one of these files (renaming PROJECT_DIR_NAME, tightening SESSION_SOURCE_TYPE_PATTERN, changing canonicalizeWorkspace's realpath step) matches none of the seven paths entries — no serve A/B fires and the admission/staging change lands unprobed; the next PR that does trigger the workflow stages fixtures under the stale scheme, the canary fails on both arms, and a red, confusing A/B is attributed to the wrong PR.
Witness (executed evidence):
Deterministic sweep at the reviewed commit: 0 of the workflow's 7 paths entries match paths.ts, storage.ts, transcript-records.ts, acp-bridge/session-source.ts, or acp-bridge/workspacePaths.ts — each verified against the call chain it decides (sanitizeCwd ↔ chatsDirFor; getProjectDir ↔ chatsDirFor; prepareTranscriptRecords ↔ reconstructHistory; parseSessionSource ↔ session.ts:2306; canonicalizeWorkspace ↔ run-qwen-serve.ts:2097).
| - 'packages/core/src/services/sessionService.ts' | |
| - 'packages/core/src/utils/jsonl-utils.ts' | |
| - 'packages/core/src/services/sessionService.ts' | |
| - 'packages/core/src/utils/jsonl-utils.ts' | |
| - 'packages/core/src/utils/paths.ts' | |
| - 'packages/core/src/config/storage.ts' | |
| - 'packages/core/src/utils/transcript-records.ts' | |
| - 'packages/acp-bridge/src/session-source.ts' | |
| - 'packages/acp-bridge/src/workspacePaths.ts' |
中文说明
问题:手工枚举的触发 paths: 列表对本 PR 现在覆盖的准入表面无法补全 —— 每一轮审计都在发现新的遗漏,依赖链的闭包没有原则性的终点。已验证的遗漏:packages/core/src/utils/paths.ts(sanitizeCwd,chatsDirFor 与之镜像)、packages/core/src/config/storage.ts(getProjectDir/PROJECT_DIR_NAME 形状被 chatsDirFor 硬编码)、packages/core/src/utils/transcript-records.ts(restore 链上的 prepareTranscriptRecords/validateTranscriptRecord)、packages/acp-bridge/src/session-source.ts(parseSessionSource,决定 session-create-source-type 所探测的 POST /session source 准入)、packages/acp-bridge/src/workspacePaths.ts(canonicalizeWorkspace,决定 staged fixture 写入的目录)。第六个候选(场景危害较弱):packages/cli/src/config/session-id.ts(normalizeSessionIdForLookup —— scenario 发送的已是小写 id,且已列入清单的 sessionService.ts 中的 findSessionIdIgnoringCase 吸收了大小写解析)。
失败场景:只改这些文件之一的 PR(重命名 PROJECT_DIR_NAME、收紧 SESSION_SOURCE_TYPE_PATTERN、改动 canonicalizeWorkspace 的 realpath 步骤)不会命中 7 个 paths 条目中的任何一个 —— serve A/B 不会触发,准入/摆放变化未经探测就落地;下一个触发 workflow 的 PR 会按陈旧方案摆放 fixture,canary 在两个 arm 上都失败,一个红色且令人困惑的 A/B 被归因到错误的 PR。
修复建议:结构性地收口,而不是逐个文件追加:要么扩大触发范围以覆盖依赖面(至少加上上面五个已验证文件),要么维持窄清单并在本 diff 已添加的注释中写明容忍的 best-effort 遗漏。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /takeover |
1 similar comment
|
@qwen-code /takeover |
Clears the capture directory before a drive writes into it, so a re-run can never let an earlier run's files stand in for scenarios this run did not capture, and writes a completion marker once every scenario is captured. A baseline without that marker is now reported as partial, because a base drive that stopped part-way leaves the scenarios it never reached rendering as "this PR adds these responses" — the same shape a genuinely new scenario produces. Both arms are driven by the head checkout's harness, so a capture pair always carries the status field on both sides and the compatibility shim for a base that predates it was unreachable; it and its tests are removed rather than left to teach a transition the wiring cannot produce. Non-object response bodies are now nested instead of spread, which dropped scalars and re-keyed arrays. The source-type scenario probed a type today's daemon does not reserve, so it never reached the refusal branch it was named for. It is split: one scenario pins the source the daemon actually reserves, the other keeps an ordinary type that a future reservation would move from admitted to refused. A second canary covers the archive directory, which nothing certified before — a drifted archive name would have left the conflict scenario loading from the active copy on both arms and diffing clean. The remaining inert request body key is gone too; the client id is read from a header, never the body. The harness tests were passing under mutations they appeared to cover: the staging routing, the projection guards and the fixed-id requirement are now pinned by assertions that fail when those are inverted.
|
R1 review actions (commit
On R1-1: I did not verify either way whether VerificationRe-ran the full A/B against #9341's build pair with the revised harness: 12 scenarios per arm, both canaries satisfied on both arms, and the same three admission tables as before ( Determinism: the same build driven twice → |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
.github/scripts/serve-ab-drive.mjs:85 — [review] no staged scenario reaches the load-time reserved-source decision (issue #9443's 'transcript carrying that source 200 → 404' divergence)
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The completion marker was declared twice, once by the writer and once by the reader, with nothing pinning the copies together: renaming one side left both suites green while CI would either flag every complete baseline as truncated or stop noticing truncated ones. The drive now owns the constant and the diff imports it. Two invariants the code asserted in comments were not enforced. A response body carrying its own status key overwrote the status the harness saw, so a status-only regression on such a route would have diffed as an unchanged body; the harness value now wins. And the canary check — the harness's only drift alarm — had no test at all: inverting it so it could never fire left every test passing. It is now a named helper with tests on both branches. The archive canary pinned an exact status, which conflates its precondition with the product's decision: if an archived-only load ever becomes loadable, the precondition still held, but the drive would abort and suppress the very row the captures already contained. It now fails only on the one answer that means the staged file was never seen. Finally, nothing pinned that a staged scenario probes an id it actually staged. Staging the wrong id answers 404 on both arms, captures identically, and drops that branch out of coverage with every test green.
|
R2 review actions (commit
While extracting the canary check I gave the capture composition the same treatment — VerificationRe-ran the full A/B against #9341's build pair: 12 scenarios per arm, both canaries satisfied, completion marker written, same three admission tables ( On the "reverse audit did not converge within the round cap of 5" note — happy to take another pass if it produces findings, but I'd rather not keep widening this PR past its stated scope. Everything beyond making the existing harness observe the admission surface is tracked in #9443. |
|
Thread housekeeping: resolved the fifteen threads whose findings landed verbatim across
Both are one-line reversals if you disagree. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.14)
The completion marker proved that some drive finished, never that this run's did. The only reset lived inside the drive script, which does not run when an arm is skipped before it starts — no merge-base resolved, the base checkout failing, or its build dying — and on the persistent pool the capture paths outlive a run. An inherited baseline then arrived complete, marker included, so neither degraded-baseline warning fired and the comment would have diffed this head against another run's base. The workflow now clears both capture paths in an unconditional step, which is the only place that covers a skipped arm. The in-script reset also turned a write-only script into an unguarded recursive delete of a path taken straight off the command line, which the documented local usage invites a reader to mistype. It now refuses any directory that holds something other than captures. The healthy canary's premise was wrong: the product validates transcripts record by record and fails open, so a fixture whose records stop validating restores as an empty session and still answers 200. Measured against a real daemon, a wholly drifted fixture passed the canary and left every staged scenario probing an empty daemon — the false all-clear this harness exists to prevent. The canary now keeps a replay-size witness in its capture and fails when it is zero. Three test gaps behind the same theme: the marker's writer, the comment subcommand that CI actually invokes, and the mixed-case scenario's existence were all unpinned, and the staged-id check asserted against the union of every scenario's staging rather than the one under test. The capture loop is extracted so its ordering is testable without a daemon.
|
R3 review actions (commit
VerificationFull A/B against #9341's build pair after the changes: 12 scenarios per arm, canaries satisfied, marker written, same three admission tables. The healthy canary's capture now reads Every mutant you supplied was re-run against the fixed code and each now fails a test; the numbers are in the table above. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 57 passed · 0 failed · 57 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:57 通过 · 0 失败 · 57 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9444 deep verification —
|
| cell | harness | daemon builds | oracle | result |
|---|---|---|---|---|
| 1 (blind spot) | base, byte-identical to HEAD^1 (cmp-verified extraction) |
head build vs mutant | serve-ab-diff.mjs comment over the captures |
0 changed fields across 4 scenarios — the admission regression is invisible |
| 2 (signal) | head (this PR) | the same pair | same | 3 changed fields, exactly one scenario: session-restore-active-and-archived — _status 409→200, code "session_conflict"→—, error "…exists in both active and archived directories…" → — |
(Side observation, by design: the cell-1 comment also carries the new truncated-baseline
warning — the legacy base captures have no .drive-complete marker — so a pre-marker
capture set is disclosed as partial rather than silently trusted.)
Witness: evidence/01-ab-blindspot-vs-signal.png (both cells as printed, plus per-scenario
status lines of each drive). This is the exact observability gap #9341 ran into, reproduced
and closed with a mechanism-equivalent build pair.
Determinism (Reviewer Test Plan step 2): the head build driven twice answered
identically on all 12 scenarios; serve-ab: 0 changed field(s) / "No response changes
against the PR base across 12 scenario(s)". Raw bytes differ only in the three
masked volatiles (workspaceCwd mkdtemp path, lastActivityAt, idleSinceMs), which
DEFAULT_VOLATILE masks — the masking is doing its designed job.
Witness: evidence/02-determinism-same-build-twice.png. Note: the description quotes
"across 10 scenario(s)" — the final set is 12 (see Finding 1).
Canaries fail loud (Reviewer Test Plan step 3), measured live against the real daemon:
| probe | mutation | observed failure | drive exit |
|---|---|---|---|
| chats-leaf drift | chatsDirFor writes chatsX/ |
session-restore-healthy canary: expected HTTP 200 but got 404 |
1 |
| archive-leaf drift | staging writes archiveX/ |
healthy canary passes; archive canary: must not answer HTTP 404 but got 404 |
1 |
| fixture rot (fail-open) | record types unknown → loader skips records |
daemon answers 200 over an empty session; _replayItems=0 witness fires: restored an EMPTY transcript |
1 |
| fixture rot (role) | role:"model"→"assistant" |
canary: expected HTTP 200 but got 500 (Role must be user or model) |
1 |
The deviating capture is written before each abort (verified on disk), so the evidence
survives the failure. Witness: evidence/03-canaries-fail-loud-on-drift.png.
Findings
No blocking findings. Two non-blocking notes:
- Nit — stale scenario count in the description. The Reviewer Test Plan quotes
No response changes against the PR base across 10 scenario(s).; the final scenario
set (after R3 split the source-type scenario into reserved + unreserved) has 12
scenarios, and the determinism run printsacross 12 scenario(s). The described
property (all scenarios identical across two arms) holds; only the quoted count is
outdated. No code change needed. - Informational, pre-existing —
health-deepcaptures a 503 warmup race. The second
scenario lands before the deferred runtime finishes bootstrapping: the daemon schedules
the fallback start at +1000 ms after listen, the firsthealthrequest pulls that
forward to +50 ms, buthealth-deepstill arrives before the bootstrap completes and
captures503 {"status":"degraded","reason":"bootstrap"}(run-qwen-serve.ts:1552).
Reproduced 4/4 drives on this loaded
runner (both head-harness runs, both base-harness runs), so the A/B arms agree here —
but the status is timing-derived: a machine on the other side of the window would flip
it to 200 and emit a spurious diff row on a scenario unrelated to most PRs. This
scenario exists unchanged atHEAD^1(the base harness captured the same 503), so it
is not introduced or worsened by this PR, and none of the PR's staged scenarios are
exposed (they run after the window; canaries certify the daemon is serving). Named for
awareness, not as a merge condition; a fix belongs to its own measured round.
Not covered
- The literal feat(cli): Add standalone conversation isolation primitives #9341 build pair from the description's evidence table could not be
built: the shallow depth-2 checkout contains only the merge commit, base tip, and PR
head. The mutant A/B above is the mechanism-equivalent substitute (same observability
question, one-decision build delta). The current-main census matches the description's
"before" column where comparable (case-twins200,unreadable404
session_not_found). - Per-commit attribution: 4 commits in the metadata, but
git rev-list HEAD^1..HEAD^2
reaches only9a5018c655(shallow boundary). Verified the aggregateHEAD^1..HEADdiff
only. - yamllint could not be installed in-container (
pip3: Permission deniedduring
node scripts/lint.js --setup); actionlint (1.7.12) and shellcheck (0.11.0) installed
and ran via the repo wrapper — exit 0, zero findings mentioning any serve-ab file (the
emitted warnings are pre-existing, in unrelated scripts). The new step'srun:block
was additionally extracted verbatim with a YAML parser, passedbash -n, and executed
under its ownset -euo pipefailcontract (clears both dirs; exit 0 when absent —
the skipped-arm case). - Repo-wide test suite and the serve route's own vitest suites: not run (this PR touches
no production code; PR CI covers them). No production-code behavior was asserted beyond
reading the admission sources to design the mutant. - macOS/Windows: the workflow is Linux-only by design; N/A.
Methodology
Environment: the CI verify container (node:22-bookworm lane runtime, Node v22.23.2,
$__w/_temp = runner temp), npm ci + npm run build pre-run at the merge commit; the
daemon entry is packages/cli/dist/index.js resolving @qwen-code/qwen-code-core via the
workspace symlink whose realpath was asserted inside the head tree before mutating dist.
Drives were run exactly as CI does (node .github/scripts/serve-ab-drive.mjs packages/cli/dist/index.js <outDir>, dummy model creds, isolated HOME/QWEN_HOME);
the mirror identity between chatsDirFor and the product's
Storage.getProjectDir()+/chats was checked over an 8-workspace sweep against the built
core (mirror-probe.mjs, 8/8). The base harness was extracted byte-identical from
HEAD^1 (git show | cmp). Harness suites ran under node --test; the mutation matrix
(mutation-matrix.sh) rebuilt each mutant in an isolated scratch dir importing the mutant
copy. All raw logs, captures, scratch variants, and probe scripts live under
tmp/pr9444-verify-20260820-011727/; assertions.mjs re-executes every check and wrote
assertions.json (57/57, evidence/05-assertions-57-of-57.png).
Flakiness gate log
rounds=5 files=2 skipped=0
file .github/scripts/serve-ab-diff.test.mjs: (cd .) node --test ./.github/scripts/serve-ab-diff.test.mjs
file .github/scripts/serve-ab-drive.test.mjs: (cd .) node --test ./.github/scripts/serve-ab-drive.test.mjs
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/serve-ab-diff.test.mjs: PPPPP
.github/scripts/serve-ab-drive.test.mjs: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/serve-ab-diff.test.mjs: P (exit 0)
round 1 · .github/scripts/serve-ab-drive.test.mjs: P (exit 0)
round 2 · .github/scripts/serve-ab-diff.test.mjs: P (exit 0)
round 2 · .github/scripts/serve-ab-drive.test.mjs: P (exit 0)
round 3 · .github/scripts/serve-ab-diff.test.mjs: P (exit 0)
round 3 · .github/scripts/serve-ab-drive.test.mjs: P (exit 0)
round 4 · .github/scripts/serve-ab-diff.test.mjs: P (exit 0)
round 4 · .github/scripts/serve-ab-drive.test.mjs: P (exit 0)
round 5 · .github/scripts/serve-ab-diff.test.mjs: P (exit 0)
round 5 · .github/scripts/serve-ab-drive.test.mjs: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
The capture loop was extracted so its ordering could be pinned without a daemon, and three of its four abort branches were covered — but not the one that fires when a scenario's setup request fails. Dropping that throw left the whole suite green while a capture would be recorded against a daemon where the setup never took effect, which is the masked diff the branch exists to prevent.
|
R4 review action (commit
Nothing else changed. ESLint and Prettier clean. That leaves the two threads from round 1 as the only open ones, both deliberate: the healthy canary's hard abort (R1-5) and the trigger-path list not being the transitive closure (R1-12). Both are reasoned about in the round-1 reply and are one-line reversals if you'd rather go the other way. |
doudouOUC
left a comment
There was a problem hiding this comment.
Review: PR #9444 — test(ci): stage on-disk session state in the serve A/B
Reviewed at commit a85a08a5. High effort, full pipeline.
Scope
7 files, +1113/-51. This PR implements the first cut of staged on-disk state for the serve A/B harness (proposal 1 from issue #9443). Six new session-admission scenarios with two canaries, an incomplete-baseline detection mechanism, HTTP status capture on every response, and a captureScenarios extraction that certifies the completion marker is written only after the last capture.
Issue Fidelity
closingIssuesReferences: empty (the PR usesRefs #9443rather than a closing link, appropriate for a feature that partially addresses the issue)- Issue #9443 documents six real response divergences on #9341 that the old harness missed. The PR's stated scope — widening what the harness can observe — is a correct first step toward solving the root cause. The
collapsedFromUpstreamflag in the fetch report indicates overlapping merged PRs have reduced the visible diff; the scope is the residual, which is correctly handled.
Verdict
No blockers found. The code is well-structured, the test coverage is thorough (500-line new test file + integration tests), the design fixes are sound (shared marker constant via import, guarded clearCaptureDir, composeCapture ensuring _status always wins, assertCanaryStatus checked after capture), and the build-test confirms no workspace code is affected (all changes are .github/scripts/ and .github/workflows/).
Key strengths
- Correctness:
DRIVE_COMPLETE_MARKERis imported from the drive script (not redeclared) to prevent drift.clearCaptureDiris guarded against accidental deletion of non-capture dirs. The canary'sexpectReplaycheck catches fixture rot that a status-only check cannot see.captureScenarioswrites the marker only after the last capture, so an abort leaves a recognisably truncated baseline. - Test coverage: 29 tests across two files. The
serve-ab-drive.test.mjsfile pins scenario names individually (not by count), tests that staged scenarios probe ids they actually staged, verifies the completion marker behaviour on abort, and includes integration tests for thecommentCLI subcommand. TheisPlainObject/composeCapturetests verify that non-object bodies are nested rather than spread. - Workflow: The unconditional
rm -rfstep inserve-ab.yml(before either drive) closes the cross-run persistence gap that the previous review rounds identified as Critical. The new trigger paths (sessionService.ts,jsonl-utils.ts,paths.ts,storage.ts) are individually named.
Suggestions (no blockers)
None. Every prior finding from R1–R4 has been addressed. The captureScenarios ordering test, the marker-abort test, and the fixture-location test that were previously flagged as missing are now present.
Not linted (tool limitation, not a blocker)
The executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— Qwen Code · /review (v0.21.14)
doudouOUC
left a comment
There was a problem hiding this comment.
Review of PR #9444 — test(ci): stage on-disk session state in the serve A/B
Verdict: COMMENTED — No new blockers found. All previous blockers resolved.
What was reviewed
7 files, +1113/-51 lines. The PR extends the serve A/B harness to stage on-disk session transcripts before running scenario probes, so the session-admission surface (case resolution, transcript integrity, archive conflicts, reserved sources) becomes reachable for A/B testing.
Blocker re-check (2 threads)
Both remaining blockers from previous review rounds were verified against the code at commit a85a08a:
-
R3-1 (serve-ab-diff.mjs:236) — Cross-run
.drive-completemarker inheritance. Fixed. The workflow now clears${RUNNER_TEMP}/beforeand${RUNNER_TEMP}/afterin an unconditional step before either drive, covering all skip variants.clearCaptureDirinsidedriveClicovers within-run re-drives. -
R1-1 (serve-ab-drive.mjs) — Canary abort leaving partial captures. Fixed.
clearCaptureDir(outDir)beforemkdirSyncindriveCli; the same unconditional workflow step covers the cross-run persistence scenario; tests verify truncated dirs are recognised asbaselineIncomplete: true.
New findings
No new blockers found. The code has been through 4 review rounds (R1-R4) with all findings addressed. The implementation is well-structured, thoroughly tested, and correct.
Test verification
- 47/49 tests pass (2 Windows-only path separator failures — expected, CI runs on Linux)
- Full coverage of:
captureScenarios,composeCapture/isPlainObject,clearCaptureDir,assertCanaryStatus,diffCaptureDirsbaseline modes, and thecommentCLI integration path - Build test confirms no workspace code is affected (all changes are
.github/scripts/and.github/workflows/)
Not linted (tool limitation)
The executable-script lint — .github/workflows/ci.yml and .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
审查 PR #9444 — test(ci): stage on-disk session state in the serve A/B
结论:COMMENTED — 未发现新的阻断问题。所有之前的阻断已被解决。
审查内容
7 个文件,+1113/-51 行。该 PR 扩展了 serve A/B 测试框架,在运行场景探测前在磁盘上准备会话转录记录,使得会话准入表面(大小写解析、转录完整性、归档冲突、保留源)可以被 A/B 测试覆盖到。
阻断复查(2 条)
之前审查轮次的两个阻断问题已在提交 a85a08a 处验证通过:
-
R3-1 (serve-ab-diff.mjs:236) — 跨运行的
.drive-complete标记继承问题。已修复。 工作流现在在任意驱动步骤之前以无条件步骤清除${RUNNER_TEMP}/before和${RUNNER_TEMP}/after,覆盖了所有跳过变体。driveCli内部的clearCaptureDir覆盖了运行内的重驱动场景。 -
R1-1 (serve-ab-drive.mjs) — 哨兵中止留下部分采集结果。已修复。
driveCli中的clearCaptureDir(outDir)在mkdirSync之前执行;相同的工作流级无条件步骤覆盖了跨运行持久化场景;测试验证了截断的目录被识别为baselineIncomplete: true。
新发现
未发现新的阻断问题。 代码已经历 4 轮审查(R1-R4),所有发现均已被处理。实现结构良好、经过充分测试且正确。
测试验证
- 47/49 测试通过(2 个 Windows 路径分隔符失败——预期内,CI 运行在 Linux 上)
- 全面覆盖:
captureScenarios、composeCapture/isPlainObject、clearCaptureDir、assertCanaryStatus、diffCaptureDirs基线模式以及commentCLI 集成路径 - 构建测试确认无工作区代码受影响(所有变更均在
.github/scripts/和.github/workflows/中)
未检查(工具限制)
The executable-script lint — .github/workflows/ci.yml 和 .github/workflows/serve-ab.yml:actionlint 嵌入式 shell 源码映射暂不支持——未检查。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
There was a problem hiding this comment.
Review of PR #9444 — test(ci): stage on-disk session state in the serve A/B
Verdict: COMMENTED — No new blockers found. All previous blockers resolved.
What was reviewed
7 files, +1113/-51 lines. The PR extends the serve A/B harness to stage on-disk session transcripts before running scenario probes, so the session-admission surface (case resolution, transcript integrity, archive conflicts, reserved sources) becomes reachable for A/B testing.
Blocker re-check (2 threads)
Both remaining blockers from previous review rounds were verified against the code at commit a85a08a:
-
R3-1 (serve-ab-diff.mjs:236) — Cross-run
.drive-completemarker inheritance. Fixed. The workflow now clears${RUNNER_TEMP}/beforeand${RUNNER_TEMP}/afterin an unconditional step before either drive, covering all skip variants.clearCaptureDirinsidedriveClicovers within-run re-drives. -
R1-1 (serve-ab-drive.mjs) — Canary abort leaving partial captures. Fixed.
clearCaptureDir(outDir)beforemkdirSyncindriveCli; the same unconditional workflow step covers the cross-run persistence scenario; tests verify truncated dirs are recognised asbaselineIncomplete: true.
New findings
No new blockers found. The code has been through 4 review rounds (R1-R4) with all findings addressed. The implementation is well-structured, thoroughly tested, and correct.
Test verification
- 47/49 tests pass (2 Windows-only path separator failures — expected, CI runs on Linux)
- Full coverage of:
captureScenarios,composeCapture/isPlainObject,clearCaptureDir,assertCanaryStatus,diffCaptureDirsbaseline modes, and thecommentCLI integration path - Build test confirms no workspace code is affected (all changes are
.github/scripts/and.github/workflows/)
Not linted (tool limitation)
The executable-script lint — .github/workflows/ci.yml and .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
审查 PR #9444 — test(ci): stage on-disk session state in the serve A/B
结论:COMMENTED — 未发现新的阻断问题。所有之前的阻断已被解决。
审查内容
7 个文件,+1113/-51 行。该 PR 扩展了 serve A/B 测试框架,在运行场景探测前在磁盘上准备会话转录记录,使得会话准入表面(大小写解析、转录完整性、归档冲突、保留源)可以被 A/B 测试覆盖到。
阻断复查(2 条)
之前审查轮次的两个阻断问题已在提交 a85a08a 处验证通过:
-
R3-1 (serve-ab-diff.mjs:236) — 跨运行的
.drive-complete标记继承问题。已修复。 工作流现在在任意驱动步骤之前以无条件步骤清除${RUNNER_TEMP}/before和${RUNNER_TEMP}/after,覆盖了所有跳过变体。driveCli内部的clearCaptureDir覆盖了运行内的重驱动场景。 -
R1-1 (serve-ab-drive.mjs) — 哨兵中止留下部分采集结果。已修复。
driveCli中的clearCaptureDir(outDir)在mkdirSync之前执行;相同的工作流级无条件步骤覆盖了跨运行持久化场景;测试验证了截断的目录被识别为baselineIncomplete: true。
新发现
未发现新的阻断问题。 代码已经历 4 轮审查(R1-R4),所有发现均已被处理。实现结构良好、经过充分测试且正确。
测试验证
- 47/49 测试通过(2 个 Windows 路径分隔符失败——预期内,CI 运行在 Linux 上)
- 全面覆盖:
captureScenarios、composeCapture/isPlainObject、clearCaptureDir、assertCanaryStatus、diffCaptureDirs基线模式以及commentCLI 集成路径 - 构建测试确认无工作区代码受影响(所有变更均在
.github/scripts/和.github/workflows/中)
未检查(工具限制)
The executable-script lint — .github/workflows/ci.yml 和 .github/workflows/serve-ab.yml:actionlint 嵌入式 shell 源码映射暂不支持——未检查。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: the entire diff — no agent reported covering it; nobody read it.
Not reviewed: the executable-script lint — the report is stale or its diff could not be verified; re-run qwen review script-lint.
Not reviewed: every dimension — none of the 10 required agents is on record as launched with a prompt this skill built, so this diff was reviewed, if at all, from prompts the run wrote for itself: no record shows the severity bar, the finding format or this project's own rules reaching an agent.
Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries.
Not reviewed: verification — its prompt was built, but no agent was launched with it, so the posted findings cannot be counted as verified.
中文说明
未审查:整个 diff——没有 agent 报告覆盖过这部分,也没有人读过它。
未审查:the executable-script lint — the report is stale or its diff could not be verified; re-run qwen review script-lint。
未审查:所有维度——10 个必需 agent 中没有任何一个有记录表明是用本 skill 构建的 prompt 启动的,这个 diff 即便被审查过,也是基于这次 run 自行编写的 prompt:没有记录表明严重级别标准、发现格式或本项目自己的规则到达过任何 agent。
未审查:反向审计——没有审计 agent 是用本 skill 构建的 prompt 启动的——负责搜寻评审其余部分遗漏问题的这道工序,即便运行过,也缺失了 brief 承载的方法。
未审查:验证——它的 prompt 已构建,但没有 agent 用它启动,发布的发现不能算作已验证。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
| */ | ||
| export function chatsDirFor(home, workspaceCwd) { | ||
| const projectId = workspaceCwd.replace(/[^a-zA-Z0-9]/g, '-'); | ||
| return join(home, '.qwen', 'projects', projectId, 'chats'); |
There was a problem hiding this comment.
[Suggestion] chatsDirFor does not lowercase workspaceCwd on Windows, diverging from sanitizeCwd
Failure scenario: If the harness is run locally on Windows, chatsDirFor produces a project directory with uppercase letters (e.g. C-Users-Foo) while sanitizeCwd in the daemon lowercases it (c:-users-foo). Staged fixture files would land in the wrong directory, causing session-restore scenarios to 404. The CI workflow runs on Linux so this does not affect CI results.
| return join(home, '.qwen', 'projects', projectId, 'chats'); | |
| const projectId = workspaceCwd.replace(/[^a-zA-Z0-9]/g, '-').toLowerCase(); | |
| return join(home, '.qwen', 'projects', projectId, 'chats'); |
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
中文说明
chatsDirFor 函数在 Windows 上不会将 workspaceCwd 转换为小写,这与 sanitizeCwd 的行为不一致。sanitizeCwd 在 Windows 上会执行 cwd.toLowerCase(),而 chatsDirFor 没有。如果有人在 Windows 上本地运行此工具,fixture 文件会写入错误的项目目录。CI 在 Linux 上运行,因此不影响 CI 结果。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
|
@qwen-code /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 冲突,直到移除标签或达到轮次上限。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review round actions — commit
|
| Item | Action |
|---|---|
| [rc:3818053687] (R5-1) — the success-path half of the setup-failure test pins what the drive writes, but not that setup requests run BEFORE the probe | Fixed. The success path now records the request order and asserts ['/session', '/health?deep=1'], the same call-order technique as the adjacent fixtures-before-request test. Verified your mutant: moving the setup loop after the probe now fails the test (was 50/50 green under it). |
[rc:3818093675] — the chatsDirFor tests hardcode Unix-style paths and fail on Windows |
Fixed. The expectations are now built with join() (like the function itself and the daemon's Storage.getProjectDir) and the segment count splits on sep, so both tests are platform-aware without weakening any assertion. |
[rc:3818093678] — composeCapture does not guarantee _status when a scenario.project projection is used |
Fixed. Reproduced first: a projection returning { _status: 999 } beat the harness's 200. The harness status is now applied after the projection — { ...scenario.project(json, res), _status: res.status } — extending the R2-2 invariant ("the harness value wins") from bodies to projections. A new test pins it; inverting the fix fails that test. No current projection changes value: they all already stamp res.status. |
[rc:3818153686] — chatsDirFor does not lowercase workspaceCwd on Windows, diverging from sanitizeCwd |
Fixed, with a corrected shape. The divergence is real — verified against sanitizeCwd in packages/core/src/utils/paths.ts, which lowercases only when os.platform() === 'win32'. chatsDirFor now takes the same platform branch, keeping the documented lockstep true on both platforms. The suggested unconditional .toLowerCase() was tested as a mutant and fails the new mirror test on Linux — sanitizeCwd does not lowercase there, so an unconditional lowercase would strand staged fixtures on the very runners this harness drives. The conditional mirror is pinned by a case-preservation assertion with platform-aware expectations. |
No action required
- rv:4978435074 / rv:4978470906 — their two findings are the same items as rc:3818093675 and rc:3818093678, addressed above.
- rv:4978433524 restates the two former Criticals (cross-run marker inheritance, partial captures) as "still open" while its own text describes both as addressed. The focused re-check at the same commit
a85a08a(rv:4978367246) verified both fixed, and this round re-confirmed in code: the unconditionalClear stale capture dirsstep runs before either drive (covering every skip-by-if:variant), anddriveClistarts with the guardedclearCaptureDir. No change made. - rv:4978503206 is a disclosure that one review run could not certify its coverage; its only inline finding is rc:3818153686, addressed above.
- Sandboxed-verification notes (ic:5349902298, ic:5350250377): the stale
across 10 scenario(s)quote lives in the PR description, not in code (no code change, and this flow does not edit PR text); thehealth-deep503 bootstrap warmup race is pre-existing atHEAD^1, timing-derived, identical on both arms, and was explicitly named for awareness rather than as a merge condition. - R1-5 / R1-12 remain open by round-1 author judgement (healthy canary keeps its hard abort; the trigger-path list is deliberately not the transitive closure). No new feedback arrived on either this round; both stay as-is.
No conflicts (--conflict false; no merge performed).
Verification
Commands actually run this round, and their results:
node --test .github/scripts/serve-ab-drive.test.mjs .github/scripts/serve-ab-diff.test.mjs— 50 passed / 0 failed (pre-change baseline: 49/49)- Mutant probes on the new pins (each applied, suite run, then reverted):
- setup loop moved after the probe → 1 fail — the new order assertion kills it
composeCapturereturning the projection as-is → 1 fail — the new projection test kills it- unconditional
.toLowerCase()inchatsDirFor(the shape the finding suggested) → 1 fail — the mirror test kills it on Linux - restored code → 50/50 green
npx prettier --checkon the two changed files — cleannpx eslinton the two changed files — cleannpm run lint— passednpm run build— passednpm run typecheck— passed (tsc --noEmitin every workspace)- Focused package Vitest — N/A: no workspace package is touched; the changed files are CI harness scripts, and their suites are the two
node --testfiles above (run in CI viaHELPER_TESTSinci.yml). Settings-schema regeneration — N/A (no settings source changed).
中文说明
审查轮次处理 — 提交 b579c043
a85a08a 之后新到 4 条行内发现(自动审查器的 R5-1,以及三条来自维护者运行审查的建议)。四条全部实现;每条新增或修改的断言都在提交前经过「杀死突变体」验证。
| 条目 | 处理 |
|---|---|
| [rc:3818053687](R5-1) —— setup 失败测试的成功路径一半只钉住了驱动写出了什么,没有钉住 setup 请求必须先于探测请求 | 已修复。 成功路径现在记录请求顺序并断言 ['/session', '/health?deep=1'],与相邻的「fixtures 先于请求」测试采用同样的调用顺序记录手法。已验证您提供的突变体:把 setup 循环移到探测之后现在会使该测试失败(突变下原套件 50/50 全绿)。 |
[rc:3818093675] —— chatsDirFor 测试硬编码 Unix 风格路径,在 Windows 上失败 |
已修复。 期望值现在用 join() 构造(与函数本身及 daemon 的 Storage.getProjectDir 一致),段数统计改用 sep 切分,两个测试因此具备平台感知能力,且没有弱化任何断言。 |
[rc:3818093678] —— 使用 scenario.project 投影时 composeCapture 不保证 _status 是 harness 看到的状态 |
已修复。 先复现:返回 { _status: 999 } 的投影确实会覆盖 harness 的 200。现在 harness 状态在投影之后强制写入 —— { ...scenario.project(json, res), _status: res.status } —— 把 R2-2 的不变量(「harness 的值胜出」)从响应体扩展到投影。新增测试钉住该行为;反转该修复会使该测试失败。现有投影的取值不受影响:它们本就写入 res.status。 |
[rc:3818153686] —— chatsDirFor 在 Windows 上不对 workspaceCwd 转小写,与 sanitizeCwd 分歧 |
已修复,且修正了建议的形态。 分歧属实 —— 已对照 packages/core/src/utils/paths.ts 中的 sanitizeCwd 核实:它仅在 os.platform() === 'win32' 时转小写。chatsDirFor 现在走同样的平台分支,使注释声明的「与 sanitizeCwd 保持同步」在两个平台上都成立。建议中的无条件 .toLowerCase() 已作为突变体实测:失败 —— Linux 上 sanitizeCwd 并不转小写,无条件转小写会把 staged fixture 摆到 daemon 读不到的目录,恰在本 harness 实际运行的 runner 上出问题。条件式镜像由一条大小写保留断言钉住(期望值按平台感知构造)。 |
无需处理
- rv:4978435074 / rv:4978470906 —— 其两条发现与 rc:3818093675、rc:3818093678 是同一内容,已在上方处理。
- rv:4978433524 把此前两个 Critical(跨运行 marker 继承、部分采集残留)复述为「仍开放」,但其自身文字又描述两者已处理。针对同一提交
a85a08a的专项复查(rv:4978367246)已验证两者已修复,本轮也在代码中再次确认:无条件的Clear stale capture dirs步骤在任一 drive 之前运行(覆盖所有被if:跳过的变体),且driveCli以带守卫的clearCaptureDir开始。无需改动。 - rv:4978503206 是一次无法证明其覆盖面的审查运行的披露说明;它产生的唯一行内发现是 rc:3818153686,已在上方处理。
- 沙箱验证说明(ic:5349902298、ic:5350250377):过期的
across 10 scenario(s)引文位于 PR 描述而非代码中(无需改代码,本流程也不编辑 PR 文本);health-deep的 503 启动窗口竞态在HEAD^1即已存在、由时序决定、两臂一致,且被明确标注为「仅供知悉、不作为合并条件」。 - R1-5 / R1-12 依第 1 轮作者判断保持开放(healthy canary 保留硬性中止;触发路径清单有意不追求传递闭包)。本轮两者均无新反馈,维持原状。
无冲突(--conflict false;未执行任何合并)。
验证
本轮实际执行的命令及结果:
node --test .github/scripts/serve-ab-drive.test.mjs .github/scripts/serve-ab-diff.test.mjs—— 50 通过 / 0 失败(改动前基线:49/49)- 针对新钉子的突变体探测(每个突变体均为:施加 → 运行套件 → 还原):
- setup 循环移到探测之后 → 1 失败 —— 新的顺序断言将其杀死
composeCapture原样返回投影结果 → 1 失败 —— 新的投影测试将其杀死chatsDirFor无条件.toLowerCase()(即发现中建议的形态)→ 1 失败 —— 镜像测试在 Linux 上将其杀死- 还原后代码 → 50/50 全绿
npx prettier --check(两个改动文件)—— 通过npx eslint(两个改动文件)—— 通过npm run lint—— 通过npm run build—— 通过npm run typecheck—— 通过(所有 workspace 的tsc --noEmit)- 聚焦 package Vitest —— 不适用:未触碰任何 workspace package;改动文件是 CI harness 脚本,其测试套件即上方两个
node --test文件(在 CI 中经ci.yml的HELPER_TESTS运行)。settings schema 重新生成 —— 不适用(未改动任何 settings 源文件)。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
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/模型 qwen3.8-max
b579c04
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
.github/scripts/serve-ab-drive.mjs:440 — [review] the session-create-* scenarios are pinned by no test — silent dropout on deletion.github/scripts/serve-ab-drive.mjs:129 — [review] the _nonJson JSON-parse fallback branch of captureScenarios is exercised by no test.github/scripts/serve-ab-diff.mjs:262 — [probe] the 'removed' pass-through in the comment CLI glue has no CLI-path test (mutation survives 25/25).github/scripts/serve-ab-diff.mjs:182 — [probe] baselineIncomplete warning never names which scenarios have no baseline
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| name: 'session-create-unreserved-source', | ||
| method: 'POST', | ||
| path: '/session', | ||
| auth: true, | ||
| body: () => ({ sourceType: 'standalone' }), |
There was a problem hiding this comment.
[Critical] This scenario sends sourceType: 'standalone', but standalone is no longer an unreserved source: main (the base arm) merged the standalone isolation work, and POST /session now refuses this body with 400 reserved_session_source (isReservedStandaloneSessionSource). The comment's premise — "an ordinary, currently-unreserved source type. Admitted today" — is the opposite of reality at this commit.
Because both arms already answer 400, the scenario's stated detection — "a PR which starts reserving one shows up here as 200 → 400" — is unreachable: any future reservation change diffs 400 → 400 = clean, blessing exactly the regression class the comment says this harness "missed once already". The scenario also becomes a near-duplicate of session-create-reserved-source, and no scenario sends a genuinely admitted named source, so a PR that starts rejecting a truly unreserved source diffs clean. There is no expectStatus/rejectStatus, so the drive never aborts — the hole is silent.
Witness (live drive of the real daemon built at this commit):
as written: captured (HTTP 400) { "_status": 400, "code": "reserved_session_source",
"error": "The requested session source is reserved for daemon-owned standalone sessions." }
flip check: body -> { sourceType: 'scheduled_task', sourceId: 'serve-ab' }
captured (HTTP 200) { "_status": 200 }
Send a genuinely unreserved source so the scenario establishes the intended 200 admission witness (or rename it to pin the standalone refusal and rewrite the comment to match):
| name: 'session-create-unreserved-source', | |
| method: 'POST', | |
| path: '/session', | |
| auth: true, | |
| body: () => ({ sourceType: 'standalone' }), | |
| name: 'session-create-unreserved-source', | |
| method: 'POST', | |
| path: '/session', | |
| auth: true, | |
| body: () => ({ sourceType: 'scheduled_task', sourceId: 'serve-ab' }), |
中文说明
问题:该场景发送 sourceType: 'standalone',但 standalone 现在已是保留 source:main(base 臂)合入 standalone 隔离改动后,POST /session 会以 400 reserved_session_source(isReservedStandaloneSessionSource)拒绝该请求体。注释中的前提——「一个普通的、当前未被保留的 source 类型,今天会被接受」——在本提交上与现实恰好相反。
由于两个臂现在都已回答 400,该场景声称的检测能力——「某个 PR 开始保留它时,这里会显示 200 → 400」——无法触发:未来任何保留该类型的改动都会 diff 出 400 → 400 = 无变化,恰好放行注释里说本 harness「已经漏过一次」的那类回归。该场景也因此与 session-create-reserved-source 近乎重复,且没有任何场景发送真正会被接受的具名 source——一个开始拒绝真正未保留 source 的 PR 会静默地 diff 为空。场景没有 expectStatus/rejectStatus,驱动不会中止——这个漏洞是无声的。
证据(用本提交构建的真实 daemon 实测驱动):按原样运行时采集到 { "_status": 400, "code": "reserved_session_source", ... };将 body 改为 { sourceType: 'scheduled_task', sourceId: 'serve-ab' } 后采集到 { "_status": 200 }。
修复:改用一个真正未被保留的 source,让场景建立预期的 200 准入见证(如上方 suggestion);或者将场景改名以锁定 standalone 拒绝本身,并相应改写注释。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #9444Feedback dispositions[rc:3820302319] — Critical (inline, The finding claims the
Fix (the finding's suggested body): the witness now sends Regression test: a new drive-test case pins that the witness exists, sends a named source, and matches neither reserved shape the route refuses today ( [rv:4981191117] — CHANGES_REQUESTED review body: no separate action. Its actionable content is the inline Critical above. The "not reviewed" (reverse audit did not converge within its cap) and "not linted" (actionlint embedded-shell source mapping unsupported) disclosures are tool limitations, not code findings. The four items listed under "Deferred under the convergence posture (round 6, not a blocker)" are the reviewer's own audit record, explicitly recorded and not requested in this round; they remain tracked in the reviewer's ledger comment on the PR, and no code, thread, or reply changes were made for them. Failed checks / still-red checks: none reported. Conflict: none ( Changes
Commit: Verification
中文说明Autofix 审查轮次 — PR #9444反馈处理结论[rc:3820302319] — Critical(行内评论, 该发现指出
修复(采用发现中建议的 body):见证场景现在发送 回归测试:新增一个 drive 测试用例,钉住该见证场景存在、发送具名 source、且不匹配路由今天拒绝的任何保留形态( [rv:4981191117] — CHANGES_REQUESTED 审查主体:无需单独处理。 其可操作内容即上面的行内 Critical。「未审查」(反向审计在轮数上限内未收敛)与「未检查」(actionlint 不支持 embedded-shell source mapping)属于工具限制,不是代码发现。列在「收敛姿态下延后(第 6 轮,非阻断)」下的 4 条是审查者自己的审计记录,明确标注已记录、本轮不要求修改;它们继续由审查者在 PR 上的台账评论跟踪,未对其做任何代码、线程或回复操作。 失败检查 / 持续失败的检查: 无。冲突: 无( 变更内容
提交: 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 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/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
.github/scripts/serve-ab-diff.mjs:53 — [probe] marker writer/reader pairing unpinned across modules — drift mutation survives 51/51.github/scripts/serve-ab-drive.mjs:425 — [probe] session-create-reserved-source pinned by no test — deletion/body flip survives 26/26.github/scripts/serve-ab-diff.test.mjs:327 — [probe] the removed flag is unpinned at the CLI level — warning can vanish, suite stays green.github/scripts/serve-ab-drive.mjs:87 — [probe] clearCaptureDir admits any all-JSON directory and deletes it wholesale.github/scripts/serve-ab-drive.mjs:421 — [review] the standalone reserved-source refusal is probed by no scenario.github/scripts/serve-ab-drive.test.mjs:112 — [probe] SCENARIOS membership pinned only in subsets — four scenarios silently droppable.github/scripts/serve-ab-drive.mjs:478 (+2 locations) — [probe] harness mkdtemp dirs never removed — unbounded growth on the persistent pool.github/scripts/serve-ab-drive.mjs:445 — [probe] unreserved witness test asserts not-reserved but not admitted — ScheduledTask mutant survives
中文说明
仅完成部分审查,审查缺口已披露。
未审查:reverse audit — did not converge within the reverse-audit round cap of 5。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/serve-ab.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — PR #9444: no action requiredThe only feedback newer than the last evaluation is the automated reviewer's round-7 review (
No code changes were made this round. The branch remains at 中文说明Autofix 评审轮次 — PR #9444:无需任何改动上次评估之后唯一更新的反馈是自动评审器的第 7 轮评审(
本轮未做任何代码改动。分支保持在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 95 passed · 0 failed · 95 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:95 通过 · 0 失败 · 95 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportSandboxed verification: ✅ passed — merge-ready (agent verdict, follow-up round) 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: 95 passed · 0 failed · 95 total Verdict: 中文摘要
Previous-round findings at the new head
Declined/deferred rows from the previous round: none. Delta since the previous roundThe old heads (
Central claim + A/B table (re-run at the new head)Central claim: without on-disk staging every probe hits an empty daemon and the session-admission surface is invisible to the A/B; a harness that stages transcripts and records
Canaries + ordering, re-run live against the real daemon (witness
All five aborted with no completion marker and their captures on disk. Mirror identity re-verified at the new head: Mutation matrix at the new head (witness Suites + flake: drive 26/26 + diff 25/25 at the new head; 5 identical rounds × 2 changed test files, no divergence ( FindingsNo new findings. The two carried notes (stale scenario count in the PR body; Not covered
MethodologyEnvironment: the CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. CI green. The completion marker is single-owned (written by the drive, imported by the diff — no drift) and the baselineIncomplete disclosure closes a real reporting hole: a base drive that stops part-way would otherwise render its unreached scenarios as 'this PR adds these responses'. The session-state staging and the diff/disclosure paths are both covered by the new tests.
|
Released in v0.21.15. |











What this PR does
The serve A/B drives its whole scenario set against a daemon that has just started with an empty home directory. Everything the daemon decides from persisted transcripts — resolving a session id whose file is spelled in another case, refusing two spellings of one id, refusing a transcript whose head recovers nothing, refusing one that exists in both the active and the archive directory, and refusing a reserved session source — is therefore never reached, and a PR that rewrites all of it diffs as "no response changes".
A scenario can now stage transcripts on disk before its request, and capture a reduced projection of the response instead of the whole session snapshot. The HTTP status is recorded on every capture, so a status-only difference is no longer invisible. Six session-admission scenarios use that: a healthy restore, the legacy uppercase spelling, case-only twins, an unreadable transcript, one id present in both the active and the archive directory, and session creation carrying a source type.
The transcripts are re-pointed copies of a committed fixture that was recorded from a real CLI turn rather than hand-written, because the loader rejects synthesized records over details that are easy to get wrong, and a fixture that quietly fails to load would turn every staged scenario into a meaningless "no such session". Staging also depends on the on-disk project layout, which the harness mirrors rather than imports; if that mirror ever drifts, the fixtures land nowhere. The healthy restore therefore doubles as a canary that fails the drive outright instead of letting the run publish a reassuring all-clear.
The trigger now also fires on the two core files the admission answers are decided by, named individually rather than by globbing the whole package, and the existing setup step drops a body key the route never read.
Why it's needed
#9341 is the worked example. Its subject is exactly this surface, and the A/B posted on it reported no response change across four scenarios. Driving the same two builds with transcripts on disk, they answer differently on six requests — including one where the new refusal explains itself with a message that does not match the situation, which the report now surfaces for free.
A verification signal that cannot reach the code under review is worse than no signal, because "no response changes" reads as a clean bill of health rather than as "nothing I did could have observed this". Widening what the harness can see is the narrow, testable half of that problem; the rest is tracked in the linked issue.
Reviewer Test Plan
How to verify
Build the CLI from #9341's head and from its merge base, then drive each with this harness and diff the captures. The base build accepts the case-only twins and the reserved source and answers "no such session" for the unreadable transcript; the head build refuses all three. On
maintoday the same two builds produce no diff at all, because none of those requests are made.Then confirm the harness has not become noisy: drive the same build twice and diff the two capture directories. All scenarios must come out identical — the staged ids are fixed rather than random precisely so the two arms, which run as separate processes, cannot disagree by accident.
Finally, break the layout on purpose — change the project-directory mirror so fixtures are written somewhere the daemon does not read — and confirm the drive fails on the healthy-restore canary instead of reporting a clean run.
Locally: both node test files pass (29 cases), ESLint and Prettier are clean on the touched files, and the drive completes against both builds with the canary satisfied.
Evidence (Before & After)
Against #9341's build pair, before this change:
After, the same build pair:
Same build twice, after this change:
No response changes against the PR base across 10 scenario(s).Tested on
Environment (optional)
Linux 6.12, Node 22.22.2, two locally built
qwen servedaemons driven with dummy model credentials, as in CI.Risk & Scope
_status, and a base built before this change carries none, so the diff drops the unpaired field for one transition rather than reporting it as added on every scenario; that shim is a no-op once both sides have it.Linked Issues
Refs #9443
Motivated by the verification round on #9341.
中文说明
本 PR 做了什么
serve A/B 的全部 scenario 都是打在一个刚启动、home 为空的 daemon 上。凡是 daemon 依据持久化 transcript 做出的决定 —— 解析文件名大小写不同的 session id、拒绝同一 id 的两种拼写、拒绝头部无法恢复任何记录的 transcript、拒绝同时存在于 active 与 archive 目录的 transcript、拒绝保留的 session source —— 因此从未被触达;一个把这些逻辑全部重写的 PR,diff 出来就是 "no response changes"。
现在一个 scenario 可以在发出请求前把 transcript 摆到磁盘上,并且采集响应的一个精简投影而不是整个 session 快照。每次采集都记录 HTTP status,因此只有状态码变化的差异不再不可见。六个 session 准入 scenario 用到了这个能力:健康的 restore、legacy 大写拼写、仅大小写不同的双份、不可读的 transcript、同时存在于 active 与 archive 的同一 id,以及携带 source type 的会话创建。
这些 transcript 是一份提交在仓库里的 fixture 的重定向副本,而该 fixture 是从一次真实 CLI 对话录制下来的,不是手写的 —— 因为 loader 会因为一些很容易写错的细节而拒绝合成记录,而一份静默加载失败的 fixture 会把每个 staged scenario 都变成没有意义的「查无此会话」。摆放状态还依赖磁盘上的 project 目录布局,而 harness 是照着抄的、不是 import 来的;一旦这个镜像发生漂移,fixture 就会落到别处。因此健康 restore 同时充当 canary:它会让整次驱动直接失败,而不是让这一轮发布一份让人安心的全绿结论。
触发条件现在也包含准入结论所依赖的两个 core 文件,逐个列出而不是 glob 整个包;同时现有 setup 步骤去掉了一个路由从来不读的 body 字段。
为什么需要
#9341 就是活生生的例子。它的主题正是这个表面,而发布在它上面的 A/B 报告是四个 scenario 全无响应变化。把同样两个构建配上磁盘上的 transcript 来驱动,它们在六个请求上给出不同结果 —— 其中一个新增拒绝所给出的解释与实际情况并不相符,而这一点现在会被报告顺带暴露出来。
一个无法触达被审查代码的验证信号,比没有信号更糟,因为 "no response changes" 读起来像健康证明,而不是「我做的任何事都不可能观察到这一点」。拓宽 harness 能看到的范围,是这个问题中范围收敛、可测试的那一半;其余部分记录在关联 issue 中。
Reviewer 测试计划
如何验证
从 #9341 的 head 与其 merge base 各构建一次 CLI,然后用本 harness 分别驱动并 diff 采集结果。base 构建会接受仅大小写不同的双份和保留 source,并对不可读 transcript 回答「查无此会话」;head 构建三者全部拒绝。而在今天的
main上,同样这两个构建根本产生不出任何 diff,因为那些请求压根不会发出。然后确认 harness 没有变得不稳定:用同一个构建驱动两次并 diff 两份采集目录,所有 scenario 必须完全一致 —— staged id 之所以是固定值而非随机值,正是为了让分处两个进程的两个 arm 不会偶然产生分歧。
最后故意破坏布局 —— 修改 project 目录镜像,让 fixture 被写到 daemon 不会读取的地方 —— 确认驱动会在健康 restore canary 上失败,而不是报出一次干净的运行。
本地结果:两个 node 测试文件全部通过(29 个用例),改动文件的 ESLint 与 Prettier 干净,针对两个构建的驱动均完成且 canary 满足。
证据(Before & After)
针对 #9341 的构建对,改动前:
改动后,同一构建对:
改动后同一构建跑两次:
No response changes against the PR base across 10 scenario(s).测试平台
环境(可选)
Linux 6.12、Node 22.22.2,两个本地构建的
qwen servedaemon,使用与 CI 相同的 dummy 模型凭据驱动。风险与范围
_status,而在此改动之前构建的 base 不带,因此 diff 会在过渡期丢弃这个无配对字段,而不是在每个 scenario 上把它报成新增;一旦两侧都带上它,这个兼容层就是空操作。关联 Issue
Refs #9443
由 #9341 的验证轮次触发。