chiga0
diff --git a/‎.github/workflows/release.yml‎
Lines changed: 7 additions & 0 deletions b/‎.github/workflows/release.yml‎
Lines changed: 7 additions & 0 deletions
diff --git a/‎docs/users/features/code-review.md‎
Lines changed: 4 additions & 0 deletions b/‎docs/users/features/code-review.md‎
Lines changed: 4 additions & 0 deletions
diff --git a/‎packages/cli/src/commands/review/drive.test.ts‎
Lines changed: 122 additions & 2 deletions b/‎packages/cli/src/commands/review/drive.test.ts‎
Lines changed: 122 additions & 2 deletions
diff --git a/‎packages/cli/src/commands/review/drive.ts‎
Lines changed: 16 additions & 3 deletions b/‎packages/cli/src/commands/review/drive.ts‎
Lines changed: 16 additions & 3 deletions
@@ -425,6 +425,13 @@ jobs:
         run: |-
           npm run build
           npm run bundle
+          # The review staleness check degrades to "could not check" without
+          # this stamp; fail here instead of shipping a release that silently
+          # lost it.
+          test -f dist/review-sources.sha256 || {
+            echo "::error::review source stamp missing — see the copy_bundle_assets warning above"
+            exit 1
+          }
           npm run prepare:package
 
       - name: 'Build Standalone Archives'
 
@@ -225,6 +225,10 @@ A finding is skipped when its fix would change intended behavior, would need cha
 
 Confirmed findings are canonicalized into `.qwen/tmp/qwen-review-<target>-findings.json` before anything else consumes them — the terminal report, the saved Markdown report, and the PR review JSON all read that one artifact instead of re-typing the list. Each finding carries a unique `id` (what outcomes and resolved anchors join on), `severity`, `confidence`, `source`, `summary`, a `shortSummary` capped at 60 characters for list rendering, `failureScenario`, and one or more `locations` — a pattern-aggregated finding keeps **one location per occurrence**, so each still gets its own inline comment.
 
+**Before anything else, the review checks that it is running your code.** Every `qwen review …` step runs the built bundle, not the working tree, so a review command edited since the last build takes no effect and the run measures the old behaviour. The build records a digest of the review sources it bundled; `parse-args` re-derives it and compares, and `drive` checks again, because the verifier brief sends agents straight there without a step 1. On a mismatch it says on stderr that the bundle was not built from these sources, and what to rebuild. The check runs when the CLI resolves to the bundled `dist/cli.js` (the `qwen` binary, or `node dist/cli.js`); launchers that run unbundled output, such as `npm start` and `npm run dev`, skip it. Two cases it cannot compare are treated differently: a checkout whose build predates the recording is told the check could not run and why, and an installed package — which has no sources to differ from — is left silent. The digest covers the review commands, the file that registers them, the review-only lease they import from outside their directory, and the bundled review skill; it does not follow those into the shared helpers they import, so a quiet run means the review code matches the bundle rather than that the whole tree does.
+
+**A Critical the base tree already failed is held back, not filed.** When a test command failed and the merge base could be built, `test-delta` records which failing files also fail without the pull request. Canonicalization reads that measurement back (`qwen review findings --test-delta`, beside `--outcomes`): a Critical whose own text names one of those files is lowered to a Suggestion, keeps its evidence, gains the measurement that demoted it and a `heldByMeasurement` field, and the demotion is announced. A test that was already red is not a test this pull request turns red — and if it now fails for a _new_ reason, say which test, quote both sides, and file it at Critical again: a finding that already carries the measurement and is raised anyway is left where you put it.
+
 The command validates on write: a duplicate id, a finding with no failure scenario, an empty locations array, or an unknown severity is an error rather than a silently mangled entry.
 
 ## Evidence Images in PR Comments
 
@@ -9,9 +9,16 @@
 // sessions: 81% waited with `sleep`, 74% captured one screenful with no way to
 // know the command had finished, 87% cleaned up by hand.
 
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { spawnSync, execFileSync } from 'node:child_process';
-import { mkdtempSync, readFileSync, writeFileSync, existsSync } from 'node:fs';
+import {
+  mkdtempSync,
+  mkdirSync,
+  readFileSync,
+  writeFileSync,
+  existsSync,
+  rmSync,
+} from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 import {
@@ -20,9 +27,28 @@ import {
   sentinelExitCode,
   trimCapture,
   shellQuote,
+  driveCommand,
   DRIVE_SENTINEL,
   type ExecResult,
 } from './drive.js';
+import {
+  writeStdoutLine,
+  writeStderrLineSafe,
+} from '../../utils/stdioHelpers.js';
+import { reviewSourceRoots, reviewSourcesDigest } from './lib/stale-bundle.js';
+import {
+  FOREIGN_DIGEST,
+  makeStaleBundleFixture,
+  stampDigest,
+} from './lib/test-utils.js';
+
+// The handler's output goes through the same helpers the parse-args suite
+// mocks; the wiring tests below intercept them so no real terminal is touched.
+vi.mock('../../utils/stdioHelpers.js', () => ({
+  writeStdoutLine: vi.fn(),
+  writeStderrLine: vi.fn(),
+  writeStderrLineSafe: vi.fn(),
+}));
 
 const ok = (stdout = ''): ExecResult => ({ status: 0, stdout, stderr: '' });
 const fail = (stderr = ''): ExecResult => ({ status: 1, stdout: '', stderr });
@@ -496,3 +522,97 @@ describe('a partial observation is never presented as a whole one', () => {
     expect(r.note).toContain('not evidence that the run produced nothing');
   });
 });
