[widechaos]

The exit-detection, rate-limit, and session-reset suites tested inline copies of loop functions, so production regressions could leave these tests green. This loads the functions directly from ralph_loop.sh using the existing extraction pattern and removes the copies, including the permission-denial variant. Production behavior is unchanged.

The explicit-false exit-signal fixture now stays below the independent five-indicator safety breaker, so it tests its intended condition against the real implementation.

Closes #374.

Validation:

• Targeted suites: 140 passed, 2 skipped.
• Independent temporary production mutations were caught in all three suites: test-saturation boundary, invocation-limit boundary, and stale exit-signal reset. The production file was restored afterward.
• Full unit/integration suite: 1348 passed, 6 skipped, 0 failures (Bash 5; GNU timeout supplied for the macOS test environment).

[frankbria]

Cross-family review — opencode (zai/glm), post-PR pass on 538372d

The review / review check fails on this PR because GitHub passes no secrets to fork PRs (job log: Secret source: None, empty ANTHROPIC_AUTH_TOKEN). This local opencode run stands in for it.

Review: PR #380 — exercise production functions in unit suites

Contract check (criterion 1): All three files load the real functions via sed -n '/^fn() {/,/^}/p' ralph_loop.sh + eval in setup(), with two guards ([[ -n "$function_body" ]] and declare -F). This closes the "silently load nothing" hole: in bats-core a non-zero setup marks the test as failed, so an empty/mis-parsed extraction fails loudly rather than falling through to a stale or undefined function. Order is also correct in all three files (helpers/libs sourced before the evals, so the real definitions win any name collision), and log_status is stubbed before tests invoke the extracted functions.

Criterion 3 (no copied bodies): All inline copies (_count_blocking_unchecked, should_exit_gracefully, should_exit_gracefully_with_denials, the four rate-limiting functions, both reset_session copies) are fully removed; the remaining log_status() { :; } is a stub, not a copy.

Test 26 fixture change (5→4 indicators): Justified, not gaming. The real function's ≥5 breaker force-exits regardless of EXIT_SIGNAL, so the old fixture would legitimately fail; 4 indicators still exercises the test's intent (≥2 heuristics + explicit false → no exit). The added comment documents the constraint.

Findings

1. Suggestion — tests/unit/test_exit_detection.bats:~466 (Test 26): The real should_exit_gracefully's ≥5-indicator safety breaker is deliberately dodged here and covered nowhere else in the file, so a mutation that deletes or weakens the breaker (e.g., threshold 5→500) would pass this suite unchanged; add a positive test (5+ indicators + exit_signal: false → exit) to pin the breaker.

2. Suggestion — tests/unit/test_exit_detection.bats:12-19, tests/unit/test_rate_limiting.bats:9-16, tests/unit/test_session_continuity.bats:49-56: The guards catch empty extraction and failed definition, but not a mis-scoped extraction — if a future edit introduces a column-0 } inside a body or indents the function's closing brace, sed truncates or overruns into the next function and eval can still define a wrong-but-callable function that passes declare -F. Failure scenario: someone reformats ralph_loop.sh, the suite silently tests a truncated shadow of the production function and passes green while production behavior has diverged; a cheap structural assertion (extracted text starts with name() { and its last non-empty line is exactly }) would make drift loud.

3. Nitpick — tests/unit/test_session_continuity.bats:~625: The second reset_session test ("manual_reset") calls the real implementation without exporting RALPH_SESSION_HISTORY_FILE (each bats test is a fresh subshell, so the export in the first test doesn't carry over), relying on the default $RALPH_DIR path; if log_session_transition ever grows set -u-style strictness this test breaks for a non-obvious reason — exporting the var in setup() would make the dependency explicit.

Vacuous-pass audit: No test becomes vacuous. Tests 36–41 switching from the synthetic should_exit_gracefully_with_denials to the real function is strictly stronger (the real one additionally runs test-saturation, done-signal, breaker, and plan checks, any of which firing wrongly would flip an expected "" to a reason string and fail). The rate-limiting and reset tests assert observable state (file contents, counters, JSON fields), so mutations to the now-real functions are killed by the existing assertions; criterion 2 is plausible from the visible tests, though it should be demonstrated by actually running the mutations per the acceptance criteria.

Verdict: APPROVE

---

Maintainer triage

• Suggestion 1 (safety breaker untested): confirmed and applied. Mutating the breaker threshold 5→500 left this suite green. A positive test (5 indicators + explicit false → safety_circuit_breaker) is pushed onto this branch and kills that mutant.
• Suggestion 2 (mis-scoped extraction): skipped. The sed range is the repo's established extraction pattern (test_tmux_integration.bats). A truncated body produces an eval syntax error, which leaves the function undefined, so the declare -F guard fails setup loudly. All 9 extracted functions are single-definition and heredoc-free today.
• Nitpick 3: not applicable. setup() already exports RALPH_SESSION_HISTORY_FILE (test_session_continuity.bats:29).

[frankbria]

Demo verification (Showboat, showboat verify passes)

Each mutant changes one real function in ralph_loop.sh, runs that function's suite, then restores the file. The same mutants were run against this branch and against main, where the suites still test inline copies.

Real function	Mutation	Suite	this PR	main
should_exit_gracefully	test-saturation -ge → -gt	test_exit_detection	caught (1)	survived
should_exit_gracefully	safety breaker 5 → 500	test_exit_detection	caught (1)*	survived
_count_blocking_unchecked	optional-section skip removed	test_exit_detection	caught (5)	survived
can_make_call	invocation limit -ge → -gt	test_rate_limiting	caught (2)	survived
increment_call_counter	+1 → +2	test_rate_limiting	caught (5)	survived
extract_token_usage	output tokens dropped	test_rate_limiting	caught (3)	survived
reset_session	exit-signal reset removed	test_session_continuity	caught (2)	survived

* caught by the test added in 29be2ae, after the opencode review.

Criterion	Evidence	Status
Each file loads the real function	for function_name in … sed-extract + eval in setup(): exit detection (3 fns), rate limiting (4), session (2); setup fails on an empty body or an undefined function	VERIFIED
A change to each real function fails a test in its file	table above: 7/7 caught on this branch, 0/7 on main	VERIFIED
No copied function bodies	the only test functions sharing a ralph_loop.sh name are log_status test doubles (a : stub, and a 3-line echo double in session_continuity), not copies of the 20-line production function	VERIFIED

Local: npm test 1355/1355.

[frankbria]

Thanks @widechaos, merged as ff9a900. Your extraction approach held up under every mutant we threw at it: all 7 caught on this branch, none on main. I pushed one follow-up commit (29be2ae). It adds a positive test for the 5-indicator safety breaker, which your test-26 fix correctly stepped around but nothing else pinned, and a CLAUDE.md note so future tests load real functions too.

[frankbria]

Post-merge disposition: filed #384 ([P8.5] skip the GLM review job on fork PRs, which have no secrets; it failed on both pushes here). Dropped as already handled: the mis-scoped-extraction suggestion (the declare -F guard already fails setup on a truncated body), and the history-file nitpick (not applicable; setup() exports it). Lessons: 2 entries appended, 1 updated in tasks/lessons.md. Post-merge main: 1361/1361.