diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index bf70b39f3..effde9269 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -104,8 +104,8 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate (the digest= on gstack-review-dispatch verify's REVIEW_EXCLUDE_RECORD: line, which the reviewers' briefs name; none without that line): must equal the digest HEAD has under the base policy's review_exclude_paths, else - RECEIPT_INVALID field=review_exclude and nothing is written; recorded as review_exclude. Carry and the queue compare that digest; carry also - carries when neither the recorded selection nor HEAD's holds a file the PR changed. + RECEIPT_INVALID field=review_exclude and nothing is written; recorded as review_exclude. Carry and the queue compare that digest; both also + bind when neither the recorded selection nor HEAD's holds a file the PR changed. The check binds the digest to the base policy, not to a review: write does not read the review ledger, so the expected= digest a refusal prints passes too. After a refusal, rerun the review and pass the digest its verify prints. --docs-post -post.json (Step 14.5's post snapshot, the file finish takes): the reviewed --review-exclude digest also binds diff --git a/changelog.d/capy-gst-1434-review-exclude-ready-scan.md b/changelog.d/capy-gst-1434-review-exclude-ready-scan.md new file mode 100644 index 000000000..a7238f402 --- /dev/null +++ b/changelog.d/capy-gst-1434-review-exclude-ready-scan.md @@ -0,0 +1,2 @@ +- **`ready` binds a receipt with an older review_exclude digest when the policy change touches none of the PR's files.** After a `review_exclude_paths` edit landed on main, `gstack-ship-queue ready` refused every receipt that had recorded the old digest, even when no file the PR changed fell under any pattern (harness-evals #274 and #276 after policy PR #278). The queue now uses carry's rule. When neither the receipt's recorded selection nor HEAD's holds a file the PR changed, the receipt binds. A selection that holds a PR file, excluded or kept, and an unreadable base policy still refuse. (GST-1434) +- **The review_exclude name scan no longer times out on large results repos.** The scan that keeps a file a program names by full path ran one `git grep` over the whole tree, so on harness-evals #269 it searched 30 MB of receipts for 4,354 candidate paths, ran 278 s past its 120 s limit and kept every file. It now greps only the code and test files that can name a candidate, so its cost follows the code, not the data the PR adds. A git failure or timeout still keeps every candidate. (GST-1435) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 1b657bfc1..19d019222 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1426,8 +1426,10 @@ harness that writes the receipts names it) keeps nothing: the plan prints an informational `REVIEW_EXCLUDE_REFERENCE:` line naming the code that reads the folder. A data file never keeps another file: a results manifest such as `docs/receipts-manifest.json` lists every receipt by full path, and it is -content, not a program that reads them. The name scan is one uncapped `git grep -F -f` over the candidates' -paths, not the collector's capped consumer scan. +content, not a program that reads them. The name scan is an uncapped `git grep -F -f` of the candidates' +paths over the consumer files alone (the head's and the merge base's trees filtered to code and tests, passed as literal +pathspecs in 24,000-byte chunks), never the data under the patterns, so its cost follows the code a results PR +can be read by and not the receipts it adds (GST-1435); it is not the collector's capped consumer scan. **What readers run.** Every reader's `DIFF_CMD:` gains one `':(top,exclude,glob)'` item per applied pattern, and the fixture @@ -1474,8 +1476,7 @@ checks the digest against the base policy, so a stale selection cannot be recorded by accident, but it does not read the review ledger: a write given the `expected=` digest passes without a review, and only the review run shows that the reviewers read that selection. -The queue makes one exception (GST-1408): a receipt with no `review_exclude`, written before the base gained the key, still binds when no file the head changes matches any current pattern, excluded or kept, because its reviewers read the same full diff; a recorded digest that differs, a matched file and an unreadable base policy still refuse. -Carry makes the same exception in both directions (GST-1415): when the digests differ but neither the receipt's recorded selection (no `review_exclude`, or one whose list hash is the empty selection's and whose digest is built from it) nor HEAD's holds a file the PR changed, the receipt carries and records HEAD's digest, so a pattern the base gains, adds, removes or reverts does not void it; a selection that holds a PR file, excluded or kept, and an unreadable base policy still refuse. +Carry and the queue make one exception (GST-1408, GST-1415, GST-1434): when the digests differ but neither the receipt's recorded selection (no `review_exclude`, or one whose list hash is the empty selection's and whose digest is built from it) nor HEAD's holds a file the PR changed, carry carries the receipt and records HEAD's digest and the queue (`ready`, train build, land) binds it, so a pattern the base gains, adds, removes or reverts does not void it; a selection that holds a PR file, excluded or kept, and an unreadable base policy still refuse. Write makes one exception for Step 14.5's docs audit, which runs after the review (GST-1429): with `--docs-post -post.json` it accepts the reviewed digest when that digest is HEAD's selection with every file under a pattern that changed since the snapshot's HEAD reset to it, each such file is uncommitted in the snapshot with the content HEAD holds, each is excluded or kept only because code names it, and every other file is excluded or kept alike on both sides. With `--docs-pre ` only the files whose content changed between the audit's two snapshots reset, to their Prepare content, so uncommitted docs the reviewers read stay as they are; content compares as git stores it or as it checks out (autocrlf, filters). It prints `REVIEW_EXCLUDE_DOCS_AUDIT:` and records HEAD's digest. Any other change to the selection, an audit file the exclusion keeps for another reason, an edit after the snapshot and a missing or unreadable snapshot still refuse. **Adopting it.** (1) Open a PR on the base that adds the patterns and diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 09d928014..e466012be 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3502,9 +3502,10 @@ removing files under a pattern, pass that run's `--docs-post -post.jso `--docs-pre` as well instead of rerunning the review; the refusal's `the docs-audit exception does not apply:` clause says why when it cannot ([ship-policy.md](ship-policy.md#review-exclude-paths), GST-1429). -The queue's check skips a receipt with `recorded=none` (written before the base gained -`review_exclude_paths`) when no file the head changes matches any current pattern, so -this line then means the PR touches a pattern, or the recorded digest differs. +The queue's check binds a receipt whose digest differs (`recorded=none`, written before the base +gained `review_exclude_paths`, or a digest from an older policy) when neither the receipt's recorded +selection nor HEAD's holds a file the PR changed, the rule carry uses (GST-1434), so +this line then means one of the two selections holds a file the PR changed. ### `REVIEW_DIFF_TOO_LARGE` diff --git a/lib/review-exclude.ts b/lib/review-exclude.ts index c43e80927..ede914888 100644 --- a/lib/review-exclude.ts +++ b/lib/review-exclude.ts @@ -79,6 +79,18 @@ export const recordSelectsNoFile = (r: Pick `'${s.replace(/'/g, "'\\''")}'`; const literal = (p: string) => `:(top,literal)${p}`; const argBytes = (paths: Iterable) => { let n = 0; for (const p of paths) n += Buffer.byteLength(shellQuote(literal(p))) + 1; return n; }; +/** `paths` in order, split so each chunk's literal pathspecs stay within KEEP_ARG_MAX_BYTES (one path past it is its own chunk). */ +function argChunks(paths: readonly string[]): string[][] { + const chunks: string[][] = []; + let bytes = Infinity; + for (const p of paths) { + const n = argBytes([p]); + if (bytes + n > KEEP_ARG_MAX_BYTES) { chunks.push([]); bytes = 0; } + chunks[chunks.length - 1]!.push(p); + bytes += n; + } + return chunks; +} function git(root: string, args: string[], input?: string): { ok: boolean; status: number | null; out: string; err: string } { const r = spawnSync('git', ['-c', 'core.quotePath=false', ...args], { cwd: root, input, encoding: 'utf8', timeout: 120_000, maxBuffer: MAX_BUFFER }); @@ -183,15 +195,7 @@ function binaryContent(i: ExcludeInput, files: ReadonlyMap, cla function attributeBinaryLines(i: ExcludeInput, paths: readonly string[], unread: Set): Record { const out: Record = {}; const range = i.head ? [i.mergeBase, i.head] : [i.mergeBase]; - const chunks: string[][] = []; - let bytes = Infinity; - for (const p of [...paths].sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b)))) { - const n = argBytes([p]); - if (bytes + n > KEEP_ARG_MAX_BYTES) { chunks.push([]); bytes = 0; } - chunks[chunks.length - 1]!.push(p); - bytes += n; - } - for (const chunk of chunks) { + for (const chunk of argChunks([...paths].sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b))))) { const r = git(i.root, ['diff', '--text', '--no-textconv', '--no-ext-diff', '--no-renames', '--src-prefix=a/', '--dst-prefix=b/', '-U0', ...range, '--', ...chunk.map(literal)]); const sections = r.ok ? r.out.split(/^(?=diff --git )/m).filter(s => s.startsWith('diff --git ')) : []; const aligned = sections.length === chunk.length && chunk.every((p, k) => /["\\\x00-\x1f\x7f]/.test(p) || sections[k]!.startsWith(`diff --git a/${p} b/${p}\n`)); @@ -236,7 +240,13 @@ function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet< const consumer = (i: ExcludeInput, f: string, candidates: ReadonlySet) => !candidates.has(f) && !f.startsWith('.gstack/tmp/') && contentKind(i, f, new Set()) === null; -/** An uncapped `git grep -F -o -f` of fixed strings at the head (or the working tree) and `revs`: string -> the consumer files holding it; null when git grep failed. */ +/** + * An uncapped `git grep -F -o -f` of fixed strings at the head (or the working tree) and `revs`: string -> the consumer files + * holding it; null when git could not list a tree, a consumer's name did not survive UTF-8 decoding (its literal pathspec would + * match nothing), or git grep failed (a timeout included). Each tree's file list is filtered to + * consumers first and grepped through literal pathspecs, so the data under the patterns is never read (GST-1435: a whole-tree + * grep for 4,354 candidate paths through 30 MB of receipts ran 278 s). + */ function grepFixed(i: ExcludeInput, strings: readonly string[], candidates: ReadonlySet, revs: readonly string[]): Map> | null { const found = new Map>(); if (!strings.length) return found; @@ -244,18 +254,22 @@ function grepFixed(i: ExcludeInput, strings: readonly string[], candidates: Read try { const list = path.join(dir, 'patterns'); fs.writeFileSync(list, strings.join('\n') + '\n'); - const runs = [i.head ? { at: i.head, args: [i.head, '--'] } : { at: '', args: ['--'] }, ...revs.map(rev => ({ at: rev, args: [rev, '--'] }))]; - for (const run of runs) { - const r = git(i.root, ['grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', '--no-color', '-f', list, ...run.args]); - if (r.status !== 0 && r.status !== 1) return null; - for (const line of r.out.split('\n')) { - const parts = line.split('\0'); - const where = parts[0]; - const match = parts.length > 1 ? parts[parts.length - 1] : undefined; - if (!where || !match) continue; - const file = run.at ? where.slice(run.at.length + 1) : where; - if (!consumer(i, file, candidates)) continue; - (found.get(match) ?? found.set(match, new Set()).get(match)!).add(file); + for (const at of [i.head ?? '', ...revs]) { + const ls = git(i.root, at ? ['ls-tree', '-r', '-z', '--name-only', at] : ['ls-files', '-z']); + if (!ls.ok) return null; + const files = [...new Set(ls.out.split('\0'))].filter(f => f && consumer(i, f, candidates)); + if (files.some(f => f.includes('\uFFFD'))) return null; + for (const chunk of argChunks(files)) { + const r = git(i.root, ['grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', '--no-color', '-f', list, ...(at ? [at] : []), '--', ...chunk.map(literal)]); + if (r.status !== 0 && r.status !== 1) return null; + for (const line of r.out.split('\n')) { + const parts = line.split('\0'); + const where = parts[0]; + const match = parts.length > 1 ? parts[parts.length - 1] : undefined; + if (!where || !match) continue; + const file = at ? where.slice(at.length + 1) : where; + (found.get(match) ?? found.set(match, new Set()).get(match)!).add(file); + } } } return found; diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index e84b47c7c..a6e70281f 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -279,20 +279,13 @@ function headInputs(root: string, baseRef: string, head: string, r?: ShipReceipt return { baseSha, c, scope: scopeOfRange(root, BIN_DIR, baseRev, head, merged), x: exclusionAt(root, baseRef, baseSha.ok ? baseSha.out : null, head, c, tagged, 'gstack-ship-queue') }; } -/** - * The queue's exclusion check: the digest the receipt recorded (none when absent) against the one `head` has under the current - * base policy. Only the digest counts (the policy's patterns and the excluded list), so a base merge or a moved base that leaves - * both unchanged binds; a changed selection needs the review rerun (write checks the same digest and does not read the - * review ledger). A receipt with no digest also binds when no file the head changed matches any current pattern, excluded - * or kept (GST-1408): the selection then equals the full diff that receipt's review read, so a pattern the base gained later - * changes nothing the reviewers saw. - */ /** No file the head changed matches any pattern the base policy declares, so each reviewer reads the full diff (GST-1408). */ const selectsNoFile = (x: Exclusion) => x.state === 'none' || x.state === 'head-only' || (x.state === 'active' && x.record.unmatched.length === x.record.patterns.length); /** - * Carry's exclusion check: the recorded digest against the one HEAD has under the current base policy, except that a policy - * change alone does not void the receipt (GST-1415): when neither the receipt's selection nor HEAD's holds a file the PR - * changed, each reviewer read the full diff and still would, whatever the patterns. A policy that cannot be read never binds. + * Carry's and the queue's exclusion check: the recorded digest against the one HEAD has under the current base policy, except + * that a policy change alone does not void the receipt (GST-1408, GST-1415, GST-1434): when neither the receipt's selection + * (none recorded, or an empty one) nor HEAD's holds a file the PR changed, each reviewer read the full diff and still would, + * whatever the patterns. A policy that cannot be read never binds. */ export function carryExcludeBinds(x: Exclusion, r: ShipReceipt): boolean { if (x.state === 'refused') return false; @@ -389,14 +382,14 @@ export function writeExcludeBinds(root: string, base: string, head: string, revi } return { ok: true, line: `REVIEW_EXCLUDE_DOCS_AUDIT: reviewed=${short(given)} head=${short(x.record.digest)} snapshot=${short(at)} files=${auditFiles.join(',')} (the selection changed only by the Step 14.5 docs audit's files under review_exclude_paths; the receipt records HEAD's)` }; } +/** The queue's refusal when carryExcludeBinds does not bind the receipt at `head`; null when it does. */ export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null { + if (carryExcludeBinds(x, r)) return null; const expected = x.state === 'active' ? x.record.digest : 'none'; const recorded = r.review_exclude?.digest ?? 'none'; - if (expected === recorded) return null; - if (recorded === 'none' && selectsNoFile(x)) return null; const source = x.state === 'active' ? x.record.source : r.review_exclude?.source ?? base; const h = head.slice(0, 12); - return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the receipt records another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr --review-exclude --body with the digest gstack-review-dispatch verify --ledger prints on REVIEW_EXCLUDE_RECORD: (write checks the digest against the base policy and does not read the review ledger, so only the rerun shows the reviewers read this selection)`; + return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the receipt records another selection than ${base}'s review_exclude_paths gives ${h}, and one of the two holds a file the PR changed; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr --review-exclude --body with the digest gstack-review-dispatch verify --ledger prints on REVIEW_EXCLUDE_RECORD: (write checks the digest against the base policy and does not read the review ledger, so only the rerun shows the reviewers read this selection)`; } /** The receipt's recorded gate scan when it names this head; otherwise null (unavailable). */ const boundScan = (r: ShipReceipt, head: string): number | null => { diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index 3514042c9..6ad1e1657 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -344,8 +344,8 @@ describe('the receipt binds the selection the reviewers read', () => { }, 240_000); // Value: protects=a base policy that gains review_exclude_paths does not void a receipt whose PR touches none of the new - // patterns, while a recorded digest that differs, a PR touching a pattern (an excluded or a kept file) and an unreadable base - // policy still refuse; fails_when=the queue compares recorded=none to the new digest without asking whether any changed + // patterns, while a PR touching a pattern (an excluded or a kept file) and an unreadable base policy still refuse (a recorded + // digest that differs: GST-1434 below); fails_when=the queue compares recorded=none to the new digest without asking whether any changed // file matches (GST-1408: harness-evals #264 stopped 9 receipted PRs), or the acceptance spreads to any of the four // refusals; why_new=GST-1408; seam=bindShippedReceipt, which gstack-ship-queue ready, train build and land call, on receipts // gstack-ship-receipt write produced; no_claim=carry (bin/gstack-ship-receipt carry keeps its own comparison) @@ -399,25 +399,48 @@ describe('the receipt binds the selection the reviewers read', () => { } }, 240_000); - test('a recorded digest that differs from the current one still refuses, though the PR touches no pattern', () => { - const { repo, body } = shipped(CODE, ['receipts/**']); - const recorded = receiptIn(body).review_exclude!.digest; - expect(bind(repo, body).ok).toBe(true); - policyOnBaseOnly(repo, policyText(['receipts/**', 'results/**'])); - const b = bind(repo, body); - expect(b.ok).toBe(false); - if (b.ok) return; - expect(b.code).toBe('RECEIPT_INVALID'); - expect(b.message).toMatch(new RegExp(`^field=review_exclude expected=[0-9a-f]{64} recorded=${recorded} `)); + // Value: protects=ready, train build and land bind a receipt whose recorded digest is older than the base policy's when + // neither the recorded selection nor HEAD's holds a PR file (carry's rule since #4030), while a selection that holds a PR + // file (excluded or kept, on either side) and an unreadable base policy still refuse; fails_when=the queue compares digests + // alone (GST-1434: harness-evals #274 and #276 refused after policy PR #278 merged, recorded=5ae7e95a07b5 expected= + // 323c17cf6249), or the acceptance spreads to a selection that holds a PR file; why_new=GST-1434; seam=bindShippedReceipt, + // which gstack-ship-queue ready, train build and land call; no_claim=carry (GST-1415 below) + test('the #274 shape: an older recorded digest binds after a policy edit on main when neither selection holds a PR file', () => { + for (const after of [['receipts/**', 'results/**'], ['results/**']]) { + const { repo, body } = shipped(CODE, ['receipts/**']); + const recorded = receiptIn(body).review_exclude!.digest; + expect(bind(repo, body).ok).toBe(true); + policyOnBaseOnly(repo, policyText(after)); + const now = JSON.parse(plan(repo, ['--json']).out).review_exclude.digest; + expect({ after, moved: typeof now === 'string' && now !== recorded }).toEqual({ after, moved: true }); + const b = bind(repo, body); + expect({ after, ok: b.ok, message: b.ok ? '' : b.message }).toEqual({ after, ok: true, message: '' }); + } }, 240_000); - test('an unreadable base policy still refuses a receipt with no digest', () => { - const { repo, body } = shipped(CODE); - policyOnBaseOnly(repo, '{ "review_exclude_paths": "receipts/**" }\n'); - const b = bind(repo, body); - expect(b.ok).toBe(false); - if (b.ok) return; - expect(b.code).toBe('POLICY_INVALID'); + test('a recorded digest that differs still refuses when the recorded or the current selection holds a PR file', () => { + const cases: Array<{ name: string; files: Record; after: string[] }> = [ + { name: 'recorded excludes a PR file', files: { 'receipts/a/result.json': '{"ok": true}\n' }, after: ['results/**'] }, + { name: 'current excludes a PR file', files: { 'results/a.json': '{"ok": true}\n' }, after: ['receipts/**', 'results/**'] }, + { name: 'both keep a PR file', files: { 'receipts/run.js': 'exports.run = 1;\n' }, after: ['receipts/**', 'results/**'] }, + ]; + for (const c of cases) { + const { repo, body } = shipped({ ...CODE, ...c.files }, ['receipts/**']); + const recorded = receiptIn(body).review_exclude!.digest; + policyOnBaseOnly(repo, policyText(c.after)); + const b = bind(repo, body); + expect({ name: c.name, ok: b.ok, code: b.ok ? '' : b.code }).toEqual({ name: c.name, ok: false, code: 'RECEIPT_INVALID' }); + if (!b.ok) expect(b.message).toMatch(new RegExp(`^field=review_exclude expected=[0-9a-f]{64} recorded=${recorded} `)); + } + }, 240_000); + + test('an unreadable base policy still refuses a receipt with no digest or an older one', () => { + for (const basePolicy of [null, ['receipts/**']]) { + const { repo, body } = shipped(CODE, basePolicy); + policyOnBaseOnly(repo, '{ "review_exclude_paths": "receipts/**" }\n'); + const b = bind(repo, body); + expect({ basePolicy, ok: b.ok, code: b.ok ? '' : b.code }).toEqual({ basePolicy, ok: false, code: 'POLICY_INVALID' }); + } }, 240_000); }); @@ -725,6 +748,55 @@ describe('a branch cannot hide code under a base pattern', () => { expect(blind.kept.filter(k => k.reason === 'gate-scan-unavailable').map(k => k.path).sort()).toEqual([...EXCLUDED, 'receipts/b/result.json', 'receipts/d/fixture.json'].sort()); }, 120_000); + // Value: protects=the named-by-code scan reads only the code and test files that can name a candidate, so its cost follows + // the consumers and not the data a results PR adds; fails_when=git grep reads the whole tree, so 4,354 receipts and 30 MB of + // data under the patterns are searched for every candidate path (GST-1435: harness-evals #269 ran 278 s past the 120 s limit + // and kept every file as consumer-scan-unavailable), or the scoped scan stops finding a full-path reference; why_new=GST-1435; + // seam=reviewExclude in head mode, observed through GIT_TRACE; no_claim=wall-clock time (the PR body has the timings) + test('the consumer scan greps only consumer files, never the data under the patterns, and still finds full-path references', () => { + const repo = census(); + const bulk: Record = {}; + for (let k = 0; k < 200; k++) bulk[`receipts/bulk/${k}/result.json`] = JSON.stringify({ next: `receipts/bulk/${k + 1}/result.json`, peer: 'receipts/b/result.json' }) + '\n'; + commit(repo, bulk, 'bulk receipts'); + const head = repo.git('rev-parse', 'HEAD'); + const baseSha = repo.git('rev-parse', 'origin/main'); + const c = collectCommit(repo.work, baseSha, head); + const trace = path.join(repo.root, 'git-trace.log'); + const prior = process.env.GIT_TRACE; + process.env.GIT_TRACE = trace; + let x: ReturnType; + try { x = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged: new Set(), context: 'test' }); } + finally { if (prior === undefined) delete process.env.GIT_TRACE; else process.env.GIT_TRACE = prior; } + expect(x.state).toBe('active'); + if (x.state !== 'active') return; + expect(x.kept.filter(k => k.reason !== 'executable')).toEqual([{ path: 'receipts/b/result.json', reason: 'named-by-code' }, { path: 'receipts/d/fixture.json', reason: 'named-by-code' }]); + expect(x.excluded.has('receipts/bulk/7/result.json')).toBe(true); + const greps = fs.readFileSync(trace, 'utf8').split('\n').filter(l => /built-in: git( -c \S+)* grep /.test(l)); + expect(greps.length).toBeGreaterThan(0); + const read = greps.map(l => [...(l.split(' -- ')[1] ?? '').matchAll(/:\(top,literal\)([^' ]+)/g)].map(m => m[1]!)); + expect(read.every(paths => paths.length > 0)).toBe(true); + const all = new Set(read.flat()); + expect(['lib/reader.js', 'test/d.test.js'].every(p => all.has(p))).toBe(true); + expect([...all].filter(p => p.startsWith('receipts/') && p !== 'receipts/run.sh')).toEqual([]); + }, 120_000); + + // GST-1435 review (outside-voice#1): a consumer whose name is not valid UTF-8 comes back from ls-tree as U+FFFD, so its literal + // pathspec matches nothing; fails_when=the scan treats that as no match and excludes the data the file names instead of keeping + // every candidate; seam=reviewExclude in head mode + test.skipIf(process.platform === 'win32' || process.platform === 'darwin')('a consumer whose name is not valid UTF-8 keeps the candidates, not excludes them', () => { + const repo = census(); + commit(repo, { 'receipts/z/result.json': '{}\n' }, 'receipt'); + fs.writeFileSync(Buffer.concat([Buffer.from(path.join(repo.work, 'lib/reader-')), Buffer.from([0xff]), Buffer.from('.js')]), "read('receipts/z/result.json')\n"); + commit(repo, {}, 'reader with a non-UTF-8 name'); + const head = repo.git('rev-parse', 'HEAD'); + const baseSha = repo.git('rev-parse', 'origin/main'); + const c = collectCommit(repo.work, baseSha, head); + const x = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged: new Set(), context: 'test' }); + expect(x.state).toBe('active'); + if (x.state !== 'active') return; + expect(x.excluded.has('receipts/z/result.json')).toBe(false); + }, 120_000); + test.skipIf(process.platform === 'win32' || process.getuid?.() === 0)('a live binary file whose bytes cannot be read is kept, not excluded', () => { const repo = census(); const gz = path.join(repo.work, 'receipts/a/shot.png');