diff --git a/lib/review-collect.ts b/lib/review-collect.ts index cb48b708f..ac16ba0d2 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -255,10 +255,11 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings }); } -/** The working tree against the merge base with origin/ (or ), untracked files counted per path; the registry is the base ref's commit. */ +/** The working tree against the merge base with origin/ (or ; a remote-qualified base reads refs/remotes/ only), untracked files counted per path; the registry is the base ref's commit. */ export function collectLive(root: string, base: string): Collected { const errors: DepthError[] = []; - const ref = [`refs/remotes/origin/${base}`, `refs/heads/${base}`].find(r => git(root, ['rev-parse', '--verify', '-q', `${r}^{commit}`]).ok) ?? base; + const qualified = base.startsWith('origin/') || git(root, ['remote']).out.split('\n').some(r => r && base.startsWith(`${r}/`)); + const ref = (qualified ? [`refs/remotes/${base}`] : [`refs/remotes/origin/${base}`, `refs/heads/${base}`]).find(r => git(root, ['rev-parse', '--verify', '-q', `${r}^{commit}`]).ok) ?? base; const registry = loadRegistryAt(root, ref); const mb = git(root, ['merge-base', ref, 'HEAD']); if (!mb.ok) { diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 7ef05ab1b..875028435 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -96,7 +96,7 @@ export function receiptReviewAtHead(repoRoot: string, base: string, binDir: stri const declares = declaresExclude(repoRoot, ref, sha); let tagged: number | null = 0; let paths: Set | null = declares ? new Set() : null; - if (gateScanNeeded(c.paths, c.root)) { + if (declares || gateScanNeeded(c.paths, c.root)) { const atHead = git(repoRoot, ['rev-parse', 'HEAD']).out === head && git(repoRoot, ['diff', '--quiet', 'HEAD', '--']).ok; const g = atHead ? runScriptSync(path.join(binDir, 'gstack-gate-diff'), [baseRev, ...(declares ? ['--format', 'jsonl'] : [])], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }) : null; const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g?.stdout ?? ''); diff --git a/test/review-collect.test.ts b/test/review-collect.test.ts index 1a64f4da8..3db4a3665 100644 --- a/test/review-collect.test.ts +++ b/test/review-collect.test.ts @@ -112,6 +112,18 @@ describe('collectLive: the working tree', () => { expect(c.paths).toEqual(['docs/a.md', 'src/a.ts', 'src/new.ts']); expect(depthOfCommitInput({ ...c, gateTagged: 0 })).toMatchObject({ depth: 'light', runtimeLines: 20 + 10 }); }); + // Value: protects=a remote-qualified base reads that remote's tracking branch by its full name, so a fetched branch called + // origin/main cannot become the merge base (an empty diff) or the registry and policy source; fails_when=collectLive tries + // refs/remotes/origin/ first for origin/main (/ship review of #3852, outside-voice#1); why_new=every other case names a + // bare base; seam=none; no_claim=remotes other than origin + test('a fetched branch named origin/main never stands in for origin/main', () => { + const r = repo({}); + r.git('update-ref', 'refs/remotes/origin/main', r.baseSha); + const head = r.commit({ 'src/a.ts': lines(3) }); + r.git('update-ref', 'refs/remotes/origin/origin/main', head); + const c = collectLive(r.dir, 'origin/main'); + expect([c.mergeBase, c.registry.sha, c.paths]).toEqual([r.baseSha, r.baseSha, ['src/a.ts']]); + }); test('no merge base is a collector error the CLI refuses on', () => { const r = repo({}); expect(collectLive(r.dir, 'no-such-branch').errors.map(e => e.code)).toEqual(['numstat_failed']); diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index a93862931..4c0695183 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -11,7 +11,7 @@ import { EXIT } from '../lib/headless-artifacts'; import { KEEP_ARG_MAX_BYTES, readSelection, reviewExclude, tooLargeLine, unreadableLine } from '../lib/review-exclude'; import { collectCommit, collectLive } from '../lib/review-collect'; import { extractReceipt } from '../lib/ship-receipt'; -import { bindShippedReceipt } from '../lib/ship-receipt-depth'; +import { bindShippedReceipt, receiptReviewAtHead } from '../lib/ship-receipt-depth'; import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable } from '../lib/ship-policy'; import { loadPolicyAt } from '../lib/ship-policy-at'; import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; @@ -253,6 +253,20 @@ describe('the receipt binds the selection the reviewers read', () => { const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; const writeReceipt = (repo: FixtureRepo, body: string, extra: string[]) => runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body, ...extra]); + // Value: protects=a docs-only range whose only file sits under a base pattern and carries a gate tag binds the same selection at + // plan and receipt write, so the receipt can be written; fails_when=receiptReviewAtHead skips the gate scan when the paths are + // docs-class and excludes the file the plan kept as gate-tagged (/ship review of #3852, red-team#1: digests 652d0aa8 vs + // 6f33bd56, RECEIPT_INVALID with no fix); why_new=every other receipt case has a code path in its range; seam=gstack-review-plan + // --json and receiptReviewAtHead on one head; no_claim=the gate scan's own tag rules + test('a docs-only range with a gate-tagged file under a pattern binds the plan\'s digest at receipt write', () => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-docs-', baseFiles: { '.gstack/ship-policy.json': policyText(['testdata/**']) } }); + repos.push(repo); + const head = commit(repo, { 'testdata/expected.txt': 'expected output\n' }); + const planned = JSON.parse(runBin(repo, 'gstack-review-plan', ['--base', 'main', '--json']).out).review_exclude; + expect(planned.kept_files).toEqual([{ path: 'testdata/expected.txt', reason: 'gate-tagged' }]); + const { exclude } = receiptReviewAtHead(repo.work, 'main', path.join(import.meta.dir, '..', 'bin'), head); + expect(exclude.state === 'active' ? exclude.record.digest : exclude.state).toBe(planned.digest); + }); // Value: protects=a receipt names the selection the rendered briefs read, a base merge that leaves the policy alone still // carries, and a policy change on the base (adding or removing a pattern) needs a new review, not a write; fails_when=write // records a digest the briefs never named, carry compares the base sha, or a removed exclusion clears by writing the old