feat(bin): safely retire task records without teardown - #2466
Open
coreldh wants to merge 9 commits into
Open
Conversation
…mic metadata publication
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
Close F1 and F2 from the independent record-retirement enforcement gate on top of PR #2458, and add focused behavioral coverage for F3, F4, and F5. Prove all five previously uncovered marker_active call sites discriminate by reverting each guard and observing RED, then restore GREEN. Stop re-deriving seen and heartbeat surfaced paths by making the producer-owned path functions the single spelling used by retirement. Individually close the four initially surviving planning mutants: F2-08 producer-path omission, F4-04 target-window normalization bypass, F4-05 competing-record normalization bypass, and F4-06 artifact-window normalization bypass; all four must be named in the PR body with their one-substitution RED verdicts and zero harness-class exits. Preserve the hard boundary: never execute fm-record-retire.sh, never run any test that invokes retirement, and touch no real state record or pooled worktree copy; safe focused library, watcher, spawn, and session-start fixtures are allowed only in scratch homes. Never weaken a property. Publish an open HTTPS pull request to kunchenguid/firstmate without merging. Upstream has zero configured checks, so treat the completed local pipeline gate as delivery evidence and do not wait for maintainer-controlled checks.
What Changed
Risk Assessment
✅ Low: Captain, both prior defects are closed at their shared boundaries: metadata publication is atomic and marker-safe, while partial-bin spawns preserve marker-free compatibility and fail closed on existing markers.
Testing
Repository and safety preflight, focused library fixtures, watcher/spawn/session-start behavioral suites, a direct live-versus-retired CLI demonstration, and six counterfactual RED substitutions all succeeded; evidence is transcript-based because this is shell/CLI behavior with no rendered UI, and the prohibited retirement executable and real fleet state were never touched.
Evidence: Live, retired, and counterfactual RED behavior demo
Evidence: Focused marker-library transcript
Evidence: Watcher and heartbeat transcript
Evidence: Spawn marker lifecycle transcript
Evidence: Session-start orphan visibility transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
bin/fm-spawn.sh:2628- The fresh-spawn path removes the retirement marker before replacement metadata is safely published. If the process is interrupted or the metadata write at line 2693 fails, no metadata and no marker remain, so late writes from the deliberately untouched retired agent become actionable again. Keep the validated marker through an atomic metadata publication, then clear it under the existing lifecycle locks only after the complete canonical record exists.🔧 Fix: Captain, preserve retirement markers through atomic metadata publication
1 error still open:
bin/fm-spawn.sh:2627- The atomic-publication fix now callsfm_record_retire_marker_pathandfm_record_retire_marker_validate_for_spawndirectly, butfm-wake-lib.sh's supported partial-bin fallback defines neither. A recovery copy withoutfm-record-retire-lib.shtherefore aborts every spawn here withcommand not found, even when no marker exists. Extend the shared fallback with path resolution and fail-closed prevalidation, preserving its rule that absent support permits marker-free spawns but refuses any existing marker.🔧 Fix: Captain, complete partial-bin retirement marker compatibility
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
git diff --stat ef35d799a846d676c2fd30b1d1e3ed47b0fb2c22..252ceef80ac9310ba4421613de31c913496fe845and targeted call-site/diff inspectionFM_RECORD_RETIRE_TEST_ONLY=<marker-safe selector> bash tests/fm-record-retire.test.shfor full and incremental decision scans, status snapshots, live-metadata audibility, producer-owned paths, normalization collisions, and queue-lock releasebash tests/fm-supervision-events.test.shbash tests/fm-spawn-dispatch-profile.test.shbash tests/fm-session-start.test.shManual scratch-home CLI probe comparing marker-plus-live-metadata behavior against post-metadata-removal retirement behaviorCounterfactualmarker_active -> marker_validsubstitutions for full, incremental, snapshot, captain-scan, wake-append, and watcher-signal surfaces; each produced the expected RED hidden-live-state verdictgit status --shortafter testing✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.