fix(tools): settle the review threads left open on merged PRs #553-#559 and #507

.claude/skills/babysit/SKILL.md

@@ -117,8 +117,9 @@ confirmation.)
    - Rounds 1 and 2 fix every valid finding, Medium and Low included, whatever the PR's size. No deferral, and every
      file a valid fix needs is in scope. A `no-change` or rejection needs a cited file:line that proves the finding
      wrong.
-   - Round 3 and later fix every valid Critical and High. A Medium or Low is fixed only when it is cheap (about 20
-     changed lines) or the same class as an earlier round's fix; every other one gets a reply (valid or not, and why
+   - Round 3 and later fix every valid Critical and High, and CodeRabbit's Major (its level above Minor; there is no
+     High in its scale). A Medium or Low is fixed only when it is cheap (about 20 changed lines) or the same class as
+     an earlier round's fix; every other one gets a reply (valid or not, and why
      it waits) and its thread stays OPEN: resolve only a thread that was acted on. Record such a deferral with the
      reply transport's `"verdict": "belongs-elsewhere"` and explain the policy in the reply; `integrate.ts` keeps it
      in the summary but posts it in a separate `post-replies.ts --no-resolve` batch, never the batch that resolves
@@ -314,10 +315,12 @@ add the correction to the affected items and to `alsoInPush`, set every item's `
 (fable-judge)"`, and render `round-summary.ts --range <old head>..<new head> --cwd <pr worktree>`. Post per review-loop
 step 4 only when `post` is true.
 
-## 7a. A PR short of three rounds: an independent review round
+## 7a. An independent review round (when the user asks for one)
 
-A round counts only when it fixed or answered real findings. When the reviewer bot has nothing left on a PR that
-still needs rounds, or a PR merged with fewer than three, run an independent read-only review: `bun
+A round counts only when it fixed or answered real findings. The round policy (step 3.5) sets no minimum number of
+rounds: after two rounds with no Critical or High, switch to merge prep (review-loop "Round policy"), and land by the
+merge bar. When the user asks for a dedicated review of a PR, before or after its merge, run an independent read-only
+review: `bun
 .claude/skills/babysit/scripts/review-brief.ts --run <dir> --pr <N> --wt <worktree> [--post-merge] [--swift]`, then
 `steer.ts` with the printed file to a worker that did NOT fix this PR (Sol; Astra only for a risky cross-system PR,
 by the model rule in step 1). Judge its findings like a bot's. Before a merge they become the next fix round (`fill-brief --judge <review
@@ -327,14 +330,14 @@ rounds" when GitHub showed two round summaries.
 
 ## 7b. Optional: land a PR in master (only with the user's explicit approval)
 
-Off unless the user approves it in their own words for this run ("drive the PRs into master after 3 rounds"), and
+Off unless the user approves it in their own words for this run ("drive the PRs into master"), and
 only for PRs the run owns: never another session's PR, never the integration branch. Ask once, early, and keep
 running review rounds while the question is open: a missing answer never blocks a round, it only means no merge.
 
