fix(review): confine every ledger and evidence read to contained regular files

.github/workflows/qwen-code-pr-review.yml

@@ -1326,6 +1326,7 @@ jobs:
           KIND=''
           run_review_once() {
             local attempt_timeout="$1"
+            local attempt_prompt="$2"
             OUTCOME='fatal'
             REASON=''
             KIND=''
@@ -1382,7 +1383,7 @@ jobs:
               --auth-type openai \
               --approval-mode yolo \
               "${MODEL_ARGS[@]}" \
-              --prompt "$PROMPT" \
+              --prompt "$attempt_prompt" \
               --output-format stream-json \
               | tee "$LOG_PATH"
             local ps=("${PIPESTATUS[@]}")
@@ -1478,16 +1479,16 @@ jobs:
 
           # Retry budget: all attempts SHARE QWEN_TIMEOUT, so two tries can never
           # exceed the single-review budget (nor the job timeout), and that
-          # shared budget is the only thing that needs to bound them. A retry
-          # re-runs the whole review from scratch rather than resuming the failed
-          # one, so on a large PR it spends minutes re-fetching, re-chunking and
-          # re-launching agents before the first finding exists — a short retry
-          # cap makes the retry die on the clock instead of clearing the
-          # transient it was meant to clear. Every attempt therefore gets the
-          # whole remaining budget. Retry only a `retryable` outcome, only once,
-          # and only when enough budget is left for the retry to plausibly
-          # finish; below that, report the transient failure so the next run
-          # starts over with a full budget.
+          # shared budget is the only thing that needs to bound them. The retry
+          # carries `--resume`: `fetch-pr` then reuses the dead attempt's
+          # worktree, plan and agent evidence when the PR head has not moved
+          # (and silently falls back to a fresh review when it has), so the
+          # retry spends its remaining budget on the work still owed instead of
+          # re-fetching, re-chunking and re-launching what already ran. Every
+          # attempt still gets the whole remaining budget. Retry only a
+          # `retryable` outcome, only once, and only when enough budget is left
+          # for the retry to plausibly finish; below that, report the transient
+          # failure so the next run starts over with a full budget.
           BUDGET_SECONDS=$(( QWEN_TIMEOUT * 60 ))
           RETRY_BACKOFF_SECONDS=60
           RETRY_MIN_SECONDS=600
@@ -1499,7 +1500,17 @@ jobs:
             if [ "$attempt_timeout" -lt 30 ]; then
               fail "${REASON:-Qwen review ran out of time budget before it could complete.}" 1 "$KIND"
             fi
-            run_review_once "$attempt_timeout"
+            # `--resume` is understood by the parser in this repository's own
+            # tree; the runner installs @qwen-code/qwen-code@latest, so between
+            # this landing and the next npm release the released parser will
+            # report `Unrecognized flag "--resume"; ignored.` and the retry runs
+            # from scratch — today's behaviour plus one warning line, and it
+            # self-heals on the first release that carries the flag.
+            ATTEMPT_PROMPT="$PROMPT"
+            if [ "$attempt" -gt 1 ]; then
+              ATTEMPT_PROMPT="$PROMPT --resume"
+            fi
+            run_review_once "$attempt_timeout" "$ATTEMPT_PROMPT"
             if [ "$OUTCOME" = "success" ]; then
               break
             fi

docs/users/features/code-review.md

@@ -18,6 +18,9 @@
 # Review local changes and apply the findings to your working tree
 /review --fix
 
+# Continue a review of the same PR that was interrupted, instead of starting over
+/review 123 --resume
+
 # Review a specific file
 /review src/utils/auth.ts
 
@@ -222,6 +225,18 @@ A finding is skipped when its fix would change intended behavior, would need cha
 
 **Every finding gets an outcome, and this is enforced rather than requested.** The ledger goes through `qwen review findings --outcomes`, which refuses a set that does not cover all of them — a fixer that applies six of nine findings and reports six has not lied about any one of them, it has silently shortened the list, and you would have no way to see the three that fell off.
 
+## Resuming an interrupted review (`--resume`)
+
+A long review that dies part-way — a dropped connection, a timeout, a killed terminal — leaves everything it had done on disk: the worktree, the captured diff, and the harness's own record of every agent that ran. `--resume` continues from there instead of starting over:
+
+```bash
+/review 123 --resume
+```
+
+It applies to **PR targets only** (a local review's diff comes from a live working tree, which has no stable interrupted state to continue), and it is safe to pass whenever you are unsure: the review rules on the on-disk state itself — the worktree still at the fetched commit and clean, the captured diff unchanged byte for byte, the PR head unmoved, the resume limit unspent — and silently starts fresh whenever anything no longer matches, telling you which check refused. A continuation reuses the earlier attempt's certified agent results, so the report says how many were recovered; it is disclosed, never a coverage gap.
+
+Two things to know. A continuation keeps the interrupted run's **effort**: passing a different `--effort` refuses the resume and runs fresh at the level you asked for, because different effort is different work. And if the PR head moved while the review was down, the resume refuses (`head-moved`) and the fresh run reviews the new commits — which is what you want, and it counts as this review's one restart.
+
 ## Findings as Data
 
 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.
@@ -371,7 +386,7 @@ Every run ends with one machine-readable line (`Review complete: <target> — <d
 `/review` is interactive. When a script or CI job needs to run a review and act on its outcome, use the headless wrapper:
 
 ```bash
-qwen review run [target] [--json] [--fail-on request-changes] [--comment] [--quiet]
+qwen review run [target] [--json] [--fail-on request-changes] [--comment] [--resume] [--quiet]
 ```
 
 `target` is a PR number, a PR URL, or a file path; omit it to review the local working tree. The command runs this build's own CLI non-interactively (with stdin closed, so slash-command detection survives), streams the child's progress to **stderr**, and prints the verdict to **stdout** — or, with `--json`, the full result object. The verdict is read from the artifact `compose-review` writes (the same JSON the skill treats as the verdict authority), never parsed from the model's prose.
@@ -386,6 +401,8 @@ The exit code is the contract a gate should read:
 
 `3` (not `2`) lets a gate distinguish "the review is blocking" from "the tool broke" — yargs already uses `1` for usage errors — without parsing any output. `--timeout-minutes` (default 120, floored at 1) terminates a hung review and exits `1`, and cancelling the command (Ctrl+C / SIGTERM) terminates the review's process group rather than orphaning it.
 
+`--resume` continues an interrupted review of the same PR instead of starting over — the exact retry shape a CI wrapper hits: the first attempt dies on a transient error, and the retry would otherwise spend its remaining shared budget re-fetching, re-chunking and re-launching agents whose work is already on disk. It is safe to pass unconditionally on a retry: `fetch-pr` rules on the on-disk state itself (worktree still at the fetched SHA and clean, diff bytes unchanged, PR head unmoved, resume cap unspent) and silently falls back to a fresh review whenever anything no longer matches, so the flag never fails a run that could start over. A continuation is pinned to the interrupted run's recorded effort — an explicitly different `--effort` refuses the resume and runs fresh at the requested level. PR targets only (a local review's diff is captured from a live working tree, which has no stable interrupted state to continue). The repository's own review workflow retries through the SLASH-COMMAND path, not this subcommand — it appends `--resume` to the `/review` prompt of its second attempt — so a change to `review run`'s flag mapping is exercised by this command's own tests, not by CI.
+
 A time-budgeted run can also export a **soft** deadline so the review stops its open-ended reverse-audit loop while there is still time to verify, compose and post: `QWEN_REVIEW_DEADLINE_EPOCH` is the Unix-seconds moment the run will be killed, and `QWEN_REVIEW_DEADLINE_RESERVE_SECONDS` (default 3600; `0` keeps only the round estimate) is the tail that must remain for the last round's verification, `compose-review` and submission. When the remaining budget no longer fits another round plus that tail, the round builder refuses to build it, and the composed verdict discloses the truncated audit (an otherwise-Approve verdict is capped at Comment). A missing or malformed deadline leaves the review ungated — the outer timeout still bounds the run.
 
 Nested inside that reserve is a smaller **compose floor**, `QWEN_REVIEW_DEADLINE_COMPOSE_FLOOR_SECONDS` (default 1200; `0` disables this gate entirely, at every point including past the deadline). The reserve is one number covering "verify the last round **plus** compose **plus** submit", which fits a normal per-finding re-trace but not a security review whose verification re-runs real filesystem/git workloads without bound. So the verifier — not the round builder — is gated on this floor: once the floor or less remains, `agent-prompt --role verify` refuses to build (a `VERIFY BUDGET:` line, exit **4**), the findings in hand keep their unverified tag (which caps the verdict), and `compose-review` and submission run. The floor is strictly below the reserve, so a healthy run hits the reverse-audit gate first and never reaches it; it is the cover for the one span the reserve cannot bound.

packages/cli/src/commands/review.test.ts

@@ -68,6 +68,7 @@ describe('reviewCommand', () => {
       'test-efficacy',
       'test-plan',
       'findings',
+      'recover-findings',
       'publish-assets',
       'compose-review',
       'save-artifact',

packages/cli/src/commands/review.ts

@@ -13,6 +13,7 @@ import { parseArgsCommand } from './review/parse-args.js';
 import { matchRemoteCommand } from './review/match-remote.js';
 import { composeReviewCommand } from './review/compose-review.js';
 import { findingsCommand } from './review/findings.js';
+import { recoverFindingsCommand } from './review/recover-findings.js';
 import { fetchPrCommand } from './review/fetch-pr.js';
 import { captureLocalCommand } from './review/capture-local.js';
 import { planDiffCommand } from './review/plan-diff.js';
@@ -79,14 +80,15 @@ export const reviewCommand: CommandModule = {
       .command(testEfficacyCommand)
       .command(testPlanCommand)
       .command(findingsCommand)
+      .command(recoverFindingsCommand)
       .command(publishAssetsCommand)
       .command(composeReviewCommand)
       .command(saveArtifactCommand)
       .command(submitCommand)
       .command(cleanupCommand)
       .demandCommand(
         1,
-        'Specify a subcommand: run, parse-args, match-remote, meta, issue-context, fetch-diff, comment-body, fetch-pr, capture-local, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
+        'Specify a subcommand: run, parse-args, match-remote, meta, issue-context, fetch-diff, comment-body, fetch-pr, capture-local, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, recover-findings, publish-assets, compose-review, save-artifact, submit, or cleanup.',
       )
       .version(false),
   handler: () => {

packages/cli/src/commands/review/cleanup.test.ts

@@ -42,6 +42,27 @@ vi.mock('node:child_process', async (importOriginal) => {
   };
 });
 
+vi.mock('./lib/contained-read.js', async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import('./lib/contained-read.js')>();
+  return {
+    ...actual,
+    // `readBudgetStopUnfenced` reads the marker through this now; the cleanup
+    // fixtures serve it via the readFileSync mock, so delegate.
+    readContainedFileOrNull: (path: string) => {
+      try {
+        return {
+          content: String(mocks.readFileSync(path)),
+          mtimeMs: 0,
+          size: 0,
+        };
+      } catch {
+        return null;
+      }
+    },
+  };
+});
+
 vi.mock('node:fs', async (importOriginal) => {
   const actual = await importOriginal<typeof import('node:fs')>();
   return {