Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
277 changes: 277 additions & 0 deletions docs/doperpowers/execplans/2026-07-14-split-review-responsibilities.md

Large diffs are not rendered by default.

51 changes: 42 additions & 9 deletions docs/doperpowers/specs/2026-07-08-pr-review-loop-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,16 @@ PR opened / ready_for_review (ida-solution)
→ GH workflow job on self-hosted runner (the Mac) [trigger — mechanical]
→ review-dispatch.sh <pr#> [assembler — mechanical]
dedupe → gather PR body + linked issue → detached
worktree at PR head SHA → daemon-spawn.sh --no-wait
→ review daemon (claude --bg, fresh context) [the worker — judgment]
orient → codex review --base <PR base> → verify
findings → route (fix / ticket / tech-debt /
rebut) → push fixes → re-review? → escalate:
worktree at PR head SHA → spawn worker --no-wait
→ review worker (Claude or Codex, fresh context) [worker — judgment]
start native Codex correctness review in background
∥ audit implementer gate/spec/decision discipline directly
→ join both streams → verify → route (fix / ticket /
tech-debt / rebut / needs-human) → push fixes
→ re-review? → escalate:
small + simple + CI green → SELF-MERGE [autonomous tier]
else → confident-ready on PR + issue [human tier]
→ human merges confident-ready PRs on wake [human — big tier only]
else → confident-ready or needs-human [human tier]
→ human handles escalated PRs on wake [human — big/decision tier]
```

Load-bearing properties:
Expand Down Expand Up @@ -111,8 +113,9 @@ whole-branch review; the escalation would have died at the board write).
branches leave `closingIssuesReferences` empty); detached worktree
(`git worktree add --detach <repo>/.claude/worktrees/review-pr-<n> <headRefOid>`
after fetching; a stale worktree with no live daemon is force-removed and
re-added); spawn-prompt assembly (protocol verbatim + PR metadata + PR body
+ linked issue body); then a no-wait spawn:
re-added); bootstrap assembly (explicit `doperpowers:reviewing-prs`
invocation + runtime bindings, PR body, and linked issue body); then a
no-wait spawn:
`daemon-spawn.sh "review-pr-<n>" "<prompt>" <worktree-path>` with the
no-wait mode selected (exact surface — flag before positionals or
`DAEMON_NOWAIT=1` env — decided at implementation; it must not collide
Expand Down Expand Up @@ -525,3 +528,33 @@ Pending — written at finish.
the skill and supplies all instance placeholders as runtime bindings. The
former operator-oriented skill body moved intact to
`references/operation-manual.md`. Review policy and routing are unchanged.
- 2026-07-14 (split review responsibilities): native Codex review returned to
pure correctness with no ticket criteria or custom developer instructions.
The outer Review Worker now starts Codex in the background, independently
audits whether the Implement Worker passed the substantive ticket gate and
escalated human-grade forks, records that audit before reading Codex output,
then joins both streams. The linked issue body is primary specification;
only documents it explicitly references are secondary, resolved from the PR
base or another immutable revision rather than PR head. Human answers
recorded on a parked ticket before resume are authoritative ticket content
for the answered fork. Protocol blockers route `needs-human`, settled
requirement mismatches are fix-required, weak process evidence alone is a
non-blocking audit note, and unverifiable claimed validation is a separate
confidence-blocking evidence finding even on ticketless PRs. Direct review also
exposed that consumer-owned `.agents/skills` can prevent discovery and can
advertise a PR-controlled same-name skill. The bootstrap therefore
unconditionally opens the same installed version's dispatcher-owned absolute
`SKILL.md`; workspace skills never define this review protocol. The dispatcher
likewise binds the canonical Implement Worker protocol so contract-only gate,
closing-artifact, and follow-up requirements are auditable. Exit review then
exposed temporal provenance gaps: the Implement Worker gate comment now
records normalized issue-body and protocol-template SHA-256 values. Review
dispatch binds current hashes; issue-body drift is resolved through GitHub
`userContentEdits`, while a protocol hash mismatch prevents newer
contract-only requirements from being enforced retroactively. Closing-artifact
inspection stays read-only while native Codex shares the worktree; local
command validation runs serially after the join. Missing Validation Evidence
has one route, `EVIDENCE FINDING`, only when a version-matched protocol,
ticket, or repo facts proves the requirement; otherwise it remains an audit
note rather than a retroactive blocker. See
`docs/doperpowers/execplans/2026-07-14-split-review-responsibilities.md`.
40 changes: 35 additions & 5 deletions docs/doperpowers/specs/2026-07-12-native-review-recovery-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,19 @@

## Purpose

Restore the review loop's product core: **a review worker calls the native
`codex exec review` engine and receives a compact structured verdict — the
PR diff never lands in the worker's own context.** The verdict→fix→re-review
→confident-ready loop then runs exactly as today.
This design restored the review loop's native Codex transport: a Review
Worker calls `codex exec review --base`, receives a compact findings file,
and retains the proven nested environment recipe. Its original responsibility
split also put ticket spec compliance into fixed developer instructions and
kept the PR diff out of the worker's context.

**Current architecture (superseded responsibility split, 2026-07-14):** the
transport and environment recipe remain, but the criteria/developer-instruction
carrier does not. Codex now performs pure native correctness review in the
background while the outer Review Worker reads the implementation through the
issue's spec and decision-discipline lens. The worker records that independent
audit before joining the compact Codex findings. See
`docs/doperpowers/execplans/2026-07-14-split-review-responsibilities.md`.

The current state deviates from that core. The codex-workers build
(2026-07-10) hit two walls — the `--base`-vs-`[PROMPT]` clap conflict, then
Expand Down Expand Up @@ -43,7 +52,11 @@ required", never "infrastructure failed".

## Ground truth (verified live 2026-07-12)

- The reviewer invocation, identical for both worker species:
The nested-execution facts below remain evidence for the environment recipe.
The criteria and developer-instruction portions describe the 2026-07-12
shipped state and are superseded by the current-architecture note above.

- The reviewer invocation, identical for both worker species at that time:

```
codex exec review --base origin/<base> \
Expand Down Expand Up @@ -178,6 +191,13 @@ same only-if-unset pattern. Worker-shell flags unchanged.

