fix(test): the acceptance harness still installed the relay the removed way - #362
Merged
Merged
Conversation
…ed way Found by a real integration run on the maintainer's host, against 2.2.0. I SHIPPED THIS BROKEN, in the release that closed the umbrella about exactly this failure. Phase E installed the relay by writing SANDY_HANDOFF_RELAY=.sandy/relay.sh into the isolated host config -- which #354 made a HARD ERROR. So the launch was correctly refused, and the harness reported the relay supervisor as broken when what was broken was the harness. Nineteen FAILs against a feature that works. It is the same shape this whole umbrella recorded: an assertion correct about a mechanism that had been replaced. I updated phases A/B/C/D/F for the removal and did not look at E, because E tests the relay and the relay survived -- but HOW you install one changed underneath it. FIXED: - phase E installs the relay as a manifest `entry` (a feature under $SANDY_HOME, privileged by where it lives, so no approval is involved); - its fixture reads SANDY_RELAY_STATE, not the retired SANDY_HANDOFF_*; - state paths move from $SBX/handoff/relay to $SBX/relay-state, which is why three lines were erroring with "No such file or directory"; - E10 breaks the ENTRY (points it at a non-executable payload file) instead of a removed config key -- which also moves that check onto the in-container refusal branch, the one the host-side path never covered; - phase E now removes its own feature before phase G, since its manifest selects every sandbox. PHASE G'S FAILURE WAS DOWNSTREAM, NOT INDEPENDENT: E10's restore put SANDY_HANDOFF_RELAY back into the host config, so every later --start in the run hard-errored. But G had a real defect of its own worth fixing anyway -- THREE OF ITS CHECKS PASSED AGAINST AN EMPTY CONTAINER ID, because `docker exec "" ...` fails and "the write FAILED" is then true for the wrong reason. That is precisely what the phase's own premise check exists to catch, and the premise DID fail -- but a phase whose premise is red must not print green assertions beneath it. Every check in G is now gated on a non-empty container id. §114(16g) is repointed at the property (refuse, name the rule, leave nothing behind) rather than at the fixture's spelling, so the next change of fixture shape does not read as a regression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sts.sh Found by the maintainer's integration run once an expired host token stopped masking everything else: acceptance-update-sessions.sh step 4 failed with `AssertionError: 3` -- a schema_version pin still asserting 2. #355 bumped schema_version and I updated the pins in run-tests.sh only. The sweep should have been the whole tree, because a pin is a pin wherever it lives, and the acceptance harnesses are exactly the ones CI never runs. Also corrects README's "Also in 2.0" consumer note, which still told readers schema_version is 2. It now names both values and says the thing the downstream consumer worked out for themselves this week: treat it as an OPAQUE TOKEN compared against a reviewed set, never with >=, so a future bump is something you read rather than something you silently accept. Verified no `schema_version == 2` pin remains anywhere in test/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The maintainer spotted this in a real run: section 23 printed nineteen FAIL
lines and the suite summary said "1 passed, 0 failed (of 1 run)".
THE SUMMARY BUG IS THE SERIOUS HALF, and it is not new -- it was latent in
all six acceptance wrappers and my broken harness merely triggered it:
set +e; bash "$harness"; _acc_rc=$?; set -e
_acc_res="$(grep -oE 'RESULT: ...' "$out" | tail -1)" # <-- unguarded
if [ "$_acc_rc" -eq 0 ]; then pass ...; else fail ...; fi
When a harness dies BEFORE printing its RESULT line, that grep matches
nothing and exits 1, which under the suite set -e aborts the whole run
BEFORE fail() is ever reached. The failure is never recorded. Demonstrated
both ways against a stub harness that exits 1: unguarded the wrapper dies
silently with nothing recorded; guarded it records the failure and
continues. All six wrappers are now guarded, and the message says "NO RESULT
LINE -- the harness died before printing one" rather than nothing.
WHY THE HARNESS DIED, which is entirely mine: replacing phase F in #355 also
deleted the file's RESULT/exit block, which lived after it. So the harness
never printed RESULT and never exited on its FAIL count -- it could not fail
the suite even in principle. Restored.
Two more stale assertions in phase E, same class as the last round:
- pgrep still matched '\.sandy/relay\.sh', the path the fixture relay lived
at when it was installed by config key. It is a feature payload now, so
every "is the relay running" check missed a relay that was running. That
is why E2/E3/E4/E7 failed while E7's state-persistence check PASSED --
the relay was writing its file the whole time.
- handoff_relay=true was asserted against the marker, and #355 removed that
field. Now asserts relay.source=manifest, which is what replaced it.
Guarded by §152, which ratchets that every wrapper guards its grep AND that
every acceptance harness still ends by printing RESULT and exiting on its
FAIL count -- the second half is what would have caught the deleted block.
Both mutations verified: un-guarding one wrapper, and deleting one harness's
RESULT line, each turn it red.
It is a STATIC ratchet deliberately: the wrappers are inline in a 2000-line
script with no extractable seam, so a behavioural check would have to
re-implement the thing under test, which is how a guard ends up asserting
its own copy (§148(12), the same week).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…se premise E1: the env-forwarding check still grepped for the relay at .sandy/relay.sh. My earlier edit to that line did not apply (the search string carried a `|| true` the file did not have) and nothing caught it, because a wrong pattern makes grep -c return 0 rather than erroring. Verified the expected value by running _sandy_fm_apply against an acc-relay-shaped feature rather than reasoning about it: the entry resolves to /opt/sandy/features/acc-relay/relay. E10: "no daemon container was left behind" was true of the HOST-side refusal, which happened before `docker run`. A manifest entry resolves to an image-only path the host cannot stat, so the refusal is IN-CONTAINER: user-setup.sh exits 1, the container dies, and --start classifies it as CRASH-LOOPING (exit 7) rather than refused-before-launch (exit 6). The old assertion was demanding a property this branch does not promise -- so it was correct about the mechanism it was written for and wrong about the one that replaced it, which is the fourth instance of that shape this week and the second one I wrote myself. It now asserts what criterion 7 actually requires: the launch reports crash-looping rather than ready, and --stop cleans it up. A container that exists but is crash-looping is LOUD; the failure the rule exists to prevent is a container that is up with nothing delivering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The structural guard demanded 'no daemon container was left behind', which is the HOST-side refusal's promise. A manifest entry is an image-only path, so the refusal is in-container: exit 7 (crash-looping) rather than exit 6 (refused before launch), and a container does exist. It now asserts what criterion 7 requires on either branch -- the launch refuses, names the rule, reports crash-looping rather than ready, and leaves nothing running. Caught by run-tests.sh immediately after the harness change, which is the structural guard doing exactly its job: it exists so a phase cannot be quietly reduced to something weaker, and a reworded phase is indistinguishable from a deleted one until someone looks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rappdw
added a commit
that referenced
this pull request
Sep 21, 2026
…emoved way (#365) Integration §25 failed all three cases at "daemon container is running", in 5s, with no cause anywhere in the log. The harness sets the relay by writing SANDY_HANDOFF_RELAY into the isolated host config -- a hard error since 2.2.0 (#354), so every --start refused before docker run and no container ever appeared. This is #362 again in a different file. That fix swept acceptance-handoff-dirs.sh and stopped there; the retired mechanism has to be grepped for in BOTH roles -- what ASSERTS it and what SETS IT UP -- across every harness, including the ones CI never runs. CI runs neither of these. Three changes: - Install the relay as a feature manifest `entry` under $SANDY_HOME, the same shape acceptance-handoff-dirs.sh now uses. No config key is involved, and a manifest under $SANDY_HOME is privileged by location so no approval prompt appears -- which is what the surrounding `env -u SANDY_AUTO_APPROVE_PRIVILEGED` exists to prove. - Move the three in-container paths off ~/.handoff/relay/, removed in 2.2.0 (#353/#355), onto /opt/sandy/relay-state/ -- the rw mount that replaced it. The sentinel, the injector log and Claude Code's --debug-file all write there, so even a launch that had succeeded would have had nowhere to put them. - KEEP THE --start OUTPUT. It was redirected to /dev/null, which is the whole reason a launch that refused in one line on stderr surfaced as three identical container-missing failures with the cause discarded. It now prints the tail of that output, and the exit code, when no container appears. Not run here: this harness needs Docker, which Claude Code inside sandy cannot reach. It needs a host run. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Merged
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.
Found by your integration run on the host, against 2.2.0.
I shipped this broken, in the release that closed the umbrella about exactly this failure.
What happened
Phase E installed the relay by writing
SANDY_HANDOFF_RELAY=.sandy/relay.shinto the isolated host config — which #354 made a hard error. So the launch was correctly refused, and the harness reported the relay supervisor as broken when what was broken was the harness. NineteenFAILs against a feature that works.It is the same shape #347 recorded throughout: an assertion correct about a mechanism that had been replaced. I updated phases A/B/C/D/F for the removal and never looked at E — because E tests the relay, and the relay survived. What changed underneath it was how you install one.
Fixed
entry(a feature under$SANDY_HOME, privileged by where it lives, so no approval is involved)SANDY_RELAY_STATE, not the retiredSANDY_HANDOFF_*$SBX/handoff/relayto$SBX/relay-state— the source of the threeNo such file or directorylinesPhase G: downstream failure, plus a real defect of its own
G's failure was not independent — E10's restore put
SANDY_HANDOFF_RELAYback into the host config, so every later--startin the run hard-errored andCGcame back empty.But G had its own defect worth fixing regardless: three of its checks passed against an empty container id, because
docker exec "" ...fails and "the write FAILED" is then true for the wrong reason.That is precisely what the phase's own premise check exists to catch — and the premise did fail. But a phase whose premise is red must not print green assertions beneath it. Every check in G is now gated on a non-empty container id.
Also
§114(16g) is repointed at the property (refuse, name the rule, leave nothing behind) rather than at the fixture's spelling, so the next change of fixture shape does not read as a regression.
Not in this PR
The §14 below-floor failure in the same run is unrelated and pre-existing since 2.0.0: it greps for
"refuses to launch against it", which last existed in v1.15.0 — #248 rewrote that message deliberately and the assertion never followed. Its sibling check is vacuous too. Worth its own PR.The fleet-update acceptance failure (
31 passed, 1 failed) is also not diagnosed here.Verification
run-tests.sh§97 3/3, §114 124/124, §123 36/36 green;lint-bash32clean. The acceptance harness itself needs a host run — it is Docker-dependent and cannot be exercised from inside sandy.🤖 Generated with Claude Code