-A PR lands when all hold: review-loop step 5a ("Before you call a PR ready") passes; at least 3 review rounds done;
-every finding required by the derived policy is fixed and judged (every valid security finding and all
+A PR lands when all hold: review-loop step 5a ("Before you call a PR ready") passes; the rounds followed the round policy
+(step 3.5), which sets no minimum count; every finding required by the derived policy is fixed and judged (every valid security finding and all
 dedicated-review findings included), so no critical or high finding is open; eve reviewed the head before the last
-fix (the merge bar, step 3.5); a PR over about 50 files has a CodeRabbit review; a reply on every thread; CI green
+fix (the merge bar, step 3.5); a PR over about 50 files has a CodeRabbit review; the thread replies of review-loop step 5a; CI green
 on the landing tree; its base is master or a parent the run merges first (a PR stacked on another session's branch
 waits for that branch). Merge without waiting for eve to review the last judged fix. Policy-deferred Medium/Low
 threads may remain OPEN. In an unattended land-it job a red CI, an open
@@ -373,8 +376,8 @@ the worker who knows the PR, as a merge-prep steer that authorizes the rebase. `
 forked from on its parent (a child stacked on a parent that kept growing forked before the parent's later commits;
 find it with `git merge-base <child> <old parent head>`). Check with `git range-diff`, run the area tests, give a
 child that changes plugin files a version above its new base, then push with `--force-with-lease`. A restack is new
-code: rerun CI and judge it before the merge; eve need not re-review the last judged fix. Keep at least one
-CodeRabbit review and a reply on every thread. When the restack carries a fix round too, push it with
+code: rerun CI and judge it before the merge; eve need not re-review the last judged fix. A PR over about 50
+files keeps its CodeRabbit review, and the thread replies of review-loop step 5a still hold. When the restack carries a fix round too, push it with
 `integrate.ts … --restacked-from <old head>`: the lease is that old head, and the gates check only the PR's own commits.
 
 ## 7c. Disk: watch it, reclaim /tmp yourself

.claude/skills/babysit/scripts/integrate.ts

@@ -23,6 +23,7 @@ import {
     symlinkSync,
     writeFileSync,
 } from "node:fs";
+import { tmpdir } from "node:os";
 import { isAbsolute, join, relative, resolve, sep } from "node:path";
 import { env } from "@genesiscz/utils/env";
 import { SafeJSON } from "@genesiscz/utils/json";
@@ -328,7 +329,10 @@ async function main(): Promise<void> {
         process.exit(0);
     }
 
-    mkdirSync(run.vault, { recursive: true });
+    // --dry is read-only for the run: its reply copy and summary preview go to a scratch folder, so a preview never
+    // replaces the vault files of a real round.
+    const outDir = dry ? mkdtempSync(join(tmpdir(), `babysit-integrate-dry-${pr}-r${round}-`)) : run.vault;
+    mkdirSync(outDir, { recursive: true });
     let text = readFileSync(repliesIn, "utf8");
 
     for (const [from, to] of Object.entries(shaMap)) {
@@ -373,18 +377,18 @@ async function main(): Promise<void> {
     // Named from the PR and round, never from the worker's file name: a reply file called `524-r2.json` made the summary
     // overwrite it, the posted marker point at the summary itself, and the round's summary and replies go unposted
     // (2026-10-15, #524 round 2).
-    const replies = join(run.vault, `${pr}-r${round}-replies.json`);
+    const replies = join(outDir, `${pr}-r${round}-replies.json`);
     writeFileSync(replies, `${SafeJSON.stringify(data, null, 2)}\n`);
-    const resolvingReplies = deferred.length ? join(run.vault, `${pr}-r${round}-resolvable.json`) : replies;
-    const deferredReplies = join(run.vault, `${pr}-r${round}-deferred.json`);
+    const resolvingReplies = deferred.length ? join(outDir, `${pr}-r${round}-resolvable.json`) : replies;
+    const deferredReplies = join(outDir, `${pr}-r${round}-deferred.json`);
     if (deferred.length) {
         writeFileSync(
             resolvingReplies,
             SafeJSON.stringify({ ...data, items: (data.items ?? []).filter((item) => !isDeferredReply(item)) }, null, 2)
         );
         writeFileSync(deferredReplies, SafeJSON.stringify({ ...data, items: deferred }, null, 2));
     }
-    const summary = join(run.vault, `${pr}-r${round}-summary.md`);
+    const summary = join(outDir, `${pr}-r${round}-summary.md`);
     const range = flag("--range") ?? (oldHead !== newHead ? `${gateFrom}..${newHead}` : undefined);
     must(
         [
@@ -398,11 +402,16 @@ async function main(): Promise<void> {
             summary,
         ],
         "round-summary",
-        run.vault
+        outDir
     );
 
+    if (dry) {
+        console.log(`dry: preview in ${outDir}, vault untouched, nothing posted: ${summary}`);
+        process.exit(0);
+    }
+
     // Check authorization immediately before the first public mutation, after saving the reviewable files.
-    if (has("--no-post") || dry || !(run.post === true || has("--post"))) {
+    if (has("--no-post") || !(run.post === true || has("--post"))) {
         console.log(`held (posting not authorized or disabled): ${summary}`);
         process.exit(0);
     }
@@ -489,15 +498,20 @@ async function main(): Promise<void> {
             `${reply.code === 0 ? "✔" : "⚠"} follow-up on ${item.ref ?? item.threadId} (was ${before}, now fixed)`
         );
     }
-    console.log(posted.code === 0 ? `✔ #${pr} round ${round} posted` : `⚠ #${pr} post-replies exit ${posted.code}`);
+    const exitCode = postingExitCode(posted.code, [...followups, deferredPosted]);
+    console.log(
+        exitCode === 0
+            ? `✔ #${pr} round ${round} posted`
+            : `⚠ #${pr} round ${round} posting incomplete: post-replies exit ${posted.code}, deferred exit ${deferredPosted.code}, ${followups.filter((f) => f.code !== 0).length} follow-up(s) failed`
+    );
     appendEvent(run.dir, {
         kind: "post",
         pr: Number(pr),
         round: Number(round),
         text: readFileSync(summary, "utf8"),
         meta: {
             replies,
-            exitCode: postingExitCode(posted.code, [...followups, deferredPosted]),
+            exitCode,
             postRepliesExitCode: posted.code,
             followups,
             postReplies: posted.out
@@ -507,7 +521,7 @@ async function main(): Promise<void> {
         },
     });
 
-    process.exitCode = postingExitCode(posted.code, [...followups, deferredPosted]);
+    process.exitCode = exitCode;
 }
 
 if (import.meta.main) {

.claude/skills/babysit/scripts/review-brief.ts

@@ -1,7 +1,7 @@
 #!/usr/bin/env bun
 /**
- * The brief for an independent read-only review round: when the reviewer bot has nothing left on a PR that still
- * needs rounds, or a PR merged with fewer than three. Its findings become the next fix round (`fill-brief --judge`)
+ * The brief for an independent read-only review round, which runs when the user asks for one (SKILL.md 7a): on an
+ * open PR, or on a PR that already merged. Its findings become the next fix round (`fill-brief --judge`)
  * or, after a merge, a follow-up PR. Writes `<run>/steer-review-<pr>.md` and prints the path for steer.ts.
  *
  *   bun review-brief.ts --run <dir> --pr 517 --wt <worktree> [--base master] [--post-merge] [--swift] [--fresh]
@@ -30,7 +30,7 @@ export interface ReviewBriefInput {
 
 export function reviewBrief({ pr, title, wt, base, scratch, postMerge, swift, fresh }: ReviewBriefInput): string {
     const why = postMerge
-        ? `PR #${pr} ("${title}") merged into master with fewer than three review rounds. This is an independent review of the MERGED code; your findings become a follow-up PR.`
+        ? `PR #${pr} ("${title}") already merged into master, and the user asked for one more review. This is an independent review of the MERGED code; your findings become a follow-up PR.`
         : fresh
           ? `PR #${pr} ("${title}") was just opened, and the reviewer bots read it at the same time as you. The lead wants an independent review next to theirs; your findings join the bots' threads in the first fix round.`
           : `The reviewer bot found nothing more to fix on PR #${pr} ("${title}"), so the lead wants an independent review. Your findings become the next fix round.`;

.claude/skills/babysit/scripts/scripts.test.ts

@@ -1509,7 +1509,7 @@ test("generated worker briefs follow the round and dedicated-review policy", ()
         );
         expect(result.exitCode).toBe(0);
         const brief = fs.readFileSync(target, "utf8");
-        expect(brief).toContain(all ? "fix EVERY valid finding" : "fix Critical and High");
+        expect(brief).toContain(all ? "fix EVERY valid finding" : "fix Critical, Major and High");
         expect(brief).toContain("A valid security finding is always fixed");
         expect(brief).toContain("Change every file the fix needs in this PR");
         if (!all) {
@@ -4565,6 +4565,62 @@ describe("round 26 posting and answering-turn fences", () => {
         }
     });
 
+    test("a dry integration previews outside the vault and never writes or posts there", () => {
+        const dir = fs.mkdtempSync(join(tmpdir(), "babysit-integrate-dry-"));
+        fs.writeFileSync(
+            join(dir, "run.json"),
+            SafeJSON.stringify({ repo: "fixture/repo", main: dir, post: true, reviewLoopScripts: join(dir, "scripts") })
+        );
+        fs.writeFileSync(
+            join(dir, "replies.json"),
+            SafeJSON.stringify({ pr: 1, round: 1, model: "fixture", intro: "fixture", items: [] })
+        );
+        const preload = join(dir, "preload.ts"),
+            callsFile = join(dir, "calls.json");
+        fs.writeFileSync(
+            preload,
+            [
+                "import * as fs from 'node:fs';const calls=[];process.on('exit',()=>fs.writeFileSync(" +
+                    SafeJSON.stringify(callsFile) +
+                    ",JSON.stringify(calls)));const ok=(out='')=>({exitCode:0,stdout:Buffer.from(out),stderr:Buffer.alloc(0)});",
+                "Bun.spawnSync=(args,opts)=>{if(args[0] === 'git'){if(args.includes('push'))throw new Error('no pushes');return ok(args.includes('rev-parse')?'fixture-head':'');}if(args[0] === 'gh'){if(args[1] === 'pr' && args[2] === 'view')return ok(JSON.stringify({headRefName:'fixture',baseRefName:'main'}));calls.push(args);return ok();}if(args.some(a=>a.endsWith('round-summary.ts'))){calls.push([...args,'cwd='+opts.cwd]);fs.writeFileSync(args[args.indexOf('--out')+1],'fixture summary');return ok();}if(args.some(a=>a.endsWith('post-replies.ts'))){calls.push(args);return ok();}throw new Error('unexpected command');};",
+            ].join("\n")
+        );
+        const result = Bun.spawnSync(
+            [
+                process.execPath,
+                "--preload",
+                preload,
+                join(import.meta.dir, "integrate.ts"),
+                "--run",
+                dir,
+                "--wt",
+                dir,
+                "--pr",
+                "1",
+                "--round",
+                "1",
+                "--replies",
+                join(dir, "replies.json"),
+                "--post",
+                "--dry",
+            ],
+            { stdout: "pipe", stderr: "pipe", env: env.getProcessEnv(), timeout: 5000 }
+        );
+        const output = result.stdout.toString() + result.stderr.toString();
+        expect(result.exitCode, output).toBe(0);
+        expect(fs.existsSync(join(dir, "vault"))).toBe(false);
+        expect(output).toContain("vault untouched");
+        const calls = SafeJSON.parse(fs.readFileSync(callsFile, "utf8"), { strict: true }) as string[][];
+        expect(calls.length).toBe(1);
+        const summary = calls[0];
+        expect(summary.some((arg) => arg.endsWith("round-summary.ts"))).toBe(true);
+        const preview = summary[summary.indexOf("--out") + 1];
+        expect(preview.startsWith(join(dir, "vault"))).toBe(false);
+        expect(fs.readFileSync(preview, "utf8")).toBe("fixture summary");
+        expect(fs.existsSync(join(dirname(preview), "1-r1-replies.json"))).toBe(true);
+    });
+
     test("a steer fence excludes the previous finished turn and accepts a reply finished before waiting", () => {
         for (const mode of ["previous", "fast"]) {
             const dir = fs.mkdtempSync(join(tmpdir(), "babysit-answer-fence-"));

docs/benchmarks-du.md

@@ -1407,3 +1407,21 @@ planning round. It now counts only the dataless folders of interior directories,
 The planning walk also runs with dataless materialization off, and a dataless scan root is never listed. The scan
 then finds nothing, as the C engine's guarded open of that root does. `cloudctl` runs stop after 60 s plus 2 s per
 path, and the call fails with that reason (an `evict` names that some files may already be evicted).
+
+## 2026-10-23 14:34 — Review follow-ups: partner and big-file queries reset the cloud policy
+
+No hot-loop code changed and no total moves, so no benchmark was rerun. The cloud policy lives in globals
+(`g_skip_cloud`, `g_walk_cloud_roots`). `clonesize_run_json` and `clonesize_cloudfiles_json` set it on every call,
+but `clonesize_partners_json` and `clonesize_bigfiles_json` did not. A partner query after an `includeCloud` scan in
+the same process inherited `g_skip_cloud = 0`: the dataless checks were skipped, `cloud_thread_guard` did nothing, and
+cloud-backed files were opened. Both FFI entries now set the stat-only policy first. The CLI keeps its `--include-cloud`
+for `--partners-of` and `--bigfiles`, because `main()` calls the inner functions directly.
+
+Check (`cloud.test.ts`, "partners never inherit --include-cloud"): on the fake File Provider fixture, a partner query
+after an opened scan reported `files_opened: 5` before the fix and 0 after it. The CLI keeps the flag: `--partners-of`
+on the same kind of fixture opens 0 files without `--include-cloud` and 2 with it.
+
+`tools du evict` now takes its top N from the iCloud Drive files only, and counts the other providers' files in one
+line. `--apply` exits 1 when a selected file, or one named by `--path`, was not evicted. The C engine, the shim and the
+binary are not compiled on a Mac without Xcode or the Command Line Tools: an error names `xcode-select --install`, and
+clang never opens the install dialog.