Skip to content

fix(runner): guard the test_terminal.eigs invocation (fixes #656)#659

Merged
InauguralPhysicist merged 1 commit into
InauguralSystems:mainfrom
Nitjsefnie-OSC:fix-656-terminal-guard
Jul 17, 2026
Merged

fix(runner): guard the test_terminal.eigs invocation (fixes #656)#659
InauguralPhysicist merged 1 commit into
InauguralSystems:mainfrom
Nitjsefnie-OSC:fix-656-terminal-guard

Conversation

@Nitjsefnie

Copy link
Copy Markdown
Contributor

Fixes #656 — thanks for first refusal. One line plus the coat removal:

TM_OUTPUT=$($EIGS_TMO ./eigenscript ../tests/test_terminal.eigs </dev/null 2>&1); TM_OUTPUT_RC=$?

matching the guarded :93 shape, and the CLAUDE.md stdin-folklore rule is deleted verbatim since it stops being true.

Your four "done when" items, all demonstrated

  • [60] Terminal Builtins: 10/10 in a normal run.
  • Full suite: 2891/2891 (base without tests: derive block tallies from their own output (fixes #654) #657's derived tallies — if that merges first, this rebases trivially; the hunks are one line apart in spirit but disjoint in practice).
  • The wedge repro, before/after: sleep 300 | bash run_all_tests.sh under a bounded outer timeout — BEFORE: did not complete, log frozen exactly at [60] Terminal Builtins, zero RESULTS lines, and zero orphaned interpreters after group-kill cleanup (pgrep -a eigenscript empty). AFTER: the same held-open-stdin invocation runs to a green RESULTS: 2891/2891. (Measurement note: with your exact repro the outer wrapper can't exit before sleep 300 does regardless of the fix — pipeline semantics — so "completes" is correctly judged by the suite emitting its RESULTS line.)
  • Mutation: reverting only the runner line (keeping the doc deletion) re-wedges the repro at [60]; restoring completes green — the guard is load-bearing, not coincidental.

bash -n clean; shellcheck absent on this box (stated, per house custom by now). One test-harness pitfall worth passing on: setsid without --wait forks-and-exits so $! tracks a dead pid — the first BEFORE attempt falsely looked "completed" with an empty log; setsid --wait + writing the inner PGID to a file for group-kill cleanup is the reliable shape.


🤖 This PR was written with AI assistance (Claude), directed and verified by a human-supervised workflow.

…s#656)

The [60] Terminal Builtins invocation was the last unguarded interpreter
call in the runner: it lacked both the $EIGS_TMO runaway backstop (InauguralSystems#616)
and </dev/null. Because test_terminal.eigs reads stdin, any environment
where stdin is a held-open pipe (a captured or backgrounded CI-ish shell)
made it block forever, wedging the whole suite rather than failing.

Give :2073 the same guarded shape as the check_eigs_suite call at :93 —
$EIGS_TMO and </dev/null — and delete the now-false CLAUDE.md stdin rule,
which documented this bug as an invocation-discipline constraint.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Verified and merging. Fast turnaround — thank you.

I checked the one claim that had to be true before deleting a documented rule, since removing the CLAUDE.md line means the runner has to be robust on its own. Your wording — "the last unguarded invocation shape in the runner" — is a bit loose: there are plenty of bare ./eigenscript invocations left (:134, :164, :201, :2103, :2509, …). But the substance is right, and that is what matters: :2073 was the last unguarded invocation of a test that actually reads stdin.

I confirmed it rather than reasoned about it, with a FIFO holding the write end open so the reader could be killed independently:

test bare invocation held-open stdin
test_ui (:2103) yes completes rc=0 in 0s
test_proc_stream (:2509) yes completes rc=0 in 0s
test_terminal (:2073) yes WEDGED, rc=124

test_ui's input of is lib/ui's input widget and proc_stream's read_line reads a subprocess pipe — neither touches stdin. So test_terminal was the only wedger, it is now guarded, and the rule is genuinely obsolete rather than merely inconvenient. Deletion approved on that basis.

Your setsid note is a good catch and worth passing on in the other direction too: I hit the sibling pitfall writing my own check — sleep 60 | timeout 20 eigenscript ... keeps the pipeline alive after timeout kills the interpreter, so the harness hangs even when the test does not. Exactly the "the outer wrapper cannot exit before sleep 300 does" semantics you flagged. A FIFO with a separately-killable holder is the shape that actually isolates the reader.

Worth noting what this issue was: a bug we had documented as a constraint and lived with. You paid 11 minutes to rediscover it because we wrote the workaround into CLAUDE.md instead of fixing the one line. Good find, and the coat removal is the better half of the PR.

If you find the project useful or enjoyed contributing, a star helps others discover it.

@InauguralPhysicist
InauguralPhysicist merged commit 5ea9d71 into InauguralSystems:main Jul 17, 2026
16 checks passed
InauguralPhysicist added a commit that referenced this pull request Jul 18, 2026
…640, #641) (#662)

Each returned a plausible-but-wrong number at rc=0 — the highest-yield
failure class. All four were found in the 2026-07 adversarial review and
each fix ships with a regression test that fails on the pre-fix library.

#639 punnett_square: Aa x Aa read 1:1:1:1 instead of the canonical 1:2:1.
  The two heterozygous boxes "Aa" and "aA" are the same genotype but were
  keyed distinctly. Canonicalize each genotype (dominant/lower-ord allele
  first) so they fold; Aa is now 0.5 with no separate "aA" key.

#640 power_iteration: returned |lambda|, not lambda. It read the eigenvalue
  off |w| = |A v|, which drops the sign — diag(-5, 1) reported 5, which is
  not an eigenvalue of the matrix at all. Use the Rayleigh quotient
  (lambda = vᵀ(A v) = v·w for unit-norm v), summed from the pre-update v.
  Positive spectra are unaffected (both agree there).

#638 correlation: with mismatched lengths it paired min(len) elements but
  centered them on the FULL-list means, so unpaired tail elements corrupted
  the covariance. Center on the paired prefix instead.

#641 two degenerate-input contracts returning plausible-but-wrong numbers:
  - factorial of a negative returned 1 (via `n <= 1`); it is undefined, so
    return the 0 sentinel — impossible for a real factorial (always >= 1),
    matching combinations' return-0-on-invalid convention.
  - time_of_flight could return a NEGATIVE time when both roots of
    x = v0 t + ½ a t² are negative (target never reached going forward).
    Return the smallest NON-NEGATIVE root, or the 0 sentinel the function
    already uses for the no-real-root (disc < 0) case.

Tests: 8 known-answer regressions added to test_stem_accuracy.eigs (each
verified to fail with its fix reverted), plus test_biology's Punnett check
strengthened to lock in 1:2:1 and assert no "aA" key. The stem-accuracy
block emits no "Tests: N" line so its runner tally is a hand-synced literal
(#654's documented-fallback class): bumped 123 -> 131, and guarded its
invocation with $EIGS_TMO + </dev/null to match #659.

Suite 3075/3075 release. No C changed; behavior is pure-.eigs library logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: run_all_tests.sh invokes test_terminal.eigs unguarded — wedges forever when stdin is a held-open pipe

2 participants