fix(main): guard the seam call sites instead of stubbing the seams - #1346
Merged
Conversation
cppcheck blocked the release run, correctly: src/main.c:283: Condition '!main_test_worker_project_lock_marker(mutation)' is always false src/main.c:2049: Condition '!worker_start_watchdog_test_descendant()' is always false Compiling the seams out left behind stubs that returned true unconditionally, so negating them was provably constant. Refactored rather than suppressed (repo rule): the stubs are deleted and the CALL SITES are guarded, so a release build does not contain the call at all -- which is what "the seam is not in the artifact" should have meant in the first place. The worker path needed care. The probe was one term in a five-term `||` chain, and its POSITION is load-bearing: it must fork AFTER the isolated process group exists (it inherits that group) and BEFORE the parent-death watchdog thread starts, because forking a multithreaded process is the bug that ordering avoids. Hoisting the call out of the chain would compile, pass tests, and silently break that invariant. So the chain is split into three ordered steps -- process group, then (test builds only) the probe, then the watchdog -- sharing one worker_containment_unavailable() failure path so every step still fails identically, with write()/_exit() rather than stdio and atexit handlers. Verified in BOTH directions, since a guard that is always off is as wrong as one always on: a TEST_SEAMS=1 build still contains the probe and tests/test_worker_watchdog.sh passes against it, while the release build carries no seam string. `make -f Makefile.cbm lint-ci` now passes locally -- the step skipped before the last push, and the reason CI found this instead of me. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This was referenced Jul 31, 2026
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.
What
cppcheck blocked release run 30498150408 at the lint gate — correctly:
#1344 compiled the test seams out by leaving stubs that returned
trueunconditionally, which makes negating them provably constant. Refactored, not suppressed (repo rule): the stubs are deleted and the call sites are guarded, so a release build doesn't contain the call at all — which is what "the seam isn't in the artifact" should have meant to begin with.The part that needed care
The probe was one term in a five-term
||chain, and its position is load-bearing: it must fork after the isolated process group exists (it inherits that group) and before the parent-death watchdog thread starts, because forking a multithreaded process is exactly the bug that ordering avoids.Hoisting the call out of the chain would compile, pass tests, and silently break that invariant. So the chain is split into three ordered steps — process group → (test builds only) probe → watchdog — sharing one
worker_containment_unavailable()failure path so all three still fail identically, usingwrite()/_exit()rather than stdio and atexit handlers because this runs after fork-sensitive setup.Verification
Checked in both directions, since a guard that's always off is as wrong as one always on:
TEST_SEAMS=1build: probe string present,tests/test_worker_watchdog.shpasses against it.make -f Makefile.cbm lint-cipasses locally — cppcheck, clang-format, NOLINT check.Process note
I skipped
make lint-cibefore the previous push, which is why CI found this rather than me. cppcheck isn't part of the test suite, so a fully green 3-OS ladder (macOS 5166/0/0, Linux 6606/0/4, Windows suites + guards, smokeSMOKE_EXIT=0) says nothing about it. Running it pre-push from here.