mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-21 22:55:16 +00:00
b286875e72
* fix(review): harden the pipeline against four live-run failures
Measured on three parallel PR reviews (qwen3.8-max, 2026-08-13, PRs
#9013/#9014/#9045) run via `qwen review run`:
- run.ts: pin the composed-verdict and report scans to the run's own
target. The generic newest-composed scan captured a concurrent run's
artifact — two of the three runs republished a neighbour PR's verdict
(one reported REQUEST_CHANGES for a review whose own report said
Comment). Also keep re-reading while the child runs: a coverage
re-check legitimately recomposed a verdict 12 minutes after the first
write, and the first-snapshot capture would republish the superseded
one.
- budget.ts: drop placeholder gaps whose completion word carries a
trailing budget adverbial. Three "none — all checks … completed
within budget" non-answers reached two posted bodies because the
completion idiom required the completion word to end the text.
- coverage.ts: label a non-chunk agent by the brief codename found
anywhere in its launch prompt. Launchers prepend context lines, so the
first-line label gave twelve finders one shared PR-summary sentence,
and every budget-gap disclosure rendered as the same truncated PR
quote instead of a name.
- copy_bundle_assets.js: emit dist/cli.js with a shebang and the execute
bit. shellContextEnv blanks a QWEN_CODE_CLI a POSIX shell cannot exec,
so every review subcommand issued from a session launched off the
bundle silently fell back to the PATH's global install — all three
runs executed the machine's auto-updated release instead of the tree
they were launched from.
* test(review): follow the codename label into compose-review's fixtures
The backtick-collapse fixture's first line was itself the brief codename
shape, so the new codename extraction labels it `agent security` and the
first-line assertion no longer holds. Keep the sanitization intent on a
prose-only first line, and pin the codename behaviour — a prepended
context line must not reintroduce the shared-PR-quote label — as its own
case.
* fix(review): classify the run target with the child's own parser
Review feedback on the target pin: prNumberFromTarget re-derived PR
classification with a narrower regex than parse-args — /pull/<n>/files
URLs went unpinned, 0042 pinned pr-0042- while the child writes pr-42-,
and docs/pull/42 pinned a file target as a PR — so a completed (and
posted) review could be reported as one that produced no verdict.
Delegate to parseReviewArgs, whose verdict is what the child names its
artifacts from, and pin the divergent shapes as tests.
Also gate the bundle's shebang/exec-bit block with a package-assets
case (mode asserted off-win32, double-run must not stack shebangs), and
document the accepted same-target residual race on composedPatternFor.
* fix(review): pin run artifacts by exact target identity, share the identity-line parser
Round-2 review feedback, all six findings:
- run.ts: replace name-shape pins with the exact composed filename each
target class produces (pr-<n> / file basename / the fixed 'local' token,
per the skill's --out template). The (?!pr-\d+-) lookahead rejected a
file run's own artifact whenever the reviewed file was named
pr-<digits>-…, the PR branch's .* wildcard claimed that same artifact,
and the pooled null class let concurrent file and no-target runs
cross-capture each other's verdicts. Target classification now comes
from classifyRunTarget (parse-args' verdict, basename for files).
- run.ts: newestArtifactSince returns {path, mtime}, so the capture poll
reuses the scan's own stat instead of re-statting the path — the
scan-vs-sweep window (and its untestable catch branch) is gone
structurally.
- lib/agent-identity.ts: one parser for the identity line agent-prompt
bakes into every launch, shared by cost-ledger's row labels and
coverage's disclosure labels — the two hand-rolled copies could drift,
and coverage's copy dropped the (round N) and owned-file suffixes,
folding reverse-audit rounds into indistinguishable disclosure lines.
cost-ledger still feeds it only the first line (quoted identity lines
below must never be credited); coverage scans for the first
line-anchored identity line (launchers prepend context lines).
- lib/budget.ts: one vocabulary for the budget-idiom family — 'below'
joins the completion tail's position words, and the stayed idiom takes
the same qualifiers ('stayed inside the tool-call budget').
- run.test.ts: handler-level assertion that the report scan is pinned
(a strictly newer neighbour report must not become this run's
reportPath), alongside the pattern-level cases for every shape the
review probed.
* fix(review): round-3 review polish — CRLF identity lines, producer-side no-gap rule, edge-case pins
- agent-identity: tolerate a trailing CR (CRLF-recorded prompts fed
through \n-splitting callers failed every parse and fell back to
first-line prose); scan the launch prompt with one multiline match
instead of materializing a line array per agent record; pin the
round-over-file precedence with a both-suffixes test.
- agent-prompt: state the no-gap rule at the producer — write NO
'Budget gap:' line when nothing was cut short — instead of leaving
each agent to improvise a 'none' phrasing the consumer-side
placeholder filter must chase forever.
- run.ts: strip trailing path separators before taking a file target's
basename (a tab-completed 'src/' pinned 'qwen-review--composed.json',
which no child artifact carries — fail-closed exit 1 on a completed
review); pin file-run reports by their filename slot so a file named
'pr-1234.md' claims its own report; document the two collision
classes the basename-keyed pin defines (same-basename files,
basenames spelling 'local'/'pr-<n>').
- budget.test: pin the parenthesis-form exception keep case beside the
dash form.
* fix(review): round-4 polish — one budget vocabulary, named pin expectation, honest prompt claim
- budget.ts: spell the budget-position vocabulary once (BUDGET_QUALIFIED /
COMPLETION_TAIL) and build PLACEHOLDER_GAP_RE from it — the literal
carried three hand-copies that had already drifted twice in two review
rounds; the space-separated 'tool call' form is pinned in both branches.
- agent-prompt.ts: the no-gap rule now states what actually happens to a
'none' disclosure — at best filtered, and any unrecognized wording is
published as a phantom coverage gap — instead of claiming the parser
treats it as a gap, which was the negation of the filter shipped beside
it.
- run.ts: derive the composed pin from composedNameFor and name the
expected filename in both the no-verdict prose and the JSON result
(expectedComposedName) — a naming drift between the pin and the skill's
template was undiagnosable once Step 9 swept the near-miss.
- compose-review.ts: publicAgentSubject's provenance note now describes
the codename labels coverage prefers, with first-line prose as the
fallback.
* test(review): guard the no-verdict diagnostic and the pinned capture; align the chunk-role grammar
- run.test.ts: assert the no-verdict report names the artifact it waited
for, in prose and as expectedComposedName — mutation-verified: dropping
the suffix now fails.
- run.test.ts: force the neighbour's composed artifact strictly NEWER in
the concurrent-run fixture. With it older, an unpinned newest-composed
scan landed on the right file anyway and the regression passed;
mutation-verified: reverting composedPatternFor to the generic scan now
fails the handler test, not only the pattern units.
- agent-identity.ts: CHUNK_ROLE_RE takes coverage's CHUNK_RE shape
(whitespace-tolerant, case-insensitive) so a hand-edited 'Chunk 3 of 7'
cannot resolve as a chunk owner in the posted body and a role agent in
the ledger row.
* fix(review): keep the bundle's write time across the shebang rewrite; hold the pins to the skill
Round-6 review feedback, all five findings:
- copy_bundle_assets.js: preserve dist/cli.js's atime/mtime across the
shebang rewrite. stampReviewSourceDigest reads that mtime as the build
time, so a bumped one certifies a bundle as newer than review sources
edited before it and the staleness warning the skill's Step 0 stops on
never fires. A full bundle stamps before reaching here, but a
standalone run of this script — a flow the gate's own comment
contemplates — was exposed.
- package-assets.test.js: pin both halves the block owes. The mtime is
asserted against a fixture built 60s in the past, and the second run
now arrives at mode 0644 so the exec bit must be re-set — demoting the
chmod inside the shebang guard previously stayed green.
- run-skill-parity.test.ts: new. composedNameFor and reportPatternFor
encode the bundled skill's Step 6 --out template and Step 8 report
stems, and were pinned only against self-referential literals. This
reads the templates out of SKILL.md and renders them per target class,
so a skill-side edit fails next to the code that must follow it
instead of silently in a later review.
- cost-ledger.test.ts: pin the first-line-only invariant — a launch
whose prepended context sits above the identity line keeps the
transcript's own id, never a label lifted from below. Consolidating
both callers on labelFromLaunchPrompt now fails.
- agent-identity.test.ts: assert the two entry points genuinely differ
on that prompt, so neither caller's policy can be collapsed into the
other unnoticed.
* fix(review): stop the mtime assertion from pinning libuv's timespec truncation
The assertion compared the recorded mtime against the Date handed to
utimesSync, so it also pinned libuv's double-seconds → timespec
conversion: about half of all millisecond values read back 1 ns low
(X - 0.001), and builtAt is a fresh Date.now() - 60_000 every run — a
~50% coin flip that would have landed intermittent reds on unrelated
PRs through test:ci.
Capture what the filesystem actually recorded after the setup and
compare against that; the invariant under test is only whether the
shebang rewrite moves the stored time. 10/10 green through the CI entry
point, and it keeps its teeth: removing the production
fs.utimesSync(cliEntry, atime, mtime) restore fails it 3/3.
|
||
|---|---|---|
| .. | ||
| installation | ||
| lib | ||
| tests | ||
| acp-http-smoke.mjs | ||
| audit-runtime-critical.js | ||
| benchmark-api-latency.mjs | ||
| build-hosted-installation-assets.js | ||
| build-standalone-release.js | ||
| build.js | ||
| build_package.js | ||
| build_sandbox.js | ||
| build_vscode_companion.js | ||
| check-build-status.js | ||
| check-desktop-isolation.js | ||
| check-i18n.ts | ||
| check-lockfile.js | ||
| check-serve-fast-path-bundle.js | ||
| check-voice-guard-sync.js | ||
| clean-package-build-artifacts.js | ||
| clean.js | ||
| cli-entry.js | ||
| copy_bundle_assets.js | ||
| copy_files.js | ||
| create-standalone-package.js | ||
| create_alias.sh | ||
| daemon-dev.js | ||
| desktop-openwork-sync.ts | ||
| dev.js | ||
| esbuild-shims.js | ||
| generate-changelog.js | ||
| generate-git-commit-info.js | ||
| generate-release-notes.js | ||
| generate-settings-schema.ts | ||
| get-release-version.js | ||
| lint.js | ||
| local_telemetry.js | ||
| measure-flicker.mjs | ||
| pre-commit.js | ||
| prepare-package.js | ||
| prepare.js | ||
| release-script-utils.js | ||
| review-audit-layers.mts | ||
| run-java-daemon-sdk-e2e.ts | ||
| sandbox_command.js | ||
| sdk-node-exporter-stub.js | ||
| sign-release.sh | ||
| start.js | ||
| sync-computer-use-schemas.ts | ||
| telemetry.js | ||
| telemetry_gcp.js | ||
| telemetry_utils.js | ||
| test-rewind-e2e.sh | ||
| test-windows-paths.js | ||
| unused-keys-only-in-locales.json | ||
| upload-aliyun-oss-assets.js | ||
| verify-capture.mjs | ||
| verify-installation-release.js | ||
| version.js | ||
| workspaces.js | ||