Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree

[philcunliffe]

Issue #786 carried two deferred findings from triage of PR #785. One is fixed here, one is deliberately left alone.

Finding 1: the stray npm-install.log (fixed, behavioural)

The file itself was already removed from fix/issue-783 in a1787e0, so nothing lands on master when #785 merges. What the issue left open is the second half of its own recommendation: "consider adding npm-install.log (or *.log) to .gitignore so a reviewer's install transcript cannot be committed again." That was never done, and the root cause is exactly there.

Nothing in the toolchain objects to a committed transcript. It is outside the package.json files allowlist so npm pack stays clean, it touches no code path so no test moves, and .gitignore had no rule for it so git status listed it as an ordinary new file. The only check that caught it was a human reading the diff, which is the check that is not there next time.

Fix. .gitignore now ignores *.log, so an install/test/typecheck transcript never reaches git add -A in the first place. test/core/repo-scratch-hygiene.test.js gates both halves: no .log is tracked, and the ignore rule actually covers the paths that show up in practice (repo root and nested). It is a lint on a property of the repository, written in the shape of the existing test/core/house-style-em-dash.test.js.

Proof. With the .gitignore change reverted and the test in place:

ok 1 - no tool transcript is tracked in the repo
not ok 2 - .gitignore refuses a tool transcript
ok 3 - the probe distinguishes ignored from tracked paths
# pass 2
# fail 1

The failure names the paths: npm-install.log, npm-test.log, typecheck.log, x/npm-test.log, src/core/cli/debug.log. With the .gitignore change applied:

ok 1 - no tool transcript is tracked in the repo
ok 2 - .gitignore refuses a tool transcript
ok 3 - the probe distinguishes ignored from tracked paths
# pass 3
# fail 0

Test 3 is the anti-vacuity check: it asserts the probe reports a tracked source file as not ignored and an already-covered pattern (*.tgz) as ignored, so a probe that answered "ignored" for everything cannot pass test 2.

