This repository was archived by the owner on Aug 25, 2026. It is now read-only.
fix: refine task intake and direct-PR recovery - #80
Merged
Merged
Conversation
… authorization tests
JTInventory
force-pushed
the
fm/firstmate-adopt-phase4-934-0726
branch
from
July 27, 2026 14:10
14dcb11 to
362d2b1
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Port owner kunchenguid#934 scout intake parallel
What Changed
Risk Assessment
✅ Low: Captain, the repair consistently applies WATCH_LOCK-to-task-lock ordering, revalidates after migration, and retains teardown locks through task-state deletion without introducing a material new risk.
Testing
The supplied baseline and final post-fix full behavior runs passed; focused intake, Herdr, teardown, and bounded watcher-lock checks passed, while generated Markdown evidence directly shows scout selection, immediate safe parallel dispatch, and direct-PR conflict ownership.
Evidence: Rendered Firstmate intake contract
Evidence: Generated direct-PR crewmate brief
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (4) ✅
bin/fm-pr-check.sh:258- Replacement publishing overwrites data, registration, then check while the old generation remains. A crash after either of the first two writes creates mixed artifacts that both recovery validators reject, permanently wedging guarded PR recovery. Retire the validated old generation immediately after publishing the replacement receipt, before publishing new artifacts.bin/fm-pr-check.sh:80- Guarded mode runs retirement recovery—which can delete poll artifacts and its receipt—before validating the live PR identity and expected head. A lookup failure or mismatch therefore changes durable state despite the new zero-write contract. Perform guarded live validation first, while retaining early recovery only for unguarded calls.🔧 Fix: Captain, harden guarded PR replacement crash recovery
3 errors still open:
bin/fm-pr-check.sh:208- An unguarded invocation can run while a guarded replacement receipt is active. If the optional head lookup fails, metadata is rewritten withoutpr_head, making every later guarded recovery reject the receipt. Refuse unguarded publication while a replacement receipt exists, or recover it using receipt-bound authority first.bin/fm-pr-lib.sh:854- Recovery validates one artifact generation, but removal recomputes hashes and identities afterward. A concurrent publication between those steps is therefore treated as deletion authority and removed despite not being the validated generation. Remove only using identities and hashes captured during validation, failing if any path changed.bin/fm-pr-check.sh:131- Active-receipt recovery validates the expected head but not the supplied prior head. A retry with the correct expected head and a wrong prior head can delete the valid prior generation before the request is rejected. Bind recovery to both receipt heads before any mutation.🔧 Fix: Captain, bind guarded recovery to validated receipt authority
2 errors still open:
bin/fm-pr-check.sh:77- The receipt refusal is only a point-in-time check. An unguarded process can pass it, wait while a guarded replacement completes, then overwrite the guarded metadata and artifacts without a head binding. Hold a per-task lock from receipt inspection through metadata and artifact publication.bin/fm-pr-lib.sh:943- Captured identities do not make deletion atomic:remove_exactchecks identity and hash before deleting by pathname. A concurrent publisher can replace the path between that check andrm, causing its new file to be deleted and leaving a partial generation. Serialize all same-task publishers and removers, or atomically quarantine the captured files before validation.🔧 Fix: Captain, serialize PR poll publication and recovery
2 errors still open:
bin/fm-pr-check-migrate.sh:1080- Migration still quarantines and republishes task poll artifacts without the new per-task lock. A standalone migration can race a locked publisher or teardown and recreate or partially remove that task’s poll. Run migration beforefm-pr-checktakes its task lock, then acquire each affected task lock during migration using one consistentWATCH_LOCK→ task-lock order.bin/fm-teardown.sh:364- Teardown releases the task lock before deleting task metadata. A waitingfm-pr-checkcan acquire the lock, republish a complete poll while metadata still exists, and then have teardown remove the metadata, leaving orphan authenticated artifacts. Hold the lock through metadata and related task-state deletion, including the child teardown path.🔧 Fix: Enforce migration and teardown task lock ordering
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed (2) ✅
bash bin/fm-run-behavior-tests.sh🔧 Fix: Fix hermetic Herdr and teardown test fixtures
1 error still open:
bash bin/fm-run-behavior-tests.sh🔧 Fix: Fix watcher migration lock ordering
✅ Re-checked - no issues remain.
bash bin/fm-run-behavior-tests.shBaseline supplied by the validator:bash bin/fm-run-behavior-tests.shbash tests/fm-backend-herdr.test.shbash tests/fm-backend.test.shandbash tests/fm-gotmp.test.sh(direct gate-worktree runs hit the expected lifecycle refusal; both passed through the isolated behavior runner)bash tests/fm-instruction-owners.test.shbash tests/fm-watcher-lock.test.shFirst aggregatebash bin/fm-run-behavior-tests.shexposed the fake-Herdr log interleavingFive consecutive post-fix runs ofbash tests/fm-backend-herdr.test.shFinal post-fixbash bin/fm-run-behavior-tests.shFM_HOME=/tmp/no-mistakes-evidence/01KYG43W7V8H31FFQDVN4AMH6D/fm-home bash bin/fm-brief.sh parallel-direct-pr direct-projectRenderedAGENTS.mdsection### Intakeinto/tmp/no-mistakes-evidence/01KYG43W7V8H31FFQDVN4AMH6D/intake-contract.md✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix: Captain: fix ShellCheck warnings
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.