feat(bin): gate no-mistakes with implementation readiness - #1402
Open
pranaypratyush wants to merge 5 commits into
Open
feat(bin): gate no-mistakes with implementation readiness#1402pranaypratyush wants to merge 5 commits into
pranaypratyush wants to merge 5 commits into
Conversation
4 tasks
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
Implement a durable Firstmate-owned implementation-readiness preflight that keeps feature completion out of no-mistakes, enforces project-adaptive evidence including Spec Kit/tests/clean committed intent trace and applicable design/runtime/Maestro grounding, and adds a safe exact-run Herdr attach monitor whose presentation-only lifecycle never duplicates AXI ownership, while preserving all other runtime backends and merge authority.
What Changed
Risk Assessment
Testing
Baseline inspection confirmed a clean target diff; focused readiness/monitor, brief, and teardown tests passed. Manual CLI evidence demonstrated pending and false Spec Kit claims being rejected, exact reviewed-byte approval reaching READY, later evidence mutation revoking it, preserved tmux/Zellij/Orca/cmux behavior, generated preflight instructions, and synthetic exact-run Herdr creation/retirement/fail-closed recovery. The worktree remained clean at target commit 803e85f; no real Herdr screenshot was captured because external runtime mutation was prohibited, leaving that compatibility evidence outstanding.
Evidence: Readiness CLI lifecycle transcript
Evidence: Synthetic exact-run Herdr monitor transcript
Evidence: Generated no-mistakes ship brief
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-no-mistakes-ready.sh:165- The intent requires “enforces project-adaptive evidence including Spec Kit/tests/clean committed intent trace and applicable design/runtime/Maestro grounding,” but the changed validator treats[ "${#evidence}" -ge 24 ]plus a short phrase blacklist as concrete evidence. A worker can enterfocused_tests pass abcdefghijklmnopqrstuvwxyzand analogous generic rows, receiveREADY, and begin no-mistakes with implementation work outstanding. Require structurally inspectable, axis-specific evidence or an explicit Firstmate semantic approval beforeREADY.bin/fm-no-mistakes-ready.sh:526- Monitor creation, failure cleanup, and retirement perform focus-sensitive Herdr mutations under only a per-task lock, bypassing the existing session/socket-wide presentation lock. Concurrent reconcile, spawn, or teardown operations can capture transient focus and restore the wrong tab. Acquirefm_backend_herdr_presentation_session_lock_patharound every create/restore and close/gone-confirmation sequence.bin/fm-no-mistakes-ready.sh:414- Process verification accepts any foreground argv containing anattachtoken and a--run <id>pair; it does not verify the no-mistakes executable or exact invocation. A repurposed pane can therefore be accepted and later closed when the run becomes terminal, contradicting the promise to preserve repurposed panes. Validate the canonical executable and exact supported argv shape.bin/fm-no-mistakes-ready.sh:599- Both monitor entrypoints interpolate the task ID into a lock path and acquire it beforefm_nm_task_loadvalidates the ID. An ID containing../can create, reclaim, or remove lock artifacts outside the task-state directory before rejection. Callfm_task_id_creation_validbefore constructing or acquiring either monitor lock.bin/fm-no-mistakes-ready.sh:443- After a journal exists, reconciliation compares the run only with the originally submitted commit and never rechecks the task worktree’s current branch and HEAD. If the task branch advances, diverges, or changes outside that run, the stale run remains accepted. Reapply the current branch/HEAD matcher in addition to the journal lineage check.🔧 Fix: Enforce exact readiness approval and monitor safety
3 issues (2 errors, 1 warning) still open:
bin/fm-no-mistakes-ready.sh:331- The required contract says “worker evidence alone must never produce READY,” butfm_nm_approveperforms no caller-authority check and writes the self-assertedauthority=firstmate. A worker already permitted to use this helper can populate evidence, invokeapprove <task-id> <HEAD>, and then receiveREADY. Require the existingfm_session_lock_owned_by_self "$STATE"Firstmate-session proof before validating or publishing approval.bin/fm-no-mistakes-ready.sh:533- Process proof requires exactly one matching attach argv but permits additional foreground processes. A pane containing the exact attach process plus another process is accepted and later closed as unrepurposed. Requireforeground_processesitself to be an array of length one, then validate that sole process’s canonical executable and four exact argv fields.docs/verification/runtime-backends.md:162- The maintainer-verification block is not reproducible: its commands rely on undefined$LAB,$LAB_HOME,$MONITOR_PANE, and$WORKSPACEstate and replace exact output with prose. Record the complete fixture/setup commands and exact output for the corrected locking and process-topology guarantees, as required by the repo’s verification-record standard.🔧 Fix: Authenticate readiness approval and exact Herdr topology
3 issues (2 errors, 1 warning) still open:
bin/fm-no-mistakes-ready.sh:361- The authorized contract requires “exact-head and reviewed-evidence-byte binding,” but the changed hunkevidence_sha256=$(fm_pr_sha256 "$FM_NM_VALIDATED_RECORD")hashes the live record after validation. A worker can replace valid evidence after Firstmate’s review, causing approval andREADYto bind unreviewed bytes. Require an explicit reviewed digest or immutable snapshot and publish only that exact snapshot.bin/fm-no-mistakes-ready.sh:785- The public monitor recovery entrypoints lack Firstmate session authentication. After an ambiguous create leaves a version-0 journal, a worker can read its token, clear the duplicate-prevention record without inspection, and allow a later reconcile to create a second attach surface. Requirefm_session_lock_owned_by_self "$STATE"before both monitor dispatchers acquire locks or mutate state.docs/verification/runtime-backends.md:159- This version-specific verification claim is supported only by synthetic fixtures: fakepane runnever launches no-mistakes, and fake process-info supplies the expected argv. Add guarded real-lab commands and exact output, or narrow the claim to synthetic contract coverage.🔧 Fix: Bind reviewed evidence and authenticate monitor recovery
2 warnings still open:
tests/fm-no-mistakes-ready.test.sh:230- The documented self-contained verification test uses GNU-onlysed -ihere and at lines 258, 272, 283, and 292. BSDsedrequires a backup-extension argument, so this fixture fails on the project’s principal Herdr platform. Use the established portablesed -i.bak ...pattern and remove the backup afterward.docs/herdr-backend.md:253- This monitor-specific cross-reference still claims “active Herdr evidence,” but its target now explicitly says it launches neither Herdr nor no-mistakes and establishes only synthetic contract coverage. Change the description to “synthetic contract evidence” so the Round-3 verification claim is consistently honest.bin/fm-no-mistakes-ready.sh:688- The Herdr lifecycle was validated only with the self-contained fake Herdr/no-mistakes runtime. A real attach TUI and screenshot were not captured because creating Herdr tabs, processes, or session state would violate the assigned workspace boundary. Actual Herdr runtime/API compatibility and rendered presentation therefore remain unverified; decide whether to accept synthetic contract evidence or authorize a guarded isolated Herdr-lab validation elsewhere.git status --shortandgit diff --stat/name-status f7d0d0a703a717880656709d7906615505b3f2c0..803e85f701b1cd9fc90e93d59a405a8270eb3f0ebin/fm-test-run.sh tests/fm-no-mistakes-ready.test.shbin/fm-test-run.sh tests/fm-brief.test.sh tests/fm-teardown.test.shbash /tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG/readiness-demo.sh "$PWD" /tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG > /tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG/readiness-cli-transcript.txt(rerun passed after correcting an evidence-only Orca metadata fixture)FM_EVIDENCE_ROOT="$PWD" TMPDIR=/tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG FM_EVIDENCE_TRACE=/tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG/herdr-monitor-cli-transcript.txt bash /tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG/herdr-monitor-demo.test.shFM_HOME=/tmp/no-mistakes-evidence/01KYVZJE6D227DNX60QKH2YQGG/brief-home bin/fm-brief.sh evidence-brief sample-projectgit rev-parse HEADandgit status --short --untracked-files=allfinal audit✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.