Repository navigation
fix: prevent duplicate portable serial shard assignments - #50
Merged
Merged
Conversation
…bash 3.2 The guard computes every serial shard list, and the whole lane list, inside one shell. Each list ran a per-script process substitution to test proven-isolated membership and to look up a duration hint. macOS /bin/bash 3.2 loses lines from later reads once one long-lived shell has run enough of them, so past a lane size the last shard picked up scripts that belonged to earlier shards and the guard refused with "portable serial shards share scripts" while every standalone --list was correct. Membership now uses a case match on a list captured once, and the hint join is one awk pass, so a shard selection costs a fixed number of processes whatever the lane size. Shard contents are unchanged. Tests build a fixture runner with extra scripts and require the guard and the shard lists to stay a disjoint cover, run under a scrubbed environment because the harness's exported variables hide the corruption, and keep a true duplicate refused.
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.
Intent
Red CI: diagnose and fix. Never waive or merge around red; is the code bad or the test bad? Fix it.
Timeouts and harness issues get a root cause and a root fix, assessed as a system: fix without breaking, fix once.
Context: firstmate pull request 22 adds 6 test scripts and now fails the test runner's coverage guard (bin/fm-test-run.sh --check-coverage) with 'portable serial shards share scripts', so CI's portable lane would fail. That pull request is not the cause: origin/main plus 5 dummy test scripts fails the same way, and removing any one of that pull request's 6 scripts makes it pass. Standalone shard lists show 0 duplicates, but inside the guard 11 scripts land in shard 9 twice. The finding is that the guard's shard assignment double-assigns scripts at some script counts.
What Changed
Risk Assessment
✅ Low: The change removes per-script process-substitution membership and weight lookups while preserving exact membership, deterministic shard packing, and existing coverage refusals, with executable-interface regression coverage for larger inventories and genuine duplicates.
Testing
Exercised the real runner CLI, five focused contracts, adversarial producer and duplicate faults, execution receipts, timing JSON, concurrency admission, and scheduling. All nine live scenarios passed. Initial broader/stress attempts hit local driver bounds; focused contracts and deadline-corrected replays completed successfully without changing product limits or guards. CLI and persisted-state evidence was saved, and all worktree fixtures were removed.
Evidence: Real CLI outputs, execution traces, refusals, and final guard results
Source: Real CLI outputs, execution traces, refusals, and final guard results
Evidence: Timing artifact generated by actual serial shard execution
Source: Timing artifact generated by actual serial shard execution
Evidence: Completed change-specific shard and coverage contracts
Source: Completed change-specific shard and coverage contracts
Evidence: Observed current-inventory coverage and partition baseline
Source: Observed current-inventory coverage and partition baseline
Evidence: Initial broader contract attempt — interrupted, not a passing result
Source: Initial broader contract attempt — interrupted, not a passing result
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
🔧 **Test** - 2 issues found → auto-fixed ✅
bin/fm-test-run.sh:934- Serial shard generation can still lose scripts while returning success on macOS Bash 3.2. With the unchanged target runner and the repository inventory plus five scripts,--list --lane portable-serial-3of9exited 0 but printed only one script and reportedprintf: write error: Interrupted system call. The combined nine-shard lists consequently omitted 24 of 220 serial scripts. Three identical-command replays each returned the expected 25 scripts, establishing an intermittent failure. This occurred while the focused runner contract was running concurrently. Eliminate the interrupted-generation failure and ensure a partial assignment stream cannot be accepted as successful; extend regression coverage to check the standalone shard union at the five- and six-script boundaries, not only at twenty. Evidence: runner-truncation.txt and runner-live-transcript.json.bin/fm-test-run.sh --check-coveragereturned FM_TEST_COVERAGE ok with total=257, serial=215, serial_shards=9./bin/bash --versionandbin/fm-test-run.sh --check-coverage: Bash 3.2.57; the current checkout's coverage guard passed.Executed base-commit and target runners against identical disposable inventories containing the repository test names plus five scripts, using subprocess environments containing only PATH, workspace-local HOME, and workspace-local TMPDIR.Executed--check-coverage,--list-lanes, the complete serial lane listing, and all nine serial shard listings for inventories with 0, 5, 6, and 20 extra scripts; compared generated outputs for duplicates, omissions, and deterministic membership.Replayed the failing five-script--list --lane portable-serial-3of9command three times and compared its recovered output with the 24 omitted scripts.Executed the complete coverage guard with five new scripts within the measured-hint policy; injected genuine parallel and serial shard overlaps into disposable runner copies and verified rejection; verified rejection ofportable-serial-1of8.Executed all nine real runner shards with six added script inputs using--lane portable-serial-<k>of9 --jobs 1 --json <evidence-path>; checked persisted execution records for exactly-once coverage of all 221 serial inputs.env HOME="$PWD/.runner-live-ltzsh8j8/home" TMPDIR="$PWD/.runner-live-ltzsh8j8/tmp" bin/fm-test-run.sh --jobs 1 --json <evidence-path> tests/fm-test-run.test.sh: all three added regression cases completed successfully, but the overall focused contract invocation exceeded its 1200-second command deadline and is not reported as passing.Executedbin/fm-test-run.sh --jobs 2 --per-script-timeout-secs 120 tests/fm-supervision-events.test.sh tests/fm-session-lock-ancestry.test.sh; ancestry exceeded the imposed cap. Re-executed the selection with a 600-second cap and JSON evidence; both scripts completed successfully, with ancestry taking approximately 163 seconds.Exercised concurrency admission through the real runner: proven scripts were admitted; new unproven scripts and an external basename collision were refused; the watcher family's four-worker cap remained enforced.Removed all disposable inventories, local homes, temporary roots, and generated execution data from the worktree after validation.🔧 Fix applied.
✅ Re-checked - no issues remain.
/bin/bash bin/fm-test-run.sh --check-coverage: current inventory passed with 257 scripts and nine serial shards.Compared--list --allwith the complete lane union and compared all nine standalone serial shard lists with--list --lane portable-serial./bin/bash tests/fm-test-run.test.sh: initial broader contract attempt exceeded the 600-second wait bound and was stopped; it is not reported as passing.Executed unchanged contract functions through a disposable copy:test_serial_shard_guard_holds_at_every_lane_size,test_serial_shard_generation_failure_is_not_a_partial_success,test_coverage_guard_passes_with_extra_scripts,test_coverage_guard_still_refuses_a_true_duplicate, andtest_portable_serial_shard_lane_refusals; all passed.python3 .fm-runner-live-vbd8ner6/live_check.py: exercised producer faults, serial execution, failure propagation, and lane refusals. Initial stress and guard attempts reached driver-only deadlines.python3 .fm-runner-live-vbd8ner6/guard_replay.py: completed two full nine-shard partitions at each added-script count of 5, 6, and 20, checked determinism and exact coverage, and exercised guard refusal and acceptance with a corrected driver-only deadline.python3 .fm-runner-live-vbd8ner6/concurrency_check.py: verified actual concurrent execution for proven scripts and repository family members, serial execution for an unknown script, and refusal of unknown, external-name-colliding, and over-cap selections.Ran--check-coverageagainst a disposable runner with one script inserted into both parallel lane inventories; it refused and named the duplicate.Exercised--list-scheduledwith six explicit script paths and with--proven-isolated; observed the expected weighted ordering and path-ordered ties.Verified serial execution receipts and the generated timing JSON, checked that no fixture processes remained, and removed all disposable drivers, inventories, homes, and generated data.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.