Fix push-check binary checks from repository subdirectories (#1825)

agents/bradford/work/queue-327/BUILD.md

@@ -84,3 +84,24 @@ Candidate-mismatched receipts are impossible through the queue API and are
 covered by a corrupt scratch queue refusing its read. The lead's full-suite gate,
 Poe first review, Dewey landing review, and adoption for Sage's next push remain
 landing stages. No hooks, role policy, credentials or other seats' paths changed.
+
+## Poe first review R1 (poe-0010) — changes
+
+cceea407e has a blocking subdirectory bypass: ls-tree without --full-tree can
+omit an existing binary outside the caller cwd, making it look deleted.
+Accepted the finding. Git now anchors all invocations at the repository root,
+ls-tree uses --full-tree, and diff/diff-tree use --no-relative. EXPECTED_LOG still
+resolves from the original caller cwd. Added outside-cwd and nested-binary cases
+under diff.relative=true: unreviewed stops, reviewed manifest/commit passes.
+No old test changed. New run requires Sage CLEAR before execution.
+
+R1 verification under sage-3999 CLEAR: test-run-4.txt records PASS 29 checks
+and KILLED all five required mutants. Syntax and diff whitespace checks passed.
+A second bounded run used the new suite against cceea407e's script copied under
+TMPDIR: r1-regression-red.txt records the outside-cwd unreviewed binary reaching
+NO_BINARY/PUSH_READY (actual exit 0, expected 1). Thus the new regression detects
+the reviewed bug, while the revised guard refuses it. No code changed between
+the green run and this old-candidate reproduction. All runs used timeout -k 10.
+
+Holding claimant queue writes per sage-3999 until Boyd finishes row 309's record.
+Last acknowledged row 327 state is in-progress (bradford-0010, revision 3868).

agents/bradford/work/queue-327/r1-regression-red.txt

@@ -0,0 +1,27 @@
+node:internal/modules/run_main:107
+    triggerUncaughtException(
+    ^
+
+AssertionError [ERR_ASSERTION]: LOG_OK
+FF_OK
+NO_BINARY
+SECRETS_CLEAN
+NO_ATTRIBUTION
+PUSH_READY 896bdf0ef56596c67d2dc427275743c46026e832..ad9f9c86112fadafef4d4854f46bd9994fe2fe5d
+
+
+
+0 !== 1
+
+    at Object.check (file:///home/jwoltje/.t3/worktrees/mosaic-stack/bradford-row327/[eval1]:44:12)
+    at file:///home/jwoltje/.t3/worktrees/mosaic-stack/bradford-row327/[eval1]:130:7
+    at process.processTicksAndRejections (node:internal/process/task_queues:104:5) {
+  generatedMessage: false,
+  code: 'ERR_ASSERTION',
+  actual: 0,
+  expected: 1,
+  operator: 'strictEqual',
+  diff: 'simple'
+}
+
+Node.js v26.8.1

agents/bradford/work/queue-327/test-run-4.txt

@@ -0,0 +1,6 @@
+PASS push-check 29 checks
+KILLED skip-hash
+KILLED accept-changes
+KILLED earlier-round
+KILLED working-tree
+KILLED no-row

docs/TOOLS.md

@@ -1370,6 +1370,8 @@ identity input and is never reused for proof.
 
 `scripts/push-check.sh SHA EXPECTED_LOG [--row N]... [--base-ref REF] [--fetch]`
 checks the repository at the caller's cwd; it never pushes or writes the queue.
+Git checks cover the full repository even from a subdirectory; EXPECTED_LOG is
+resolved relative to the original caller. Non-UTF-8 git paths refuse with exit 2.
 EXPECTED_LOG must contain exactly `git log --oneline BASE..SHA` (including its
 newlines). BASE defaults to `refs/remotes/origin/refactor`; `--base-ref` overrides
 it. Only `--fetch` fetches origin, before resolving BASE.

scripts/push-check.sh

@@ -9,8 +9,9 @@ import { join } from 'node:path';
 import { pathToFileURL } from 'node:url';
 
 const stop = (reason, code = 1) => { console.log(`STOP ${reason}`); process.exit(code); };
+let gitRoot = process.cwd();
 const git = (...args) => {
-  const r = spawnSync('git', args, { maxBuffer: 256 * 1024 * 1024 });
+  const r = spawnSync('git', args, { cwd: gitRoot, maxBuffer: 256 * 1024 * 1024 });
   if (r.error || r.status !== 0) throw new Error('git');
   return r.stdout;
 };
@@ -33,7 +34,13 @@ while (argv.length) {
   else stop('usage', 2);
 }
 let sha, base, expected;
-try { sha = resolve(ref); expected = readFileSync(expectedFile); }
+try {
+  // Resolve the log against the original caller, then anchor every Git command
+  // at the repository root. Queue reads retain their production cwd checks.
+  expected = readFileSync(expectedFile);
+  gitRoot = text('rev-parse', '--show-toplevel');
+  sha = resolve(ref);
+}
 catch { stop('sha or expected log unavailable', 2); }
 if (fetch) { try { git('fetch', '-q', 'origin'); } catch { stop('fetch', 2); } }
 try { base = resolve(baseRef); } catch { stop('base ref unavailable', 2); }
@@ -45,11 +52,11 @@ try {
 
   // --no-renames keeps -z numstat to exactly one path per record. Never parse
   // quoted path output or consult working-tree bytes.
-  const binaries = pathsText(git('diff', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', '--numstat', '-z', base, sha))
+  const binaries = pathsText(git('diff', '--no-relative', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', '--numstat', '-z', base, sha))
     .split('\0').filter(Boolean).filter(s => s.startsWith('-\t-\t')).map(s => s.slice(4));
   const tree = commit => {
     const entries = new Map();
-    for (const entry of pathsText(git('ls-tree', '-r', '-z', commit)).split('\0').filter(Boolean)) {
+    for (const entry of pathsText(git('ls-tree', '--full-tree', '-r', '-z', commit)).split('\0').filter(Boolean)) {
       const tab = entry.indexOf('\t');
       const [mode, type, oid] = entry.slice(0, tab).split(' ');
       entries.set(entry.slice(tab + 1), { mode, type, oid });
@@ -83,7 +90,7 @@ try {
     let parent;
     try { parent = text('rev-parse', '--verify', `${c.digest}^1`); }
     catch { parent = text('hash-object', '-t', 'tree', '/dev/null'); }
-    const changed = new Set(pathsText(git('diff-tree', '-r', '--no-renames', '--name-only', '-z', parent, c.digest)).split('\0').filter(Boolean));
+    const changed = new Set(pathsText(git('diff-tree', '--no-relative', '-r', '--no-renames', '--name-only', '-z', parent, c.digest)).split('\0').filter(Boolean));
     candidates.set(c.digest, { candidateTree, changed });
   }
   for (const path of present) {
@@ -99,7 +106,7 @@ try {
   }
   console.log(present.length ? `BINARY_REVIEWED ${present.length}` : 'NO_BINARY');
 
-  const diff = git('diff', '--no-color', '--no-ext-diff', '--no-textconv', base, sha).toString('utf8');
+  const diff = git('diff', '--no-relative', '--no-color', '--no-ext-diff', '--no-textconv', base, sha).toString('utf8');
   const secret = /BEGIN [A-Z ]*PRIVATE KEY|ghp_[A-Za-z0-9]{20}|sk-ant-[A-Za-z0-9-]{10}|xox[bp]-|AKIA[0-9A-Z]{16}|(token|secret|password)["' ]*[:=] *["'][A-Za-z0-9_\-]{24,}/i;
   if (diff.split('\n').some(line => line.startsWith('+') && secret.test(line))) stop('secrets');
   console.log('SECRETS_CLEAN');

scripts/test-push-check.sh

@@ -5,7 +5,7 @@ ROOT=$(cd -- "$(dirname -- "$0")/.." && pwd)
 exec node --input-type=module - "$ROOT" "$@" <<'JS'
 import assert from 'node:assert/strict';
 import { spawnSync } from 'node:child_process';
-import { readFileSync, writeFileSync, mkdtempSync, rmSync } from 'node:fs';
+import { readFileSync, writeFileSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { pathToFileURL } from 'node:url';
@@ -42,10 +42,10 @@ function fixture() {
     repo.g('commit', '-q', '-m', message);
     return repo.g('rev-parse', 'HEAD').trim();
   };
-  const check = (sha, extra = [], expectedCode = 0, reason = null, script = target) => {
+  const check = (sha, extra = [], expectedCode = 0, reason = null, script = target, cwd = repo.root) => {
     const log = join(repo.base, 'expected.log');
     writeFileSync(log, repo.g('log', '--oneline', `${base}..${sha}`));
-    const r = spawnSync('bash', [script, sha, log, ...extra], { cwd: repo.root, env: repo.env, encoding: 'utf8' });
+    const r = spawnSync('bash', [script, sha, log, ...extra], { cwd, env: repo.env, encoding: 'utf8' });
     assert.equal(r.status, expectedCode, `${r.stdout}\n${r.stderr}`);
     const lines = r.stdout.trim().split('\n');
     if (expectedCode === 0) assert.match(lines.at(-1), /^PUSH_READY /);
@@ -128,6 +128,25 @@ try {
     f.repo.g('add', '-A'); f.repo.g('commit', '-q', '-m', 'invalid UTF-8 path');
     f.check(f.repo.g('rev-parse', 'HEAD').trim(), [], 2, /^STOP git or environment error$/);
   }
+  {
+    const f = fixture(), sha = f.commit('asset.bin', binary);
+    const cwd = join(f.repo.root, 'nested'); mkdirSync(cwd);
+    f.repo.g('config', 'diff.relative', 'true');
+    f.check(sha, [], 1, /^STOP binary asset.bin /, target, cwd);
+    f.round(f.manifest());
+    assert.match(f.check(sha, ['--row', '9'], 0, null, target, cwd), /BINARY_REVIEWED 1\n/);
+    const relativeLog = spawnSync('bash', [target, sha, '../../expected.log', '--row', '9'], { cwd, env: f.repo.env, encoding: 'utf8' });
+    assert.equal(relativeLog.status, 0, relativeLog.stdout + relativeLog.stderr);
+    assert.match(relativeLog.stdout.trim().split('\n').at(-1), /^PUSH_READY /); count++;
+  }
+  {
+    const f = fixture(), cwd = join(f.repo.root, 'nested'); mkdirSync(cwd);
+    const sha = f.commit('nested/asset.bin', binary);
+    f.repo.g('config', 'diff.relative', 'true');
+    f.check(sha, [], 1, /^STOP binary nested\/asset.bin /, target, cwd);
+    f.round(sha);
+    assert.match(f.check(sha, ['--row', '9'], 0, null, target, cwd), /BINARY_REVIEWED 1\n/);
+  }
   {
     const f = fixture();
     f.commit('asset.bin', binary);