Consolidate the stub platform guard, and record why it was needed - #121
Merged
b-macker merged 2 commits intoAug 4, 2026
Merged
Conversation
The 29 guards added in #120 carried "pending stall bisect" in their comments. The bisect is finished: excluding all 29 took "CLI tests — shell suites" from a 47-minute hang to 2m04s, confirming the diagnosis that had sat in test_absorption_degenerate.sh alone. Leaving hypothesis language in the tree is how that finding went unapplied to 28 other suites in the first place. Replaces 29 near-copies with tests/helpers/stub_platform.sh: source "$SCRIPT_DIR/../helpers/stub_platform.sh" skip_if_no_stub_support "<suite>.sh" 762 lines removed, 58 added. The point is not the line count — it is that the reason this took four stalls to fix was a finding recorded in one file's comment where 28 other callers could not see it. One definition is what makes the next such finding reach all of them. The helper states what is known rather than what is suspected, and says plainly that it is a WORKAROUND: the launcher's signal and cleanup behaviour under MSYS2 is the actual defect and remains open. Coverage is unchanged — build-linux and Build & Test run all 29 in full. Verified: all 29 observed taking the skip under a simulated MINGW uname AND under $WINDIR with a normal uname; the guard confirmed NOT firing on Linux (test_quarantine_corroboration.sh still 5/5). Suite total unchanged at 441 with 0 unexpected, 874 leak checks / 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
NAAb Governance Report
All governance checks passed! Generated by NAAb Governance Engine v4.0 |
Adds to docs/governance-campaign-findings.md, which exists so the next person does not rediscover what was already settled — the exact failure this entry is about. New section "A fix that reached one caller". The Windows stall's diagnosis was in the tree before any of the work chasing it: test_absorption_degenerate.sh carried a comment naming signal propagation and process cleanup under MSYS2 as the cause. It sat in 1 of the 29 suites that launch the stub. The same shape had already happened once, in this file's own table: 081f460 fixed the stub's one-shot port pick and reached 2 of 29. Measured, not estimated. The point is not that the helper is duplicated. It is that a finding was recorded where the other callers could not see it, and the cost of that was four stalls, three misattributed Linux failures, one incorrect bisect and a reverted fix — all downstream of a correct diagnosis nobody could find. Two withdrawn proposals, kept because the reasoning against them is the useful part: 5. Check allowed_actions in agent.commit(). Reported as a gap in the same family as #8 and #13; it does not exist. The reload-generation check already invalidates any proposal whose config could have changed. 6. Give the stub launcher a port retry. Fixed Linux, hung Windows, and the one-commit bisect that seemed to prove it looked conclusive and was coincidence — the next commit restored the Windows path and stalled anyway. Fifth method note: a grep defines what you are able to see. Three conclusions in one session were wrong because the pattern that produced the evidence was narrower than the thing being reasoned about, and absence from a grep result reads exactly like absence from the code. Tabulated with what each pattern hid. Corrected one overstatement while writing it: the platform guard is now one definition, but the launcher is still 29 near-copies and the port retry still reaches only some of them. That is deliberately not consolidated — the correct Windows behaviour of that code is what is still unknown, and consolidating would freeze one guess into 29 callers. Every figure verified against the tree: 1 of 29, 2 of 29, 29 launchers, H-01/H-02 present. 441 tests / 0 unexpected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ELUfjXZvx8kzXo1UJjrAhC
b-macker
marked this pull request as ready for review
August 4, 2026 02:27
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
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.
Summary
Two commits: the consolidation, and the campaign record of what made it necessary.
1. Consolidate the guard (
4bad165)The 29 guards added in #120 carried
pending stall bisectin their comments. The bisect is finished — excluding all 29 tookCLI tests — shell suitesfrom a 47-minute hang to 2m04s — so the comments described a hypothesis that had been settled.Replaces 29 near-copies with one helper:
762 lines removed, 58 added. The line count is not the point. The reason this took four stalls to diagnose is that the finding was recorded in one file's comment, where the 28 other callers of the same launcher could not see it:
The helper states what is known rather than what was suspected, and says plainly that it is a workaround, not a fix — the launcher's signal and cleanup behaviour under MSYS2 is the actual defect and is still open.
2. Record it (
642cce5)docs/governance-campaign-findings.mdexists so the next person does not rediscover what was already settled — which is precisely the failure this entry is about. 522 → 613 lines.New section, "A fix that reached one caller." The same shape had already happened once, in that file's own table:
081f460fixed the stub's one-shot port pick. Measured, not estimated:081f460)Cost of the second one: four stalls, three misattributed Linux failures, one incorrect one-commit bisect, and a fix that had to be reverted — all downstream of a correct diagnosis nobody could find.
Two withdrawn proposals, in the section kept because the reasoning against them is the useful part:
allowed_actionsinagent.commit()— reported as a gap in the same family as Re-check the standing lease when a proposal is committed #105/Hold the CRITICAL suspension at the commit boundary #114; it does not exist.agentCommitalready comparesgetReloadCount()against the count stamped at propose time, andreload_count_++fires only on an accepted reload, so any config change that could remove the action has already invalidated the proposal.Fifth method note — "A grep defines what you are able to see." Three wrong conclusions in one session, tabulated with what each pattern hid: the
reload_countguard, 20 of 29 stub suites, and the second taint violation message format. Absence from a grep result reads exactly like absence from the code.One overstatement corrected while writing: the platform guard is now one definition, but the launcher is still 29 near-copies and the port retry still reaches only some of them. Deliberately not consolidated — the correct Windows behaviour of that code is what remains unknown, and consolidating would freeze one guess into 29 callers. Left recorded as an open exposure rather than rounded up.
Test Plan
bash run-all-tests.shwith no new failures — 441 tests, 0 unexpected failures (total unchanged)bash tests/security/test_error_msg_leaks.sh— 874 checks, 0 failuresBoth directions verified, since a guard that never fires and a guard that always fires look identical from a green suite:
MINGW64_NTuname$WINDIRset, normal unametest_quarantine_corroboration.shstill 5/5Every figure in the documentation was checked against the tree: 1 of 29, 2 of 29, 29 launchers,
H-01/H-02present. Nopending stall bisecttext remains intests/.build-windowshas now passed three consecutive times since the exclusion (e5c6f00,4bad165,642cce5).Follow-ups not in this PR
test_absorption_degenerate.shretains anIS_WINDOWSbranch at ~line 265 that is now unreachable. Left alone rather than widening this change.Related Issues
Follow-up to #120, which established the result this PR records.
Generated by Claude Code