Plan lifecycle: comment-backed reviews, bounded repair loops, implement-start branch

.claude-plugin/marketplace.json

@@ -66,7 +66,7 @@
     {
       "name": "plan-lifecycle",
       "source": "./plugins/plan-lifecycle",
-      "description": "Cross-tool GitHub-issue plan lifecycle: a marker-based v3 issue-body record, six phases (decide, draft, research, one plan review, implement, code review), and two read-only reviewer wrappers, plan-reviewer and code-reviewer.",
+      "description": "Cross-tool GitHub-issue plan lifecycle with marker-based plan bodies, comment-backed review records, implement-start linked branches, bounded plan and code review repair loops, and two read-only reviewer wrappers.",
       "version": "0.7.0",
       "author": {
         "name": "Eduardo Marquez"

.codex/agents/code-reviewer.toml

@@ -46,14 +46,25 @@ Verdicts. Select exactly one:
   and does not change reviewed bytes after the pass; they never trigger a
   re-review.
 - `fixes-required`: At least one evidenced `CRITICAL` or `HIGH` defect. The
-  manager fixes it and dispatches exactly one repair re-review.
+  manager fixes every named defect and dispatches a fresh re-review on a fresh
+  diff.
 - `blocked`: Required review input is unreadable or contradictory, so no safe
   verdict can be reached.
 A pass block carries only advisory MEDIUM and LOW lines, or none. A
 fixes-required or blocked block has at least one finding line.
 
 Remain read-only. Never apply a fix and never ask for approval to apply one.
-Return one readable `Code-review:` markdown block to the manager. These inline
-Standards buckets, severity caps, and Spec axis keep this wrapper complete when
-the runtime skills are unavailable.
+Return exactly one readable `Code-review:` markdown block to the manager, which
+posts that whole block unchanged as one issue comment and owns fixes and fresh
+re-review dispatch. The block must be the parser-compatible comment record, and
+this exact shape overrides any report layout from a loaded `code-review` skill:
+
+    ### Code review round <n> — <UTC YYYY-MM-DD>
+    Code-review: <pass|fixes-required|blocked>
+    - <CRITICAL|HIGH|MEDIUM|LOW> · <Bug|Security|Performance|Maintainability|Spec> · <locator> — <defect> — <fix>
+
+Use the round number the manager supplies, one finding per line, and no prose
+outside the block. These inline Standards buckets, severity caps, Spec axis,
+and record shape keep this wrapper complete when the runtime skills are
+unavailable.
 """

.codex/agents/plan-reviewer.toml

@@ -1,5 +1,5 @@
 name = "plan-reviewer"
-description = "Use when plan-manager needs one read-only pre-implementation review of a canonical plan against repository facts and official documentation. Not for code review, plan edits, implementation, user decisions, lifecycle changes, or direct user invocation."
+description = "Use when plan-manager needs a read-only pre-implementation review round for a canonical plan against repository facts and official documentation. Not for code review, plan edits, implementation, user decisions, lifecycle changes, or direct user invocation."
 sandbox_mode = "read-only"
 developer_instructions = """
 # Plan Reviewer
@@ -11,6 +11,8 @@ Read the plan body from the export path the manager supplies; it is an absolute
 A plan-review finding is exactly one of `goal_fit`, `research_gap`, or `security_risk`; nothing else is a finding. A sufficient plan passes.
 
 Remain read-only. Never write, dispatch an agent, run a mutating command, or ask
-the user. Return one readable `Plan-review:` markdown block to the manager. The
-canonical skill owns the review workflow and output contract.
+the user. Return exactly one readable `Plan-review:` markdown block to the
+manager, which posts that whole block as one issue comment and owns repairs and
+fresh re-review dispatch. The canonical skill owns the review workflow and
+output contract.
 """

AGENTS.md

@@ -83,11 +83,12 @@ The plan record is a GitHub issue. Its body starts with
 sections; it has no frontmatter. GitHub owns title, open-work phase, owner,
 timestamps, and completion, and no plan markdown is tracked in the repository.
 Exactly three skills own the workflow: `plan-workspace` maintains the workspace;
-main-context `plan-manager` runs six phases — decide, draft, research, one plan
-review, implement, code review — and archives; internal `plan-reviewer` returns
-a readable pre-implementation verdict. Two read-only reviewer wrappers ship,
-`plan-reviewer` and `code-reviewer`, and nothing else in the lifecycle has a
-wrapper.
+main-context `plan-manager` runs six phases — decide, draft, research, plan
+review, implement, code review — with bounded repair and fresh re-review in both
+review phases, then archives; internal `plan-reviewer` returns one readable
+pre-implementation verdict block per round. Two read-only reviewer wrappers
+ship, `plan-reviewer` and `code-reviewer`, and nothing else in the lifecycle has
+a wrapper.
 </constraint>
 
 After the marker and blank line, the record carries exactly `## Goal`,
@@ -96,17 +97,36 @@ After the marker and blank line, the record carries exactly `## Goal`,
 and once each. `## Goal` carries exactly one mode line. Open-work phase is one
 of `drafting`, `planned`, `ongoing`, or `blocked` in a `plan:<phase>` label; a
 blocked plan starts `## Open questions` with `Blocked: <one-line reason>`.
-Closed completion derives from GitHub `state` and `stateReason`. The record
-carries no hash, permit, run identity, lock, or bundle, and the `plan.mjs`
-shipped inside the installed `plan-lifecycle` plugin is the only lifecycle tool.
-An `export` writes the sha256 of the body it copied beside the copy so a stale
-copy cannot revert the record; that digest detects staleness and authorizes
-nothing. Routine plan issue publication and post-review landing through a
-branch, commit, normal push, and closing pull request carry the settled mode's
-authorization and need no repeated prompt. After the checks policy passes, the
-manager asks immediately before merge. Without a fresh `Merge now` answer, it
-leaves the pull request and issue open. `plan.mjs archive` verifies the merged
-closing pull request after landing.
+Closed completion derives from GitHub `state` and `stateReason`. `## Review`
+contains exactly `_Review records are stored in issue comments._`. Each reviewer
+returns one markdown block, and the manager posts that whole block as one issue
+comment. The latest trusted well-formed record per review kind wins; its author
+must equal the plan's sole assignee. A legacy body verdict is consulted only
+when no trusted comment record exists for that kind. Both review phases use
+fresh inputs and run at most five rounds, stopping on pass, no progress, a
+finding surviving its fix, or `repair` or `fixes-required` in round five. A
+plan-review `blocked` verdict always routes its user-only decision through
+`## Open questions` and `ask`.
+
+The record carries no hash, permit, run identity, lock, or bundle, and the
+`plan.mjs` shipped inside the installed `plan-lifecycle` plugin is the only
+lifecycle tool. An `export` writes the sha256 of the body it copied beside the
+copy so a stale copy cannot revert the record; that digest detects staleness and
+authorizes nothing. Routine plan issue publication, implement-start linked
+branch creation, commits, normal pushes, and the closing pull request carry the
+settled mode's authorization and need no repeated prompt. Before any branch
+checkout, including `gh issue develop --checkout`, require
+`git status --porcelain` to be empty. If it is dirty, never stash, move, or
+commit ambient work; set the plan `blocked` and name the dirty paths, or use an
+authorized clean worktree.
+Immediately after setting the plan `ongoing`, every `gh issue develop` call uses
+`--repo`; the manager reuses a linked branch or creates one with
+`--base <default> --checkout`, then re-lists and recovers after failure.
+Implementation stops when no linked branch can be verified; there is no local
+fallback. After the checks policy passes, the manager asks immediately before
+merge. Without a fresh `Merge now` answer, it leaves the pull request and issue
+open. `plan.mjs archive` verifies the latest trusted code-review result and
+merged closing pull request after landing.
 
 Every Steps row carries an `Effect` of exactly
 `local|probe|production_access|publish|push|release|deploy`. A step whose
@@ -118,8 +138,9 @@ actions are outside the Steps table.
 
 Render a plan body verbatim only when the user names that plan and asks to see it. After a write, report the one-line header strip and the changed lines only; a write never re-renders the body.
 
-`docs/plans/finished/` is frozen pre-GitHub history: read it as history, never as
-a source of truth, and never parse or migrate it. The complete contract lives in
+`docs/plans/finished/` is frozen pre-GitHub history. Humans may read it as
+history, but it is not a source of truth. No lifecycle command or workspace
+migration operation opens or inventories it. The complete contract lives in
 `docs/PLAN.md`; `docs/AGENTS.md` routes to it and `docs/CLAUDE.md` contains only
 `@AGENTS.md`.
 

README.md

@@ -70,15 +70,16 @@ a canonical plan for explicit planning, multi-commit/cross-repository work,
 scheduling, cold handoff, unresolved decisions, cross-subsystem/public-contract
 changes, security-sensitive/destructive work, or an external effect.
 
-The three lifecycle skills, shipped `plan.mjs`, markdown-only v2 contract
+The three lifecycle skills, shipped `plan.mjs`, marker-based contract
 reference, and two read-only reviewer wrappers ship as the self-versioned
 `plan-lifecycle` plugin (`plugins/plan-lifecycle/`), installable from this same
-marketplace. Plan records live in GitHub issue bodies.
+marketplace. Plan bodies and comment-backed review records live on GitHub
+issues.
 
 | Owner | Skill | Invocation | Responsibility |
 |---|---|---|---|
 | Workspace | `plan-workspace` | Public | Create the plan label set and maintain `docs/PLAN.md` plus the `docs/AGENTS.md`/`docs/CLAUDE.md` pair; never mutate an individual plan issue |
-| Orchestration | `plan-manager` | Public, main context | Decide → draft → research → one plan review → implement → code review; archive after a passing review and a merged pull request closes the issue |
+| Orchestration | `plan-manager` | Public, main context | Decide → draft → research → plan review → implement → code review; repair and freshly re-review both review phases, then archive after a pass and a merged closing pull request |
 | Plan review | `plan-reviewer` | Internal, read-only | Check only `goal_fit`, `research_gap`, and `security_risk` before implementation |
 | Code review | `code-reviewer` | Internal, read-only | Review the implemented diff against code standards and the plan |
 
@@ -87,13 +88,16 @@ seeded as thin Claude/Codex wrappers; main context invokes `plan-manager`
 directly. The docks pipelines route to these skills and stop, naming the missing
 `plan-lifecycle` plugin, when they are unavailable.
 
-The lifecycle runs six phases: decide, draft, research, one plan review,
-implement, and one post-implementation code review. After review passes, it
-commits and pushes the reviewed branch, opens the closing pull request, and
-waits for repository CI. It then asks `Merge now` or
-`Leave pull request open`. Without a fresh `Merge now` answer, it leaves the
-pull request and issue open. After an approved merge, `plan.mjs archive`
-verifies the merged closing pull request.
+The lifecycle runs six phases: decide, draft, research, plan review, implement,
+and code review. Plan repairs are re-reviewed from fresh exports, and code fixes
+are re-reviewed from fresh diffs, with a five-round ceiling in each review
+phase. Each reviewer returns one markdown block that the manager stores as one
+issue comment. When implementation starts, the manager reuses or creates the
+GitHub-linked plan branch. After code review passes, it commits and pushes any
+remaining reviewed bytes, opens the closing pull request, and waits for
+repository CI. It then asks `Merge now` or `Leave pull request open`. Without a
+fresh `Merge now` answer, it leaves the pull request and issue open. After an
+approved merge, `plan.mjs archive` verifies the merged closing pull request.
 
 Every Steps row has `Effect` exactly
 `local|probe|production_access|publish|push|release|deploy`. Each non-`local`
@@ -112,7 +116,7 @@ The complete contract lives in `docs/PLAN.md`.
 │   │   ├── .claude-plugin/plugin.json
 │   │   ├── skills/                    ← cross-tool skills
 │   │   └── README.md                  ← plugin-facing docs
-│   ├── plan-lifecycle/                ← GitHub-issue plan lifecycle plugin (three skills + plan.mjs + v2 contract + two read-only reviewer wrappers)
+│   ├── plan-lifecycle/                ← GitHub-issue plan lifecycle plugin (three skills + plan.mjs + marker contract + two read-only reviewer wrappers)
 │   └── effect-kit/                    ← Effect-TS skill kit plugin
 ├── scripts/                           ← plugin-author tooling (NOT shipped to users)
 │   ├── ci.mjs / release.mjs           ← orchestrators (the gate ci.yml runs)