From 4b1187174ebf39619eb48179227607f726703dc2 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:27:13 -0300 Subject: [PATCH 1/2] fix(plan-lifecycle): bound code review convergence Advisory findings should not force repair rounds, while repeated high-impact failures need a deterministic stop. Define the verdict threshold and pin both reviewer wrappers so contract drift fails CI. Refs #18 --- .codex/agents/code-reviewer.toml | 11 +++++ docs/PLAN.md | 24 ++++++---- .../plan-lifecycle/agents/code-reviewer.md | 12 +++-- .../skills/productivity/plan-manager/SKILL.md | 20 ++++---- .../plan-manager/references/plan-contract.md | 13 +++-- .../productivity/plan-workspace/SKILL.md | 2 +- .../references/plan-md-template.md | 24 ++++++---- scripts/tests/plan-cli.mjs | 47 +++++++++++++++++++ scripts/tests/plan-skill-phases.mjs | 16 ++++++- 9 files changed, 135 insertions(+), 34 deletions(-) diff --git a/.codex/agents/code-reviewer.toml b/.codex/agents/code-reviewer.toml index 87b78692..3088db43 100644 --- a/.codex/agents/code-reviewer.toml +++ b/.codex/agents/code-reviewer.toml @@ -40,6 +40,17 @@ the other. Use `Bug`, `Security`, `Performance`, or `Maintainability` for Standards findings. Use `Spec` for a plan mismatch. +Verdicts. Select exactly one: +- `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` and + `LOW` lines may ride along on a `pass`: the manager records them and fixes + them at its judgment, and 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. +- `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 diff --git a/docs/PLAN.md b/docs/PLAN.md index f63771cb..0ea63d13 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -206,7 +206,9 @@ Legal step transitions are `planned → in-flight | done | blocked | skipped`, ` all Steps rows to be terminal (`done` or `skipped`), a line matching exactly `Code-review: pass` in `## Review`, and an issue already closed as completed by an eligible merged pull request. It writes no status. On success it removes any -stale phase label and prints `plan # finished (closed by )`. +stale phase label and prints `plan # finished (closed by )`. The pass +line may carry advisory `MEDIUM` and `LOW` finding lines beneath it; only an +unfixed `CRITICAL` or `HIGH` keeps a plan from archiving. The verifier reads the issue's `closedByPullRequestsReferences` with `excludeUserLinked: true`. It accepts only keyword-linked merged pull requests. @@ -289,9 +291,15 @@ Code-review: pass|fixes-required|blocked - HIGH · Security · plugins/x/y.mjs:41 — user input reaches a shell command unquoted — pass an argument array ``` -A `pass` record has no finding lines. Every other verdict has at least one -finding line. A plan-review finding is exactly one of `goal_fit`, `research_gap`, -or `security_risk`; nothing else is a finding. A sufficient plan passes. +A code-review `pass` means no `CRITICAL` or `HIGH` finding stands unfixed; it +carries only advisory `MEDIUM` and `LOW` lines, or none, and the manager fixes +those at its judgment without a re-review. `fixes-required` names at least one +evidenced `CRITICAL` or `HIGH` defect and forces exactly one repair re-review; +if that re-review still returns `fixes-required`, the manager appends +`Code-review: blocked` and sets the plan `blocked`. A `blocked` verdict has at +least one finding line. A plan-review finding is exactly one of `goal_fit`, +`research_gap`, or `security_risk`; nothing else is a finding. A sufficient plan +passes. ## Phases @@ -300,16 +308,16 @@ or `security_risk`; nothing else is a finding. A sufficient plan passes. 3. **Research.** Verify repository facts and external claims, record their sources, choose the durable fix, bind the exact files, complete Acceptance, pass `plan.mjs check`, and set the plan `planned`. 4. **Plan review.** Dispatch exactly one pre-implementation review. Append its verdict and findings. Fix reproduced findings before implementation. A user-only decision goes in `## Open questions`. A plan-only run stops at `planned` after this review. 5. **Implement.** Set the plan `ongoing`, move each step through its legal states, and record real Acceptance output in `## Verification Results` before the closing merge. -6. **Code review.** Review the declared change, fix every critical and high finding, and review again only after such a fix. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. +6. **Code review.** Review the declared change, fix every critical and high finding, and run exactly one repair re-review after such a fix; if that re-review still returns fixes-required, append `Code-review: blocked` and set the plan `blocked`. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. Build the review diff from what actually changed: `git status --porcelain` names the paths and the diff covers exactly those. Name every changed path that no Steps `Files` cell mentions in the review request, so the reviewer judges undeclared scope instead of the manager blocking on bookkeeping. -If a code-review round returns the same finding-id set as the previous round and -no file changed between the two rounds, stop, append `Code-review: blocked` -naming that set, and set the plan `blocked`. +If that repair re-review again returns `fixes-required`, stop: append +`Code-review: blocked` naming the surviving findings, and set the plan +`blocked`. A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` diff --git a/plugins/plan-lifecycle/agents/code-reviewer.md b/plugins/plan-lifecycle/agents/code-reviewer.md index 177d9b42..d31c1e6a 100644 --- a/plugins/plan-lifecycle/agents/code-reviewer.md +++ b/plugins/plan-lifecycle/agents/code-reviewer.md @@ -79,8 +79,11 @@ buckets or the Spec axis defined below. 8. Re-read every locator and verify that the proposed fix resolves the defect without violating another plan step or invariant. 9. Select one verdict: - - `pass`: Neither axis has a finding. - - `fixes-required`: At least one evidenced defect can be fixed by the manager. + - `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` + and `LOW` lines may ride along on a `pass`: the manager records them and + fixes them at its judgment, and 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. - `blocked`: Required review input is unreadable or contradictory, so no safe verdict can be reached. 10. Order findings `CRITICAL`, `HIGH`, `MEDIUM`, then `LOW`. Return once. @@ -101,8 +104,9 @@ Each finding uses one line: SEVERITY · CATEGORY · file:line — defect — fix ``` -Use only `pass`, `fixes-required`, or `blocked`. A `pass` verdict has no finding -lines. A non-passing verdict has at least one finding line. Use `Bug`, +Use only `pass`, `fixes-required`, or `blocked`. A `pass` verdict carries only +advisory `MEDIUM` and `LOW` lines, or none. A `fixes-required` or `blocked` +verdict has at least one finding line. Use `Bug`, `Security`, `Performance`, or `Maintainability` for Standards findings. Use `Spec` for a plan mismatch. Keep both analysis axes distinct even though the single review record orders all findings by severity. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md index f54e104b..20ea411a 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-08-21" - content_hash: "cae1f4289cd368270eb9cb21138bc81a001ed0d689c8792fdd1ce0f84c744d34" + content_hash: "44df74ed5d00700a9d68679062e6e57f406549fabe5f509ebc0d277a1db090f3" --- # Plan Manager @@ -141,13 +141,17 @@ with `Blocked:`. Dispatch `code-reviewer` with the absolute diff path, the absolute export path, and the issue number. Append its report to `## Review` under - `### Code review round — `. Fix every `CRITICAL` and `HIGH` - finding. Record each `MEDIUM` and `LOW` finding, then fix it at your judgment. - Re-review only after fixing a `CRITICAL` or `HIGH`. - - This is a progress guard, not a budget. If a code-review round returns the same finding-id set as the previous round and no file changed between the two rounds, stop, append `Code-review: blocked` naming that set, and set the plan `blocked`. - - When a round returns `Code-review: pass`, report that the work is ready for + `### Code review round — `. A round that returns + `Code-review: pass` carries no unfixed `CRITICAL` or `HIGH` finding; it may + still carry advisory `MEDIUM` and `LOW` lines. Record each advisory line, + then fix it at your judgment; an advisory line never triggers a re-review. + A round that returns `Code-review: fixes-required` names at least one + evidenced `CRITICAL` or `HIGH` defect: fix every one of them, then dispatch + exactly one repair re-review. If that repair re-review again returns + `fixes-required`, stop: append `Code-review: blocked` naming the surviving + findings, and set the plan `blocked`. + + A `pass` round means the work is ready for the user's landing actions. Run `plan.mjs archive ` only after the user has landed the closing pull request. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md index 33fe15bc..89d0a2e7 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md @@ -168,8 +168,13 @@ Code-review: fixes-required ``` `Plan-review:` is exactly `pass`, `repair`, or `blocked`. `Code-review:` is -exactly `pass`, `fixes-required`, or `blocked`. `pass` has no finding lines; the -others have at least one. +exactly `pass`, `fixes-required`, or `blocked`. A code-review `pass` means no +`CRITICAL` or `HIGH` finding stands unfixed; it carries only advisory `MEDIUM` +and `LOW` lines, or none, and the manager fixes those at its judgment without a +re-review. `fixes-required` names at least one evidenced `CRITICAL` or `HIGH` +defect and forces exactly one repair re-review; if that re-review still returns +`fixes-required`, the manager appends `Code-review: blocked` and sets the plan +`blocked`. A `blocked` verdict has at least one finding line. A plan-review finding is exactly one of `goal_fit`, `research_gap`, or `security_risk`; nothing else is a finding. A sufficient plan passes. @@ -267,7 +272,9 @@ ownership. `archive` is a verifier, not a status writer. It requires the issue already closed with `stateReason: COMPLETED`, every Steps row terminal (`done` or -`skipped`), and a line exactly `Code-review: pass` in `## Review`. +`skipped`), and a line exactly `Code-review: pass` in `## Review`. That pass +line may carry advisory `MEDIUM` and `LOW` finding lines beneath it; only an +unfixed `CRITICAL` or `HIGH` keeps a plan from archiving. `archive` also requires a merged closing pull request into the target repository's default branch. It reads `closedByPullRequestsReferences` with diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md index 5088d6a5..ebd958ce 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-08-21" - content_hash: "15326383147ec4fd0125bd985eff6657d1b2f24449a68080320e1df251bff66f" + content_hash: "59cab3fc49f767eb557270c6187fd5b1fdd555ffb8ddc73d4a1f5e2cdeefa84e" --- # Plans Workspace diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md index 4b6d81a1..0c1a33f2 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md @@ -210,7 +210,9 @@ Legal step transitions are `planned → in-flight | done | blocked | skipped`, ` all Steps rows to be terminal (`done` or `skipped`), a line matching exactly `Code-review: pass` in `## Review`, and an issue already closed as completed by an eligible merged pull request. It writes no status. On success it removes any -stale phase label and prints `plan # finished (closed by )`. +stale phase label and prints `plan # finished (closed by )`. The pass +line may carry advisory `MEDIUM` and `LOW` finding lines beneath it; only an +unfixed `CRITICAL` or `HIGH` keeps a plan from archiving. The verifier reads the issue's `closedByPullRequestsReferences` with `excludeUserLinked: true`. It accepts only keyword-linked merged pull requests. @@ -293,9 +295,15 @@ Code-review: pass|fixes-required|blocked - HIGH · Security · plugins/x/y.mjs:41 — user input reaches a shell command unquoted — pass an argument array ``` -A `pass` record has no finding lines. Every other verdict has at least one -finding line. A plan-review finding is exactly one of `goal_fit`, `research_gap`, -or `security_risk`; nothing else is a finding. A sufficient plan passes. +A code-review `pass` means no `CRITICAL` or `HIGH` finding stands unfixed; it +carries only advisory `MEDIUM` and `LOW` lines, or none, and the manager fixes +those at its judgment without a re-review. `fixes-required` names at least one +evidenced `CRITICAL` or `HIGH` defect and forces exactly one repair re-review; +if that re-review still returns `fixes-required`, the manager appends +`Code-review: blocked` and sets the plan `blocked`. A `blocked` verdict has at +least one finding line. A plan-review finding is exactly one of `goal_fit`, +`research_gap`, or `security_risk`; nothing else is a finding. A sufficient plan +passes. ## Phases @@ -304,16 +312,16 @@ or `security_risk`; nothing else is a finding. A sufficient plan passes. 3. **Research.** Verify repository facts and external claims, record their sources, choose the durable fix, bind the exact files, complete Acceptance, pass `plan.mjs check`, and set the plan `planned`. 4. **Plan review.** Dispatch exactly one pre-implementation review. Append its verdict and findings. Fix reproduced findings before implementation. A user-only decision goes in `## Open questions`. A plan-only run stops at `planned` after this review. 5. **Implement.** Set the plan `ongoing`, move each step through its legal states, and record real Acceptance output in `## Verification Results` before the closing merge. -6. **Code review.** Review the declared change, fix every critical and high finding, and review again only after such a fix. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. +6. **Code review.** Review the declared change, fix every critical and high finding, and run exactly one repair re-review after such a fix; if that re-review still returns fixes-required, append `Code-review: blocked` and set the plan `blocked`. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. Build the review diff from what actually changed: `git status --porcelain` names the paths and the diff covers exactly those. Name every changed path that no Steps `Files` cell mentions in the review request, so the reviewer judges undeclared scope instead of the manager blocking on bookkeeping. -If a code-review round returns the same finding-id set as the previous round and -no file changed between the two rounds, stop, append `Code-review: blocked` -naming that set, and set the plan `blocked`. +If that repair re-review again returns `fixes-required`, stop: append +`Code-review: blocked` naming the surviving findings, and set the plan +`blocked`. A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` diff --git a/scripts/tests/plan-cli.mjs b/scripts/tests/plan-cli.mjs index 43a51bc3..7b6ea9fa 100755 --- a/scripts/tests/plan-cli.mjs +++ b/scripts/tests/plan-cli.mjs @@ -1201,6 +1201,53 @@ try { assert.equal(inexactReviewArchive.status, 1, 'changing the exact Code-review pass line must be refused'); assert.match(inexactReviewArchive.stderr, /Code-review: pass/); + const advisoryPassNumber = createPlan('advisory-pass-archive'); + makeValid(advisoryPassNumber); + setIssueStatus(advisoryPassNumber, 'ongoing'); + updateIssue(advisoryPassNumber, (entry) => { + entry.body = replaceStepStatus(entry.body, 'done').replace( + '_No review yet._', + 'Code-review: pass\n- MEDIUM · Maintainability · src/example.mjs:9 — duplicated guard clause — extract a named predicate', + ); + entry.state = 'CLOSED'; + entry.stateReason = 'COMPLETED'; + entry.closedByPullRequestsReferences = [ + { + number: 43, + url: 'https://github.com/DocksDocks/fixture/pull/43', + repository: 'DocksDocks/fixture', + userLinked: false, + }, + ]; + }); + updateState((state) => { + state.prs.push({ + number: 43, + repository: 'DocksDocks/fixture', + mergedAt: '2026-08-20T21:30:00Z', + state: 'MERGED', + baseRefName: 'main', + url: 'https://github.com/DocksDocks/fixture/pull/43', + }); + }); + const advisoryPassArchive = run('archive', String(advisoryPassNumber)); + expectSuccess(advisoryPassArchive, 'archive accepts a pass carrying only an advisory line'); + + const mediumOnlyRequiredNumber = createPlan('medium-only-fixes-required'); + makeValid(mediumOnlyRequiredNumber); + setIssueStatus(mediumOnlyRequiredNumber, 'ongoing'); + updateIssue(mediumOnlyRequiredNumber, (entry) => { + entry.body = replaceStepStatus(entry.body, 'done').replace( + '_No review yet._', + 'Code-review: fixes-required\n- MEDIUM · Maintainability · src/example.mjs:9 — duplicated guard clause — extract a named predicate', + ); + entry.state = 'CLOSED'; + entry.stateReason = 'COMPLETED'; + }); + const mediumOnlyRequiredArchive = run('archive', String(mediumOnlyRequiredNumber)); + assert.equal(mediumOnlyRequiredArchive.status, 1, 'a fixes-required verdict must refuse archive'); + assert.match(mediumOnlyRequiredArchive.stderr, /Code-review: pass/); + const wrongBranchNumber = createPlan('wrong-branch-archive'); makeValid(wrongBranchNumber); setIssueStatus(wrongBranchNumber, 'ongoing'); diff --git a/scripts/tests/plan-skill-phases.mjs b/scripts/tests/plan-skill-phases.mjs index 88323974..d3852ea7 100755 --- a/scripts/tests/plan-skill-phases.mjs +++ b/scripts/tests/plan-skill-phases.mjs @@ -16,6 +16,8 @@ const WORKSPACE_SKILL = 'plugins/plan-lifecycle/skills/productivity/plan-workspa const WORKSPACE_TEMPLATE = 'plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md'; const PLAN_CONTRACT = 'plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md'; const PLAN_MD = 'docs/PLAN.md'; +const CODE_REVIEWER_AGENT = 'plugins/plan-lifecycle/agents/code-reviewer.md'; +const CODE_REVIEWER_CODEX = '.codex/agents/code-reviewer.toml'; const V3_PINNED_CLAUSES = [ { @@ -63,10 +65,20 @@ const V3_PINNED_CLAUSES = [ files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], }, { - name: 'code-review-progress-guard', - text: 'If a code-review round returns the same finding-id set as the previous round and no file changed between the two rounds, stop, append `Code-review: blocked` naming that set, and set the plan `blocked`.', + name: 'code-review-repair-bound', + text: 'If that repair re-review again returns `fixes-required`, stop: append `Code-review: blocked` naming the surviving findings, and set the plan `blocked`.', files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], }, + { + name: 'code-review-pass-verdict', + text: '- `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` and `LOW` lines may ride along on a `pass`: the manager records them and fixes them at its judgment, and they never trigger a re-review.', + files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX], + }, + { + name: 'code-review-fixes-required-verdict', + text: '- `fixes-required`: At least one evidenced `CRITICAL` or `HIGH` defect. The manager fixes it and dispatches exactly one repair re-review.', + files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX], + }, { name: 'review-scope-guard', text: 'Build the review diff from what actually changed: `git status --porcelain` names the paths and the diff covers exactly those. Name every changed path that no Steps `Files` cell mentions in the review request, so the reviewer judges undeclared scope instead of the manager blocking on bookkeeping.', From 6aafb47093dc63c9eb55f6678e4621e1fa65b345 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:35:37 -0300 Subject: [PATCH 2/2] feat(plan-lifecycle)!: default landing to reviewed PR Routine plan publication and post-review branch landing no longer require repeated approval. Preserve one explicit user boundary immediately before default-branch merge, and pin the complete candidate, CI, and head-SHA safeguards. Refs #20 --- .codex/agents/code-reviewer.toml | 5 +- AGENTS.md | 16 +-- README.md | 9 +- docs/PLAN.md | 71 ++++++--- plugins/docks/README.md | 11 +- .../skills/engineering/refactor/SKILL.md | 4 +- .../refactor/references/executor-dispatch.md | 8 +- .../plan-lifecycle/agents/code-reviewer.md | 5 +- plugins/plan-lifecycle/skills/AGENTS.md | 11 +- .../skills/productivity/plan-manager/SKILL.md | 135 ++++++++++++------ .../references/github-issue-publication.md | 85 ++++++++--- .../plan-manager/references/plan-contract.md | 47 ++++-- .../productivity/plan-workspace/SKILL.md | 18 +-- .../references/codex-agent-templates.md | 11 ++ .../references/plan-md-template.md | 71 ++++++--- scripts/tests/plan-skill-phases.mjs | 105 ++++++++++++-- 16 files changed, 451 insertions(+), 161 deletions(-) diff --git a/.codex/agents/code-reviewer.toml b/.codex/agents/code-reviewer.toml index 3088db43..ad3acc4e 100644 --- a/.codex/agents/code-reviewer.toml +++ b/.codex/agents/code-reviewer.toml @@ -42,8 +42,9 @@ findings. Use `Spec` for a plan mismatch. Verdicts. Select exactly one: - `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` and - `LOW` lines may ride along on a `pass`: the manager records them and fixes - them at its judgment, and they never trigger a re-review. + `LOW` lines may ride along on a `pass`: the manager records them as follow-ups + 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. - `blocked`: Required review input is unreadable or contradictory, so no safe diff --git a/AGENTS.md b/AGENTS.md index c1912fa5..3c6009a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,20 +101,20 @@ 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. This lifecycle creates zero commits and never pushes. Commit when the -user asks, under `docks:commit-discipline`. +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. Every Steps row carries an `Effect` of exactly `local|probe|production_access|publish|push|release|deploy`. A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` and the plan reason becomes the first `## Open questions` line, `Blocked: `. -Persisted effects record intent only. - -Work lands through a pull request whose body carries `Closes #` and whose -base is the repository default branch. `plan.mjs archive` is a verifier: it -requires completed closure, terminal steps, an exact `Code-review: pass` line, -and a merged closing pull request into that branch; it writes no status. +Persisted effects record intent only. Routine issue publication and landing +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. diff --git a/README.md b/README.md index 550dfadb..b42358bf 100644 --- a/README.md +++ b/README.md @@ -88,9 +88,12 @@ 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. Plans use a markdown-only v2 -record with `plan_contract: v2` frontmatter and eight sections. The lifecycle -creates zero automatic commits and never pushes. +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. Every Steps row has `Effect` exactly `local|probe|production_access|publish|push|release|deploy`. Each non-`local` diff --git a/docs/PLAN.md b/docs/PLAN.md index 0ea63d13..cfa37d1b 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -292,9 +292,11 @@ Code-review: pass|fixes-required|blocked ``` A code-review `pass` means no `CRITICAL` or `HIGH` finding stands unfixed; it -carries only advisory `MEDIUM` and `LOW` lines, or none, and the manager fixes -those at its judgment without a re-review. `fixes-required` names at least one -evidenced `CRITICAL` or `HIGH` defect and forces exactly one repair re-review; +carries only advisory `MEDIUM` and `LOW` lines, or none. Record each advisory as +a follow-up and do not change reviewed bytes after a pass; an advisory never +triggers a re-review. +`fixes-required` names at least one evidenced `CRITICAL` or `HIGH` defect and +forces exactly one repair re-review; if that re-review still returns `fixes-required`, the manager appends `Code-review: blocked` and sets the plan `blocked`. A `blocked` verdict has at least one finding line. A plan-review finding is exactly one of `goal_fit`, @@ -310,10 +312,17 @@ passes. 5. **Implement.** Set the plan `ongoing`, move each step through its legal states, and record real Acceptance output in `## Verification Results` before the closing merge. 6. **Code review.** Review the declared change, fix every critical and high finding, and run exactly one repair re-review after such a fix; if that re-review still returns fixes-required, append `Code-review: blocked` and set the plan `blocked`. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. -Build the review diff from what actually changed: `git status --porcelain` names -the paths and the diff covers exactly those. Name every changed path that no -Steps `Files` cell mentions in the review request, so the reviewer judges -undeclared scope instead of the manager blocking on bookkeeping. +Build the review diff from the complete candidate pull request, not only the +dirty worktree. Resolve and fetch the repository default branch, then compute +`` with `git merge-base HEAD`. Cover one net +tracked candidate with `git diff -- `. Add one +`git diff --no-index /dev/null ` hunk for each untracked path. +`git status --porcelain` still names dirty paths. Name every changed path that +no Steps `Files` cell mentions in the review request. + +After pull-request creation, record `headRefOid` and compare the changed paths +and hunks from `gh pr diff` with the reviewed net candidate. Any mismatch +invalidates the pass and blocks merge. If that repair re-review again returns `fixes-required`, stop: append `Code-review: blocked` naming the surviving findings, and set the plan @@ -323,22 +332,44 @@ A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` and `Blocked: ` is recorded first in `## Open questions`. -This lifecycle creates zero commits and never pushes. +Routine plan issue publication is authorized by the settled mode and needs no +repeated repository picker. ## Landing -Work lands through a pull request whose body carries `Closes #` and whose base is the repository default branch, because GitHub interprets a closing keyword only in a pull request that targets the default branch. `plan.mjs archive` verifies that merged pull request rather than performing the merge. - -Only the pull request that lands the completed work carries `Closes #`. A -partial pull request carries a plain `Refs #` instead, because GitHub -closes the issue as soon as the first pull request carrying a closing keyword -merges into the default branch. - -Landing sits outside the six phases. This lifecycle creates zero commits and -never pushes, so the branch, the commits, the push, the pull request, and the -merge are the user's to run, on request, under `docks:commit-discipline`. No -Steps row exists for them, and `archive` reads the result rather than causing -it. A plan that never lands is retired, not archived. +Work lands through a pull request whose body carries `Closes #` and whose +base is the repository default branch. + +After `Code-review: pass`, the manager runs landing without another prompt: +ensure a non-default branch, commit exactly the reviewed bytes under +`docks:commit-discipline`, push normally, and create or update one pull request +that carries `Closes #` and targets the repository default branch. + +Never treat an empty first checks result as success. Retry +`gh pr checks --json name,bucket` at most 12 times with a 10-second delay until +checks appear. If required checks exist, run +`gh pr checks --watch --required`; if CI checks exist but none are required, +run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks +merge. If no checks appear, continue only when repository inspection confirms +that no pull-request CI is configured; otherwise leave the pull request open +with a named no-checks blocker and do not show the merge prompt. + +When the checks policy passes and GitHub reports the pull request mergeable, +ask immediately with exactly two options: `Merge now` or +`Leave pull request open`. Merge only on that fresh answer. If the user +declines, or `ask` is unavailable, leave the pull request and the issue open +and report the pull request URL. Never auto-merge, force-push, bypass branch +protection, or merge on a stale or assumed answer. + +Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA +or diff changed, block merge. Invoke `gh pr merge` with +`--match-head-commit ` and the repository's configured merge +strategy only after the fresh `Merge now` answer. + +Only the pull request that lands the completed work carries `Closes #`. +A partial pull request carries plain `Refs #`. `archive` verifies the +merged result rather than causing it. A plan that never lands is retired, not +archived. ## Portability diff --git a/plugins/docks/README.md b/plugins/docks/README.md index a52b163e..ad9cc38e 100644 --- a/plugins/docks/README.md +++ b/plugins/docks/README.md @@ -60,10 +60,13 @@ Plus `write-skill`, `multi-tool-bridge`, and `zoom-out` under `productivity/`. The plan lifecycle ships separately as the self-versioned `plan-lifecycle` plugin in this same marketplace. It includes three skills, the shipped -`plan.mjs`, a markdown-only v2 plan contract, and the read-only `plan-reviewer` -and `code-reviewer` wrappers. Its six phases are decide, draft, research, one -plan review, implement, and code review, with zero automatic commits. Every -docks route into that lifecycle is fail-loud: +`plan.mjs`, a markdown-only v3 plan contract, and the read-only `plan-reviewer` +and `code-reviewer` wrappers. After review passes, the manager commits and +pushes the reviewed branch, opens the closing pull request, waits for repository +CI, and 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 docks +route into that lifecycle is fail-loud: Prerequisite: `plan-lifecycle` must be installed. If `plan-workspace` or `plan-manager` is unavailable, STOP, name the missing `plan-lifecycle` plugin, and do not create or mutate a plan. diff --git a/plugins/docks/skills/engineering/refactor/SKILL.md b/plugins/docks/skills/engineering/refactor/SKILL.md index 61cf659a..9278d1cf 100644 --- a/plugins/docks/skills/engineering/refactor/SKILL.md +++ b/plugins/docks/skills/engineering/refactor/SKILL.md @@ -4,8 +4,8 @@ description: "Use when auditing a codebase for structural issues — dead code, user-invocable: true metadata: pattern: pipeline - updated: "2026-08-20" - content_hash: "c496105fbaf3735e9dd2c189e5a011970ce02516c4cc10087a28739c123b85b9" + updated: "2026-08-21" + content_hash: "3c2e953bdcd16540ffb49f86bc4b157b3072e1a0793c755f5d4c9b1d20620f43" --- # Refactor (cross-tool pipeline) diff --git a/plugins/docks/skills/engineering/refactor/references/executor-dispatch.md b/plugins/docks/skills/engineering/refactor/references/executor-dispatch.md index 6765b840..9aa1263a 100644 --- a/plugins/docks/skills/engineering/refactor/references/executor-dispatch.md +++ b/plugins/docks/skills/engineering/refactor/references/executor-dispatch.md @@ -88,7 +88,7 @@ A *documented* deviation is judged on merit, not reflex-blocked; an | Verdict | When | Action | |---|---|---| -| **APPROVE** | plan identity matches, criteria pass, scope is clean, and quality holds | Return the reviewed diff and executor result to main-context `plan-manager`. It applies the reviewed diff, reruns verification, records `## Verification Results`, dispatches the single post-implementation code review, and archives with `plan.mjs archive ` once that review returns `Code-review: pass`. | +| **APPROVE** | plan identity matches, criteria pass, scope is clean, and quality holds | Return the reviewed diff and executor result to main-context `plan-manager`. It applies the reviewed diff, reruns verification, records `## Verification Results`, dispatches the single post-implementation code review, and follows the manager's full Landing flow. It archives only after an approved merge lands the closing pull request. | | **REVISE** | fixable gaps | Send the same executor specific, actionable feedback. Allow at most two executor revision rounds, then return a failure result to `plan-manager`. | | **BLOCK** | STOP hit, scope violated unrecoverably, or revisions exhausted | Return the evidence to `plan-manager`; it sets the plan `blocked` with a reason and does not re-run the plan review. | @@ -100,8 +100,8 @@ approved diff to the main working tree. - The executor never writes lifecycle state, `## Review`, or `## Verification Results`; main-context `plan-manager` owns them. -- The executor creates no commit. This lifecycle creates zero commits and never - pushes, merges, or applies directly to the main working tree. Its only handoff - is the reviewed diff and result. +- The executor creates no commit, pushes nothing, and never merges or applies + directly to the main working tree. Its only handoff is the reviewed diff and + result; main-context `plan-manager` owns landing. - Off Claude, or when worktree isolation is unavailable, this mode is skipped; `plan-manager` runs Phases 7–8 in context. diff --git a/plugins/plan-lifecycle/agents/code-reviewer.md b/plugins/plan-lifecycle/agents/code-reviewer.md index d31c1e6a..054217a3 100644 --- a/plugins/plan-lifecycle/agents/code-reviewer.md +++ b/plugins/plan-lifecycle/agents/code-reviewer.md @@ -80,8 +80,9 @@ buckets or the Spec axis defined below. without violating another plan step or invariant. 9. Select one verdict: - `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` - and `LOW` lines may ride along on a `pass`: the manager records them and - fixes them at its judgment, and they never trigger a re-review. + and `LOW` lines may ride along on a `pass`: the manager records them as + follow-ups 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. - `blocked`: Required review input is unreadable or contradictory, so no safe diff --git a/plugins/plan-lifecycle/skills/AGENTS.md b/plugins/plan-lifecycle/skills/AGENTS.md index 91737131..c047b6e5 100644 --- a/plugins/plan-lifecycle/skills/AGENTS.md +++ b/plugins/plan-lifecycle/skills/AGENTS.md @@ -60,11 +60,12 @@ The synchronized surface is the v3 first-line marker, no-frontmatter rule, eight sections, both table headers, four open-work phases (`drafting`, `planned`, `ongoing`, `blocked`), the GitHub `state` + `stateReason` derivation truth table, the three plan-review kinds (`goal_fit`, `research_gap`, `security_risk`), the -two review-record shapes, the zero-commit rule, lifecycle-tool ownership, issue -write preconditions, archive-as-verifier landing proof, and marker-only -classification: any body without the exact v3 opening is unreadable and no -parser is attempted. Legacy workspace migration leaves old files unopened and -creates fresh records only for user-restated goals. +two review-record shapes, default issue and pull-request publication, explicit +merge confirmation, lifecycle-tool ownership, issue write preconditions, +archive-as-verifier landing proof, and marker-only classification: any body +without the exact v3 opening is unreadable and no parser is attempted. Legacy +workspace migration leaves old files unopened and creates fresh records only +for user-restated goals. When any part changes, synchronize the three skills, `plan-manager/references/plan-contract.md`, diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md index 20ea411a..9ab872d6 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-08-21" - content_hash: "44df74ed5d00700a9d68679062e6e57f406549fabe5f509ebc0d277a1db090f3" + content_hash: "0f0f3ca545c47bbd5314402248b3047d6a3e50e3604d4e6fd0da4d7862844d4b" --- # Plan Manager @@ -38,6 +38,9 @@ immediately before it runs; when `ask` is unavailable the step is set `blocked` and the first line of `## Open questions` becomes `Blocked: ` naming the unconfirmed effect. Only a blocked plan may open `## Open questions` with `Blocked:`. +Routine plan issue publication and the landing actions in `## Landing` carry +the settled mode's authorization; they are not Steps rows and never need this +confirmation. ## Six-phase flow @@ -54,14 +57,15 @@ with `Blocked:`. unresolved decision, a cross-subsystem or public-contract change, security-sensitive or destructive work, or any non-`local` effect. When `ask` is unavailable in a subagent, headless run, or `-p` run, take the direct - path only for a clear reversible local diff. Otherwise the issue-creation - preflight blocks canonical planning; do not silently substitute a tracked - file. State a direct-path assumption in the final report. A direct run - creates no plan issue. + path only for a clear reversible local diff. Otherwise canonical planning + continues unless an issue-publication safeguard needs an answer; do not + silently substitute a tracked file. State a direct-path assumption in the + final report. A direct run creates no plan issue. 2. **Draft.** Complete the preflight in - [`references/github-issue-publication.md`](references/github-issue-publication.md), - then run + [`references/github-issue-publication.md`](references/github-issue-publication.md). + The settled mode authorizes the issue write, so ask again only for an + ambiguous repository or a sensitive public disclosure. Then run `plan.mjs new --title --goal <goal> --mode <plan-and-implement|plan-only>`. Report the returned issue number. Use the edit flow below to write the full outcome and one Mode line in `## Goal`, the hypothesis in `## Research`, and @@ -125,15 +129,24 @@ with `Blocked:`. and write its real output into `## Verification Results` through the edit flow. -6. **Code review.** Build the review diff from what actually changed: `git status --porcelain` names the paths and the diff covers exactly those. Name every changed path that no Steps `Files` cell mentions in the review request, so the reviewer judges undeclared scope instead of the manager blocking on bookkeeping. - - Resolve the scratch directory with `git rev-parse --git-path docks-review` and - create it with mode `0700`. Write the review input to - `<that directory>/<issue>-<round>.diff`. Resolving through git keeps a linked - worktree working, where `.git` is a file and a literal `.git/` path does not - exist. The directory is untracked and discarded with the clone. Cover exactly the changed paths with - `git diff -- <those paths>` and `git diff --cached -- <those paths>`. Add one +6. **Code review.** Build the review diff from the complete candidate pull + request, not only the dirty worktree. Resolve and fetch the repository + default branch, then compute `<merge-base>` with + `git merge-base <default-remote-ref> HEAD`. Cover one net tracked candidate + with `git diff <merge-base> -- <changed paths>`. Add one `git diff --no-index /dev/null <path>` hunk for each untracked path. + `git status --porcelain` still names dirty paths. Name every changed path + that no Steps `Files` cell mentions in the review request. + + Resolve the scratch directory with `git rev-parse --git-path docks-review` + and create it with mode `0700`. Write the review input to + `<that directory>/<issue>-<round>.diff`. Resolving through git keeps linked + worktrees valid, where `.git` is a file and a literal `.git/` path does not + exist. The directory is untracked and discarded with the clone. + + After pull-request creation, record `headRefOid` and compare the changed paths + and hunks from `gh pr diff` with the reviewed net candidate. Any mismatch + invalidates the pass and blocks merge. Re-export the record before this dispatch too, exactly as in phase 4, so the reviewer reads current bytes rather than a stale export. @@ -143,17 +156,17 @@ with `Blocked:`. report to `## Review` under `### Code review round <n> — <UTC date>`. A round that returns `Code-review: pass` carries no unfixed `CRITICAL` or `HIGH` finding; it may - still carry advisory `MEDIUM` and `LOW` lines. Record each advisory line, - then fix it at your judgment; an advisory line never triggers a re-review. - A round that returns `Code-review: fixes-required` names at least one - evidenced `CRITICAL` or `HIGH` defect: fix every one of them, then dispatch - exactly one repair re-review. If that repair re-review again returns - `fixes-required`, stop: append `Code-review: blocked` naming the surviving - findings, and set the plan `blocked`. - - A `pass` round means the work is ready for - the user's landing actions. Run `plan.mjs archive <issue>` only after the - user has landed the closing pull request. + still carry advisory `MEDIUM` and `LOW` lines. Record each advisory as a + follow-up and do not change reviewed bytes after a pass; an advisory never + triggers a re-review. A round that returns `Code-review: fixes-required` + names at least one evidenced `CRITICAL` or `HIGH` defect: fix every one of + them, then dispatch exactly one repair re-review. If that repair re-review + again returns `fixes-required`, stop: append `Code-review: blocked` naming + the surviving findings, and set the plan `blocked`. + + A `pass` round ends review and starts landing. Follow `## Landing`. Run + `plan.mjs archive <issue>` only after an approved merge lands the closing + pull request. ## Plan contract @@ -210,12 +223,16 @@ and is refused. Every command exits 0 on success and 1 on a usage or validation failure. Failure messages keep their current wording wherever the check is unchanged. -## Issue creation +## Issue publication Every canonical plan crosses the GitHub issue boundary. Before `plan.mjs new`, -follow +run the preflight in [`references/github-issue-publication.md`](references/github-issue-publication.md). -When its confirmation cannot be obtained, report the blocker and create nothing. +The settled plan mode authorizes routine creation and update of the plan issue +in the repository that the preflight resolved. Do not ask again for that +publication or show a repository picker that repeats a resolved fact. Ask only +for an ambiguous repository or a sensitive public disclosure. When such an ask +cannot be obtained, report the blocker and create nothing. ## Reading and writing @@ -223,16 +240,38 @@ Render a plan body verbatim only when the user names that plan and asks to see i ## Landing -Work lands through a pull request whose body carries `Closes #<issue>` and whose base is the repository default branch, because GitHub interprets a closing keyword only in a pull request that targets the default branch. `plan.mjs archive` verifies that merged pull request rather than performing the merge. - -Only the pull request that lands the completed work carries `Closes #<issue>`. A partial pull request carries a plain `Refs #<issue>` instead, because GitHub closes the issue as soon as the first pull request carrying a closing keyword merges into the default branch. - One writer owns a plan issue at a time, recorded in the issue's own GitHub assignee field. `plan.mjs new` claims ownership at creation and `plan.mjs claim <issue>` claims an existing plan. Ownership is a precondition, not advice: every mutating command refuses a plan owned by another login, writes nothing when it refuses, and claims an unassigned plan in the same write. Read-only commands never check ownership. Taking a plan from another owner is a deliberate manual GitHub action; no lifecycle command transfers ownership. -Landing sits outside the six phases. The branch, commits, push, pull request, -and merge are the user's to run, on request, under `docks:commit-discipline`. -No Steps row exists for them; `archive` reads the result rather than causing it. -A plan that never lands is retired, not archived. +After `Code-review: pass`, the manager runs landing without another prompt: +ensure a non-default branch, commit exactly the reviewed bytes under +`docks:commit-discipline`, push normally, and create or update one pull request +that carries `Closes #<issue>` and targets the repository default branch. + +Never treat an empty first checks result as success. Retry +`gh pr checks --json name,bucket` at most 12 times with a 10-second delay until +checks appear. If required checks exist, run +`gh pr checks --watch --required`; if CI checks exist but none are required, +run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks +merge. If no checks appear, continue only when repository inspection confirms +that no pull-request CI is configured; otherwise leave the pull request open +with a named no-checks blocker and do not show the merge prompt. + +When the checks policy passes and GitHub reports the pull request mergeable, +ask immediately with exactly two options: `Merge now` or +`Leave pull request open`. Merge only on that fresh answer. If the user +declines, or `ask` is unavailable, leave the pull request and the issue open +and report the pull request URL. Never auto-merge, force-push, bypass branch +protection, or merge on a stale or assumed answer. + +Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA +or diff changed, block merge. Invoke `gh pr merge` with +`--match-head-commit <reviewed-head-sha>` and the repository's configured merge +strategy only after the fresh `Merge now` answer. + +Only the pull request that lands the completed work carries `Closes #<issue>`. +A partial pull request carries plain `Refs #<issue>`. `archive` reads the +merged result rather than causing it. A plan that never lands is retired, not +archived. ## Frozen history @@ -241,8 +280,11 @@ or migrate it through this lifecycle. It is not a plan source of truth. ## Git boundary -This lifecycle creates zero commits and never pushes. -Commit when the user asks, under `docks:commit-discipline`. +Routine landing Git work is in scope: branch, commit, push, and pull request +after `Code-review: pass`, under `docks:commit-discipline`. The merge itself +needs the fresh `Merge now` answer. Force-push, history rewrite, branch +deletion, and every other destructive Git action stay out of scope without an +explicit user request. ## BAD / GOOD @@ -253,6 +295,17 @@ GOOD: Confirm the hypothesis, cite the source, and choose the durable fix. BAD: Run implementation after a plan-only delivery without a new instruction. GOOD: Stop at the reviewed planned issue and report its number and verdict. -BAD: Treat repeated review output as permission to loop forever. -GOOD: Stop on identical findings with no changed files and record the block. +BAD: Treat a failed repair re-review as permission to dispatch another round. +GOOD: After one repair re-review still returns `fixes-required`, record the + block and stop. + +BAD: Ask a second time which repository receives the plan issue. +GOOD: Publish to the repository the preflight resolved and report it. + +BAD: Merge the pull request because the required checks turned green. +GOOD: Ask `Merge now` or `Leave pull request open`, then act on that answer. + +BAD: Stop at a passed review and hand the user an uncommitted tree. +GOOD: Commit, push, open the closing pull request, watch the required checks, + then stop at the merge question. ``` diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/github-issue-publication.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/github-issue-publication.md index 60c596fe..c9ac6ca7 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/github-issue-publication.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/github-issue-publication.md @@ -1,26 +1,77 @@ -# GitHub issue creation +# GitHub issue publication Every canonical plan begins as a GitHub issue. Creating it is a repository-visible write, so complete this preflight before `plan.mjs new`. -A failure creates no issue and writes nothing: +A failed preflight creates no issue and writes nothing: 1. `gh auth status` succeeds. 2. The checkout has a GitHub remote. 3. From that checkout, `gh repo view --json nameWithOwner,visibility,defaultBranchRef` resolves the exact repository, its visibility, and its default branch. -4. An in-session `ask` confirmation names that exact repository and confirms - creation of the plan issue there. -5. When the repository is public and the proposed plan names a vulnerability, a - credential location, or another sensitive finding, a second explicit - confirmation states that the issue body is public. - -When `ask` is unavailable in a subagent, headless run, or `-p` run, do not run -`plan.mjs new`. Report that issue creation is blocked because the exact -repository confirmation could not be obtained. Never substitute a tracked plan -file or create the issue speculatively. - -The pull request that lands the work must carry `Closes #<issue>` in its body -and target the repository default branch. Landing is not a lifecycle action or -a Steps row; the user runs it on request, and `plan.mjs archive` only verifies -the resulting merged pull request. + +## Authorization + +The settled plan mode authorizes routine creation and update of the plan issue +in the repository that the preflight resolved. Do not ask again for that +publication or show a repository picker that repeats a resolved fact. + +Two safeguards still require an in-session `ask` before the write: + +- **Ambiguity.** The preflight resolves no repository, resolves more than one + candidate GitHub remote, or resolves a repository that contradicts the user's + stated target. Ask which exact repository receives the issue. +- **Sensitive public disclosure.** The repository is public and the proposed + plan names a vulnerability, a credential location, or another sensitive + finding. Ask one explicit question stating that the issue body is public. + +When `ask` is unavailable in a subagent, headless run, or `-p` run and a +safeguard applies, do not run `plan.mjs new`. Report that issue publication is +blocked and name the safeguard that could not be answered. Never substitute a +tracked plan file and never publish a sensitive body speculatively. When no +safeguard applies, the settled mode is sufficient and publication proceeds. + +## Landing + +After `Code-review: pass`, the manager runs landing without another prompt: +ensure a non-default branch, commit exactly the reviewed bytes under +`docks:commit-discipline`, push normally, and create or update one pull request +that carries `Closes #<issue>` and targets the repository default branch. + +Never treat an empty first checks result as success. Retry +`gh pr checks --json name,bucket` at most 12 times with a 10-second delay until +checks appear. If required checks exist, run +`gh pr checks --watch --required`; if CI checks exist but none are required, +run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks +merge. If no checks appear, continue only when repository inspection confirms +that no pull-request CI is configured; otherwise leave the pull request open +with a named no-checks blocker and do not show the merge prompt. + +When the checks policy passes and GitHub reports the pull request mergeable, +ask immediately with exactly two options: `Merge now` or +`Leave pull request open`. Merge only on that fresh answer. If the user +declines, or `ask` is unavailable, leave the pull request and the issue open +and report the pull request URL. Never auto-merge, force-push, bypass branch +protection, or merge on a stale or assumed answer. + +Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA +or diff changed, block merge. Invoke `gh pr merge` with +`--match-head-commit <reviewed-head-sha>` and the repository's configured merge +strategy only after the fresh `Merge now` answer. + +`plan.mjs archive <issue>` stays a post-merge verifier: it verifies the merged +closing pull request and never performs the merge. + +```text +BAD: Ask again which repository receives the issue after the preflight + resolved exactly one. +GOOD: Publish to the resolved repository and report its name with the issue + number. + +BAD: Merge the pull request because the required checks turned green. +GOOD: Ask `Merge now` or `Leave pull request open`, then act on that answer. + +BAD: Leave the work uncommitted and tell the user to push it. +GOOD: Commit, push, and open the closing pull request, then stop at the merge + question. +``` diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md index 89d0a2e7..f9e392e1 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md @@ -170,9 +170,10 @@ Code-review: fixes-required `Plan-review:` is exactly `pass`, `repair`, or `blocked`. `Code-review:` is exactly `pass`, `fixes-required`, or `blocked`. A code-review `pass` means no `CRITICAL` or `HIGH` finding stands unfixed; it carries only advisory `MEDIUM` -and `LOW` lines, or none, and the manager fixes those at its judgment without a -re-review. `fixes-required` names at least one evidenced `CRITICAL` or `HIGH` -defect and forces exactly one repair re-review; if that re-review still returns +and `LOW` lines, or none. Record each advisory as a follow-up and do not change +reviewed bytes after a pass; an advisory never triggers a re-review. +`fixes-required` names at least one evidenced `CRITICAL` or `HIGH` defect and +forces exactly one repair re-review; if that re-review still returns `fixes-required`, the manager appends `Code-review: blocked` and sets the plan `blocked`. A `blocked` verdict has at least one finding line. @@ -253,13 +254,37 @@ The header strip is `#<issue> · <status> · <title> · <url>`. ## Landing Work lands through a pull request whose body carries `Closes #<issue>` and whose -base is the target repository's default branch. `plan.mjs archive` verifies that -merged pull request rather than performing the merge or closing the issue. +base is the target repository's default branch. + +After `Code-review: pass`, the manager runs landing without another prompt: +ensure a non-default branch, commit exactly the reviewed bytes under +`docks:commit-discipline`, push normally, and create or update one pull request +that carries `Closes #<issue>` and targets the repository default branch. + +Never treat an empty first checks result as success. Retry +`gh pr checks --json name,bucket` at most 12 times with a 10-second delay until +checks appear. If required checks exist, run +`gh pr checks --watch --required`; if CI checks exist but none are required, +run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks +merge. If no checks appear, continue only when repository inspection confirms +that no pull-request CI is configured; otherwise leave the pull request open +with a named no-checks blocker and do not show the merge prompt. + +When the checks policy passes and GitHub reports the pull request mergeable, +ask immediately with exactly two options: `Merge now` or +`Leave pull request open`. Merge only on that fresh answer. If the user +declines, or `ask` is unavailable, leave the pull request and the issue open +and report the pull request URL. Never auto-merge, force-push, bypass branch +protection, or merge on a stale or assumed answer. + +Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA +or diff changed, block merge. Invoke `gh pr merge` with +`--match-head-commit <reviewed-head-sha>` and the repository's configured merge +strategy only after the fresh `Merge now` answer. Only the pull request that lands the completed work carries `Closes #<issue>`. -A partial pull request carries a plain `Refs #<issue>` instead. Landing sits -outside the six phases: branch, commits, push, pull request, and merge are the -user's to run, on request, under `docks:commit-discipline`. +A partial pull request carries plain `Refs #<issue>`. `archive` verifies the +merged result rather than causing it. One writer owns a plan issue at a time, recorded in the issue's GitHub assignee field. `plan.mjs new` claims ownership at creation and `plan.mjs claim <issue>` @@ -320,9 +345,9 @@ does not certify permission. There is deliberately no self-certifying ## What the lifecycle never does The record carries no hash, no permit, no run identity, no lock file, no sealed -review bundle, no automatic commit, no automatic push, no external-authority -object, and no tracked plan file. The lifecycle does not weaken the -agent-enforced boundary above. +review bundle, no external-authority object, and no tracked plan file. The +lifecycle never auto-merges, force-pushes, or bypasses branch protection. It +does not weaken the agent-enforced boundary above. One digest exists, and it grants nothing. `export` writes the sha256 of the body it copied beside the copy, and `edit` refuses a file derived from a superseded diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md index ebd958ce..04c0ac9b 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-08-21" - content_hash: "59cab3fc49f767eb557270c6187fd5b1fdd555ffb8ddc73d4a1f5e2cdeefa84e" + content_hash: "44018dca60d44a64ebc6b3473caaae9f9d766c9aead7d1f4ee4eddd1430cbf4d" --- # Plans Workspace @@ -228,20 +228,20 @@ 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. This lifecycle creates zero commits and never pushes. Commit when the -user asks, under `docks:commit-discipline`. +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. Every Steps row carries an `Effect` of exactly `local|probe|production_access|publish|push|release|deploy`. A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` and the plan reason becomes the first `## Open questions` line, `Blocked: <reason>`. -Persisted effects record intent only. - -Work lands through a pull request whose body carries `Closes #<issue>` and whose -base is the repository default branch. `plan.mjs archive` is a verifier: it -requires completed closure, terminal steps, an exact `Code-review: pass` line, -and a merged closing pull request into that branch; it writes no status. +Persisted effects record intent only. Routine issue publication and landing +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. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/codex-agent-templates.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/codex-agent-templates.md index c803f1ff..5fecc8bc 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/codex-agent-templates.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/codex-agent-templates.md @@ -72,6 +72,17 @@ the other. Use `Bug`, `Security`, `Performance`, or `Maintainability` for Standards findings. Use `Spec` for a plan mismatch. +Select one verdict: + +- `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` + and `LOW` lines may ride along on a `pass`: the manager records them as + follow-ups 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. +- `blocked`: Required review input is unreadable or contradictory, so no safe + verdict can be reached. + 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 diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md index 0c1a33f2..34d09a78 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/references/plan-md-template.md @@ -296,9 +296,11 @@ Code-review: pass|fixes-required|blocked ``` A code-review `pass` means no `CRITICAL` or `HIGH` finding stands unfixed; it -carries only advisory `MEDIUM` and `LOW` lines, or none, and the manager fixes -those at its judgment without a re-review. `fixes-required` names at least one -evidenced `CRITICAL` or `HIGH` defect and forces exactly one repair re-review; +carries only advisory `MEDIUM` and `LOW` lines, or none. Record each advisory as +a follow-up and do not change reviewed bytes after a pass; an advisory never +triggers a re-review. +`fixes-required` names at least one evidenced `CRITICAL` or `HIGH` defect and +forces exactly one repair re-review; if that re-review still returns `fixes-required`, the manager appends `Code-review: blocked` and sets the plan `blocked`. A `blocked` verdict has at least one finding line. A plan-review finding is exactly one of `goal_fit`, @@ -314,10 +316,17 @@ passes. 5. **Implement.** Set the plan `ongoing`, move each step through its legal states, and record real Acceptance output in `## Verification Results` before the closing merge. 6. **Code review.** Review the declared change, fix every critical and high finding, and run exactly one repair re-review after such a fix; if that re-review still returns fixes-required, append `Code-review: blocked` and set the plan `blocked`. Every step must be terminal and code review must pass before the closing merge; archive verifies those facts afterward. -Build the review diff from what actually changed: `git status --porcelain` names -the paths and the diff covers exactly those. Name every changed path that no -Steps `Files` cell mentions in the review request, so the reviewer judges -undeclared scope instead of the manager blocking on bookkeeping. +Build the review diff from the complete candidate pull request, not only the +dirty worktree. Resolve and fetch the repository default branch, then compute +`<merge-base>` with `git merge-base <default-remote-ref> HEAD`. Cover one net +tracked candidate with `git diff <merge-base> -- <changed paths>`. Add one +`git diff --no-index /dev/null <path>` hunk for each untracked path. +`git status --porcelain` still names dirty paths. Name every changed path that +no Steps `Files` cell mentions in the review request. + +After pull-request creation, record `headRefOid` and compare the changed paths +and hunks from `gh pr diff` with the reviewed net candidate. Any mismatch +invalidates the pass and blocks merge. If that repair re-review again returns `fixes-required`, stop: append `Code-review: blocked` naming the surviving findings, and set the plan @@ -327,22 +336,44 @@ A step whose `Effect` is not `local` requires an in-session `ask` confirmation immediately before it runs; when `ask` is unavailable the step is set `blocked` and `Blocked: <unconfirmed effect>` is recorded first in `## Open questions`. -This lifecycle creates zero commits and never pushes. +Routine plan issue publication is authorized by the settled mode and needs no +repeated repository picker. ## Landing -Work lands through a pull request whose body carries `Closes #<issue>` and whose base is the repository default branch, because GitHub interprets a closing keyword only in a pull request that targets the default branch. `plan.mjs archive` verifies that merged pull request rather than performing the merge. - -Only the pull request that lands the completed work carries `Closes #<issue>`. A -partial pull request carries a plain `Refs #<issue>` instead, because GitHub -closes the issue as soon as the first pull request carrying a closing keyword -merges into the default branch. - -Landing sits outside the six phases. This lifecycle creates zero commits and -never pushes, so the branch, the commits, the push, the pull request, and the -merge are the user's to run, on request, under `docks:commit-discipline`. No -Steps row exists for them, and `archive` reads the result rather than causing -it. A plan that never lands is retired, not archived. +Work lands through a pull request whose body carries `Closes #<issue>` and whose +base is the repository default branch. + +After `Code-review: pass`, the manager runs landing without another prompt: +ensure a non-default branch, commit exactly the reviewed bytes under +`docks:commit-discipline`, push normally, and create or update one pull request +that carries `Closes #<issue>` and targets the repository default branch. + +Never treat an empty first checks result as success. Retry +`gh pr checks --json name,bucket` at most 12 times with a 10-second delay until +checks appear. If required checks exist, run +`gh pr checks --watch --required`; if CI checks exist but none are required, +run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks +merge. If no checks appear, continue only when repository inspection confirms +that no pull-request CI is configured; otherwise leave the pull request open +with a named no-checks blocker and do not show the merge prompt. + +When the checks policy passes and GitHub reports the pull request mergeable, +ask immediately with exactly two options: `Merge now` or +`Leave pull request open`. Merge only on that fresh answer. If the user +declines, or `ask` is unavailable, leave the pull request and the issue open +and report the pull request URL. Never auto-merge, force-push, bypass branch +protection, or merge on a stale or assumed answer. + +Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA +or diff changed, block merge. Invoke `gh pr merge` with +`--match-head-commit <reviewed-head-sha>` and the repository's configured merge +strategy only after the fresh `Merge now` answer. + +Only the pull request that lands the completed work carries `Closes #<issue>`. +A partial pull request carries plain `Refs #<issue>`. `archive` verifies the +merged result rather than causing it. A plan that never lands is retired, not +archived. ## Portability diff --git a/scripts/tests/plan-skill-phases.mjs b/scripts/tests/plan-skill-phases.mjs index d3852ea7..e10f2bac 100755 --- a/scripts/tests/plan-skill-phases.mjs +++ b/scripts/tests/plan-skill-phases.mjs @@ -18,6 +18,15 @@ const PLAN_CONTRACT = 'plugins/plan-lifecycle/skills/productivity/plan-manager/r const PLAN_MD = 'docs/PLAN.md'; const CODE_REVIEWER_AGENT = 'plugins/plan-lifecycle/agents/code-reviewer.md'; const CODE_REVIEWER_CODEX = '.codex/agents/code-reviewer.toml'; +const CODE_REVIEWER_TEMPLATE = + 'plugins/plan-lifecycle/skills/productivity/plan-workspace/references/codex-agent-templates.md'; +const ISSUE_PUBLICATION = + 'plugins/plan-lifecycle/skills/productivity/plan-manager/references/github-issue-publication.md'; +const ROOT_AGENTS = 'AGENTS.md'; +const README_MD = 'README.md'; +const EXECUTOR_DISPATCH = 'plugins/docks/skills/engineering/refactor/references/executor-dispatch.md'; +const LIFECYCLE_SKILLS_AGENTS = 'plugins/plan-lifecycle/skills/AGENTS.md'; +const DOCKS_README = 'plugins/docks/README.md'; const V3_PINNED_CLAUSES = [ { @@ -60,9 +69,9 @@ const V3_PINNED_CLAUSES = [ files: [MANAGER_SKILL], }, { - name: 'zero-commits', - text: 'This lifecycle creates zero commits and never pushes.', - files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], + name: 'routine-issue-publication', + text: 'The settled plan mode authorizes routine creation and update of the plan issue in the repository that the preflight resolved. Do not ask again for that publication or show a repository picker that repeats a resolved fact.', + files: [MANAGER_SKILL, ISSUE_PUBLICATION], }, { name: 'code-review-repair-bound', @@ -71,17 +80,27 @@ const V3_PINNED_CLAUSES = [ }, { name: 'code-review-pass-verdict', - text: '- `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` and `LOW` lines may ride along on a `pass`: the manager records them and fixes them at its judgment, and they never trigger a re-review.', - files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX], + text: '- `pass`: No `CRITICAL` or `HIGH` finding stands unfixed. Advisory `MEDIUM` and `LOW` lines may ride along on a `pass`: the manager records them as follow-ups and does not change reviewed bytes after the pass; they never trigger a re-review.', + files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX, CODE_REVIEWER_TEMPLATE], }, { name: 'code-review-fixes-required-verdict', text: '- `fixes-required`: At least one evidenced `CRITICAL` or `HIGH` defect. The manager fixes it and dispatches exactly one repair re-review.', - files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX], + files: [CODE_REVIEWER_AGENT, CODE_REVIEWER_CODEX, CODE_REVIEWER_TEMPLATE], + }, + { + name: 'advisory-pass-immutable', + text: 'Record each advisory as a follow-up and do not change reviewed bytes after a pass; an advisory never triggers a re-review.', + files: [MANAGER_SKILL, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], }, { name: 'review-scope-guard', - text: 'Build the review diff from what actually changed: `git status --porcelain` names the paths and the diff covers exactly those. Name every changed path that no Steps `Files` cell mentions in the review request, so the reviewer judges undeclared scope instead of the manager blocking on bookkeeping.', + text: 'Build the review diff from the complete candidate pull request, not only the dirty worktree. Resolve and fetch the repository default branch, then compute `<merge-base>` with `git merge-base <default-remote-ref> HEAD`. Cover one net tracked candidate with `git diff <merge-base> -- <changed paths>`. Add one `git diff --no-index /dev/null <path>` hunk for each untracked path. `git status --porcelain` still names dirty paths. Name every changed path that no Steps `Files` cell mentions in the review request.', + files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], + }, + { + name: 'reviewed-pr-diff-match', + text: 'After pull-request creation, record `headRefOid` and compare the changed paths and hunks from `gh pr diff` with the reviewed net candidate. Any mismatch invalidates the pass and blocks merge.', files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], }, { @@ -115,9 +134,49 @@ const V3_PINNED_CLAUSES = [ files: [MANAGER_SKILL, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], }, { - name: 'landing-linkage', - text: 'Work lands through a pull request whose body carries `Closes #<issue>` and whose base is the repository default branch, because GitHub interprets a closing keyword only in a pull request that targets the default branch. `plan.mjs archive` verifies that merged pull request rather than performing the merge.', - files: [MANAGER_SKILL, PLAN_MD, WORKSPACE_TEMPLATE], + name: 'default-pr-landing', + text: 'After `Code-review: pass`, the manager runs landing without another prompt: ensure a non-default branch, commit exactly the reviewed bytes under `docks:commit-discipline`, push normally, and create or update one pull request that carries `Closes #<issue>` and targets the repository default branch.', + files: [MANAGER_SKILL, ISSUE_PUBLICATION, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], + }, + { + name: 'ci-check-discovery', + text: 'Never treat an empty first checks result as success. Retry `gh pr checks --json name,bucket` at most 12 times with a 10-second delay until checks appear. If required checks exist, run `gh pr checks --watch --required`; if CI checks exist but none are required, run `gh pr checks --watch` to wait for all reported CI. Any failed check blocks merge. If no checks appear, continue only when repository inspection confirms that no pull-request CI is configured; otherwise leave the pull request open with a named no-checks blocker and do not show the merge prompt.', + files: [MANAGER_SKILL, ISSUE_PUBLICATION, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], + }, + { + name: 'merge-approval-boundary', + text: 'When the checks policy passes and GitHub reports the pull request mergeable, ask immediately with exactly two options: `Merge now` or `Leave pull request open`. Merge only on that fresh answer. If the user declines, or `ask` is unavailable, leave the pull request and the issue open and report the pull request URL. Never auto-merge, force-push, bypass branch protection, or merge on a stale or assumed answer.', + files: [MANAGER_SKILL, ISSUE_PUBLICATION, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], + }, + { + name: 'merge-head-revalidation', + text: "Immediately before merge, re-read `headRefOid` and `gh pr diff`. If the head SHA or diff changed, block merge. Invoke `gh pr merge` with `--match-head-commit <reviewed-head-sha>` and the repository's configured merge strategy only after the fresh `Merge now` answer.", + files: [MANAGER_SKILL, ISSUE_PUBLICATION, PLAN_CONTRACT, PLAN_MD, WORKSPACE_TEMPLATE], + }, + { + name: 'repository-landing-summary', + text: "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.", + files: [WORKSPACE_SKILL, ROOT_AGENTS], + }, + { + name: 'readme-landing-summary', + text: '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.', + files: [README_MD], + }, + { + name: 'shipped-readme-landing-summary', + text: 'After review passes, the manager commits and pushes the reviewed branch, opens the closing pull request, waits for repository CI, and 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.', + files: [DOCKS_README], + }, + { + name: 'lifecycle-authoring-landing-boundary', + text: 'default issue and pull-request publication, explicit merge confirmation', + files: [LIFECYCLE_SKILLS_AGENTS], + }, + { + name: 'executor-landing-handoff', + text: "follows the manager's full Landing flow. It archives only after an approved merge lands the closing pull request.", + files: [EXECUTOR_DISPATCH], }, { name: 'label-set', @@ -348,9 +407,29 @@ function assertBoundedWorkflows() { previousIndex = index; } - const zeroCommits = V3_PINNED_CLAUSES.find(({ name }) => name === 'zero-commits').text; - assert.ok(normalizeContract(manager).includes(zeroCommits), `${MANAGER_SKILL} is missing zero-commits`); - assert.ok(manager.includes('references/plan-contract.md'), `${MANAGER_SKILL} must link the v3 contract`); + const issuePublication = V3_PINNED_CLAUSES.find(({ name }) => name === 'routine-issue-publication').text; + assert.ok( + normalizeContract(manager).includes(issuePublication), + `${MANAGER_SKILL} is missing routine-issue-publication`, + ); + for (const file of [ + MANAGER_SKILL, + PLAN_CONTRACT, + WORKSPACE_SKILL, + WORKSPACE_TEMPLATE, + PLAN_MD, + ROOT_AGENTS, + README_MD, + EXECUTOR_DISPATCH, + DOCKS_README, + ]) { + const normalized = normalizeContract(read(file)); + assert.doesNotMatch( + normalized, + /This lifecycle creates zero commits and never pushes\.|creates zero automatic commits and never pushes|branch, commits, push, pull request, and merge are the user's|no automatic commit, no automatic push/i, + `${file} retains the retired user-owned landing boundary`, + ); + } // Ban only machinery identifiers. `permit` and `reserved` are ordinary words the // body needs in order to say the budget model is gone ("It has no hashes or // permits."), so banning them would make this lock fight the documentation.