Skip to content

fix: harden supervision feedback and worker launches - #31

Merged
BohnBawerick merged 24 commits into
mainfrom
fm/fm-contributions-read-cap-false-unavailable
Oct 4, 2026
Merged

BohnBawerick merged 24 commits into
mainfrom
fm/fm-contributions-read-cap-false-unavailable

Conversation

@BohnBawerick

@BohnBawerick BohnBawerick commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Intent

[captain] The captain's standing rule for all projects, verbatim: "Fix what you find. Lint failures and flaky tests get fixed even when the current task did not cause them."

Context, found 2026-10-03: bin/fm-contributions.sh observes each owned PR with one core read and then six parallel forge reads, each capped at 5 seconds. For #20 (head 72dbec0) the commit check-runs read with filter=all returns about 67 KB and takes 1 to 3 seconds alone, but when it runs beside the other five reads it hits the 5 second cap about one poll in four on this host. A timeout that is not the poll budget's own deadline counts as forge-unavailable, so the PR's record gets error "forge observation unavailable or changed during read", its observation goes stale, and the supervisor gets a "contributions: observation unavailable" check wake each failure episode. A traced poll showed rc=124 on one parallel read with every other read returning 0; running the six reads in parallel by hand four times gave one rc=124 on a commits read. https://github.com/BohnBawerick/VoiceMaster/pull/35 shows the same steady error.

The same file has a second known host fault, filed as fm-contributions-shasum-missing-on-linux on 2026-09-19: publish_pending computes its wake key with shasum -a 256, and on this Arch host shasum lives only in /usr/bin/core_perl, which may be absent from the watcher's PATH, giving an empty key; tests/fm-contributions.test.sh failed locally for that reason.

