fix(review): report what the transcripts prove; build the roster in one call - #7033
Merged
Conversation
…hand-assemble
Dogfooded on a real 3A review of a live PR, and the run talked its way past the gate:
compose-review printed: Verdict: Comment — an Approve was NOT available: a
dimension nobody reviewed
the run's next thought: "the compose-review flagged reverse audit as unreviewed
(transcript visibility issue — the reverse audit did run substantively with two
dry rounds). Let me proceed."
the run then reported, and saved: Verdict: Approve
The gap was right and its wording was wrong, and the wording is what let the run
dismiss it. Two auditors HAD run — 16 and 23 tool calls each — and both HAD opened
their brief. What had actually happened is that the orchestrator skipped `--findings`
and hand-wrote their launches, keeping only the brief pointer, so no agent was
launched with the prompt the CLI built. The gap said "no agent was launched with it
that opened its brief", which is false as written, and "a transcript visibility
issue" is what a reader concludes from a message that does not describe what
happened.
So the floor tells the four shapes apart instead of collapsing them into one
boolean, and each says what happened and what to do:
not-built — the step was skipped; `agent-prompt --role <r>` never ran
not-launched — the prompt was built and nothing was launched with it
rewritten — an agent ran and opened its brief, but no agent got the built
prompt: the launch was written by hand instead of pasted
brief-unread — an agent got the built prompt and never opened the brief
`rewritten` is the one that just happened, and it is now un-dismissable: it concedes
the agent ran and read its brief, and names the orchestrator's own edit as the defect.
And the path that produced it is gone: `--findings` is now REQUIRED for a role that
takes findings. There is no bare-block-plus-hand-assembly path left — the command
refuses, and prints one block to paste. An early reverse-audit round with nothing
confirmed yet passes an empty file, which the command renders as "Nothing is
confirmed yet".
SKILL: Step 4/5 say `--findings` is required. Step 6 gains the second half of the
lesson — you may not overrule the line compose-review gives you; a cap you can
explain is still a cap, and the fix is to make the step verifiable and re-run, not to
keep the verdict you preferred. Step 8's report interpolates the verdict out of the
composed JSON (`jq -r .event`) instead of typing it, because the terminal is prose
and the archive is forever.
Collaborator
|
Thanks for the PR! Template: the body uses a custom format rather than the PR template headings, but it provides comprehensive information — six numbered sections with motivation, before/after evidence, and a Chinese translation. Non-blocking. Problem: observed, not theoretical. All six changes trace back to specific failures found by dogfooding Direction: aligned. The Size: 25 files, ~1269 production lines (excluding ~1812 test lines). Core path changes are minimal: Approach: the six changes are well-separated and each targets a specific failure. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板：body 使用自定义格式而非 PR 模板标题，但提供了全面的信息——六个编号章节包含动机、before/after 证据和中文翻译。非阻塞性。 问题：已观测，非理论性。 所有六个修改都来自在真实 PR 上 dogfood 方向：对齐。 规模：25 个文件，约 1269 生产行（排除约 1812 测试行）。核心路径改动极小。1000+ 大 PR 建议适用，但范围合理——六个不同失败模式，各有对应修复。 方案：六个修改分离良好，各自针对特定失败。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Contributor
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. |
Collaborator
Code ReviewIndependent proposal: before reading the diff, I'd approach this as: (1) add a Comparison: the PR's approach matches my proposal and exceeds it in several ways. The bipartite matching (Kuhn's algorithm) for transcript-to-requirement assignment is something I wouldn't have thought of — it solves the problem where a greedy pass can permanently cap a compliant repair. The Reuse check: No Critical issues found. The code is well-structured, thoroughly commented (every decision explains why), and the test coverage is comprehensive (238 CLI tests + 22 core tests + 516 script tests, all passing). Real-Scenario TestingDev build ( Unit test results (all passing): The one script test failure ( Files changed (25 files)
中文说明代码审查独立方案对比： PR 的方法与我的独立方案一致，并在多个方面超越。二分匹配（Kuhn 算法）用于 transcript-to-requirement 分配是我没想到的——它解决了贪心分配可能永久封顶合规修复的问题。 未发现 Critical 问题。代码结构良好，注释详尽（每个决策都解释为什么），测试覆盖全面（238 CLI + 22 core + 516 scripts，全部通过）。 真实场景测试Dev build 无法在此 CI 环境中运行——workspace 依赖 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This PR has been through 22+ rounds of review — human and automated — and every finding has been addressed. The latest head ( The six changes share one principle: the review run may only claim what its records prove, said to the reader who can act on it. Each change targets a specific, observed failure mode found by dogfooding Key strengths:
No concerns. Shipping this. 中文说明置信度：5/5 — 各阶段均通过；可以毫不犹豫地合并。 这个 PR 经历了 22+ 轮审查——人工和自动——每个发现都已被解决。 六个修改共享一个原则：review run 只能声明其记录能证明的，而且要说给能据此行动的人听。每个修改针对一个在真实 PR 上 dogfood 关键优势：
无顾虑。批准合并。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 16, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…inds Reported from a real session: `npm run dev:daemon`, `/review 6998 --comment` in the web shell, and the run died on Missing required argument: chunk with a help screen for a command that has no `--role` at all. The daemon was running the checkout — the skill it loaded is the current one, and says `--role 0` — but the skill shells out to `qwen review agent-prompt …`, and `qwen` on that machine is `/usr/bin/qwen` → a v0.19.10 global install whose `agent-prompt` predates #6892 entirely. The skill and the CLI it was talking to were different programs. The skill assumed `qwen` on PATH is the build running it. That holds for a single install and breaks for exactly the people most likely to run a dev daemon. It is also invisible when it breaks: the error names an argument, not a version. So the entry is passed down instead of rediscovered. `scripts/cli-entry.js` is the executable entry and the one thing that knows its own path, so it publishes it as `QWEN_CODE_CLI` (`||=`, so the relaunch into dist/cli.js keeps pointing callers back at the wrapper with the shebang, not at itself). `daemon-dev.js` sets it too — the dev daemon is started as `node scripts/dev.js` and never passes through the wrapper, which is why this bit there first. `getShellContextEnvVars` passes it to every shell subprocess, beside the session and project-dir vars that are already handed down for the same reason. The skill's 23 command sites now read `"${QWEN_CODE_CLI:-qwen}" review …`; the fallback keeps hosts that do not export it on the old behaviour. PATH was the other candidate and was rejected: prepending a shim dir means writing an executable at spawn time and overriding `PATH` in an env that `normalizePathEnvForWindows` has already normalised — a `Path`/`PATH` collision on Windows in exchange for saving one variable. The env var is isolated in `shellContextEnv.test.ts` the way the session id already is: the CLI now exports it to every shell it spawns, so `npm test` run from inside a qwen session inherits it, and the exact-equality assertion would have failed on a variable the test never set. Verified by running the suite with it set.
wenshao
changed the title
…not dist
Verifying the previous commit on a real `npm run dev:daemon` caught it doing a
smaller version of the bug it fixes. The daemon runs the TypeScript **source**
through tsx; `cli-entry.js` runs `dist/cli.js`. Pointing QWEN_CODE_CLI there traded
"the subprocess is a whole major version behind" for "the subprocess is however
stale the last build was" — measured on the box that reported this, dist was **105
source files** behind the daemon. Same bug, smaller hat.
`scripts/dev.js` is the entry that runs what the daemon itself runs, so the dev
daemon points there. It gains a shebang and the exec bit, which is what lets a
caller invoke it as `"${QWEN_CODE_CLI}" review …` without knowing it needs node —
the same shape `cli-entry.js` already has for the published path.
Verified end to end on a headless box: started the dev daemon, read
/proc/<pid>/environ (QWEN_CODE_CLI=<repo>/scripts/dev.js, -rwxr-xr-x), and ran the
command that started this whole thread. Before: `Missing required argument: chunk`.
After: `agent-prompt: --role 0 needs a plan with prNumber and ownerRepo` — the role
is understood, and the complaint is about the fixture, which is the correct answer.
wenshao
commented
Jul 16, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
… can act on it A role with no recorded prompt proves one thing: the brief never reached an agent. The roster check claimed more than that — "no prompt was built for it (`agent-prompt --role 0` never ran)" — and on #7012 it said that about all twelve dimensions of a review that had just posted two Criticals with line numbers. The agents were in the same comment the gate was calling empty. Both failures are real and neither is the other. An orchestrator that writes the launch by hand gets an agent that runs, reads the diff and finds things, having never seen the severity bar, the finding format or this project's rules — all of which live in the brief it was never given. That is worth blocking on. It is not "nobody looked", and a check may not report the reading it cannot see. Three changes, one shape: - The per-role text says the brief never reached an agent, and that the dimension was reviewed "if at all" from a prompt the run wrote for itself. It no longer speaks for the agent's existence. - Every role briefless collapses to one line. It is one failure — the run did not use the prompt builder — and saying it twelve times buries the fact that explains all twelve. - The public body drops the internal command. `agent-prompt --role 2` is not something a PR author can run; on #7012 fourteen lines of it were the whole CHANGES_REQUESTED while the findings sat inline below the fold. The call survives in check-coverage's stderr, where the orchestrator reads it, and the role number is already in each label. check-coverage no longer leads with a count: the collapsed line covers the whole roster, so "1 required brief" would undercount it by the size of the review. Behaviour is unchanged — the gate fires on exactly the same runs and still caps the verdict. Only the sentence changes, and only where it was overclaiming or talking to the wrong reader.
"The prompt builder never ran" and "the prompt builder ran against a different --plan" arrive at this check as the same thing — an absent file — and they are fixed differently. Nothing in the error told them apart. The record directory hangs off the plan path as given, so a relative --plan resolves against the caller's cwd, and the skill runs Steps 2-6 from inside the worktree it just created. Two cwds, one relative path, two directories. Proven locally: the same `--plan .qwen/tmp/p.json` from a repo root and from a worktree under it yields two record dirs. That is not a reason to resolve the path differently — resolving a relative path against the cwd is what a relative path means, and the mismatch mostly fails loudly, because the plan is not in the worktree either and the read errors. It is a reason to print where it looked. One line, on stderr, where the orchestrator reads it; the PR author gets no path to a temp directory.
wenshao
changed the title
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
previously approved these changes
Jul 16, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…decays per call The launch prompts are already small — a role line, the brief pointer, the diff reads — and it did not save the run that stopped building them. Dogfooded on one PR, the same environment went from a clean review to "no prompt was built for any of twelve roles" over three reviews in a day. The per-agent form asks the orchestrator for ~30 build-then-launch round trips on a large review, and that is a compliance cost paid per agent, per review, forever; what decays under repetition eventually decayed. `agent-prompt --roster` builds every prompt the plan requires — chunk agents, dimension agents, invariants — in one call: one labelled block per agent, each recorded under the key `check-coverage` will look it up by. The list is `requiredAgents(plan)`, the same list the coverage gate reads, so what gets built is exactly what gets checked; a key the two derive differently is refused at build time rather than surfacing later as "brief never reached an agent" on a compliant run. The blocks are separated by lines that are visibly not prompt text, and a block copied lazily — separator included — still passes the add-only delivery check. That is load-bearing: if honest-but-sloppy copying read as a rewrite, the gate would punish exactly the behaviour this call exists to buy. The per-agent forms stay, for rebuilding a single prompt after Step 3D names a gap. Step 4/5 verify and reverse-audit are untouched: they are built per round, with the findings folded in. SKILL.md's Step 3A and 3B now ask for the roster once instead of one call per agent, and check-coverage's missing-brief error names the one-call fix first.
wenshao
changed the title
Collaborator
Author
Code review — PR #7033OverviewSix coordinated changes to What I verified (isolated worktree at
|
…ing story
Review feedback on this PR found five places where the fix stopped short of its
own thesis. All five, addressed:
1. Four copyable SKILL.md commands had missed the QWEN_CODE_CLI sweep —
`pr-context` (lightweight mode), `cleanup` (cache hit), `capture-local
--file` (file-path reviews), `agent-prompt --whole-diff` (Agent 8). On a
skewed host those modes died exactly the way the motivating run did. All
four now carry the prefix; the remaining bare mentions are prose.
2. check-coverage's own stderr recommended recovery with a bare `qwen` — the
message is the interface the orchestrator acts on, and on a skewed host the
recommended recovery reproduced the skew. All four recommendation sites now
print the prefixed form, and the rebuild hint covers `--chunk <id>`, which a
missing chunk agent needs and `--role` cannot express.
3. Ambient inheritance could silently re-point an entry at another session's
CLI. A dev daemon started from inside another qwen session's shell — the
usual dogfooding flow — inherited that session's QWEN_CODE_CLI through
`??`/`||=` and called the OUTER build: the same skew, one level up, and
silent. Every entry now stamps itself unconditionally; nested sessions each
call their own build. The `||=` comment in cli-entry.js also claimed a
relaunch hazard that does not exist (the relaunch child runs dist/cli.js and
never re-executes the wrapper) — the comment now states the real reason.
4. The third dogfooding entry point was still unpinned: `npm run dev` and
`npm start` published nothing, so a /review from a plain dev TUI fell back
to PATH. `scripts/dev.js` now stamps the variable in the env it spawns with
— which also covers the daemon, since daemon-dev launches serve through it,
and the daemon's own deferring copy is gone (one writer, not two).
`scripts/start.js` does the same and gains the shebang and exec bit that
make it callable as the entry it now names.
5. `"${VAR:-fallback}"` is POSIX parameter expansion, which cmd.exe passes
through literally and PowerShell rejects. The skill was already POSIX-bound
(Step 0 pipes through `tee`); the requirement is now total, and SKILL.md
says so where the variable is introduced: on Windows, run the review from
git-bash.
The unconditional stamp is pinned by a test that inherits a foreign
QWEN_CODE_CLI and asserts the spawned child gets this checkout's dev.js;
flipping the assignment back to `??` turns exactly that test red.
Collaborator
Author
Re-review at
|
…t rarer than a space Round-22 review: the bare '…' wrap from the previous commit closed at an embedded apostrophe, so ~/Documents/John's Projects broke where it had worked unquoted — one breakage class traded for another instead of both closed. A shared shellQuotePath (the same '\'' dance as utils/standalone-update.ts) now serves all six repair-printing sites, and a test drives verificationGaps from a plan under an apostrophe directory, asserting the escaped form and rejecting the naive wrap.
Collaborator
Author
|
Fixed in the commit just pushed — you're right that the bare wrap traded one breakage class for another rather than closing both. A shared |
Collaborator
Author
Re-review at
|
qwen-code-ci-bot
previously approved these changes
Jul 17, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
wenshao
commented
Jul 17, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 5 — the agent made no tool call: it read nothing. Not reviewed: chunk 5 — never opened its brief (/Users/wenshao/git/qwen-code-x7/.qwen/tmp/qwen-review-pr-7033-fetch-prompts/chunk-5.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: verification — the review posts findings, but no verifier ran (Step 4 builds its prompt with agent-prompt --role verify; none was recorded, so the findings were not verified).
— qwen-latest-series-invite-beta-v92 via Qwen Code /review
… the half-identity Round-24/25 reviews, four small items. selectorOf now shell-quotes the --file path — the same copy-paste contract the --plan quoting just earned, on the one selector that carries a path. RULES_MARKER moves above writeBrief's JSDoc, which it had been silently stealing. The check-build-status test's reject guard was dead (execFile always delivers string stdout, so an ENOENT resolved and the empty-stdout assertion passed on a script that never ran) — it now rejects on spawn-level errors, which carry string codes, while non-zero exits still resolve. And the roster's ownerRepo guard gets the independent test it never had: a plan with prNumber but no ownerRepo requires no Agent 0, since the brief builder cannot serve half an identity.
Collaborator
Author
Re-review at
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
doudouOUC
approved these changes
Jul 18, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Re-verified rounds 21–25 against the head (90cc220d9b) — all substantive changes hold:
shell-quote.ts:shellQuotePathis the canonical POSIX'\''quoting (identical toshellQuoteForSh), applied uniformly across the repair-printing sites — check-coverage, coverage (--plan×2 andselectorOf's--file), and compose-review'splanRef. The round-22 apostrophe regression is closed and pinned (John's Projects).- check-build-status test: the previously-dead reject guard (
typeof stdout !== 'string', which execFile never satisfied) now rejects on spawn-level errors via the stringerr.code, while non-zero exits (numeric codes) still resolve — the ENOENT-resolves-green hole is closed. RULES_MARKERmove offwriteBrief's JSDoc and the explicitPlanDiffResultfield declaration are both correct; the new tests for the executable shebang-less filter and the apostrophe plan path are present.
No unresolved threads, ubuntu unit suite green. Approving.
One non-blocking follow-up (deferred, recorded here per the review-round policy): the quoting sweep did not reach the two command blocks printed into Agent 7's brief — build-test and test-efficacy (agent-prompt.ts:829-831 and :850-853) still interpolate --plan / --worktree / --out unquoted. It is pre-existing (main has the same at 791-794), so not a regression from this branch, but it is the same space/apostrophe breakage the sweep closed everywhere else, and the fix is a one-liner with the shellQuotePath helper this PR just added. Reasonable to handle in a follow-up rather than widen this PR further at round 25.
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
reviewed
Jul 18, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Summary
This is a substantial hardening PR touching the review pipeline's core trust model. Five interlocking changes address real dogfooded failures:
--roster— builds all required prompts in one call, eliminating per-agent round trips that measurably decay compliance- Digest-keyed findings records — the record IS the printed prompt (findings included), keyed by SHA-256 digest, so a launch that drops findings matches no record
QWEN_CODE_CLIpropagation — every entry point stamps its own path; subprocesses call THIS build, not whatever PATH resolves- Remediation channel — author-facing body vs operator-facing stderr, preventing internal commands from leaking into public PR reviews (#7012's 14-line public body of plumbing)
- Bipartite matching — maximum matching (Kuhn's) for transcript-to-roster assignment, preventing greedy claim order from blocking compliant repairs
What's good
- The digest-keyed records close a real attack surface: the old findings-free record was a receipt a partial delivery could satisfy. Now each shard gets its own record, cross-delivery fails, and a launch without findings matches nothing.
inertPath()sanitization handles PR-controlled filenames that could inject forged roster boundaries or close Markdown code spans — well-tested with adversarial inputs.- The
writeBriefrules-downgrade guard catches a subtle silent failure: rebuilding without--ruleswould leave delivery checks passing while project rules vanish. - Signal handling fix in
dev.js/start.js(code ?? 0→ re-raise signal) correctly prevents OOM-killed children from reporting success. - Test coverage is thorough — many tests pin specific dogfooded regressions with detailed rationale.
Suggestions
-
Duplicate
shellQuotePath(lib/shell-quote.ts): The comment acknowledgesshellQuoteForShinutils/standalone-update.tsdoes the same'\''dance. Two implementations will drift. Consider extracting to a shared utility. -
isUnusableScriptEntrycache never invalidates (shellContextEnv.ts): Module-levelMap<string, boolean>with no TTL or invalidation. In a long-running daemon, if a CLI entry is replaced (standalone update), cached usability persists. Low risk given typical process lifetimes, but worth noting for the standalone update path. -
Bipartite matching inlined in
coverageFromTranscripts(coverage.ts): ~60 lines of Kuhn's algorithm inlined into an already ~400-line function. Extracting to a named helper (maximumMatching(candidates, opened)) would improve readability.
Verdict
No blockers found. Well-structured hardening that addresses measured failures. The suggestions above are quality-of-life, not merge blockers.
— Qwen Code review
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six changes to
/review, all of the same shape: the run is allowed to say what its records prove, and no more — to the reader who can act on it.Each was found by dogfooding the skill on a real PR and reading what it posted.
1. A rewritten launch is named as itself
compose-reviewsaid an auditor "never ran" when the auditor had run and its launch had been written by hand. The orchestrator read that, concluded "transcript visibility issue", and overrode the CLI's verdict — the CLI saidComment, the model reportedApprove. A wrong diagnosis is not a smaller version of the right one; it is an invitation to argue.Step 4/5 delivery is now four shapes with four different fixes:
not-built,not-launched,rewritten,brief-unread.2.
--findingsis required where the prompt folds findings inThe one thing left to hand-assemble was the thing that got paraphrased. Roles whose brief declares
acceptsFindingsnow refuse to build without--findings <file>; an early round with nothing confirmed passes an empty file.3. The CLI that is running is the CLI that gets called
A
/reviewlaunched from a dev daemon issuedqwen review agent-prompt --role 0,PATHfound a v0.19.10 global whoseagent-promptpredates--role, and the run died onMissing required argument: chunk. The skill and the CLI running it were different programs.QWEN_CODE_CLIis passed to every shell subprocess and preferred over a bareqwenat every call site — the fenced blocks, the four inline commands review feedback caught (pr-context,cleanup,capture-local --file,--whole-diff), andcheck-coverage's own recovery hints, which are the interface the orchestrator acts on. Every entry — the bin wrapper,scripts/dev.js,scripts/start.js— stamps the variable unconditionally: an inherited value is another session's CLI, and honouring it (the first cut's??/||=) re-created the skew one level up when a dev daemon was started from inside another qwen session's shell. Nested sessions each call their own build, and a test pins it. Dev entries point at source, notdist/— measured,dist/was 105 source files behind, which is the same bug in a smaller hat.${…:-…}is POSIX expansion, so SKILL.md now states the total POSIX-shell requirement (git-bash on Windows).4. A missing record proves the brief never arrived — not that nobody looked
The roster check claimed more than it could see:
On #7012 it said that about all twelve dimensions of a review that had just posted two Criticals with line numbers. The agents were in the same comment the gate was calling empty.
Both failures are real and neither is the other. An orchestrator that writes the launch by hand gets an agent that runs, reads the diff and finds things — having never seen the severity bar, the finding format or this project's rules, all of which live in the brief it was never given. That is worth blocking on. It is not "nobody looked", and a check may not report the reading it cannot see.
Three changes, one shape:
agent-prompt --role 2is not something a PR author can run; on fix(web-shell): batch transcript dispatch to avoid tab-return freeze #7012 fourteen lines of it were the entireCHANGES_REQUESTEDwhile the findings sat inline, below the fold. The call survives incheck-coverage's stderr, where the orchestrator reads it — the role number is already in each label.Before, and after:
check-coverageno longer leads with a count: the collapsed line covers the whole roster, so "1 required brief" would undercount it by the size of the review.Behaviour is unchanged — the gate fires on exactly the same runs and still caps the verdict. Only the sentence changes, and only where it was overclaiming or talking to the wrong reader.
5. The directory the briefs were missing from
"The builder never ran" and "the builder ran against a different
--plan" arrive as the same thing — an absent file — and are fixed differently.The record dir hangs off the plan path as given, so a relative
--planresolves against the caller's cwd, and the skill runs Steps 2-6 from inside the worktree it just created. Two cwds, one relative path, two directories. Proven locally.That is not a reason to resolve the path differently — resolving a relative path against the cwd is what a relative path means, and the mismatch mostly fails loudly anyway. It is a reason to print where it looked. One line, on stderr; the PR author gets no path to a temp directory.
6. The whole roster, one call
The launch prompts being small did not save the run that stopped building them. The per-agent form asks for ~30 build-then-launch round trips on a large review, and that cost is paid per agent, per review, forever — what decays under repetition eventually decayed (see the timeline in §4).
agent-prompt --rosterbuilds every prompt the plan requires — chunk agents, dimension agents, invariants — in one call: one labelled block per agent, each recorded under the keycheck-coveragelooks it up by. The list isrequiredAgents(plan), the same list the coverage gate reads, so what gets built is exactly what gets checked — and a key the two files would derive differently is refused at build time instead of surfacing later as "brief never reached an agent" on a compliant run.A block copied lazily — separator line included — still passes the add-only delivery check, and there is a test that keeps it that way: if honest-but-sloppy copying ever read as a rewrite, the gate would punish exactly the behaviour this call exists to buy. The per-agent forms stay for single relaunches after Step 3D; Step 4/5 verify and reverse-audit are untouched (built per round, findings folded in). SKILL.md's Steps 3A/3B now ask for the roster once.
Verification
shellContextEnvtests, 511 script tests.tscandeslintclean on every touched file.Looked for them in:line, and pointing it at a wrong dir each turn exactly one test red. The roster call was mutation-tested too: silently dropping the invariant agents, truncating a printed block while keeping its record, and drifting the key derivation each turn at least one test red.QWEN_CODE_CLIchain checked end-to-end on a livedev:daemonvia/proc/<pid>/environ.中文说明
六个修改,同一个形状:这次 review 只能说它的记录能证明的事,而且要说给能据此行动的人听。 每一个都来自在真实 PR 上 dogfood 这个 skill、再去读它发出来的东西。
compose-review说 auditor "never ran",而它其实跑了、只是 launch 是手写的。模型读到这句,判定「transcript visibility issue」,推翻了 CLI 的结论(CLI 说 Comment,模型报了 Approve)。错误的诊断不是正确诊断的弱化版,而是在邀请对方跟你争。现在拆成四态四修法。--findings强制。 唯一还需要手工拼装的东西,正是被改写的那个。/review调qwen,PATH 找到 v0.19.10 全局版,死在Missing required argument: chunk——skill 和跑它的 CLI 是两个程序。指向源码而非dist/(实测 dist 落后 105 个文件)。评审反馈后收尾:所有入口(cli-entry.js/dev.js/start.js)无条件盖章——继承值是外层会话的 CLI,尊重它就是同一偏差上移一层;SKILL.md 四处漏网的内联命令与 check-coverage 的恢复提示全部加前缀;POSIX shell 要求(Windows 用 git-bash)已注明。--plan上」到达这里长得一模一样。这不是改resolve()语义的理由(相对路径按 cwd 解析就是它的语义),而是打印一行路径的理由。--roster一次构建 plan 要求的全部 prompt,名单与check-coverage读的是同一份requiredAgents(plan)——建什么与查什么是同一个东西;偷懒连分隔行一起复制也照样通过 add-only 校验,并有测试钉住这一点。