Repository navigation
feat: add shared CPU pass pool and host load reporting - #53
Merged
Merged
Conversation
Host CPU oversubscription (load 250-482 on 18 cores) was the shared condition behind the 2026-10-07 agent timeouts: every worktree's test runner sized itself to the whole host and nothing owned total demand. This is Phase 1 F2 of that root-cause plan. It bounds CPU-heavy test bursts, not agents: interactive agents, lanes and sessions never take a pass and are never queued or capped. - bin/fm-cpu-pass.sh (+ .py engine): `run [--passes K] -- CMD` holds K passes from one host-wide pool while CMD runs; `status`; `size`. Passes are fcntl.flock locks on slot files in $HOME/.cache/fm-cpu-pool, pool size = CPU count, so a killed holder never leaks a pass. A request queues and never fails; a broken pool degrades to running without a pass, with one notice. - bin/fm-test-run.sh: each executed script takes one pass, outside its per-script bound, so waiting never trips that bound; notices go to the runner's stderr, never into captured script output. Runs nested inside a pass, with FM_CPU_POOL=off, without python3, or from a copy without the tool run directly. - docs/cpu-pass-pool.md owns the cross-repository protocol, so Vernant's pytest -n sizing (d7) can join the same pool. - bin/fm-load-report.sh (+ .py engine): records load samples and reports load p95 against 2x CPUs plus pipeline fix-round convergence and timeout-class run errors from the no-mistakes database (read-only), to judge Phase 1 after 24-48 h. - Thirteen portable-serial duration hints from green CI run 37404365422 keep the unhinted share under the coverage guard's 15% limit now that the serial lane gains the two new tests (provenance in docs/fm-test-portable-shards.md). F6 (omp advisor and eager sub-agents off for pipeline agents only) is not in this repository: the gate overlay is written by the no-mistakes binary. It ships as a no-mistakes binary change applied by Main to the private build; this change does not depend on it. No timeout was raised and the shared daemon was not touched. System check: - Callers of bin/fm-test-run.sh that execute scripts: CI lanes in .github/workflows/ci.yml (all serial on their own runners, so one pass at a time from an uncontended pool), no-mistakes Test agents and workers running it locally (now take turns host-wide), and the runner's own fixture tests (copies without the tool run directly). Inspection callers (bin/fm-test-isolation-proof.sh --list modes, tests/fm-ci-workflow.test.sh --list-lanes) execute nothing and are unchanged. - Blast radius: a local run may now wait for a pass when the host is busy; the wait is outside per-script bounds but inside a caller's own command limit and --max-wall-ms. --jobs above the pool size runs only pool-size scripts at once. Timing durations include any wait. - Deadlock guards: nested runners inherit FM_CPU_PASS_HELD and take no pass; multi-pass requests collect behind one turnstile. - Tests: tests/fm-cpu-pass.test.sh (exclusive passes, queued waiter, multi-pass, SIGKILL release, TERM keeps pass with running work, waiter TERM never runs, nested and opt-out, degraded and no-python paths, runner wait outside the bound, gate-skip detection unchanged, bound still fires, nested runner) and tests/fm-load-report.test.sh, plus the existing runner, fixture, isolation-proof, timeout-lib, documentation and CI-workflow suites.
Only an unset or blank FM_CPU_POOL_SIZE falls back to the CPU count; any other non-positive-integer value is a usage error, as bin/fm-cpu-pass.sh already enforces. Wording only.
…onvergence cohort
…figurable cohort size
…ation holder diagnostics
…the CPU-pass wrapper's inherited slot-lock descriptor shifted a live extension claim handle onto fd 7, which the capture handoff then overwrote, preventing terminal source retirement. Reproduced the failure locally with fd 4 occupied. The shared Perl handoff now reserves destination descriptors before allocating input handles, fixing both result.silent and result.terminal paths. Added public-command regression coverage for terminal retirement and silent acknowledgement with inherited descriptors, and updated verification documentation. Verification passed: the complete fm-extension-binding suite through the CPU-pass-enabled fm-test-run.sh runner, tests/fm-cpu-pass.test.sh, shellcheck -x on the changed test, and Perl syntax checking. No timeouts or budgets changed. The remote CI check was not rerun; verification was local on macOS
…n bin/fm-test-run.sh: raised the stale fm-supervision-host duration hint from 41512 to 877426 ms and added fm-skill-pick at 120549 ms. Both hints remain sorted and unique. Updated docs/fm-test-portable-shards.md with one-sentence-per-line provenance citing supervision-host measurements from main runs 37772520915 (562966 ms), 37764027766 (853538 ms), and 37774962436 (877426 ms), plus PR runs 37774432736 (549 s) and 37778222434 (859832 ms); skill-pick measured 120549 ms in run 37778222434. These measurements and run IDs are included here for the outer executor's commit message. Verified the packer's public --list-scheduled interface over a disposable three-way merged snapshot of this branch and available origin/main (32fbe8e), including main's skill-pick test. Per-shard hinted totals in milliseconds, shards 1–9: 877426, 864666, 864675, 864667, 864672, 864654, 864669, 864670, 864637. These are 14.41–14.62 minutes, with a 12789 ms spread and at least 15m22.574s estimated headroom below 30 minutes. Supervision-host occupies shard 1 alone; skill-pick is on shard 3. The merged partition is complete and disjoint. These are packing estimates, not measured CI runtimes. Verification passed: full bash tests/fm-test-run.test.sh on the branch; --check-coverage on both branch and merged snapshot; bash -n bin/fm-test-run.sh. The initial suite invocation encountered a local Python-launcher issue before its first assertion; the complete suite passed using a resolved Python binary through a temporary worktree-local PATH entry. All verification scaffolding was removed. No test code, timeout, --max-wall-ms budget, or shard count changed. No remote CI or pipeline-control command was invoked
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 Changed
fm-test-run.sh, keeping queue waits outside script timeouts and limiting nested concurrency to inherited passes.Risk Assessment
✅ Low: The Firstmate-side change is bounded, incorporates the recorded decisions, and has no substantiated material source defects; documentation correctly conditions Phase 1 completion and measurement on Vernant participation.
Testing
Both focused public-command scenario scripts passed, followed by manual live checks of the real host-sized pool, nested and queued runner execution, recorder output, and convergence transitions. CLI transcripts, host samples, and JSON reports were preserved; disposable fixtures were removed. Private F6 operation and the post-Vernant full-fleet measurement were not validated in this phase.
Evidence: Host-sized CPU-pool CLI transcript
Source: Host-sized CPU-pool CLI transcript
Evidence: Nested and queued runner transcript
Source: Nested and queued runner transcript
Evidence: Actual host-load samples
Source: Actual host-load samples
Evidence: Pending convergence cohort
Source: Pending convergence cohort
Evidence: Settled unsuccessful convergence cohort
Source: Settled unsuccessful convergence cohort
Evidence: Unchanged cohort after later successes
Source: Unchanged cohort after later successes
Evidence: Above-budget load report
Source: Above-budget load report
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
docs/fm-test-portable-shards.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 7 issues found → auto-fixed (5) ✅
bin/fm-test-run.sh:2538- The intent requires "Phase 1, which must land together: F2 ... and F6", including Vernant worker reservations, gate sub-agent concurrency 1, and advisor/eager-sub-agent disabling for pipeline agents only. This diff implements Firstmate reservations, while docs/cpu-pass-pool.md:53 merely tells other repositories how to participate. No companion integration or gate configuration is established by the change; commit 3a70ee8 explicitly says F6 is external and "this change does not depend on it", contradicting the required coordinated cutover. Establish the companion implementation and joint-cutover dependency, or obtain authorization to split Phase 1.bin/fm-cpu-pass.py:243- Only the wrapper owns the locks, and close_fds=True prevents its command from retaining them. If the wrapper crashes or receives SIGKILL while CPU-heavy work continues, the kernel immediately frees its passes and another burst starts alongside the surviving work. The new test constructs exactly this state at tests/fm-cpu-pass.test.sh:169-173: it asserts zero held passes before explicitly killing the still-running child. This violates the reservation-until-work-ends invariant in docs/cpu-pass-pool.md:4 and bin/fm-cpu-pass.py:10. Related ownership sites are bin/fm-cpu-pass.py:116 (close-on-exec locks) and :323 (wrapper-owned release); bin/fm-test-run.sh:2538 uses this wrapper for every script. Make reservation ownership survive with executing work at the shared child-launch boundary, and correct the crash assertion.tests/fm-cpu-pass.test.sh:26- Failure cleanup sends TERM only to recorded wrapper PIDs, but bin/fm-cpu-pass.py:239 deliberately ignores TERM while its child runs. An assertion failure while a holder is waiting for its release file therefore leaves both wrapper and workload alive; deleting the fixture removes the possibility of release. Their inherited output descriptors can also keep the runner's streaming tee waiting indefinitely. Affected holder launches are tests/fm-cpu-pass.test.sh:99, :139, :163, :183, :205, :231, :244 and :304; other tracked background launches are :110, :144, :210 and :309. The crash case's child cleanup at :172-173 occurs only on success. Track and terminate the fixture workloads or their isolated groups, then wait for termination before removing fixtures.bin/fm-load-report.py:163- A run that reached review, was cancelled or failed for a non-timeout reason, and had zero to two review-fix invocations is reported as converged. Ten such unsuccessful runs produce converged_within_2_fix_rounds=true. Additionally, the selection at :141 includes pending runs, which bin/fm-nm-run-lib.sh:125-128 explicitly classifies as live. Determine eligibility and successful convergence at pipeline_section's shared boundary: exclude live runs and do not mark failed/cancelled runs as successful. Related changed consumers are bin/fm-load-report.py:196 (aggregate verdict), :226 (text verdict), :27-34 (documented selection), docs/cpu-pass-pool.md:62 (measurement recipe), and tests/fm-load-report.test.sh:29, :76-77 (fixture statuses and verdict expectations).bin/fm-cpu-pass.py:83- Simplification: FM_CPU_POOL_SIZE introduces a production sizing authority beyond the required pool "with its size taken from hw.ncpu". For example, two participants using the same directory with sizes 18 and 36 can hold up to 36 distinct slots on an 18-core host; the turnstile does not reconcile their budgets. No intent requirement needs this override. Remove it and use the host CPU count consistently. Related sites are bin/fm-cpu-pass.py:297 and :332 (run/status sizing), docs/cpu-pass-pool.md:26-27 (override contract), and tests/fm-cpu-pass.test.sh:67-71 plus its per-case FM_CPU_POOL_SIZE assignments.bin/fm-cpu-pass.py:293- Simplification: FM_CPU_POOL=off adds an unconditional bypass of the host-wide budget, including extra case-insensitive and whitespace-normalized spellings. A heavy suite can execute immediately while every pass is held, as tests/fm-cpu-pass.test.sh:244-251 demonstrates. The intent exempts interactive agents, not CPU-heavy test bursts, and supplies no requirement for this opt-out. Remove the bypass while retaining the separately necessary nested-work handling. Related sites are bin/fm-test-run.sh:127 and :2402, docs/cpu-pass-pool.md:37 and :54, and tests/fm-cpu-pass.test.sh:249-255.bin/fm-cpu-pass.py:301- Simplification: silently clamping an oversized reservation is not required by the CPU-budget intent and can under-account the command's actual workers. On a two-core host, the documentedrun --passes 4 -- pytest -n 4takes only two passes but still starts four workers; clamping the reservation does not resize pytest. Remove silent clamping and use the narrower exact-count contract: callers size their workers usingsizeand reserve that same positive count. Related sites are bin/fm-cpu-pass.py:30 and :375, docs/cpu-pass-pool.md:34 and :48, and tests/fm-cpu-pass.test.sh:80-84, which currently endorses oversized clamping.🔧 Fix applied.
3 warnings still open:
bin/fm-load-report.py:163- A run that reached review, was cancelled or failed for a non-timeout reason, and had zero to two review-fix invocations is reported as converged. Ten such unsuccessful runs produce converged_within_2_fix_rounds=true. Additionally, the selection at :141 includes pending runs, which bin/fm-nm-run-lib.sh:125-128 explicitly classifies as live. Determine eligibility and successful convergence at pipeline_section's shared boundary: exclude live runs and do not mark failed/cancelled runs as successful. Related changed consumers are bin/fm-load-report.py:196 (aggregate verdict), :226 (text verdict), :27-34 (documented selection), docs/cpu-pass-pool.md:62 (measurement recipe), and tests/fm-load-report.test.sh:29, :76-77 (fixture statuses and verdict expectations).bin/fm-cpu-pass.sh:14- Round 1 fixed reservation validation in the Python engine but left the no-python sibling behind. This loop discards --passes without validating it, so with python3 unavailable,fm-cpu-pass.sh run --passes 0 -- COMMANDexecutes COMMAND and returns its status instead of refusing work with 125. Negative, malformed, and oversized counts likewise bypass validation, in both split and equals forms. Validate the count before the degraded exec, preserving the authorized fallback for valid requests. Related invariant sites: bin/fm-cpu-pass.sh:29 (unchecked execution), bin/fm-cpu-pass.py:290 (normal-path validation), docs/cpu-pass-pool.md:36 (invalid counts never start work), tests/fm-cpu-pass.test.sh:124 and :129 (normal and nested refusal cases), and tests/fm-cpu-pass.test.sh:327 (no-python case currently exercises only a valid request).bin/fm-load-report.py:144- The intent requires that "the next 10 runs converge in at most 2 fix rounds", but the added query usesORDER BY created_at DESC LIMIT ?, measuring the latest ten instead. With twenty eligible runs after the recorded cutover epoch, a three-round failure in the first ten disappears when the last ten converge, and the report returns true despite the required cohort failing. Round 1's false-convergence fix corrected terminal-status eligibility but left this independent window-selection issue unchanged. Use the first ten eligible runs after --since for the acceptance verdict; retaining a separate rolling cohort would need authorization. Related sites: bin/fm-load-report.py:26 (most-recent selection contract), :196 (aggregate verdict), docs/cpu-pass-pool.md:69-71 (fixed cutover epoch paired with latest-ten reporting), and tests/fm-load-report.test.sh:75-83 (exactly ten eligible fixtures cannot distinguish the cohorts).🔧 Fix applied.
4 warnings still open:
bin/fm-load-report.py:163- A run that reached review, was cancelled or failed for a non-timeout reason, and had zero to two review-fix invocations is reported as converged. Ten such unsuccessful runs produce converged_within_2_fix_rounds=true. Additionally, the selection at :141 includes pending runs, which bin/fm-nm-run-lib.sh:125-128 explicitly classifies as live. Determine eligibility and successful convergence at pipeline_section's shared boundary: exclude live runs and do not mark failed/cancelled runs as successful. Related changed consumers are bin/fm-load-report.py:196 (aggregate verdict), :226 (text verdict), :27-34 (documented selection), docs/cpu-pass-pool.md:62 (measurement recipe), and tests/fm-load-report.test.sh:29, :76-77 (fixture statuses and verdict expectations).bin/fm-load-report.py:144- The intent requires that "the next 10 runs converge in at most 2 fix rounds", but the added query usesORDER BY created_at DESC LIMIT ?, measuring the latest ten instead. With twenty eligible runs after the recorded cutover epoch, a three-round failure in the first ten disappears when the last ten converge, and the report returns true despite the required cohort failing. Round 1's false-convergence fix corrected terminal-status eligibility but left this independent window-selection issue unchanged. Use the first ten eligible runs after --since for the acceptance verdict; retaining a separate rolling cohort would need authorization. Related sites: bin/fm-load-report.py:26 (most-recent selection contract), :196 (aggregate verdict), docs/cpu-pass-pool.md:69-71 (fixed cutover epoch paired with latest-ten reporting), and tests/fm-load-report.test.sh:75-83 (exactly ten eligible fixtures cannot distinguish the cohorts).bin/fm-load-report.py:146- Round 2's oldest-first fix still recomputes the cohort from current eligibility. Concrete sequence: an older run remains running while ten newer runs complete successfully; the report returns true. When the older run subsequently fails after reaching review, it enters the oldest-first cohort, displaces a successful run, and changes the verdict to false. This contradicts the recorded requirement: "once 10 eligible runs exist the verdict is fixed." Related sites: bin/fm-load-report.py:143 (live-run exclusion), :197 (recomputed verdict), :202 (JSON cohort), :225 (text verdict), :35 (stability promise); docs/cpu-pass-pool.md:74 (same promise); tests/fm-load-report.test.sh:37 and :83 (older live fixtures never transition). Address this at pipeline_section's cohort-selection boundary. Freezing cohort membership and its verdict requires durable state keyed by cutover; that remedy, rather than the defect, needs authorization because the reporter currently promises read-only operation.bin/fm-load-report.py:276- Simplification: the new --runs option makes the acceptance cohort arbitrarily configurable, although the recorded decision requires "the first 10 eligible runs after --since." With one successful eligible run, report --runs 1 returns converged_within_2_fix_rounds=true without the required ten-run evidence. No stated intent requires an alternate acceptance cohort. Remove this option and use the narrower fixed-ten contract, rather than maintaining another verdict mode. Related sites: bin/fm-load-report.py:8 and :26 (public option contract), :146 (variable query limit), :197 and :201 (variable verdict threshold and output), :251 (argument forwarding), :297 (option validation).🔧 Fix applied.
2 issues (1 error, 1 warning) still open:
docs/cpu-pass-pool.md:64- The intent requires Phase 1 to "land together" and explicitly includes a CPU pool "used by Vernant's pytest -n worker sizing ... and by Firstmate's fm-test-run.sh". This added line instead defers Vernant participation to a follow-up, while docs/cpu-pass-pool.md:63 declares the coordinated cutover satisfied by Firstmate plus F6 alone. Round 1 introduced this cutover section, but it leaves a required CPU-demand source outside the documented rollout. Coordinate the Vernant participant before declaring Phase 1 complete, or obtain explicit authorization for narrower Firstmate-only containment.bin/fm-cpu-pass.py:198- Collected slots retain their previous holder records until the entire reservation is acquired. With two slots, let an earlier run leave its label in slot 1, another run hold slot 0, and a two-pass request collect slot 1 while waiting for slot 0. Status and periodic wait notices now identify the finished earlier run as a current holder, rather than the waiting collector; this persists throughout the wait and misdirects timeout diagnosis. Write the current reservation metadata when each slot is acquired. Related sites: bin/fm-cpu-pass.py:215-220 (deferred metadata publication), :147 (reading stale records), :175 (wait-notice consumer), :335-344 (status consumers), and tests/fm-cpu-pass.test.sh:195 (partial-collection coverage currently checks only the held count).🔧 Fix applied.
1 error still open:
bin/fm-test-run.sh:2402- Nested execution bypasses reservations without limiting concurrency to the inherited pass count. Concrete intended path: an outer runner reserves one pass for tests/fm-test-run.test.sh; that test invokes the real runner with --jobs 2 over fm-session-lock-ancestry.test.sh and fm-task-inbox.test.sh (tests/fm-test-run.test.sh:1628, executed at :2052). Both nested scripts run concurrently with FM_CPU_PASS_HELD=1 and take no additional passes. With the other host slots occupied, the fleet executes more scripts than its CPU budget while status reports a full, correctly sized pool. Round 1's reservation-validation fix left this sibling invariant unchecked: bin/fm-cpu-pass.py:289 validates against host size, but :292 permits a nested --passes 2 request under an inherited one-pass reservation. Related changed sites: bin/fm-test-run.sh:2537 (reservation bypass), docs/cpu-pass-pool.md:36-40 (exact worker-count and nesting contracts), and tests/fm-cpu-pass.test.sh:483-498 (nested coverage exercises only one worker). Enforce the inherited reservation at the shared nested-dispatch boundary: nested concurrency must not exceed its reserved count, and callers needing parallel work must reserve sufficient passes before starting the outer workload rather than acquiring more while holding an insufficient reservation.🔧 Fix applied.
✅ Re-checked - no issues remain.
🔧 **Test** - 2 issues found → auto-fixed ✅
bin/fm-cpu-pass.sh:75- Live execution with Python absent from a restricted PATH ran the workload with FM_CPU_PASS_HELD=0 but produced empty stderr instead of the promised degradation notice. The group's 2>/dev/null also redirects the notice when log_fd defaults to 2. An explicit --log-fd 1 emitted the notice, isolating this to default-stderr handling. This silently removes the CPU budget without telling the operator, undermining timeout diagnosis. Preserve the original notice destination while suppressing only write errors, and add a public-command regression that checks default stderr rather than only a separate log fd.bash tests/fm-cpu-pass.test.shwith a worktree-local TMPDIR; passed.bash tests/fm-load-report.test.shwith a worktree-local TMPDIR; passed.python3 .live-validation/drive.pydrove the real CPU-pool and reporting commands with an isolated pool at the actual 18-CPU host size; retained the failed default-notice observation and completed the remaining scenarios.fm-test-run.sh --jobs 2 tests/fm-brief.test.sh tests/fm-composer-lib.test.shin a disposable repository containing unchanged product scripts: observed two separate one-pass reservations while 16 other passes were held, concurrent execution, and propagation of a deliberate worker exit 7.fm-load-report.sh report --samples <disposable.tsv> --since 1000 --nm-db <disposable.sqlite>in JSON and text modes: exercised pending, true, and false convergence, cohort stability, timeout classification, load thresholds, and byte-for-byte database read-only behavior.fm-load-report.sh report --runs 1refused the removed option; reporting against an absent database exited 1, retained load facts, and did not create the database.Confirmed the isolated live pool had zero held passes, then removed all disposable drivers, fixture repositories, pools, databases, restricted-PATH tools, and temporary data.🔧 Fix applied.
✅ Re-checked - no issues remain.
TMPDIR=<worktree disposable tmp> bash tests/fm-cpu-pass.test.shTMPDIR=<worktree disposable tmp> bash tests/fm-load-report.test.shbin/fm-cpu-pass.sh sizeandstatus --jsonagainst a private pool: confirmed the actual 18-CPU host size.Public CPU-pool commands with a 17-pass holder, a partially collecting two-pass request, and a queued waiter; inspected status and notices, cancelled the waiter, and released the holder.Killed a CPU-pool wrapper while its child remained running; inspected the retained reservation, terminated the child, and observed release.Exercised invalid counts, nested over-requests, malformed inherited markers, an unusable pool directory, and a restricted PATH without Python.Ran the real runner in a disposable repository under a real one-pass reservation with--jobs 2; inspected execution events, notice output, and timing JSON.Ran the real runner with--per-script-timeout-secs 3while all 18 passes were occupied; released the pool after the script bound had elapsed, then separately exercised a hung script with a one-second bound.bin/fm-load-report.sh recordandwatch --interval .2against a private samples file; inspected actual host samples.Ran text and JSON reports against disposable samples and SQLite state through pending, failed, and later-successful run transitions; checked database hashes for read-only behavior.Exercised load-threshold verdicts, successful and excessive-fix cohorts, rejected--runsoptions, and an absent database.Stopped validation processes, confirmed zero held passes, and removed all disposable worktree fixtures.✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.