fix: keep binary files at code paths under review_exclude_paths

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/gst-508-review-exclude-paths.md

@@ -2,7 +2,7 @@
 
 ### Added
 - **`review_exclude_paths` in `.gstack/ship-policy.json`** (`["receipts/**"]`, with `min_gstack_version` at least the release that adds it). It is read from the base branch only, so a branch cannot exclude its own files; a branch that adds the key gets a normal review and `REVIEW_EXCLUDE: none (head sets review_exclude_paths; …)`. Patterns are git glob pathspecs, root-anchored, and git itself lists the matches.
-- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary (a branch's `.gitattributes` cannot make a text file count as binary). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too. A basename, a directory reference or a data manifest that lists receipts does not keep anything.
+- **Only content is excluded**: data files, docs, `.log` files and files whose bytes are binary and whose extension is not source or script (a branch's `.gitattributes` cannot make a text file count as binary, and a NUL byte cannot hide a `.js` file). Untracked files are never part of the selection. Code, tests, executables, symlinks, submodule commits, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too. A basename, a directory reference or a data manifest that lists receipts does not keep anything.
 - **`gstack-review-plan` prints `REVIEW_EXCLUDE:`** with the counts, the excluded files by kind, the kept files and a digest. Every reader's `DIFF_CMD:`, the fixture command the security specialist and the Red Team use, and the outside voice's diff leave the files out; kept files come back through `DIFF_KEEP:`. Every brief says what was left out. The kept-file command is bounded at 24,000 argument bytes, and a pattern over that bound is not applied and the plan says why.
 - **Excluded lines no longer trigger the numeric-claim scan or the targeted money, gate and auth classes**, and they count as docs for the review depth.
 - **The receipt records `review_exclude`.** `gstack-review-dispatch verify` prints `REVIEW_EXCLUDE_RECORD: digest=<d>` from the briefs the readers got, and `gstack-ship-receipt write --review-exclude <d>` must match the digest HEAD has under the base policy (`RECEIPT_INVALID field=review_exclude`, nothing written). Carry and the queue compare the digest, not the base commit, so a base merge that leaves the policy alone still carries.

docs/ship-policy.md

@@ -1130,7 +1130,7 @@ A pattern that matches no changed file is listed as `unmatched=` on the plan lin
 **What is excluded, and what is kept.** A matched file is excluded only when it
 is content: a data file (JSON, JSONL, CSV, TSV, images), a docs-class file, a
 `.log` file, or a file git reports as binary (`-\t-` in `--numstat`; readers would only see `Binary files differ`) whose bytes hold a NUL in their first 8,000, so a branch's `.gitattributes` cannot make a text file binary content. A binary file is content even under
-`config/` or in a skill directory; only the first four reasons below keep it.
+`config/` or in a skill directory; only the first five reasons below, or a source or script extension (`code`), keep it.
 Everything else stays in every reader's diff with one reason code:
 
 | Reason | Kept because |
@@ -1142,7 +1142,7 @@ Everything else stays in every reader's diff with one reason code:
 | `agent-prose` | it is a skill, section, resolver, template or rule file |
 | `config` | it is a data-shaped config file (`package.json`, `tsconfig*.json`, `config/`, dotfiles) |
 | `gate-tagged` | the gate scan tagged one of its hunks |
-| `code` | it is code or a test, not content |
+| `code` | it is code or a test, not content; a binary file with a source or script extension (`.js`, `.py`, `.sh`, `.wasm`) counts as code |
 | `secret-line` | an added line reads a credential (the targeted pass's secret rule) |
 | `named-by-code` | a code or test file that stays in review (not docs, a data file, a `.log` or binary, and not itself excluded) names its full repository path, at the head or the merge base |
 | `consumer-scan-unavailable` | the `git grep` for those names failed, so every candidate is kept |

lib/review-exclude.ts

@@ -165,6 +165,9 @@ function contentKind(i: ExcludeInput, p: string, binary: ReadonlySet<string>): C
   return classifyPath(p, i.classRoot) === 'docs' ? 'docs' : null;
 }
 
+/** Source and script extensions: a NUL in one does not make it excludable binary content. */
+const CODE_EXT = /\.(?:[cm]?[jt]sx?|py|rb|go|rs|java|kts?|scala|swift|exs?|php|cs|c|h|cc|cpp|hpp|mm?|dart|pl|pm|lua|sh|bash|zsh|fish|ps1|bat|cmd|vue|svelte|html?|wasm)$/i;
+
 /** Why a matched file is kept by its path, mode and lines alone; null for content the policy may exclude. A binary file readers would only see as "Binary files differ" is content unless its directory, mode or gate tags keep it. */
 function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet<string>): KeptReason | null {
   if (/^\.(gstack|github)\//.test(p)) return 'policy-dir';
@@ -172,7 +175,7 @@ function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet<
   if (m.oldMode === '100755' || m.newMode === '100755') return 'executable';
   if ([m.oldMode, m.newMode].some(x => x === '120000' || x === '160000')) return 'link';
   if (i.gateTagged?.has(p)) return 'gate-tagged';
-  if (binary.has(p)) return null;
+  if (binary.has(p)) return CODE_EXT.test(p) ? 'code' : null;
   if (isAgentProse(p)) return 'agent-prose';
   if (isDataConfig(p)) return 'config';
   if (contentKind(i, p, binary) === null) return 'code';

test/review-exclude.test.ts

@@ -279,7 +279,7 @@ describe('the receipt binds the selection the reviewers read', () => {
 describe('a branch cannot hide code under a base pattern', () => {
   // Value: protects=a branch cannot hide code or a link under a base-declared pattern, and untracked files never move the
   //   selection away from the committed head the receipt reads; fails_when=the branch's .gitattributes makes a code file
-  //   binary content, a symlink is excluded as data, or an untracked file under a pattern changes the plan's digest;
+  //   binary content, a NUL-bearing code file is excluded as binary, a symlink is excluded as data, or an untracked file under a pattern changes the plan's digest;
   //   why_new=/ship pass-1 security and Red Team findings on GST-508, each reproduced before the fix; seam=bin/gstack-review-plan;
   //   no_claim=gitlinks (the same mode rule, no fixture)
   test('a branch .gitattributes cannot make code binary, a symlink is kept, and an untracked file leaves the digest alone', () => {
@@ -297,5 +297,16 @@ describe('a branch cannot hide code under a base pattern', () => {
     write(repo.work, 'receipts/stray/result.json', '{"ok": true}\n');
     expect(JSON.parse(plan(repo, ['--json']).out).review_exclude.digest).toBe(p.review_exclude.digest);
   }, 120_000);
+  test('a binary file at a code path stays kept and still forces full depth; a .gz stays binary content', () => {
+    const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo);
+    fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true });
+    fs.writeFileSync(path.join(repo.work, 'assets/loader.js'), Buffer.concat([Buffer.from('/* \0 */ require("child_process").execSync("id");\n')]));
+    fs.writeFileSync(path.join(repo.work, 'assets/real.gz'), GZ);
+    commit(repo, {});
+    const p = JSON.parse(plan(repo, ['--json']).out);
+    expect(p.review_exclude.kept_files).toEqual([{ path: 'assets/loader.js', reason: 'code' }]);
+    expect(p.review_exclude.content).toMatchObject({ binary: 1 });
+    expect(p).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'assets/loader.js' })]) });
+  }, 120_000);
 });