+
+describe('drive warns when the bundle is not built from these sources', () => {
+  // The wiring is what is under test: the notice derives from
+  // `process.argv[1]` and leaves through `writeStderrLineSafe` BEFORE
+  // `runDrive` runs — a missing seam there would ship while every `runDrive`
+  // test above stayed green. tmux is gated off through the exec seam, so the
+  // drive itself goes nowhere and nothing real is spawned.
+  let repo: string;
+  let argv1: string;
+
+  const exec = (cmd: string, args: string[]): ExecResult =>
+    cmd === 'tmux' && args[0] === '-V'
+      ? { status: 1, stdout: '', stderr: '' }
+      : { status: 0, stdout: '', stderr: '' };
+
+  // Real bindings by construction: this file never mocks `node:fs`.
+  const realFs = { mkdtempSync, mkdirSync, writeFileSync };
+
+  beforeEach(() => {
+    ({ repo, argv1 } = makeStaleBundleFixture(realFs, 'drive-stale-'));
+    vi.mocked(writeStderrLineSafe).mockClear();
+    vi.mocked(writeStdoutLine).mockClear();
+  });
+  afterEach(() => rmSync(repo, { recursive: true, force: true }));
+
+  const stamp = (digest: string) => stampDigest(realFs, repo, digest);
+  const run = () => {
+    const originalArgv = process.argv[1];
+    const originalExit = process.exitCode;
+    process.argv[1] = argv1;
+    try {
+      (driveCommand.handler as (a: unknown) => void)({
+        script: 'true',
+        cwd: repo,
+        readyTimeout: 1,
+        timeout: 1,
+        server: 'wiring',
+        exec,
+        _: ['review', 'drive'],
+      });
+    } finally {
+      process.argv[1] = originalArgv;
+      process.exitCode = originalExit;
+    }
+  };
+
+  it('warns when the stamp does not match the sources', () => {
+    stamp(FOREIGN_DIGEST);
+    run();
+    expect(vi.mocked(writeStderrLineSafe).mock.calls[0]?.[0]).toContain(
+      'NOT built from the review sources',
+    );
+    // …and BEFORE the first result: relocating the loop below `runDrive`
+    // keeps every substring assertion green while the warning lands only once
+    // the reviewer has already consumed results measured from the stale
+    // bundle — the failure mode this check exists to prevent.
+    expect(
+      vi.mocked(writeStderrLineSafe).mock.invocationCallOrder[0],
+    ).toBeLessThan(vi.mocked(writeStdoutLine).mock.invocationCallOrder[0]);
+  });
+
+  it('says nothing when the stamp matches', () => {
+    stamp(reviewSourcesDigest(repo, reviewSourceRoots(repo))!);
+    run();
+    expect(writeStderrLineSafe).not.toHaveBeenCalled();
+  });
+
+  it('says it could not check when sources exist but the stamp does not', () => {
+    // The brief unmeasured form: the state of every existing checkout the
+    // day this ships — sources on disk, no stamp beside the bundle yet.
+    run();
+    const line = vi.mocked(writeStderrLineSafe).mock.calls[0]?.[0] as string;
+    expect(line).toContain('could not check whether the bundle is current');
+    expect(line).toContain('Rebuild with `npm run bundle` to record one.');
+  });
+
+  it('prints the one-line form — the full paragraph belongs to parse-args', () => {
+    // One review can invoke `drive` many times, and each invocation prints
+    // into an agent's tool output; the repeat keeps the trigger and the
+    // remedy and drops the explanation.
+    stamp(FOREIGN_DIGEST);
+    run();
+    const line = vi.mocked(writeStderrLineSafe).mock.calls[0]?.[0] as string;
+    expect(line).toContain('NOT built from the review sources');
+    expect(line).toContain('npm run bundle');
+    expect(line).not.toContain('runs the BUILT bundle, not the working tree');
+  });
+
+  it('still drives — the notice is a diagnostic, not a gate', () => {
+    stamp(FOREIGN_DIGEST);
+    run();
+    expect(writeStdoutLine).toHaveBeenCalled();
+  });
+});
@@ -37,6 +37,7 @@
 // the whole story.
 
 import type { CommandModule } from 'yargs';
+import { bundleStalenessNotices } from './lib/stale-bundle.js';
 import { spawnSync } from 'node:child_process';
 import {
   mkdirSync,
@@ -48,7 +49,11 @@ import {
 } from 'node:fs';
 import { dirname, join, resolve } from 'node:path';
 import { tmpdir } from 'node:os';
-import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js';
+import {
+  writeStdoutLine,
+  writeStderrLine,
+  writeStderrLineSafe,
+} from '../../utils/stdioHelpers.js';
 
 /** Why a drive stopped. Every value is a fact about the run, not a verdict. */
 export type DriveOutcome =
@@ -468,9 +473,17 @@ export const driveCommand: CommandModule = {
         describe: 'Write the JSON report here',
       }),
   handler: (argv) => {
-    // Caught like `base-tree` and `test-plan`: the messages above are written
-    // for the caller, and a stack trace re-frames every one of them as a crash.
+    // Caught like `base-tree` and `test-plan`: the messages this handler
+    // writes are for the caller, and a stack trace re-frames every one of
+    // them as a crash.
     try {
+      // The verifier brief sends agents straight here, so this can run without
+      // `parse-args` ever running first — and it is where the long work
+      // starts, which makes a stale bundle costliest here. The one-line form:
+      // a fresh review already heard the full paragraph at `parse-args`, and
+      // a repeated paragraph becomes wallpaper.
+      const bundleNotice = bundleStalenessNotices(process.argv[1], true);
+      if (bundleNotice) writeStderrLineSafe(bundleNotice);
       const args = argv as unknown as DriveArgs & { readyTimeout: number };
       const report = runDrive(args);
       if (args.out) {