## Acceptance

These are the acceptance conditions used to ship the 2026-07-12 recovery.
Criteria/compliance findings in the native verdict are historical; current
acceptance instead requires no criteria or custom developer instructions in
the native invocation, plus a worker-owned audit recorded before the join.
The compact output, nested execution, outage recovery, and no-fallback clauses
remain active.

- A codex review worker's transcript shows the PR verdict arriving as a
compact findings file from `review-engine.sh` — no full-diff dump into
the worker's main context; the trail comment names the native engine.
Expand Down Expand Up @@ -356,3 +376,13 @@ plan re-execution; everything else executed as written.
the former operator-oriented skill body moved to
`references/operation-manual.md`. The dispatcher now invokes the skill via
a thin runtime-binding bootstrap; engine policy and behavior are unchanged.
6. **2026-07-14 (responsibility split).** Live use showed that even the minimal
spec/decision addendum could distract the native correctness review. The
criteria interface and custom developer instructions were therefore removed
from `review-engine.sh`; its nested transport and environment recipe remain.
The outer Review Worker now starts Codex in the background, audits the
Implement Worker's gate and human-escalation discipline against the issue
body and issue-referenced documents, records that audit independently, and
joins both streams. This intentionally supersedes Notes 2–4's final carrier
while preserving them as the evidence trail that led here. See
`docs/doperpowers/execplans/2026-07-14-split-review-responsibilities.md`.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ be written down as self-contained child pre-specs right now?
VERDICT IS YOUR FIRST BOARD WRITE. Dispatch wrote nothing.
- Pass → {{BOARD_SCRIPTS}}/board-transition.sh {{ISSUE_NUMBER}} in-progress
then a one-line gate comment:
gh issue comment {{ISSUE_NUMBER}} --body "[gate] pass — {{ENGINE_NAME}}/<mode>: <one line>"
gh issue comment {{ISSUE_NUMBER}} --body "[gate] pass — {{ENGINE_NAME}}/<mode>: <one line> | issue-body-sha256={{ISSUE_BODY_SHA256}} | implement-protocol-sha256={{IMPLEMENT_PROTOCOL_SHA256}}"
The hashes identify the exact issue-body snapshot and unrendered protocol
template supplied at dispatch. Keep the original hashes on a bound-session
re-pass; timestamped human answers amend that original ticket snapshot.
- Fail → the park state itself, with the required note. Park discriminant —
WHO UNPARKS IT:
- The human as themselves — a decision only they can make, or a
Expand Down
8 changes: 6 additions & 2 deletions skills/issue-tracker/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,12 @@ pick by repo visibility:
`references/implement-worker-protocol.md`. Substitute every
`{{PLACEHOLDER}}` (`ISSUE_NUMBER`, `ISSUE_URL`, `ISSUE_TITLE`, `REPO`,
`BOARD_SCRIPTS` = this skill's scripts dir, `ISSUE_BODY` = the full
issue body from `gh issue view <n> --json body`, `ENGINE_NAME` = the
engine, `REPO_FACTS` = `git show origin/<default-branch>:.doperpowers/repo-facts.md`
issue body from `gh issue view <n> --json body`, `ISSUE_BODY_SHA256` =
lowercase SHA-256 of that body after CRLF→LF normalization and removing
trailing line endings, `IMPLEMENT_PROTOCOL_SHA256` = lowercase SHA-256 of
the exact unrendered `references/implement-worker-protocol.md` bytes,
`ENGINE_NAME` = the engine, `REPO_FACTS` =
`git show origin/<default-branch>:.doperpowers/repo-facts.md`
(or a "(no repo-facts manifest)" note when absent), and — implement
protocol only — `EXECUTION_BLOCK` = the engine's
`references/engine-blocks/execution-<engine>.md` and `DECOMPOSE_DOC` =
Expand Down
Loading