test(image-reuse): the up-path wiring check matched a commented-out call - #118
Open
dlovell wants to merge 3 commits into
Open
test(image-reuse): the up-path wiring check matched a commented-out call#118dlovell wants to merge 3 commits into
dlovell wants to merge 3 commits into
Conversation
dlovell
added a commit
that referenced
this pull request
Aug 4, 2026
…ugh them Round 4 claimed to fix every sibling and did not. Review found two more, both green: a comment on the `stages:` KEY line (the parser's own sed required whitespace before the `#`, which the awk had already eaten), and the bare grep for the `dc exec ... sh -c "$script"` line — the ONE line that runs the injected script — which read the raw file and then extracted the argv0 out of comment text. That is the fifth and sixth instance of this class in this PR, so this round stops fixing instances. All four remaining raw reads in these two suites now go through shell_strip_comments, and setup_body goes through shell_function_body instead of a seventh hand-rolled range. There is one comment-stripper and one body extractor, and nothing in these files reads a source without them. shell_function_body moves here from #118, where it did not belong — the normalizer should be complete in the PR that introduces it. Also from review: - assert_shell_wired PASSED on an empty pattern (`grep -qF -- ""` matches every line) and treated a multi-line pattern as an OR. Both are what an extracted pattern looks like when its extraction broke. Both now fail. - The header claimed "a match means live code". It does not: it means the text is not in a comment. `if false; then ...; fi` and `echo "skipping <name>"` both satisfy these assertions with the wiring dead. Recorded as accepted limits of grep-over-source, with the alternative named. - The `ls-files -- .claude/agents` pin false-FAILed on an ordinary requote. Quoting-tolerant now — a guard that reddens on a no-op reformat trains people to edit the test. Both halves of the amended §2 pair now hold in both suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Live on main: `assert_contains "ensure_image" "$ensure_up_body"` matched comment text, so commenting out ensure_up()'s calls left the suite at 14 passed / 0 failed and `tests/run-all` at exit 0, with no image made present before `dc up`. Uses assert-on-stripped-source and the shared shell_function_body, both from tests/lib/shellsrc.sh (added by #110, extended here with the body extractor). The extractor matters independently: this file had TWO hand-rolled `sed -n '/^f()/,/^}$/p'` ranges, and reformatting `ensure_up()` to `ensure_up ()` — valid bash, shellcheck-clean — emptied them, turning assertions red for the wrong reason and making "dc_up no longer passes --build" vacuous. One spelling now, returning non-zero rather than empty when the function is absent. Correction to the audit that found this: its mutation commented ONE of two ensure_image call sites, so its stated consequence was overstated — the other site still called it, and the assertion was right to stay green. Commenting both is the honest demonstration and is what the header records. Stacked on #110 because assert_shell_wired/shell_function_body live there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…assert one Review finding: assert_contains needs only ONE live ensure_image, but ensure_up() has two call sites guarding two different paths — cold start, and the recreate arm. Deleting either left the suite at 16/0. Losing the recreate one is the consequential case: `dc down; dc_up` then runs with the new fingerprint tag absent, so compose's fallback build fires — unlocked, without ensure_image's nix-base pull or its unhashable-input refusal. That is the degraded path dc_up()'s own header reserves for a rare eviction window, silently promoted to the normal recreate path. Now every live dc_up in ensure_up() must be immediately preceded by a live ensure_image, with both derived from the extracted body. Deleting either site is 17/1; the form-only signature reformat still holds at 18. Also documents two edges of shell_function_body that the header omitted: it stops at the first column-0 `}` (so a heredoc containing one truncates the body) and a one-liner definition over-captures. Neither is reachable in this repo today, but truncation is the unsafe direction — a negative assertion over a truncated body can pass for the wrong reason — so negative assertions need a non-empty guard, as this suite has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dlovell
force-pushed
the
fix/image-reuse-wiring
branch
from
August 4, 2026 18:39
1f2c3d9 to
3e571bd
Compare
…d's counts Review found the pairing walk false-FAILs on plausible refactors: it compares against the previous non-blank line only, so `ensure_image && dc_up` and `ensure_image; dc_up` — genuinely paired — were 17/1. Both spellings are now admitted explicitly. Detection is unchanged: deleting either call site 17/1, commenting both out 16/2, form-only reformat green at 18. The conditional and intervening-statement cases stay red by design; a textual guard cannot follow control flow, and widening further would start admitting the dead-code spellings. Also: the retained header block from the previous round quoted counts (15/1, 16) measured before the pairing assertions existed, undated as such, so replaying them did not reproduce. Labelled as the prior round with the current numbers named. Records the dead-code limit explicitly against issue #123 rather than leaving it implied by shellsrc.sh's header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #110 —
assert_shell_wiredandshell_function_bodylive intests/lib/shellsrc.shthere. Base isfix/guard-the-wiring; review the incremental diff.Live defect on merged
main.assert_contains "ensure_image" "$ensure_up_body"matched comment text, so commenting outensure_up()'s calls left the suite at 14 passed / 0 failed andtests/run-allat exit 0 — with no image made present beforedc up.Verified (ADR-0005 §2, using the amended mutation pair from #117)
ensure_imagecall sites inensure_up()ensure_up()→ensure_up ()dc_up()→dc_up ()The form-only pair is why the shared extractor matters independently of the comment bug: this file had two hand-rolled
sed -n '/^f()/,/^}$/p'ranges, andensure_up ()— valid bash, shellcheck-clean — emptied them. That turned assertions red for the wrong reason and made "dc_up no longer passes --build" vacuous, since an empty body contains no--build.Correction to the audit that found this
The audit's mutation commented one of two
ensure_imagecall sites and claimed "no image is built beforedc up." Both sites are insideensure_up(), so the other still called it and the assertion was right to stay green. Commenting both is the honest demonstration; that is what the header records. The class is real, the specific consequence was overstated.tests/run-all28 suites / exit 0;pre-commit run --all-filesclean.🤖 Generated with Claude Code