feat(spawn): add preflight resolver to validate harness and project before worktree creation#7
Merged
Conversation
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
The developer wanted to evaluate their AI agent fleet from a productivity standpoint and fix the root cause of agents failing to parallelize file analysis - not through text-instruction band-aids that bloat context windows, but through a structural architectural solution. They opted in permanently to using the Workflow tool for multi-file analysis tasks and asked firstmate to help design a measurable productivity metric framework based on output rate and attention cost (escalations per task). They approved establishing a temporary weekly productivity log at data/productivity-log.md with a schema clean enough to later migrate to Linear or Notion. Finally, they decided to create a personal GitHub fork of the firstmate repository (ryxli/firstmate), directly merge the herdr migration branch into main, and update the local origin remote to point at their fork rather than the upstream kunchenguid/firstmate.
What Changed
bin/fm-resolve-spawn.sh, a preflight script that verifies the harness binary is on PATH, warns when the target project is not in the registry, and confirms the worktree base is writable - aborting before any git or herdr state is created on failure.bin/fm-spawn.shso every non-secondmate spawn runs the preflight automatically.tests/fm-resolve-spawn.test.sh) covering harness-missing, unregistered-project, and happy-path cases; fixed PATH coupling intests/fm-spawn-batch.test.shso batch tests no longer assume a specific harness is installed on the host.Risk Assessment
✅ Low: Both round-1 findings were correctly fixed: the batch tests now create a hermetic fake harness binary instead of relying on
ompbeing installed, and the redundant grep pattern was removed. No new issues introduced.Testing
All 5 new resolver tests and all 7 batch tests pass. End-to-end verification confirms the preflight catches a missing harness binary with a clear error before any git worktree or herdr pane is created, and that unregistered projects emit a non-fatal warning while valid harness+project pairs pass silently.
Evidence: Spawn preflight CLI transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
tests/fm-spawn-batch.test.sh:96- tests/fm-spawn-batch.test.sh lines 96 and 114 switched from 'codex' to 'omp' as the harness argument to keep reaching the brief-check stage after the new preflight. But unlike the new resolver tests (which create a fakebin directory), these two tests rely on 'omp' being present on PATH at test time. If CI runs without omp installed, the resolver exits early with "spawn harness binary 'omp' was not found on PATH" and the grep for "error: no brief at ..." fails, making the test assertion wrong rather than gracefully skipped. The resolver tests themselves demonstrate the correct pattern: create a fake harness binary in a temp dir and prepend it to PATH.bin/fm-resolve-spawn.sh:52- fm-resolve-spawn.sh line 52: the second grep -e pattern "-e &fix(watcher): lossless check wakes via watcher-side suppression + enqueue-before-suppress kunchenguid/firstmate#34;- ${project_name} [&fix(watcher): lossless check wakes via watcher-side suppression + enqueue-before-suppress kunchenguid/firstmate#34;" is redundant. Any registry line matching "- name [" also contains the literal "- name " (the space before '[' satisfies the first pattern), so the first -e always catches both cases. Only the first pattern is needed.bin/fm-resolve-spawn.sh:31- When no harness override is given, fm-harness.sh crew is now invoked twice per non-secondmate spawn: once inside fm-resolve-spawn.sh (line 31) and again inside fm-spawn.sh (line 131). This doubles the subprocess cost for the common code path. Not a correctness issue given spawning is a low-frequency operation.🔧 Fix: fix batch test PATH coupling and redundant grep -e pattern
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-resolve-spawn.test.shbash tests/fm-spawn-batch.test.shManual CLI transcript:fm-resolve-spawn.sh alpha codexwith codex absent (exits 1 with actionable error)Manual CLI transcript:fm-resolve-spawn.sh alpha ompwith valid harness (exits 0 silently)Manual CLI transcript:fm-resolve-spawn.sh projects/unknown ompunregistered project (exits 0 with warning)Manual CLI transcript:fm-spawn.sh mytest-k7 projects/alpha codexwith missing harness confirms no worktree is created on failure✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.