From cf4de1f3660b4706383ee5b84f519186e729d92e Mon Sep 17 00:00:00 2001 From: test Date: Mon, 17 Aug 2026 20:35:25 +0000 Subject: [PATCH 1/3] Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree Triage of PR #785 found an npm-install.log committed at the repo root. It was removed by hand, but nothing in the toolchain had objected to it: it sits outside the package files allowlist, touches no code path, and .gitignore carried no rule for tool transcripts, so git status listed it as an ordinary new file. Only a human reading the diff caught it. This closes the class from both ends. .gitignore now ignores *.log, so an install/test/typecheck transcript never reaches git add. test/core/ repo-scratch-hygiene.test.js asserts both halves: no .log is tracked, and the ignore rule actually covers the paths that showed up (root and nested). Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 7 +++ test/core/repo-scratch-hygiene.test.js | 81 ++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 test/core/repo-scratch-hygiene.test.js diff --git a/.gitignore b/.gitignore index f862ff94..38c175bb 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,10 @@ types/ # Donor/reference codebase kept out of the active HypAware repo collectivus/ + +# Tool transcripts: install, test, and typecheck output redirected to a file +# while working. Never source, and nothing else in the toolchain objects to one +# (they are outside the package `files` allowlist and touch no code path), so a +# `git add -A` used to be enough to commit one. `test/core/repo-scratch-hygiene.test.js` +# holds the other half of this: no `.log` is tracked. +*.log diff --git a/test/core/repo-scratch-hygiene.test.js b/test/core/repo-scratch-hygiene.test.js new file mode 100644 index 00000000..5b0bd3b8 --- /dev/null +++ b/test/core/repo-scratch-hygiene.test.js @@ -0,0 +1,81 @@ +// @ts-check + +// Triage of PR #785 found an `npm-install.log` committed at the repo root: a +// reviewer's install transcript, 11 lines ending `INSTALL EXIT=0`, swept up by +// a `git add -A`. Nothing in the toolchain objected. It is excluded from the +// published file set (`package.json` `files`), so `npm pack` stays clean; it +// touches no code path, so no test moved; and `.gitignore` carried no rule for +// tool transcripts, so `git status` listed it as an ordinary new file. The only +// thing that caught it was a human reading the diff, which is exactly the check +// that is not there next time. +// +// This is the gate for both halves of that, per issue #786: the tree carries no +// tool transcript today, and `.gitignore` refuses one tomorrow. The second half +// is the one that matters - a lint that only notices after the fact still needs +// someone to run it on the right branch, while an ignore rule means the file +// never reaches `git add` in the first place. +// +// It is a lint on a property of the repository rather than a behavior check, in +// the shape of `house-style-em-dash.test.js`. + +import test from 'node:test' +import assert from 'node:assert/strict' +import path from 'node:path' +import { execFileSync, spawnSync } from 'node:child_process' +import { fileURLToPath } from 'node:url' + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..') + +/** @returns {string[]} repo-relative paths of every tracked file */ +function trackedFiles() { + const out = execFileSync('git', ['ls-files', '-z'], { cwd: REPO_ROOT, encoding: 'utf8' }) + return out.split('\0').filter(f => f !== '') +} + +/** + * Whether `.gitignore` (and friends) would keep `relPath` out of a commit. + * + * `git check-ignore` exits 0 when a path is ignored and 1 when it is not, so + * the status is the answer and a nonzero exit is not a failure to report. + * `--no-index` is deliberately not passed: the probe paths below do not exist + * and are not tracked, so the plain query answers the question a contributor + * actually faces, which is what `git add` will do with a fresh file. + * + * @param {string} relPath + * @returns {boolean} + */ +function isIgnored(relPath) { + const result = spawnSync('git', ['check-ignore', '-q', '--', relPath], { cwd: REPO_ROOT }) + return result.status === 0 +} + +test('no tool transcript is tracked in the repo', () => { + const found = trackedFiles().filter(f => f.endsWith('.log')) + assert.deepEqual(found, [], 'a `.log` file is a tool transcript, not source; ' + + `delete it and let \`.gitignore\` hold the line:\n ${found.join('\n ')}`) +}) + +test('.gitignore refuses a tool transcript', () => { + // The root case is the one that actually happened; the nested cases prove the + // rule is not anchored to the root, because scratch lands in subdirectories + // at least as often as it lands beside `package.json`. + const probes = [ + 'npm-install.log', + 'npm-test.log', + 'typecheck.log', + 'x/npm-test.log', + 'src/core/cli/debug.log', + ] + const unguarded = probes.filter(p => !isIgnored(p)) + assert.deepEqual(unguarded, [], 'these paths would be committable by a stray ' + + `\`git add -A\`; \`.gitignore\` needs a rule covering them:\n ${unguarded.join('\n ')}`) +}) + +test('the probe distinguishes ignored from tracked paths', () => { + // A probe that answered "ignored" for everything would pass the rule above + // forever, including on a repo with no `.gitignore` at all. + assert.ok(!isIgnored('package.json'), 'expected a tracked source file to read as not ignored') + assert.ok(isIgnored('hypaware-9.9.9.tgz'), 'expected an existing ignore rule (`*.tgz`) to read as ignored') + const files = trackedFiles() + assert.ok(files.length > 500, `expected the tracked tree, found ${files.length} files`) +}) From c4a5a62e71427451d923fc0c55a36138819e5b80 Mon Sep 17 00:00:00 2001 From: neutral Date: Mon, 17 Aug 2026 20:58:36 +0000 Subject: [PATCH 2/3] Ignore-rule probe reads the rules, not the index, so a tracked .log names the right fix --- test/core/repo-scratch-hygiene.test.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/core/repo-scratch-hygiene.test.js b/test/core/repo-scratch-hygiene.test.js index 5b0bd3b8..bbb72137 100644 --- a/test/core/repo-scratch-hygiene.test.js +++ b/test/core/repo-scratch-hygiene.test.js @@ -37,15 +37,21 @@ function trackedFiles() { * * `git check-ignore` exits 0 when a path is ignored and 1 when it is not, so * the status is the answer and a nonzero exit is not a failure to report. - * `--no-index` is deliberately not passed: the probe paths below do not exist - * and are not tracked, so the plain query answers the question a contributor - * actually faces, which is what `git add` will do with a fresh file. + * + * `--no-index` is passed so the answer is about the ignore *rules* alone. By + * default `check-ignore` consults the index first and reports any tracked path + * as not ignored, whatever `.gitignore` says. That is the wrong question here, + * and it fails in the one case this file exists for: once a `.log` is tracked, + * the rule probe would flip to "unguarded" and send the reader off to add a + * `.gitignore` rule that is already there. Tracked transcripts are the other + * test's job, and its message names the fix (delete the file). * * @param {string} relPath * @returns {boolean} */ function isIgnored(relPath) { - const result = spawnSync('git', ['check-ignore', '-q', '--', relPath], { cwd: REPO_ROOT }) + const result = spawnSync('git', ['check-ignore', '-q', '--no-index', '--', relPath], { cwd: REPO_ROOT }) + if (result.error) throw result.error return result.status === 0 } @@ -71,10 +77,12 @@ test('.gitignore refuses a tool transcript', () => { `\`git add -A\`; \`.gitignore\` needs a rule covering them:\n ${unguarded.join('\n ')}`) }) -test('the probe distinguishes ignored from tracked paths', () => { +test('the probe distinguishes ignored from unignored paths', () => { // A probe that answered "ignored" for everything would pass the rule above - // forever, including on a repo with no `.gitignore` at all. - assert.ok(!isIgnored('package.json'), 'expected a tracked source file to read as not ignored') + // forever, including on a repo with no `.gitignore` at all. Under + // `--no-index` both assertions read the rules and nothing else, so neither + // can be satisfied by the path's tracked status. + assert.ok(!isIgnored('package.json'), 'expected a source file no rule matches to read as not ignored') assert.ok(isIgnored('hypaware-9.9.9.tgz'), 'expected an existing ignore rule (`*.tgz`) to read as ignored') const files = trackedFiles() assert.ok(files.length > 500, `expected the tracked tree, found ${files.length} files`) From 599a08f587c2c353b9be2f6567ef00a077eb86ff Mon Sep 17 00:00:00 2001 From: test Date: Mon, 17 Aug 2026 21:59:28 +0000 Subject: [PATCH 3/3] Ignore probe answers from the committed .gitignore, not a per-clone ignore source --- test/core/repo-scratch-hygiene.test.js | 53 +++++++++++++++++++++----- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/test/core/repo-scratch-hygiene.test.js b/test/core/repo-scratch-hygiene.test.js index bbb72137..5a42e60c 100644 --- a/test/core/repo-scratch-hygiene.test.js +++ b/test/core/repo-scratch-hygiene.test.js @@ -33,10 +33,14 @@ function trackedFiles() { } /** - * Whether `.gitignore` (and friends) would keep `relPath` out of a commit. + * Which ignore rule, if any, would keep `relPath` out of a commit. * * `git check-ignore` exits 0 when a path is ignored and 1 when it is not, so - * the status is the answer and a nonzero exit is not a failure to report. + * the status is the answer and exit 1 is not a failure to report. Anything else + * is a real git failure (a `safe.directory` refusal, a checkout with no `.git`) + * and is raised rather than collapsed into "not ignored", which would fail the + * rule test below with a message sending the reader to fix a `.gitignore` that + * is already correct. * * `--no-index` is passed so the answer is about the ignore *rules* alone. By * default `check-ignore` consults the index first and reports any tracked path @@ -46,13 +50,31 @@ function trackedFiles() { * `.gitignore` rule that is already there. Tracked transcripts are the other * test's job, and its message names the fix (delete the file). * + * The matching rule's *source* is returned, not just a yes/no, because the whole + * ignore stack answers this question and only one layer of it is the repo's. + * `*.log` is common in a personal `core.excludesFile` (it ships in widely copied + * global templates), and `.git/info/exclude` is per-clone too. Either would + * stand in for the committed rule, so a `.gitignore` that had lost it would + * still read green on that machine while every fresh checkout was unguarded. + * The global file is pinned to `/dev/null` here and the source is asserted + * below, so only the committed `.gitignore` can satisfy the gate. + * * @param {string} relPath - * @returns {boolean} + * @returns {{ ignored: boolean, source: string, pattern: string }} */ -function isIgnored(relPath) { - const result = spawnSync('git', ['check-ignore', '-q', '--no-index', '--', relPath], { cwd: REPO_ROOT }) +function ignoreRule(relPath) { + // `-z` output requires `--stdin`, and it is worth the stdin round trip: the + // default format is `source:line:pattern\tpath`, which is ambiguous for any + // source or pattern containing a colon. + const args = ['-c', 'core.excludesFile=/dev/null', 'check-ignore', '-v', '-z', '--no-index', '--stdin'] + const result = spawnSync('git', args, { cwd: REPO_ROOT, input: `${relPath}\0`, encoding: 'utf8' }) if (result.error) throw result.error - return result.status === 0 + if (result.status !== 0 && result.status !== 1) { + throw new Error(`git check-ignore exited ${result.status}: ${(result.stderr || '').trim()}`) + } + if (result.status !== 0) return { ignored: false, source: '', pattern: '' } + const [source, , pattern] = result.stdout.split('\0') + return { ignored: true, source, pattern } } test('no tool transcript is tracked in the repo', () => { @@ -72,7 +94,12 @@ test('.gitignore refuses a tool transcript', () => { 'x/npm-test.log', 'src/core/cli/debug.log', ] - const unguarded = probes.filter(p => !isIgnored(p)) + const unguarded = [] + for (const probe of probes) { + const rule = ignoreRule(probe) + if (!rule.ignored) unguarded.push(`${probe} (no rule matches)`) + else if (rule.source !== '.gitignore') unguarded.push(`${probe} (matched by ${rule.source}, which is not committed)`) + } assert.deepEqual(unguarded, [], 'these paths would be committable by a stray ' + `\`git add -A\`; \`.gitignore\` needs a rule covering them:\n ${unguarded.join('\n ')}`) }) @@ -80,10 +107,16 @@ test('.gitignore refuses a tool transcript', () => { test('the probe distinguishes ignored from unignored paths', () => { // A probe that answered "ignored" for everything would pass the rule above // forever, including on a repo with no `.gitignore` at all. Under - // `--no-index` both assertions read the rules and nothing else, so neither + // `--no-index` these read the rules and nothing else, so neither direction // can be satisfied by the path's tracked status. - assert.ok(!isIgnored('package.json'), 'expected a source file no rule matches to read as not ignored') - assert.ok(isIgnored('hypaware-9.9.9.tgz'), 'expected an existing ignore rule (`*.tgz`) to read as ignored') + assert.equal(ignoreRule('package.json').ignored, false, + 'expected a source file no rule matches to read as not ignored') + // The other direction, deliberately not borrowed from an unrelated rule such + // as `*.tgz`: a probe stuck on "ignored" would have to name this file's own + // rule to get here, and a maintainer reorganizing pack output cannot redden a + // transcript-hygiene test. + assert.equal(ignoreRule('npm-install.log').pattern, '*.log', + 'expected the transcript rule itself to be the one doing the work') const files = trackedFiles() assert.ok(files.length > 500, `expected the tracked tree, found ${files.length} files`) })