feat(cli): add AXI supervision and guarded branch sync#4
Conversation
|
Important Review skippedToo many files! This PR contains 68 files, which is 18 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (68)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
* fix(pipeline): ignore deferred delivery findings at pre-push review Review runs before push/PR/CI, so an authoritative intent like "Open PR A unmerged" must not park the run when the PR or remote branch this same pipeline will create is still absent. Add a phase ownership clause, narrow intent conformance to source-verifiable criteria, and strip findings that only claim later pipeline-owned delivery is missing. External or pre-existing lifecycle requirements stay enforceable; push/PR/CI remain strict after their stages run. * no-mistakes(review): Harden deferred delivery finding classification * no-mistakes(review): Require explicit delivery ownership classification * no-mistakes(review): Recompute review metadata after delivery filtering * no-mistakes(review): Preserve source risk through delivery filtering * no-mistakes(document): Document review delivery ownership boundary * no-mistakes: apply CI fixes
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…id#496) * feat(sync): guard local branch reconciliation * no-mistakes(review): Clear stale CI readiness states * no-mistakes(document): Align AXI sync guidance
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…#499) * fix(pipeline): harden step git environment * no-mistakes(document): Clarify step git environment
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…unchenguid#502) * feat(branchsync): guarded custody recovery for terminal unpublished runs A run that goes terminal (cancelled, failed, or completed without a push stage) after moving the pipeline head used to leave the branch pipeline_owned forever: sync --check reported blocked_pipeline_owned with no safe exit, while the fix commits existed only in the local gate branch (v1.38.1 dogfood catch, run 01KXN8YJ6DWF8XPP582DWQC3HV). Terminal-run classification now reports safety blocked_pipeline_owned_recoverable with the run's status and a structured recover_custody next action; active runs keep the plain wait block. The new guarded recovery (sync --recover on the human CLI and AXI, u in the TUI) verifies the preserved head at the gate branch, anchors it under refs/no-mistakes/recover/<run> in the invoking repository, fast-forwards only a clean behind worktree, and stamps runs.custody_returned_at so a fresh run can start. Dirty and diverged worktrees refuse with the explicit choices; --recover --keep-local returns custody at the current head without touching the worktree and moves the gate branch to it with an atomic compare-and-swap, staging objects via gate-side fetch so the receive hook never fires. The full relation matrix and fail-safe rules live in the Recover doc comment in internal/branchsync/sync.go. Recovered never-pushed runs classify as custody_returned (exit 0, run_pipeline next action); recovered pushed runs fall through to their ordinary binding classification such as local_ahead. Guidance is synced across the skill body, agents guide, and live axi strings, pinned by the canonical-phrase drift test. Regressions: internal/branchsync/recover_test.go (terminal x relation x cleanliness matrix, gate divergence fail-closed, CAS race), internal/cli/sync_test.go, internal/tui/branch_sync_test.go, db custody column migration tests, and e2e TestAxiCustodyRecoveryJourney which reproduces the stranded state with the real binary and recovers it end to end. * no-mistakes(review): Harden branch recovery fast-forward preconditions * no-mistakes(review): Anchor reachable recovery commits locally * no-mistakes(review): Clarify custody recovery anchor guidance * no-mistakes(document): Clarify custody recovery docs
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aa8413f to
f54822f
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a guarded local branch synchronization feature (branchsync) to handle terminal pipeline runs and ensure safe custody recovery. It adds new CLI commands (axi sync, axi watch, axi supervise) and updates the TUI to provide branch synchronization status and recovery actions. Additionally, it improves CI monitoring by persisting readiness state and separating native check runs from legacy commit statuses. My feedback highlights that several calls to store.UpdateForSession in internal/cli/axi_supervise.go ignore potential errors, which could lead to silent failures and state inconsistencies; these should be logged to ensure observability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
internal/cli/axi_supervise.go (483-486)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (489-493)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (497-501)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (508-512)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (514-518)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (525-529)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
internal/cli/axi_supervise.go (668-672)
The error returned by store.UpdateForSession is being ignored. While it's important for the hook not to fail loudly to the calling agent, completely ignoring the error can lead to silent failures and an inconsistent state in the supervision store. This could make debugging issues with supervision difficult.
Consider logging the error to a dedicated log file for no-mistakes or to stderr. This would preserve the hook's contract with the agent while making potential file system or state corruption issues observable.
|
Superseded by upstream PR kunchenguid#506, which contains the clean, rebased change set. |
Intent
Make No-Mistakes natively supervise an already armed AXI run after a Codex or Claude Code Stop event. Preserve the upstream branch-sync behavior, require explicit opt-in local hooks, never alter users hook configuration automatically, continue only technical events, and keep ask-user and terminal states safely bounded. Submit the reusable implementation and tests as a focused upstream PR; exclude local planning notes and internal evidence artifacts.
What Changed
axi supervisehooks for armed Codex and Claude Code sessions; technical Stop events can continue work while ask-user and terminal states remain bounded.sync/axi syncand TUI controls that inspect branch state, fast-forward safely, and recover custody of terminal pipeline-owned work.Risk Assessment
Testing
Der bereitgestellte Race-Baseline-Lauf sowie gezielte Unit- und reale E2E-Flüsse für Codex und Claude bestanden: explizites Arming, unveränderte Hook-Konfiguration, technische Fortsetzung, stille Ask-user- und unvollständige Ereignisse, terminaler Abschluss und der separate Branch-Sync-Nutzerpfad. Keine visuelle Evidenz erforderlich, da dies eine CLI-/Hook-Funktion ohne gerenderte Oberfläche ist.
Evidence: Native AXI supervision E2E transcript
Codex and Claude E2E CLI transcripts: arm requires a hook; only technical gates emitnm_event=technical_gate; duplicate events are quiet; terminal events complete supervision; ask-user events remain quiet.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
internal/cli/axi.go- merge conflict rebasing onto origin/main🔧 **Review** - 5 issues found → auto-fixed (9) ✅
internal/branchsync/sync.go:1- Der angegebene Bereich ist kein fokussierter Supervisions-PR: Er fügt zusätzlich die neue Branch-Sync-/Custody-Implementierung hinzu. Das widerspricht dem Kriterium „focused upstream PR“; bitte bestätigen, dass diese fachfremden Änderungen tatsächlich mitgeliefert werden sollen..no-mistakes/evidence/fm/nm-ci-autofix-git-env/focused-regression-tests.txt:1- Der Diff fügt interne Test- und Laufzeitevidenz hinzu (insgesamt vier Dateien unter.no-mistakes/evidence/fm/). Das widerspricht unmittelbar dem Kriterium „exclude local planning notes and internal evidence artifacts“.internal/scm/github/github.go:520- Klassische GitHub-Branch-Protection-Kontexte ohne App-ID werden grundsätzlich als unbeobachtbar behandelt. Selbst ein erfolgreiches gleichnamiges Check Run erzeugt dadurch zusätzlich einen synthetischen blockierenden Pending-Check; betroffene CI-Monitore erreichen niechecks-passed. Soll diese bewusst fail-closed gewählte, aber für Legacy-contextsdauerhaft blockierende Semantik gelten?internal/cli/axi_supervise.go:245- Eine aus einem Unterverzeichnis gestartete Codex- oder Claude-Session wird nie gebunden:armspeichert den Git-Root, der Hook sucht aber mit dem unveränderten Provider-CWD exakt danach und endet still. Den Hook-CWD vorFindByCWDauf den Git-Root normalisieren.internal/supervision/store.go:238- Zwei verschiedene Claude-Stop-Turns mit derselben letzten Assistant-Nachricht (z. B. zweimal „Done.“) erhalten dieselbe Handoff-ID und der zweite wird dauerhaft als Duplikat verworfen. Die Deduplizierung sollte zusätzlich den bereits gespeicherten Event-Fingerprint berücksichtigen.🔧 Fix: Fix supervision hook binding and event deduplication
5 errors still open:
internal/branchsync/sync.go:1- Der angegebene Bereich enthält zusätzlich die neue Branch-Sync-/Custody-Implementierung. Das widerspricht dem Kriterium „focused upstream PR“; bitte bestätigen, dass diese fachfremden Änderungen Teil dieses Supervisions-PRs sein sollen..no-mistakes/evidence/fm/nm-ci-autofix-git-env/focused-regression-tests.txt:1- Der Diff fügt interne Test- und Laufzeitevidenz unter.no-mistakes/evidence/fm/hinzu. Das widerspricht unmittelbar dem Kriterium „exclude local planning notes and internal evidence artifacts“.internal/scm/github/github.go:520- Klassische GitHub-Branch-Protection-contextsohne App-ID gelten immer als unbeobachtbar. Ein erfolgreicher gleichnamiger Check Run erzeugt daher zusätzlich einen synthetischen Pending-Check, sodass betroffene CI-Monitore niechecks-passederreichen. Soll diese fail-closed Semantik für Legacy-Kontexte bewusst dauerhaft blockieren?internal/cli/axi_supervise.go:423- Die neue Deduplizierung ist nicht idempotent: Der Event-Fingerprint enthält die veränderliche Heartbeat-Deadline, die nach der ersten Übergabe gespeichert wird. Eine erneute Zustellung desselben Stop-Turns erhält dadurch einen anderen Fingerprint und erzeugt erneut eineblock-Continuation. Den Fingerprint aus stabilen Provider-Ereignisdaten bilden.internal/daemon/daemon.go:565- Die persistierteCIReady-Information wird über IPC geliefert, aber beim Aufbau des AXI-Views verworfen; der Supervisor prüft ausschließlich historische CI-Logs. Bei einem frischen Attach ohne verfügbare Logdatei endet die Supervision deshalb mit Heartbeats/Pause stattchecks_passed.CIReadydurch den AXI-View führen und bei der Readiness-Entscheidung berücksichtigen.🔧 Fix: Stabilize supervision retries and CI readiness
4 errors still open:
internal/branchsync/sync.go:1- Der deklarierte Bereich enthält zusätzlich die neue Branch-Sync-/Custody-Implementierung. Das widerspricht dem Kriterium „focused upstream PR“; bitte bestätigen, dass diese fachfremden Änderungen Teil dieses Supervisions-PRs sein sollen..no-mistakes/evidence/fm/nm-ci-autofix-git-env/focused-regression-tests.txt:1- Der Diff enthält weiterhin interne Test- und Laufzeitevidenz unter.no-mistakes/evidence/fm/. Das widerspricht unmittelbar dem Kriterium „exclude local planning notes and internal evidence artifacts“.internal/scm/github/github.go:520- Klassische GitHub-Branch-Protection-contextsohne App-ID gelten immer als unbeobachtbar. Auch ein erfolgreicher gleichnamiger Check Run erzeugt dadurch einen synthetischen Pending-Check, sodass betroffene CI-Monitore niechecks-passederreichen. Soll diese fail-closed Semantik für Legacy-Kontexte bewusst dauerhaft blockieren?internal/cli/axi_supervise.go:465- Die stabilisierte Deduplizierung kann unterschiedliche Claude-Stop-Turns verwerfen: Claude bildet die Handoff-ID nur aus Session und letzter Assistant-Nachricht, während der neue Fingerprint bei unverändertem AXI-Status ebenfalls gleich bleibt. Zwei aufeinanderfolgende identische Antworten stoppen damit die Heartbeat-Kette, statt bis zur konfigurierten Stale-Grenze weiterzuarbeiten. Bitte die gewünschte Abwägung zwischen Zustellungs-Deduplizierung und Wiederholung identischer Claude-Turns bestätigen.🔧 Fix: Distinguish identical Claude Stop turns
1 error still open:
internal/cli/axi_supervise.go:241- Die neue Turn-Kennung ist nicht zuverlässig:transcript_pathwird laut Claude-Code-Hook-Referenz asynchron geschrieben und kann beim Stop noch hinterherhinken. Dadurch kann eine verzögert erneut zugestellte Stop-Nachricht nach einem Metadata-Update eine neue ID erhalten und erneut blockieren; umgekehrt kann ein echter Folgeturn noch die alte Metadata tragen und verworfen werden. Eine echte, atomar verfügbare Turn-Grenze verwenden oder die Deduplizierung so gestalten, dass sie nicht von asynchronen Dateimetadaten abhängt.🔧 Fix: Use Claude transcript UUIDs for Stop deduplication
1 error still open:
internal/cli/axi_supervise.go:253- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ bleibt bei einem frisch bewaffneten Lauf unerfüllt: Beim ersten Stop istpreviousleer, daher akzeptiert die Funktion sofort die letzte bereits persistierte Assistant-UUID, statt auf die zum aktuellen Stop gehörende Nachricht zu warten. Claude dokumentiert ausdrücklich, dass das Transcript beim Stop hinterherhinken kann; der spätere Flush wird dann als weiterer Turn behandelt und kann eine zweite Continuation erzeugen. Die aktuelle Stop-Nachricht zur Zuordnung behalten und bis zu ihrer UUID warten.🔧 Fix: Match Claude Stop messages to transcript UUIDs
2 errors still open:
internal/cli/axi_supervise.go:255- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ bleibt bei normalem Transcript-Lag unerfüllt: Der neue Hunk gibt beihandoffID == ""sofort zurück, statt bis zur konfigurierten Deadline auf die UUID der aktuellen Nachricht zu warten. Ein Stop vor dem asynchronen Transcript-Flush wird dadurch still verworfen.internal/cli/axi_supervise.go:255- Die explizite Vorgabe, echte nachfolgende Claude-Stopps auch bei identischem Nachrichtentext zu unterscheiden, ist beim ersten Handoff noch nicht erfüllt: Mit leerempreviousakzeptiert dieser Hunk eine bereits im Transcript vorhandene frühere gleichlautende Assistant-UUID als aktuellen Stop. Nach dem Flush des echten Turns wird dessen UUID als zweite Continuation verarbeitet. Es braucht eine beim Armen persistierte Transcript-Grenze oder eine andere kausale Turn-ID.🔧 Fix: Bound Claude transcript handoffs causally
2 errors still open:
internal/cli/axi_supervise.go:579- Das Kriterium „keep ask-user … safely bounded“ wird verletzt: Derask_user-Rückweg überspringtPrepareHandoff, den einzigen neuen Pfad zum Speichern vonevent.TranscriptOffset. Nachaxi respondist die alte Ask-user-Nachricht damit weiterhin hinter der Boundary; bei identischem Folgetext und Transcript-Lag wird sie als neuer Stop zugeordnet und kann erneut blockieren. Den Offset auch beim Ask-user-Übergang fortschreiben.internal/cli/axi_supervise.go:308- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ bleibt für große Turns unerfüllt: Der neue 1-MiB-Reader erreicht eine spätere Assistant-Zeile nach großem Tool-Output nicht. Jeder Poll beginnt wieder beim unveränderten Offset, liest denselben Präfix und verwirft den Stop nach Ablauf der Frist. Den Scan innerhalb der begrenzten Wartezeit fortschreiten lassen oder den Overflow sicher behandeln.🔧 Fix: Advance Claude transcript cursors safely
1 error still open:
internal/cli/axi_supervise.go:315- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ bleibt bei Transcript-Deltas über 8 MiB unerfüllt: Dieser Hunk beendet den Scan mit leerer Handoff-ID, ohne Cursor oder Pausenstatus zu persistieren. Liegt die aktuelle Assistant-UUID hinter dem Limit, wird jeder spätere Stop vom alten Offset aus erneut über denselben Präfix gescannt und dauerhaft verworfen. Einen verifizierten Fortschritt persistieren oder den Overflow sichtbar und sicher pausieren.🔧 Fix: Persist verified Claude transcript scan progress
2 errors still open:
internal/cli/axi_supervise.go:482- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ wird durch verzögerte Duplikate verletzt: Ein verspäteter T1-Stop kann einen bereits geschriebenen, anderslautenden T2-Record scannen, auf die alte T1-ID zurückfallen und hier den Scan-Offset hinter T2 persistieren. Der echte T2-Stop beginnt dann nach seiner UUID und wird still verworfen. Den Cursor beimprevious-Fallback nicht fortschreiben.internal/cli/axi_supervise.go:480- Das Kriterium „Make No-Mistakes natively supervise … after a … Claude Code Stop event“ bleibt für Assistant-Records über dem Record-Limit unerfüllt: Der Scanner verwirft sie, behandelt ihr Zeilenende aber als geprüften Fortschritt und persistiert hier den Offset. Ist dies der aktuelle Stop-Record, wird seine UUID nie geprüft und für alle späteren Zustellungen dauerhaft übersprungen. Einen verworfenen Record sichtbar pausieren oder erst nach vollständiger Prüfung fortschreiben.🔧 Fix: Guard Claude cursor advancement and oversized records
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed (3) ✅
go test -race ./...🔧 Fix: Isolate branchsync hook tests from global configuration
1 error still open:
internal/cli/axi_supervise.go:259- Codex- und Claude-Stop-Payloads ohnecwdwerden trotzdem dem Prozessverzeichnis zugeordnet und sendennm_event=technical_gate. Unvollständige Ereignisse müssen fail-closed ignoriert werden.go test -race ./...go test -race ./...(vom Auftrag als erfolgreiche Baseline angegeben)go test -race ./internal/cli ./internal/supervisiongo test -tags=e2e -count=1 -timeout 300s -run '^(TestAxiAgentJourney|TestAxiBranchSyncJourney)$' ./internal/e2ego test -tags=e2e -count=1 -timeout 300s -run '^TestAxiNativeAgentSupervisionJourney/(codex|claude)/(technical-and-terminal|ask-user-stops)$' -v ./internal/e2ego test -tags=e2e -count=1 -timeout 300s -run '^TestAxiNativeAgentSupervisionJourney$' -v ./internal/e2e(Negativtest ohnecwd, reproduzierbar fehlgeschlagen)🔧 Fix: Reject Stop hooks missing cwd
1 error still open:
.no-mistakes/evidence- The target diff adds four tracked internal evidence artifacts under.no-mistakes/evidence/, contrary to the explicit requirement to exclude internal evidence artifacts from the focused upstream PR. Remove these files from this PR.go test -race ./...go test -tags=e2e -count=1 -run '^TestAxiNativeAgentSupervisionJourney$' -v ./internal/e2ego test -race ./internal/cli ./internal/supervisiongo test -tags=e2e -count=1 -run '^TestAxiBranchSyncJourney$' -v ./internal/e2egit diff --name-only 2bbbc143bd4520056e97957883a02615657b2a62 99c0f3eadc5d2ca720fa087b9ba41cafbbd58756 -- .no-mistakes/evidence🔧 Fix: Remove internal evidence artifacts
✅ Re-checked - no issues remain.
go test -race ./...Baseline (bereits erfolgreich):go test -race ./...go test -tags=e2e -count=1 -timeout 300s -run '^TestAxiNativeAgentSupervisionJourney$' -v ./internal/e2ego test -tags=e2e -count=1 -timeout 300s -run '^TestAxiBranchSyncJourney$' -v ./internal/e2ego test -count=1 -run '^(TestCodexHookIgnoresNonStopEvents|TestClaudeHookIgnoresMalformedAndNonStopPayloads|TestClassifySupervisorRunFailsClosedForAskUserAndMalformedGates|TestClassifySupervisorRunRecognizesTechnicalGateAndTerminal|TestApplySupervisorOutcomeDeduplicatesRepeatedStopEvent|TestApplySupervisorOutcomeAskUserAdvancesClaudeTranscriptOffset|TestApplySupervisorOutcomeBoundsReasonsAndPausesAfterStaleBudget)$' ./internal/cligo test -count=1 ./internal/supervision ./internal/branchsyncManuelle Diff- und Scope-Prüfung: keine internen Evidence- oder Planungsartefakte im Ziel-Diff.🔧 **Document** - 1 issue found → auto-fixed ✅
internal/cli/axi_supervise.go:111- The liveaxi supervise --helpdescription still says Codex-only although Claude Code is supported; changing that executable help text exceeds the documentation/doc-comment-only scope.🔧 Fix: Name Codex and Claude in supervise help
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.