No @ref was added. No LLP governs repository hygiene (the hypignore documents cover the product's capture-exclusion feature, not .gitignore), and per the house rule a ref must tell you something the code and filename do not.

Finding 2: LLP 0190 #eof-everywhere placement (deliberately unchanged, not a defect)

The issue records this as "closed as no-change", and the maintainer note repeats it: 0190's Systems: Onboarding, CLI line is narrower than the tree-wide EOF rule appended to it, but that is a discoverability preference, not a defect, and the named future move is to flip 0190 to Accepted when the wizard work settles and mint the next EOF-class change as its own LLP. That decision is a maintainer's to make and is gated on work that has not settled.

It is also not actionable from master yet: the #eof-everywhere section exists only on fix/issue-783, so nothing on master currently contains the text in question. Left untouched.

Note for #785

The new lint will fail on fix/issue-783 as it stands: that branch adds an x/ scratch directory containing x/npm-test.log and x/typecheck.log (alongside x/burst.mjs, x/pty_variants.mjs, x/msg.txt and others). That is the same class as the npm-install.log this issue is about, so the gate is doing its job. x/ itself is not added to .gitignore here, since a bare directory name that generic is too broad to claim tree-wide without a maintainer's call.

Verification

• npm test: 4219 pass, 0 fail, 1 skipped.
• npm run typecheck: clean.

One environment note, for anyone reproducing: a checkout whose node_modules lacks the dev dependencies fails 22 unrelated tests (parquet predicate pushdown and report rendering) and fails typecheck with TS2688: Cannot find type definition file for 'node'. Both are artifacts of the missing install, identical on unmodified master, and both clear after a real npm install. The numbers above are from a fully installed tree.

Fixes #786

[philcunliffe]

Verdict

The change is sound and the diagnosis is right: the root cause of npm-install.log really was the missing .gitignore rule, not the file, and the issue's own recommendation asked for exactly this. The two-halves framing (an ignore rule so it never reaches git add, plus a lint so a tracked one is caught) is the correct shape, and the sibling test/core/house-style-em-dash.test.js is the right model to copy.

Two findings. One was a real defect in the probe and is fixed and pushed. One is a merge-ordering hazard that this PR cannot fix by itself and that I want louder than a paragraph in the PR body.

---

Finding 1 (Medium, fixed): the ignore probe read the index, so a tracked .log reported the wrong fix

test/core/repo-scratch-hygiene.test.js:40-49 (at cf4de1f3) deliberately did not pass --no-index, on the stated grounds that "the probe paths below do not exist and are not tracked".

That premise is false in precisely the situation the file exists to catch. git check-ignore consults the index first and reports any tracked path as not ignored, whatever .gitignore says. So the moment a .log is tracked, the probe for that same path flips to "unguarded" and test 2 fails alongside test 1 - with a message that sends the reader to fix .gitignore, which is already correct and will not fix anything.

This is not hypothetical. Merging this PR into fix/issue-783 (PR #785, which tracks x/npm-test.log and x/typecheck.log) and running the lint at cf4de1f3:

not ok 1 - no tool transcript is tracked in the repo
    a `.log` file is a tool transcript, not source; delete it and let `.gitignore` hold the line:
not ok 2 - .gitignore refuses a tool transcript
    these paths would be committable by a stray `git add -A`; `.gitignore` needs a rule covering them:
# pass 1
# fail 2

Test 2's failure there is a false positive with an actively misleading remedy.

Fix (pushed as c4a5a62e): isIgnored now passes --no-index, so it answers the question its test's name asks - does a rule cover this pattern - and tracked transcripts stay test 1's job, whose message already names the right fix (delete the file). result.error is now rethrown rather than silently collapsing a spawn failure into "not ignored".

Verified in all four states:

• Clean branch: 3 pass, 0 fail.
• .gitignore rule reverted to origin/master, test kept: test 2 fails alone (the anti-vacuity proof still holds after the change).
• Merged with fix/issue-783: now one accurate failure naming x/npm-test.log and x/typecheck.log, with "delete it" as the remedy. Tests 2 and 3 pass.
• --no-index probe answers spot-checked directly: npm-install.log -> ignored, x/npm-test.log -> ignored, package.json -> not ignored, *.tgz -> ignored, identical on both trees.

Side benefit: test 3's !isIgnored('package.json') was previously satisfiable purely because package.json is tracked. Under --no-index both of its assertions read the rules and nothing else, so the anti-vacuity check is now strictly stronger than it was. It is renamed accordingly ("distinguishes ignored from unignored paths").

Finding 2 (Medium, deliberately left, needs a maintainer): merge order against #785

The PR body's "Note for #785" is accurate but understates the consequence, and it is disclosed only here, on the PR that is not blocked.

Ground truth: PR #785 (fix/issue-783, head a1787e0e) is labelled neutral:approved and held for a maintainer merge. It tracks a scratch directory x/ containing x/burst.mjs, x/invert.mjs, x/line_asker.fixed.js, x/line_asker.orig.js, x/msg.txt, x/order.mjs, x/pty_prog.mjs, x/pty_variants.mjs, and the two transcripts x/npm-test.log and x/typecheck.log. If #796 lands first, #785 goes red on no tool transcript is tracked in the repo, after approval, through no change of its own.

I agree with the PR's judgement that this is the gate doing its job rather than a reason to weaken it: those two files are the same class as the npm-install.log that issue #786 is about, and the tree-wide lint is worth more than the convenience of one in-flight branch. I also agree with not adding a bare x/ to .gitignore - a directory name that generic is a maintainer's call, not a reviewer's.

What I am not willing to call adequately handled is the disclosure. Nothing on #785 says this, and its neutral:approved label implies "ready to merge as-is". Concretely, for whoever merges:

• Merge Close the rl.question EOF-hang class: the wizard fork menu, both y/N confirms, and the OAuth paste #785 first, then Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree #796. This is the cheap path and needs nothing else.
• Or, if Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree #796 goes first, Close the rl.question EOF-hang class: the wizard fork menu, both y/N confirms, and the OAuth paste #785 needs git rm -r x/ (or at minimum git rm x/npm-test.log x/typecheck.log) before it can be merged. After the fix above, the failure is a single test naming exactly those two paths, so the required action is legible from the CI log.

I have deliberately not touched #785, per the scope of this round.

Left alone, with reasons

• No exemption mechanism. The sibling house-style-em-dash.test.js carries an EXEMPT_PREFIXES list with a documented rationale, so a legitimate exception lands as "a visible diff with an argument attached". This lint has no equivalent, so a future committed fixture that happens to end in .log (a captured daemon log used as parser input, say) would need both git add -f and a code change here. Low: there is no such fixture today, every .log the code writes goes to HYP_HOME/temp (src/core/daemon/logs.js:36, src/core/daemon/{macos,linux}.js:98-99), and inventing an unused exemption list now would be speculative. Noting it so the next person hitting it knows the design was considered.
• Blast radius of unanchored *.log. Checked, not a finding: zero tracked .log files on master or on this branch, no fixture depends on one, and the rule is outside the package.json files allowlist so npm pack is unaffected. The unanchored pattern (matching at any depth) is the right call given the actual incident was root-level but scratch typically is not.
• No @ref added. Correct as argued. Worth recording for the next reader that the sibling's // @ref LLP 0001#tooling [tests] is itself broken on master - llp/0001-adopting-llp.plan.md has no tooling anchor - so copying it would have propagated a dangling ref. That is a pre-existing master issue, out of scope here, and not something I changed.
• Finding 2 of issue Follow-up: deferred review findings from PR #785 #786 (LLP 0190 #eof-everywhere placement). Agreed and unchanged. The section only exists on fix/issue-783, so it is not editable from master, and 0190 is Status: Draft besides. Nothing to do.

House style

Checked against CLAUDE.md: no semicolons, no em dash anywhere in the diff (the repo-wide em-dash lint passes on the fixed tree), JSDoc types with no @typedef and no inline import() types, no new type imports needed.

---

Head after fix: c4a5a62e71427451d923fc0c55a36138819e5b80. The only file touched is test/core/repo-scratch-hygiene.test.js; .gitignore is unchanged from what this PR proposed.

[philcunliffe]

Verdict

Round 2 of 2. Round 1's --no-index fix is correct and complete - I re-verified it in every state it claimed, and it holds. But the anti-vacuity claim it signed off ("the anti-vacuity proof still holds after the change") was verified on one machine and does not hold in general: the probe read the whole ignore stack, so a contributor's global core.excludesFile could stand in for the committed rule. That is fixed and pushed. One finding remains open by design: the merge-ordering hazard against #785, which this PR cannot fix by itself.

The change is still the right shape and the diagnosis is still right. Approving the substance; the remaining item is a maintainer's merge decision, not a code defect.

---

Round 1's fix: re-verified, complete

ignoreRule (was isIgnored) passing --no-index is the right call and behaves as round 1 described. Re-checked all four states on c4a5a62e:

• Clean branch: 3 pass, 0 fail.
• .gitignore rule reverted to origin/master: test 2 fails alone, 2 pass / 1 fail.
• Merged with fix/issue-783: exactly one failure, test 1, naming x/npm-test.log and x/typecheck.log with "delete it" as the remedy. Tests 2 and 3 pass. This is the property round 1 was buying, and it is intact.
• Full suite on the branch: 4219 pass, 0 fail, 1 skipped. npm run typecheck clean.

No regression, nothing half-applied. The result.error rethrow is also present.

---

Finding 1 (Medium, fixed): the ".gitignore refuses a tool transcript" test did not test .gitignore

test/core/repo-scratch-hygiene.test.js:52-56 (at c4a5a62e) probed with a bare git check-ignore. That reads the entire ignore stack, not the repo's file: core.excludesFile (the user's global gitignore) and .git/info/exclude both answer it, and both are per-clone and uncommitted.

*.log is one of the most common lines in a personal global gitignore - it ships in widely copied global templates - so this is not a corner case. Demonstrated on the branch:

=== rule reverted, global ignore has *.log ===
ok 1 - no tool transcript is tracked in the repo
ok 2 - .gitignore refuses a tool transcript
ok 3 - the probe distinguishes ignored from unignored paths
# pass 3
# fail 0

Three green with the guard deleted from .gitignore. The failure mode is concrete: someone tidies .gitignore and drops the rule, npm test is green on their machine, and every fresh checkout is unguarded. It also means this PR's own reproduction recipe ("revert the rule, watch test 2 fail") was machine-dependent, and that test 3 - advertised as the anti-vacuity check - only excluded a probe that says "ignored" for everything, not one answered by an uncommitted source.

Fix (pushed as 599a08f5): the probe now pins core.excludesFile=/dev/null and returns the matching rule's source and pattern rather than a bare boolean, and test 2 asserts the source is .gitignore. Only the committed file can satisfy the gate; .git/info/exclude is excluded by the source assertion, the global file by both.

Verified across four states, with the hole state now behaving identically to the honest one:

state	before	after
clean branch	3 pass	3 pass
rule reverted, no global ignore	test 2 fails	tests 2+3 fail, each path tagged (no rule matches)
rule reverted, global *.log	3 pass (hole)	tests 2+3 fail, identical to above
rule present, global *.log	3 pass	3 pass
merged with fix/issue-783	1 accurate failure	1 accurate failure, unchanged

The result is now machine-independent, which is what the PR body already claims for it.

Finding 2 (Low, fixed): a git fatal was silently read as "not ignored"

Same line: return result.status === 0 collapsed every status other than 0/1 into false. check-ignore exits 128 on a real failure - a safe.directory / dubious-ownership refusal, or a tree with no .git (confirmed: fatal: not a git repository, exit=128). Test 2 would then fail with ".gitignore needs a rule covering them", pointing at a file that is already correct, and test 3's negative assertion would pass for the wrong reason.

This is the same class as the bug round 1 found - a probe failure disguised as a rule failure, with a misleading remedy - so it is worth closing alongside it. Status outside {0, 1} now throws with the captured stderr.

Finding 3 (Low, fixed): the anti-vacuity assertion was coupled to an unrelated rule

test/core/repo-scratch-hygiene.test.js:86 leaned on isIgnored('hypaware-9.9.9.tgz'), i.e. on *.tgz staying in .gitignore. A maintainer moving pack output under a /dist/ rule would redden a transcript-hygiene test with a message about *.tgz and no hint that the transcript rule is fine. Replaced with an assertion the test owns: the rule matching npm-install.log reports pattern *.log. Same anti-vacuity force (the probe must be able to answer "ignored"), no borrowed dependency.

---

Finding 4 (Medium, open, needs a maintainer): merge order against #785

Re-checked at review time and unchanged since round 1, so the disclosure is still accurate and still owed:

• PR Close the rl.question EOF-hang class: the wizard fork menu, both y/N confirms, and the OAuth paste #785 (fix/issue-783, head a1787e0e) is OPEN, MERGEABLE, labelled neutral:approved, held for a maintainer merge.
• It still tracks x/npm-test.log and x/typecheck.log (in an x/ scratch dir alongside x/burst.mjs, x/invert.mjs, x/line_asker.{fixed,orig}.js, x/msg.txt, x/order.mjs, x/pty_prog.mjs, x/pty_variants.mjs).

If #796 lands first, #785 goes red after approval, through no change of its own. I agree with this PR's judgement that this is the gate working, not a reason to weaken it - those two files are the same class as the npm-install.log issue #786 is about - and I agree with not adding a bare x/ to .gitignore, which is too generic a claim for a reviewer to make. But neutral:approved reads as "ready to merge as-is", and nothing on #785 says otherwise.

For whoever merges:

• Merge Close the rl.question EOF-hang class: the wizard fork menu, both y/N confirms, and the OAuth paste #785 first, then Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree #796. Cheap path, needs nothing else.
• Or if Tool transcripts cannot be committed again: .gitignore refuses *.log, with a lint on the tree #796 goes first, Close the rl.question EOF-hang class: the wizard fork menu, both y/N confirms, and the OAuth paste #785 needs git rm -r x/ (or at minimum git rm x/npm-test.log x/typecheck.log) first. Verified just now on the merged tree: the failure is a single test naming exactly those two paths with "delete it" as the remedy, so the required action is legible straight from the CI log.

I have deliberately not touched #785, per this round's scope.

Left alone, with reasons

• No exemption mechanism. The sibling test/core/house-style-em-dash.test.js carries EXEMPT_PREFIXES so an exception lands as "a visible diff with an argument attached". This lint has none, so a future legitimate .log fixture would need git add -f plus a code change here, against a message that only says "delete it". Low: no such fixture exists, every .log the code writes goes to HYP_HOME/temp, and inventing an unused exemption list now would be speculative. Round 1 reached the same conclusion; recording it so the next person knows it was considered twice, not missed.
• Case sensitivity. f.endsWith('.log') and the *.log pattern are both case-sensitive, so NPM-INSTALL.LOG is uncovered by both halves. Consistent rather than defective, and not worth a case-folding pass.
• files.length > 500. 1118 tracked files today, so the sanity floor has real headroom.
• Blast radius of unanchored *.log. Re-confirmed: zero tracked .log on master or this branch, and the package.json files allowlist (bin/, src/, hypaware-core/, types/, one .d.ts) contains none, so npm pack is unaffected.
• No @ref added. Correct as argued, and worth repeating that the sibling's // @ref LLP 0001#tooling [tests] is itself dangling on master (llp/0001-adopting-llp.plan.md has no tooling anchor), so copying it would have propagated a broken ref. Pre-existing, out of scope, unchanged.
• Finding 2 of issue Follow-up: deferred review findings from PR #785 #786 (LLP 0190 #eof-everywhere). Agreed and unchanged; the section only exists on fix/issue-783 and 0190 is still Draft.

House style

No semicolons, no em dash anywhere in the diff (repo-wide em-dash lint passes on the fixed tree), JSDoc types with no @typedef and no inline import() types. The new JSDoc return type is a structural literal, no type import needed. CI is ubuntu-latest only, so core.excludesFile=/dev/null is portable where it runs.

---

Head after fix: 599a08f587c2c353b9be2f6567ef00a077eb86ff. Only test/core/repo-scratch-hygiene.test.js changed (+43/-10); .gitignore is untouched from what this PR proposed. npm test 4219 pass / 0 fail, npm run typecheck clean.

Round 2 of a 2-round cap. Findings 1-3 are fixed here. Finding 4 is not a code defect and cannot be fixed from this branch - it is a merge-order call for a maintainer.

[philcunliffe]

Triage at head 6091db0cf34ae5889c8d445a87f34f00a86859ca (the post-review motion is only neutral's merge of green master; no PR content changed after round 2).

All in-review fixes re-verified against the current tree, including the round 2 machine-independence claim: with the .gitignore rule reverted and a live global *.log ignore that a bare check-ignore honors, the lint still fails 2 of 3, so only the committed rule can satisfy the gate. The hygiene test passes 3/3 at head, CI is green, and the tree tracks no .log.

Every residual finding is non-blocking. They are recorded in #826, including the one that needs a human decision: merge ordering against #785, which is still OPEN, neutral:approved, and still tracks x/npm-test.log and x/typecheck.log, so it goes red if this PR lands first. Merge #785 first, or git rm its two transcripts after. That is the gate working as designed, not a defect here.