Conversation
… avoid jq argv overflow (#10) * fix: bound startup reconciliation and large fleet input * no-mistakes(review): Drop redundant contribution-input EXIT trap in fleet snapshot * no-mistakes(test): Widen cleanup deadline test budget to avoid load flakes * no-mistakes(document): Document startup summary deferral and herdr cleanup deadline * no-mistakes(ci): Lint 1 failed because ShellCheck SC2329 ("function never invoked") fired at tests/fm-herdr-session-cleanup.test.sh:356. That line is a subshell copy of fixture_workspaces that replaces the file's main version. The fake herdr command calls fixture_workspaces indirectly when it answers `workspace list` and `api snapshot`, and ShellCheck can't see that call. The fix is one comment line above the replacement: `# shellcheck disable=SC2329 # invoked indirectly by the fake herdr workspace list.` The same file already does this for its other indirectly-called replacements (lines 43 and 49), as do tests/fm-daemon.test.sh and tests/fm-bootstrap.test.sh. No behavior changed. Checked locally: `bin/fm-lint.sh tests/fm-herdr-session-cleanup.test.sh` passes with pinned ShellCheck 0.11.0 and full extended analysis, and `bash tests/fm-herdr-session-cleanup.test.sh` passes every test, including the journal-read-count, deadline, lock and identity tests. The change is not committed
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.
Intent
Send the fork's genuine bug fixes back upstream to Ken's repository (kunchenguid/firstmate), as small separate PRs, and keep fleet-specific changes in the fork. The startup repair in twilwa#10 bounds session-start projection cleanup and deferred startup reconciliation, defers summary publication, and avoids jq argument-list overflow; the deadline must also reclaim locks held by the terminated cleanup worker. The fm-send fix from fork PR #3 rejects unknown flags and stray --key arguments. The dispatch fix from fork PR #1 makes the receipt hash match the exact brief text sent to the resolver, without the fork's Jev model version pin. Fork PR #2 covers dated captain deferrals, and fork PR #5 routes no-mistakes ask-user gates back to firstmate as needs-decision; send either upstream only if upstream does not already cover it.
What Changed
bin/fm-herdr-session-cleanup.shnow does the herdr projection cleanup pass in a worker process with a time limit. The limit isFM_HERDR_SESSION_CLEANUP_TIMEOUT(default 30s). The worker is skipped when there are no journals or whenherdr/jqis missing. It checks the home's journals once to find candidates, but rereads them before every locked change. Each candidate runs in a subshell that releases its locks when it exits. Before taking the task lock and presentation lock, the worker writes a record of the lock paths plus its own PID and start time. If the worker times out or fails, the parent removes only the locks still held by that same stopped worker, per the record. Unfinished candidates are left alone, with a warning that cleanup coverage is unconfirmed.bin/fm-session-start.sh. It now starts from the deferred worker inbin/fm-startup-network.sh, in the background and in parallel with the network checks. It runs withFM_HOME_SUMMARY_IF_IDLE=1, and its time limit is capped at the startup stage budget. The network result is published before the summary process is waited on. The in-progress digest text now lists home-summary publication.contribution-inputmode,bin/fm-fleet-snapshot.shnow writes the backlog and task JSON to temporary files and reads them withjq --slurpfileinstead of--argjson. This avoids overflowing the argument list on large inputs.AGENTS.md,docs/configuration.md, anddocs/herdr-backend.mdare updated to match, and the new behavior is covered in the herdr cleanup, session-start, startup-network, and contributions test scripts.Risk Assessment
✅ Low: The fix-round changes are small and correctly ordered: the lock record is emptied only after both lock releases, so recovery data survives if the worker is killed; the journal check now runs before the temp file and timed worker are created; the test-only argument is gone and the tests exercise the real functions.
Testing
Ran the real fm-fleet-snapshot and fm-herdr-session-cleanup executables in isolated FM_HOMEs. Two scenarios passed live: the ~149 KB contribution input (base b42d4fa printed 'Argument list too long', exited 0 with empty output; target returned all 72 records and cleaned its temp dir) and the no-journal fast path (about 100 ms, no Herdr calls, no worker, no temp file, even with a read-only state dir). The deadline, stuck-worker lock reclaim, adversarial foreign-holder and finished-candidate scenarios ran against the real cleanup executable, real timeouts, real processes and real lock dirs, with a PATH herdr shim that hangs. All passed, and the base commit blocked 21 s in the same setup. They are reported untested because bin/fm-herdr-lab.sh refused to provision: it needs a running default Herdr session and this host's default session is stopped (the fleet runs in the 'firstmate' session), which the runbook forbids touching. Summary deferral is covered by existing automated tests, not a live session start. The three test files for the changed scripts all passed (fm-herdr-session-cleanup, fm-startup-network, fm-session-start). No source changes; worktree clean; temp dirs removed.
Evidence: Fleet snapshot oversized contribution input, base vs target
Source: Fleet snapshot oversized contribution input, base vs target
Evidence: Herdr cleanup deadline / lock reclaim transcript (S1-S5 + base comparison)
Source: Herdr cleanup deadline / lock reclaim transcript (S1-S5 + base comparison)
Evidence: Driver script for cleanup scenarios
Source: Driver script for cleanup scenarios
Evidence: Herdr lab provisioning refusal (no running default session)
Source: Herdr lab provisioning refusal (no running default session)
Evidence: fm-herdr-session-cleanup test output
Source: fm-herdr-session-cleanup test output
Evidence: fm-startup-network test output
Source: fm-startup-network test output
Evidence: fm-session-start test output
Source: fm-session-start test output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
bin/fm-herdr-session-cleanup.sh:352- The lock record keeps the last finished candidate, so timeout recovery acts on an unrelated task. The pass uses one lock-record file, and fm_herdr_cleanup_one writes to it only after a candidate matches this home's journals. Nothing clears it once the EXIT trap releases that candidate's locks. Workspaces from other homes, and titles that don't match a journal, return before anything is recorded, so on a large fleet the record usually still names an earlier task that is already done. Concrete sequence: task A is cleaned up and releases its locks, but the record still names A. The pass then moves on to other homes' workspaces and hits the deadline. In another process, a spawn or wake presentation is legitimately holding the shared per-session presentation lock (or A's .spawn-A.lock). Recovery reads A's record. fm_lock_try_acquire fails, and the recorded pid doesn't match the holder, so it warns 'A timed out; its presentation lock could not be reclaimed safely'. That warning names the wrong task and suggests a stuck lock that doesn't exist. Otherwise recovery needlessly takes and releases locks it never needed to touch. The same stale record is used when the worker exits with a non-124 error, and those warnings still say 'timed out'. Fix: in the fm_herdr_cleanup_one EXIT trap, after both releases, empty the record ([ -z "$FM_HERDR_CLEANUP_LOCK_RECORD" ] || : > "$FM_HERDR_CLEANUP_LOCK_RECORD") so recovery only sees a candidate that was interrupted.bin/fm-herdr-session-cleanup.sh:462- The test-only--_recover-interruptedcommand-line mode is not required by the intent. The intent requires the deadline to reclaim locks held by the killed cleanup worker. The parent process already does this by calling fm_herdr_cleanup_recover_interrupted_candidate directly (line 476). The only caller of the new public--_recover-interrupted <record>argument is tests/fm-herdr-session-cleanup.test.sh:492. That test already loads the script's functions (it calls fm_herdr_cleanup_process_identity directly), so it can call the recovery function the same way. The extra mode lets any caller point recovery at a record file (checked only by a path-prefix match), which adds surface without meeting any requirement. Suggested fix: remove the--_recover-interruptedbranch and have the test call the function from the loaded script.bin/fm-herdr-session-cleanup.sh:467- The timed worker and state-dir temp file are created even when no herdr journals exist. Every locked full session start runs this script before bootstrap, while the digest is still blocked, whatever the backend. Before this change, a home with no *.herdr-presentation journal returned almost immediately. Now the wrapper first runs mktemp in $STATE, then starts a timeout wrapper plus a second bash that reloads all the libraries, and only that worker finds there is nothing to do. On tmux or zellij homes this adds process startup to the digest's blocking path for nothing. If $STATE can't be written to, every session start also warns 'deadline lock recovery could not be prepared' even though herdr isn't in use. Suggested fix: move the existing journal check (and optionally the herdr/jq availability check) from fm_herdr_session_cleanup into the wrapper, before the mktemp and fm_run_timed calls.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-herdr-session-cleanup-e2e.test.sh(blocked: lab helper requires a running default Herdr session)bin/fm-fleet-snapshot.sh --contribution-inputwith a 148845-byte backlog on base b42d4fa (git archive) and target 5fae598, plus an isolated-TMPDIR leak checkbash evidence/drive-herdr-cleanup-deadline.sh <worktree>scenarios S1-S5 against the real bin/fm-herdr-session-cleanup.shBase b42d4fabin/fm-herdr-session-cleanup.shagainst the same hanging-Herdr home (blocked 21 s, no deadline)bash tests/fm-herdr-session-cleanup.test.shbash tests/fm-startup-network.test.shbash tests/fm-session-start.test.sh(includes test_summary_refresh_never_blocks_the_digest)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.