Firstmate specification (requirements, not the captain's literal words):

  1. Load the firstmate-coding-guidelines skill before editing.
  2. Make a slow but healthy forge read stop producing a false "observation unavailable" in bin/fm-contributions.sh. Read the file's header and poll/observe/forge/wait_forges first; the header owns the observation bounds. Choose the smallest change that keeps the header's guarantees honest, for example a per-read cap that fits the existing per-URL reserve, or treating a per-read timeout like the budget-cut "unmeasured" case so the prior record is kept and no wake fires. A genuine forge error (nonzero exit other than a timeout) must still count as unavailable. Update the header text to match.
  3. Reproduce first: show the false unavailable with a test double for gh that sleeps past the cap on one parallel read, then make it pass. Add that as a regression test in tests/fm-contributions.test.sh.
  4. Fold in fm-contributions-shasum-missing-on-linux: check whether shasum is reachable on the watcher's PATH on this host; if the wake key can come out empty, use the repo's existing hashing helper if one exists, otherwise fall back to sha256sum and fail with a named error when neither exists. Grep every other shasum caller in bin/ for the same fault and fix the ones that share it. If it is not reproducible, say so in the PR body with the evidence and change nothing for it.
  5. Out of scope: any other change to the observer, its schema, or its wake policy.

What Changed

  • Parse table and list-form Lavish feedback, preserve Unicode and nested metadata, reject incomplete reads, and match Pi's stock argument and preview rendering.
  • Gate Codex max effort on installed catalog support during validation and launch, with warnings relayed through local and remote recovery paths.
  • Treat capped contribution reads as unmeasured instead of unavailable, and add portable SHA-256 fallbacks for contribution wake keys and pending-reply IDs.

Risk Assessment

✅ Low: The timeout and hashing changes are bounded, satisfy the stated intent, preserve genuine forge failure handling, and add no unnecessary behavior.

Testing

Drove a healthy baseline and all four required boundary cases through isolated product paths against real PR 20, captured the CLI transcript, ran both focused regression scripts, and confirmed cleanup. Everything passed.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
A normal poll observes the real PR and persists its current head without error ✅ pass live Live transcript records a healthy observation of head 72dbec0 on the first attempt.
A slow healthy forge read keeps the prior record and emits no unavailable notification ✅ pass live Live transcript shows identical prior and final record hashes, empty poll output, and zero wake rows after the check-runs read exceeded its cap.
A genuine non-timeout forge error records unavailable evidence and emits its notification ✅ pass live Live transcript shows the targeted wrapper exiting 1, the unavailable notification, and the expected persisted error against the real PR head.
When shasum is absent, sha256sum generates usable contribution and pending-reply identifiers ✅ pass live Live transcript shows shasum unreachable, a 64-character contribution wake key from the real poll, and a 16-character pending-reply ID.
When no SHA-256 hasher exists, correlation-ID generation returns the named error ✅ pass live Live transcript shows exit status 1 and fm-pending-reply: no SHA-256 hasher available (need shasum or sha256sum).
Evidence: Live PR 20 contribution and hashing validation

Source: Live PR 20 contribution and hashing validation

Live target
  URL: https://github.com/BohnBawerick/firstmate/pull/20
  observed head: 72dbec0bbadb19d00052ec70f74f4a7bbc9f98fb
  healthy baseline attempts: 1

Scenario: slow healthy check-runs read exceeds the five-second cap
  command: real bin/fm-contributions.sh poll with a gh wrapper that sleeps 6 seconds only for check-runs, then would exec the real gh
  wrapper: slow wrapper: sleep 6 before real gh for repos/BohnBawerick/firstmate/commits/72dbec0bbadb19d00052ec70f74f4a7bbc9f98fb/check-runs?filter=all&per_page=100
  elapsed: 7s
  poll stdout: <empty>
  prior record sha256: 723291ae33235609ecec2f3d2a15ec3d80c2f1fb4b033102945813e16e6b56cb
  final record sha256: 723291ae33235609ecec2f3d2a15ec3d80c2f1fb4b033102945813e16e6b56cb
  record unchanged: yes
  unavailable line printed: no
  wake queue rows: 0

Scenario: genuine non-timeout check-runs failure
  command: real bin/fm-contributions.sh poll with a gh wrapper that exits 1 only for check-runs
  wrapper: failure wrapper: exit 1 for repos/BohnBawerick/firstmate/commits/72dbec0bbadb19d00052ec70f74f4a7bbc9f98fb/check-runs?filter=all&per_page=100
  poll stdout: contributions: observation unavailable for https://github.com/BohnBawerick/firstmate/pull/20
  persisted result: {"checked_at":"2026-10-03T12:40:00Z","error":"forge observation unavailable or changed during read","head":"72dbec0bbadb19d00052ec70f74f4a7bbc9f98fb"}

Scenario: shasum absent, sha256sum available
  command: real bin/fm-contributions.sh poll on a pruned PATH, plus fm_pending_reply_new_id on a SHA-256-only PATH
  shasum reachable: no
  sha256sum: /usr/bin/sha256sum
  real poll attempts: 1
  contribution wake key: 14d86f6d00cc132bf750047a2333808fedf161ced5bad16ae6d88a6a13ea07fd
  pending-reply id: c96ecfe5f0d90650

Scenario: no SHA-256 hasher available
  command: fm_pending_reply_new_id with openssl, shasum, and sha256sum absent from PATH
  exit status: 1
  stderr: fm-pending-reply: no SHA-256 hasher available (need shasum or sha256sum)
- Outcome: 🔧 1 issue found → no changes applied ✅ across 2 runs (25m35s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

⚠️ **Rebase** - 1 warning
  • ⚠️ .agents/skills/harness-adapters/references/harness/codex.md - branch carries 21 commit(s) that exist on your local main branch but were never pushed to origin/main; these may be unintended bundled work (proposed PR changes 28 file(s)):
  • 72dbec0 no-mistakes(ci): Fixed both CI failures. Updated the Lavish text-range assertion to match nested path metadata output and added fm-codex-catalog-lib.sh to the synthetic remote-root fixture. Both affected test files pass locally. Bash syntax and git diff checks also pass
  • d393b4c no-mistakes(document): Document Codex catalog validation and fallback
  • 68b48e7 no-mistakes(review): Validate Codex catalog schema before enabling max
  • 5bf3d87 no-mistakes(document): Document Codex catalog helper
  • fce2fe2 no-mistakes(review): Reject truncated Lavish list items as incomplete
  • ca58502 no-mistakes(review): Relay Codex max warnings through secondmate restarts
  • 112c121 no-mistakes(document): Document catalog validation and feedback metadata
  • 5298637 no-mistakes(review): Relay Codex max downgrade warnings through recovery
  • 25ab3ef no-mistakes(review): Preserve Lavish metadata and harden Codex catalog validation
  • 4299989 Validate Codex max effort from catalog
  • 61985d8 fix(pi): support Pi 1.0 rendering contracts
  • 48b50ae no-mistakes(document): Document catalog-based Codex max effort
  • 10d8297 no-mistakes(document): Align Codex and Lavish documentation
  • 4ea5d84 no-mistakes(review): Reject malformed Lavish lists and isolate Codex catalogs
  • c25c34e Pass supported Codex max effort to workers
  • 8a87193 no-mistakes(document): Document Lavish result decoding guarantees
  • 2a37594 Preserve Lavish UTF-8 feedback
  • e66a604 no-mistakes(ci): Updated the malformed-capture regression to require read's nonzero incomplete verdict while retaining output assertions. tests/fm-procevent.test.sh, tests/fm-bearings-board-render.test.sh, bin/fm-lint.sh, and git diff --check pass. Serial 8 was a transient runner failure and passed six local runs
  • c1e01b4 no-mistakes(document): Document Lavish list-form feedback parsing
  • 9147025 no-mistakes(review): Parse table rows beyond declared counts
  • e9d8cc2 Handle Lavish list-form feedback

Confirm these commits belong in this PR before approving, or manually separate the intended work onto origin/main before gating.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • ⚠️ bin/fm-pending-reply-lib.sh:163 - Requirement 4 requires shared shasum callers to receive a working fallback. Under the library's documented set -u contract, a caller whose PATH has sha256sum but lacks openssl and shasum aborts while expanding unset raw before reaching this new branch. If openssl fails and neither hasher exists, unset hex also aborts without the required named error. Initialize both variables and explicitly handle the no-hasher case.

🔧 Fix applied.
✅ Re-checked - no issues remain.

🔧 **Test** - 1 issue found → no changes applied ✅
  • ⚠️ live validation verdict: inconclusive (1 of 5 scenarios were driven live against the product); untested: A slow but healthy parallel forge read exceeds its five-second cap, leaves the prior record unchanged, and produces no unavailable notification, A genuine non-timeout forge error still records unavailable evidence and emits the expected notification, When shasum is absent, contribution wake keys and pending-reply correlation IDs use sha256sum successfully, When no SHA-256 hasher exists, pending-reply ID generation returns a named error instead of aborting on an unset variable
  • Live validation: ⚠️ inconclusive - 1 of 5 scenarios driven live against the product
Scenario Result Live Evidence
A slow but healthy parallel forge read exceeds its five-second cap, leaves the prior record unchanged, and produces no unavailable notification ⏸️ untested no The prior payload recorded only regression and focused test logs, not a result driven against the live product.
An authenticated contribution poll repeatedly reads #20 and persists a coherent healthy observation for its reported head and 19 checks ✅ pass live Live GitHub polling log
A genuine non-timeout forge error still records unavailable evidence and emits the expected notification ⏸️ untested no The prior payload recorded only a focused test log, not a result driven against the live product.
When shasum is absent, contribution wake keys and pending-reply correlation IDs use sha256sum successfully ⏸️ untested no The prior payload recorded only focused test logs, not a result driven against the live product.
When no SHA-256 hasher exists, pending-reply ID generation returns a named error instead of aborting on an unset variable ⏸️ untested no The prior payload recorded only a focused test log, not a result driven against the live product.
  • bash tests/fm-contributions.test.sh
  • bash tests/fm-pending-reply.test.sh
  • Replayed tests/fm-contributions.test.sh against archived base commit 71bd89cd2ba7819d71e647849c163e695371106b to confirm the regression fails before the fix
  • Ran four isolated bin/fm-contributions.sh poll calls through the authenticated real GitHub CLI against https://github.com/BohnBawerick/firstmate/pull/20
  • Checked host PATH resolution for /usr/bin/core_perl/shasum and /usr/bin/sha256sum
  • Confirmed the source tree remained clean and temporary test directories were removed

🔧 No changes applied.
✅ Re-checked - no issues remain.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
A normal poll observes the real PR and persists its current head without error ✅ pass live Live transcript records a healthy observation of head 72dbec0 on the first attempt.
A slow healthy forge read keeps the prior record and emits no unavailable notification ✅ pass live Live transcript shows identical prior and final record hashes, empty poll output, and zero wake rows after the check-runs read exceeded its cap.
A genuine non-timeout forge error records unavailable evidence and emits its notification ✅ pass live Live transcript shows the targeted wrapper exiting 1, the unavailable notification, and the expected persisted error against the real PR head.
When shasum is absent, sha256sum generates usable contribution and pending-reply identifiers ✅ pass live Live transcript shows shasum unreachable, a 64-character contribution wake key from the real poll, and a 16-character pending-reply ID.
When no SHA-256 hasher exists, correlation-ID generation returns the named error ✅ pass live Live transcript shows exit status 1 and fm-pending-reply: no SHA-256 hasher available (need shasum or sha256sum).
  • bash .live-contributions-validation.sh 2&gt;&amp;1 | tee ~/.no-mistakes/evidence/01M40RQWFX32PEFM1FFR0B4EXA/live-contributions-pr20.txt using an isolated FM_HOME and real PR 20
  • bash tests/fm-contributions.test.sh
  • bash tests/fm-pending-reply.test.sh
  • Inspected the saved evidence for record preservation, notification behavior, identifier formats, and the named missing-hasher error
  • Confirmed the transient validation directory and driver were removed and git status --short --untracked-files=all was empty
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…read's nonzero incomplete verdict while retaining output assertions. `tests/fm-procevent.test.sh`, `tests/fm-bearings-board-render.test.sh`, `bin/fm-lint.sh`, and `git diff --check` pass. Serial 8 was a transient runner failure and passed six local runs
…e assertion to match nested path metadata output and added fm-codex-catalog-lib.sh to the synthetic remote-root fixture. Both affected test files pass locally. Bash syntax and git diff checks also pass
A forge read killed by its five-second cap was recorded as forge
unavailable, staled the observation, and rang a false supervision wake
whenever a healthy read ran slow beside its five parallel siblings.
Treat every deadline-cut read, budget deadline or per-read cap, as
unmeasured: the prior record is kept and the URL is observed first on
the next poll. A genuine forge failure still records the error and
wakes once per episode.

publish_pending hashed its wake key with shasum, which this Arch host
carries only under /usr/bin/core_perl; a watcher PATH without it minted
empty wake keys and broke wake dedup. Hash through shasum or sha256sum
and fail with a named error when neither exists. The same sha256sum
fallback is folded into the pending-reply correlation-id fallback.

Regression tests cover both: a slow parallel read keeps the prior
record without a wake, and a PATH without core_perl still publishes a
64-hex wake key exactly once.
… test for Pi 1.0's renderer API while retaining older compatibility, and increased two test-only process startup windows to avoid loaded-runner flakes. All three focused tests pass; timing-sensitive tests passed five repeated runs each. ShellCheck, actionlint, Bash syntax, and git diff checks pass
@BohnBawerick
BohnBawerick merged commit 1726136 into main Oct 4, 2026
19 checks passed
@BohnBawerick
BohnBawerick deleted the fm/fm-contributions-read-cap-false-unavailable branch October 4, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant