Skip to content

Close the review loop — approve/merge, truthful outcomes, interdiff, real GitHub reviews#12

Merged
alejandro-nexcade merged 17 commits into
mainfrom
alejandro/close-the-loop
Jul 2, 2026
Merged

Close the review loop — approve/merge, truthful outcomes, interdiff, real GitHub reviews#12
alejandro-nexcade merged 17 commits into
mainfrom
alejandro/close-the-loop

Conversation

@alejandro-nexcade

@alejandro-nexcade alejandro-nexcade commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Why

A deep-research pass on the AI-PR review bottleneck (105 verified-claim agents; synthesis in cockpit-docs/RESEARCH_REVIEW_BOTTLENECK.md) plus three independent code audits found that cockpit's loop could not actually conclude: no Approve/Merge existed in the UI, comment line numbers were fragment-relative fiction, failed agents reported success and destroyed the reviewer's comments, and re-review meant re-reading the whole diff every cycle. This PR fixes all confirmed defects and lands the research + roadmap docs (cockpit-docs/ROADMAP.md).

What (by SPEC section)

Loop can conclude (SPEC §7, §12, §14)

  • GateState::Merged terminal state + Review::mark_merged
  • Approve button (InReview) and confirmed squash Merge (gh pr merge --squash --delete-branch) for authored/frontier PRs; managed-worktree GC on merge
  • Descendants marked stale on dispatch and merge (restack graph helpers promoted from test-only to production)

Truthful agent outcomes (SPEC §7)

  • apply_agent_completion: git-HEAD-authoritative — HEAD advanced → Reworked; failed/no-op → back to InReview with comments preserved (previously a failed agent flipped to Reworked and wiped them)
  • Kill switch: Stop button (confirmed) + straggler-completion suppression; plan ingest failure-aware

Correct anchors (audit P0)

  • Diff parser maps fragment↔real file lines per side; comments, rework prompts, and GitHub mirrors now carry real line numbers; deletions commentable on the original editor (side-tagged anchors)

Cheap re-review (SPEC §8 direction)

  • DispatchSnapshot (read-only, single-cycle — Invariant §0.4 respected) + get_interdiff; Reworked reviews default to "Since your review" with the addressed-requests list

Intent visible (SPEC §9)

  • Review.title/body populated and shown (header + Intent panel); fixer prompts get title+body intent and the approved plan (was hardcoded None); implement prompts too

Teammate reviews (SPEC §12)

  • submit_github_review: line-anchored review comments + APPROVE/REQUEST_CHANGES/COMMENT verdict in one confirmed submission (replaces top-level comment spam); comment mirroring now confirms too

Batch & stacks (SPEC §13)

  • Non-blocking plan approve: implementers fan out in background waves (lag-safe reconciliation against the store; fixes a latent stdout-pipe deadlock path)
  • Imported PRs get real stack edges (base↔branch derivation); board renders stacks as connected rail-grouped units in dependency order
  • PR ingestion enrich/diff fetches now bounded-concurrent (was 2N+1 serial subprocesses)

Local-first durability (Invariant §0.1)

  • ~/.cockpit/state.json: atomic, revision-driven persistence; on load, dead Dispatched reviews return to InReview with comments intact
  • Shell tab + fixer dispatch materialize the PR branch worktree for imported PRs (were running against the main checkout)

Removed: dead workflow automation engine (would have violated SPEC §14), get_version, dead frontier store slice.

How verified

  • cargo fmt --check / clippy --all-targets --all-features -D warnings / cargo test --all — green (360 unit + e2e suites incl. fix_loop_e2e, batch_fan_out_e2e)
  • tsc --noEmit strict / vitest run 124 tests / vite build — green
  • Two independent reviewer-agent passes (Rust wave, then FE wave + cross-cutting): zero blocking findings; all §0 invariants and §9 guardrails verified (merge/submit/mirror/approve-plan each behind explicit click + confirm; nothing auto-fires)

Known minors (documented, non-blocking): join-shaped DAGs (a PR with two independent parents) can render in two stack containers; AgentPanel per-review timeline buffers are retained for the app session.

Runtime paths needing a live eyeball (cargo tauri dev): merge/submit against a real PR, interdiff after a real rework, stack rail rendering in both densities.

🤖 Generated with Claude Code

LifeLex and others added 17 commits July 1, 2026 23:54
@LifeLex @claude
core: model foundations — Merged state, intent fields, dispatch snaps…
10c0c5f 
…hot, anchor side

Adds GateState::Merged + mark_merged, apply_agent_completion (HEAD-authoritative agent outcome), snapshot_dispatch, Review.title/body, Anchor side. Regenerates TS bindings; minimal FE Merged cases to keep the tree green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core: state persistence + store revisions and stale propagation helper
84943ef 
Adds persist.rs (atomic state.json save/load with corrupt-file sidecar,
versioned), revision counters + hydrate on both stores, and a cycle-safe
ReviewStore::mark_descendants_stale. Fixes the audit finding that all gate
state, comments, and plans were lost on app restart (Invariant §0.1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core: promote restack graph helpers to production API
aa1b632 
mark_descendants_stale and dependency_order lose their #[cfg(test)] gate so
the loop can actually mark descendants stale (SPEC §7/§13). Hardened: cycle
termination via visited sets, dangling child edges skipped, VecDeque BFS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core: github adapter — merge, real reviews, stack edges, intent, conc…
8ef3426 
…urrency

- merge_pr (gh pr merge --squash/--merge/--rebase --delete-branch)
- submit_review: line-anchored review comments + APPROVE/REQUEST_CHANGES/
  COMMENT verdict via gh api pulls/{n}/reviews, with client-side validation
  against the diff (replaces top-level comment spam)
- wire_stack_edges: derive parent/child from base<->branch so imported
  gh-stack stacks are no longer flat
- PrData.body + title/body now populate Review intent fields
- bounded-concurrency enrichment (JoinSet, 8) replacing 2N+1 serial calls
- compare() for cross-repo interdiff

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core: git diff_range for interdiff re-review
e31da4b 
git2 tree-to-tree unified diff between two revisions of a worktree; feeds
the 'changes since your last dispatch' view.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
docs: review-bottleneck research synthesis + product roadmap
5db2073 
Verified external evidence (DORA/DX/Meta RADAR/Atlassian/MSR/EASE 2026) and
the phased roadmap (close the loop -> verification-over-reading -> risk
routing -> rework-cycle-zero -> teammate flows) derived from it plus three
internal audits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core: remove dead workflow automation engine
4b9a47d 
workflow.rs (Rule/Action/evaluate_rules) and its gate::transition_event
feeder had zero production callers, and Action::UpdateLinearStatus would
have violated SPEC §14 (cockpit writes nothing to Linear). 12 dead tests
removed with their subjects; all gate state-machine tests intact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app: HEAD-authoritative agent completion + scoped agent events
e8290d3 
Fix/Restack completions now reconcile the worktree HEAD (spawn_blocking)
and route through apply_agent_completion: advanced HEAD -> Reworked,
unchanged/failed -> InReview with the reviewer's comments preserved. A
failed or no-op agent can no longer masquerade as success and destroy
comments. Plan ingest is failure-aware. agent-event now carries
{object_id, event} so timelines can be scoped per review; agent-completed
carries an outcome.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app: merge command, stale propagation, state persistence, real intent
f381c2a 
- merge_review: Approved-only, authored/frontier-only, gh pr merge --squash,
  marks descendants stale, GCs managed worktrees (never the main checkout)
- request_changes/fix_ci now mark descendants stale on dispatch (SPEC §7)
- state.json load-on-start (dead Dispatched -> InReview, comments intact,
  agent handles dropped) + revision-driven 1s background flush
- fixer intent is now PR title + body + issue ref instead of a bare ref
- restack completions keyed by PrRef (were silently unresolvable)
- get_frontier excludes Merged; dead get_version removed

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app: non-blocking batch, github reviews, kill, interdiff, worktree fixes
fc4503f 
- plan_approve returns immediately; implementers fan out in background
  waves via the streaming spawn path, gated on completion events
- submit_github_review: line-anchored review + verdict, local Approve
  transition for review-requests, submitted comments cleared
- kill_agent: SIGTERM + settle to InReview, comments preserved; straggler
  completions are silent no-ops (double-fire hardened)
- get_interdiff: worktree diff_range or gh compare fallback
- ensure_review_worktree: imported PRs get a real branch worktree before
  shell/fixer use so HEAD-based outcomes are truthful
- approved plan threaded into implement + rework prompts (SPEC §9)
- dispatch snapshots captured at request_changes/fix_ci

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app(fe): real-line maps in the diff parser
ddf584d 
parseDiff now parses @@ headers and records fragment<->real line maps per
file/side (fragmentToReal/realToFragment). Foundation for correct comment
anchors, rework prompts, and GitHub review lines.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
core+app: fix review findings — fan-out liveness, comment leak, minors
85da410 
- run_fan_out reconciles pending against the store on Lagged and on a 15s
  safety tick, so a dropped broadcast completion can no longer stall the
  batch build permanently
- fix_ci validates state before pushing its synthetic comment and rolls
  it back on transition failure
- store revision only bumps when an update actually found its key
- author self-exclusion is case-insensitive; poison-expect justified;
  apply_agent_completion validates before mutating

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app(fe): DiffView closes the loop — approve/merge, real anchors, inte…
98b3121 
…rdiff, intent, GitHub reviews

- comments anchor to real file lines via the parser line maps
- header leads with PR title; collapsible Intent panel (plain-text body)
- Approve (InReview) and confirmed squash Merge (Approved) for authored PRs
- SubmitReviewControl: verdict + summary -> line-anchored GitHub review
- Reworked defaults to 'Since your review' interdiff with read-only
  addressed-requests list (single-cycle, per Invariant §0.4)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app: side-aware comments, scoped agent timelines, stop/log, real shel…
b238071 
…l worktrees

- add_comment threads DiffSide; deletions are commentable on the original
  editor with old-line anchors
- AgentPanel scopes to its review via the event envelope (per-object
  buffers survive switching), gains Stop (confirmed kill) and Open log
- Shell tab materializes the PR branch worktree before mounting
- completion notifications distinguish failed (comments preserved) from
  reworked; straggler events after a Stop are suppressed

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app(fe): stack-grouped board + dead frontier slice removal
7ae11b5 
Stacks render as connected units (rail + depth indent, parent-first,
health dot, teal frontier segment) via the stack-tree lib — now fed by
real derived edges. Fixed its latent parent-not-in-set drop bug. Merged
is a first-class filter chip. Dead frontier store slice removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
app(fe): confirm comment mirroring; guard fixCi activeReview update
4dbae30 
Mirror is an outward GitHub publish — SPEC §12 asks for explicit
confirmation like merge/submit. fixCi now follows the guarded
activeReview pattern of the other actions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LifeLex @claude
docs: mark roadmap Phase A as implemented (PR #12)
c64d176 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alejandro-nexcade alejandro-nexcade merged commit 4fdae01 into main Jul 2, 2026
2 checks passed
@alejandro-nexcade alejandro-nexcade deleted the alejandro/close-the-loop branch July 2, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alejandro-nexcade @LifeLex