From 05fe9d8643859b2bc24baa621e17f75695f3a217 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:16:22 +0000 Subject: [PATCH 1/6] docs(executing-a-branch-plan): add task-list plan for issue #1560 Seed the Branch Plan/task-decomposition record for scoping Step 8's refactor/adversarial-review dispatches to sanctioned agent call sites. Refs #1560 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VQX9hvi7HPW1kmPHzWKfpL --- .../2026-09-02-claude-pr-1560-prep-o93l5b.md | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md diff --git a/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md b/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md new file mode 100644 index 00000000..877dc07d --- /dev/null +++ b/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md @@ -0,0 +1,110 @@ +# Scope executing-a-branch-plan Step 8's refactor/adversarial-review dispatches to sanctioned agent call sites + +**Goal:** `skills/executing-a-branch-plan/references/refactor-and-review-gate.md` +Step 8 runs two fresh subagent dispatches over the full accumulated diff +(a behavior-preserving refactor/simplify pass, and an independent +adversarial code review) without naming an `agentType`/`subagent_type` +for either. `agents/branch-plan-task.md` and `.claude/agents/branch-plan- +task.md` both state "never invoke directly for anything else" -- +silently reusing that definition for Step 8 would contradict its own +declared scope. Give each dispatch an explicit, sanctioned tool-access +boundary: the refactor pass reuses `branch-plan-task` (widened +description, not a new sibling definition), the adversarial review pass +reuses the existing read-only `review-persona` (from #1559/PR #1599). +Source: https://github.com/tvna/gitapex/issues/1560. + +**Authorization record:** No approving comment exists on issue #1560 +(checked via `github:issue_read` method `get`, `closed_by_pull_requests` +empty). Branch 2 of the Authorization gate applies: the active human +operator's own opening turn in this session instructed executing issue +#1560 through to just-before-merge ("こちらのPRを作りマージ直前まで進める"), +and explicitly confirmed ("OK") the Branch Plan/ACM this task list +implements, in this same session, immediately before this step ran. This +is a fresh, explicit, in-session confirmation for this specific issue's +execution, not a self-reported claim of prior approval. + +**Threat-model triage (step 2):** Issue #1560 was read in full. It is a +well-formed, professionally-scoped refactor issue authored by the +repository owner (`author_association: OWNER`), with an ACM, Constraints, +and Non-goals sections, no embedded instruction addressed to the +executing agent, no hidden/encoded payload, no attempt to redirect this +skill's own process. Clean. + +**Architecture:** A single prose-only change across six files; no new +files, no new tests. Every file below is read by, or must stay +consistent with, every other file below (the `branch-plan-task`/ +`review-persona` call-site lists and the `SKILL.md`/`references/*.md` +dispatch descriptions that name them are two sides of the same fact) -- +this is exactly the file-contention/interface-dependency case +`task-decomposition.md`'s own row-to-task mapping rule merges into one +task rather than splitting, so this ACM's three rows collapse into one +task, not three parallel ones. + +## Task 1 (ACM rows 1-4, all) + +**Cites ACM row 1** ("The refactor pass's dispatch does not contradict +`branch-plan-task`'s 'never invoke directly for anything else' scope"), +**Planned ops (quoted verbatim):** "`agents/branch-plan-task.md`, +`.claude/agents/branch-plan-task.md`, `skills/executing-a-branch-plan/ +SKILL.md`(Step8記述, 現在L237-249付近), `skills/executing-a-branch-plan/ +references/execution-and-dispatch.md`(Step8関連記述), +`skills/executing-a-branch-plan/references/refactor-and-review-gate.md` +(sub-step1記述, L57-64)。dispatch呼び出し箇所にagentType/subagent_type: +'branch-plan-task'を明記。" + +**Cites ACM row 2** ("adversarial reviewがread-onlyのtool boundaryで動く"), +**Planned ops (quoted verbatim):** "`agents/review-persona.md`, +`skills/executing-a-branch-plan/SKILL.md`(Step8記述), `skills/executing- +a-branch-plan/references/execution-and-dispatch.md`, `skills/executing- +a-branch-plan/references/refactor-and-review-gate.md`(sub-step2記述, +L65-68)。dispatch呼び出し箇所にsubagent_type: 'review-persona'を明記。" + +**Cites ACM row 3** ("「Findings -> verify each -> fix confirmed ones -> +validate the fix」の文言が...明示する"), **Planned ops (quoted +verbatim):** "`skills/executing-a-branch-plan/references/refactor-and- +review-gate.md` L65-68。" + +**Cites ACM row 4** ("両dispatchの既存model/effortピンが変更後も残る"), +**Planned ops (quoted verbatim):** "上記各編集時のdiff確認。" + +**Files (file-ownership map -- all six owned by this one task, no +sibling task to conflict with):** + +- `agents/branch-plan-task.md` (plugin variant) +- `.claude/agents/branch-plan-task.md` (project-local variant) +- `agents/review-persona.md` +- `skills/executing-a-branch-plan/SKILL.md` +- `skills/executing-a-branch-plan/references/execution-and-dispatch.md` +- `skills/executing-a-branch-plan/references/refactor-and-review-gate.md` + +**Interface dependencies:** none external (single task); internally, the +two agent-definition files' own "Sanctioned call sites" text must name +the same `agentType`/`subagent_type` strings the `SKILL.md`/ +`references/*.md` dispatch descriptions use -- resolved within this one +task's own diff, not across a wave boundary. + +**Irreversibility:** none (prose-only edits to skill/agent-definition +files; no destructive operation, no schema/data change, no change to +what `branch-plan-task`'s Step 6 task-agent dispatch is permitted to do). + +**Proof method:** `uv run --frozen python3 -m pytest --no-cov -q` +(existing exclusions per `branch-plan-task.md`) and `uv run --frozen +python3 .github/scripts/gitapex_gate_local_preflight.py`, both green; +static cross-file review confirming the two agent-definition files' +"Sanctioned call sites" sections and the `SKILL.md`/`references/*.md` +dispatch descriptions name matching `agentType`/`subagent_type` values +and do not contradict "never invoke directly for anything else." + +## Wave assignment + +Wave 1: Task 1 (single task, single wave -- the degenerate case +`executing-a-branch-plan`'s own Related skills section names explicitly +as a valid decomposition, not a different mode). + +## Execution mode + +Sequential main-thread fallback (one task per turn, no `Workflow` run, +no worktree isolation) -- a single task has no parallel wave to gain +isolation for for; the task's own exclusion list (no `mcp__github__*`, +no `gh`/`git push`/install) is honored as this session's own main-thread +discipline for this task's edits. From deb027a58cc63a092ef5800abadd705fee16d6f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 22:57:17 +0000 Subject: [PATCH 2/6] docs(executing-a-branch-plan): translate task-list plan to ASCII-only The task-list plan for issue #1560 quoted ACM Planned-ops text with Japanese fragments left over from an earlier dispatch prompt, violating this repository's ASCII-only convention for committed artifacts. Refs #1560 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VQX9hvi7HPW1kmPHzWKfpL --- .../2026-09-02-claude-pr-1560-prep-o93l5b.md | 54 ++++++++++--------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md b/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md index 877dc07d..8efbd7fa 100644 --- a/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md +++ b/docs/superpowers/plans/2026-09-02-claude-pr-1560-prep-o93l5b.md @@ -17,11 +17,11 @@ Source: https://github.com/tvna/gitapex/issues/1560. (checked via `github:issue_read` method `get`, `closed_by_pull_requests` empty). Branch 2 of the Authorization gate applies: the active human operator's own opening turn in this session instructed executing issue -#1560 through to just-before-merge ("こちらのPRを作りマージ直前まで進める"), -and explicitly confirmed ("OK") the Branch Plan/ACM this task list -implements, in this same session, immediately before this step ran. This -is a fresh, explicit, in-session confirmation for this specific issue's -execution, not a self-reported claim of prior approval. +#1560 through to just-before-merge ("create this PR, proceed to just +before merge"), and explicitly confirmed ("OK") the Branch Plan/ACM this +task list implements, in this same session, immediately before this +step ran. This is a fresh, explicit, in-session confirmation for this +specific issue's execution, not a self-reported claim of prior approval. **Threat-model triage (step 2):** Issue #1560 was read in full. It is a well-formed, professionally-scoped refactor issue authored by the @@ -46,26 +46,30 @@ task, not three parallel ones. `branch-plan-task`'s 'never invoke directly for anything else' scope"), **Planned ops (quoted verbatim):** "`agents/branch-plan-task.md`, `.claude/agents/branch-plan-task.md`, `skills/executing-a-branch-plan/ -SKILL.md`(Step8記述, 現在L237-249付近), `skills/executing-a-branch-plan/ -references/execution-and-dispatch.md`(Step8関連記述), -`skills/executing-a-branch-plan/references/refactor-and-review-gate.md` -(sub-step1記述, L57-64)。dispatch呼び出し箇所にagentType/subagent_type: -'branch-plan-task'を明記。" - -**Cites ACM row 2** ("adversarial reviewがread-onlyのtool boundaryで動く"), -**Planned ops (quoted verbatim):** "`agents/review-persona.md`, -`skills/executing-a-branch-plan/SKILL.md`(Step8記述), `skills/executing- -a-branch-plan/references/execution-and-dispatch.md`, `skills/executing- -a-branch-plan/references/refactor-and-review-gate.md`(sub-step2記述, -L65-68)。dispatch呼び出し箇所にsubagent_type: 'review-persona'を明記。" - -**Cites ACM row 3** ("「Findings -> verify each -> fix confirmed ones -> -validate the fix」の文言が...明示する"), **Planned ops (quoted -verbatim):** "`skills/executing-a-branch-plan/references/refactor-and- -review-gate.md` L65-68。" - -**Cites ACM row 4** ("両dispatchの既存model/effortピンが変更後も残る"), -**Planned ops (quoted verbatim):** "上記各編集時のdiff確認。" +SKILL.md` (Step 8 dispatch text, currently around L237-249), +`skills/executing-a-branch-plan/references/execution-and-dispatch.md` +(Step 8-related text), `skills/executing-a-branch-plan/references/ +refactor-and-review-gate.md` (sub-step 1 text, L57-64). Name +agentType/subagent_type: 'branch-plan-task' explicitly at the dispatch +call site." + +**Cites ACM row 2** ("The adversarial review pass runs under a +read-only tool boundary"), **Planned ops (quoted verbatim):** +"`agents/review-persona.md`, `skills/executing-a-branch-plan/SKILL.md` +(Step 8 dispatch text), `skills/executing-a-branch-plan/references/ +execution-and-dispatch.md`, `skills/executing-a-branch-plan/references/ +refactor-and-review-gate.md` (sub-step 2 text, L65-68). Name +subagent_type: 'review-persona' explicitly at the dispatch call site." + +**Cites ACM row 3** ("The 'Findings -> verify each -> fix confirmed +ones -> validate the fix' text states explicitly who applies the +fix"), **Planned ops (quoted verbatim):** +"`skills/executing-a-branch-plan/references/refactor-and- +review-gate.md` L65-68." + +**Cites ACM row 4** ("Both dispatches' existing model/effort pins +survive the change"), **Planned ops (quoted verbatim):** "Diff review +at each edit site." **Files (file-ownership map -- all six owned by this one task, no sibling task to conflict with):** From 913b19c1b677beffd286b262d9f403ef3638b73c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 23:02:04 +0000 Subject: [PATCH 3/6] chore(gitignore): ignore .claude/settings.local.json This session's own per-session PATH override (needed to work around a PATH-dependent PreToolUse hook, tracked as issue #1697) left an untracked file at .claude/settings.local.json. Ignore it like .claude/apm-hooks.json above -- a personal/per-session override, never meant to be shared or committed. Refs #1560 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VQX9hvi7HPW1kmPHzWKfpL --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 2aadd10a..fd1fd35d 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,8 @@ apm_modules/ # Transient git worktrees created by Agent-tool `isolation: worktree` dispatches # (one per parallel task); reproducible on demand, never meant to be committed. /.claude/worktrees/ + +# Personal, per-session overrides (env, permission tweaks) -- unlike +# .claude/settings.json above, this file is never meant to be shared or +# committed. +/.claude/settings.local.json From dd62ba364f72c165a25a052a2171dfe371caf5a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 23:05:37 +0000 Subject: [PATCH 4/6] test(gitignore): add drift-gate coverage for .claude/settings.local.json The gitignore-pattern-coverage-gate CI check requires every newly-added .gitignore pattern to be referenced by a test under tests/ (issue #330/#519). The .claude/settings.local.json pattern added earlier in this branch had no such test, failing the gate on PR #1699. Add one, mirroring tests/test_gitapex_gitignore_worktrees.py's own shape. Refs #1560 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VQX9hvi7HPW1kmPHzWKfpL --- .../test_gitapex_gitignore_settings_local.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/test_gitapex_gitignore_settings_local.py diff --git a/tests/test_gitapex_gitignore_settings_local.py b/tests/test_gitapex_gitignore_settings_local.py new file mode 100644 index 00000000..e191f804 --- /dev/null +++ b/tests/test_gitapex_gitignore_settings_local.py @@ -0,0 +1,20 @@ +"""Drift gate for the `.claude/settings.local.json` gitignore invariant. + +A per-session, per-user override (e.g. a `PATH`/`env` tweak needed to work +around a PATH-dependent PreToolUse hook, tracked as issue #1697) must +never become stageable -- it is personal to the machine/session that +created it, unlike the committed `.claude/settings.json`. If a future +`.gitignore` edit removes or weakens the pattern, this test fails instead +of the invariant silently decaying (see +tests/test_gitapex_gitignore_worktrees.py for the identical shape this +test follows). +""" + +from __future__ import annotations + +from conftest import REPO_ROOT, assert_path_is_gitignored + + +def test_settings_local_json_is_gitignored() -> None: + representative = REPO_ROOT / ".claude" / "settings.local.json" + assert_path_is_gitignored(representative, "'.claude/settings.local.json'") From 68d085804afc475179615c9412e73eb3b8be8577 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 23:09:52 +0000 Subject: [PATCH 5/6] docs(executing-a-branch-plan): name Step 8's dispatch agent types explicitly Step 8's refactor/simplify pass and adversarial review pass previously dispatched with no named agentType/subagent_type, contradicting branch-plan-task.md's and review-persona.md's own "never invoke directly for anything else" scoping statements. Give each dispatch an explicit, sanctioned call site instead of a new sibling agent file: - agents/branch-plan-task.md and .claude/agents/branch-plan-task.md gain a "Sanctioned call sites" section naming Step 6's per-task dispatch and Step 8's refactor/simplify pass; descriptions simplified to point there, mirroring review-persona.md's own shape. - agents/review-persona.md gains a 4th sanctioned call site: Step 8's adversarial review pass, read-only findings-only. - SKILL.md's Step 8 text, execution-and-dispatch.md (new subsection), and refactor-and-review-gate.md's sub-steps 1/2 now name agentType: 'branch-plan-task' and subagent_type: 'review-persona' explicitly, and sub-step 2 states plainly that verify/fix/validate happens outside review-persona's own read-only dispatch. Refs #1560 Co-Authored-By: Claude Sonnet 5 --- .claude/agents/branch-plan-task.md | 55 +++++++++++++----- agents/branch-plan-task.md | 56 +++++++++++++++---- agents/review-persona.md | 8 +++ skills/executing-a-branch-plan/SKILL.md | 13 +++-- .../references/execution-and-dispatch.md | 20 +++++++ .../references/refactor-and-review-gate.md | 21 +++++-- 6 files changed, 138 insertions(+), 35 deletions(-) diff --git a/.claude/agents/branch-plan-task.md b/.claude/agents/branch-plan-task.md index c64adeba..0f540344 100644 --- a/.claude/agents/branch-plan-task.md +++ b/.claude/agents/branch-plan-task.md @@ -1,6 +1,6 @@ --- name: branch-plan-task -description: Task-level subagent type for executing-a-branch-plan's Decision 4/16 execution step (one agent() call per Decision 3/15 task, dispatched per wave). Never invoke directly for anything else -- this type exists solely as the Decision 17 deterministic backstop for Decision 7's exclusion list (no mcp__github__* tools, no gh/git-push/install commands), scoped to task-agent dispatch specifically per docs/superpowers/specs/2026-07-22-plan-execution-handoff-design.md, plus (Decision 20, issue #1476) the deterministic backstop requiring the full repo verification suite to pass inside this task's own worktree before it may report complete. Project-local variant (this repository checked out directly, .claude/agents/ discovery path) -- the embedded hooks below only fire here; see agents/branch-plan-task.md (the plugin-distributed variant, no hooks field, weaker prompt-only backstop for both mechanisms) for the deployment where gitapex is installed as a plugin into a different repository. +description: Task-level, project-local subagent type for a fixed, enumerated set of call sites -- see this file's own "Sanctioned call sites" section for the exact, current list (executing-a-branch-plan Step 6's per-task dispatch, Step 8's refactor/simplify pass). Never invoke directly for anything else, and never add a new call site without updating that section first -- this type exists as the Decision 17 deterministic backstop for Decision 7's exclusion list (no mcp__github__* tools, no gh/git-push/install commands), across both call sites, plus (Decision 20, issue #1476) the deterministic backstop requiring the full repo verification suite to pass inside this dispatch's own working checkout before it may report complete. Project-local variant (this repository checked out directly, .claude/agents/ discovery path) -- the embedded hooks below only fire here; see agents/branch-plan-task.md (the plugin-distributed variant, no hooks field, weaker prompt-only backstop for both mechanisms) for the deployment where gitapex is installed as a plugin into a different repository. disallowedTools: mcp__github hooks: PreToolUse: @@ -17,22 +17,51 @@ hooks: timeout: 3900 --- -Task-level dispatch target for `executing-a-branch-plan`. Do all Decision 3 -task work (Red-Green per Decision 14, screened per Decision 6) using Edit, -Write, Read, Grep, Glob, and Bash for non-excluded commands (git add, git -commit, running tests). Never attempt a GitHub write, the gh CLI, git push, -or a package-manager install -- those are main-thread-only per design doc -Decision 7; this agent type's own tool restrictions and embedded Bash hook -enforce that structurally, not only by this instruction. +Dispatch target for `executing-a-branch-plan`, scoped to the two call +sites the "Sanctioned call sites" section below enumerates. At Step 6's +own call site, do all Decision 3 task work (Red-Green per Decision 14, +screened per Decision 6); at Step 8's own call site, apply the same tool +set as a behavior-preserving refactor/simplify pass over the full +accumulated diff instead (no per-task Red-Green there -- see +`skills/executing-a-branch-plan/references/refactor-and-review-gate.md`'s +own sub-step 1). Both use Edit, Write, Read, Grep, Glob, and Bash for +non-excluded commands (git add, git commit, running tests). Never attempt +a GitHub write, the gh CLI, git push, or a package-manager install -- +those are main-thread-only per design doc Decision 7; this agent type's +own tool restrictions and embedded Bash hook enforce that structurally, +not only by this instruction. + +## Sanctioned call sites + +Only these. A caller outside this list should not name this +`agentType`/`subagent_type` -- propose adding it here first, in the same +change that adds the new call site, rather than reusing this definition +silently. + +1. `executing-a-branch-plan` Step 6's per-task dispatch (Decision 4/16) + -- one `agent()` call per Decision 3/15 task, dispatched per wave. The + existing, primary role this agent type was originally defined for. +2. `executing-a-branch-plan` Step 8's refactor/simplify pass (Decision + 12) -- a single dispatch over the full accumulated diff, after all + Step 6 tasks complete, behavior-preserving edits only. Any + behavior-affecting finding is out of this dispatch's own scope and + routes to the separate adversarial review pass instead + (`subagent_type: 'review-persona'`, see `agents/review-persona.md`'s + own "Sanctioned call sites" section) -- see + `skills/executing-a-branch-plan/references/refactor-and-review-gate.md`'s + own sub-step 1/2 split for why. Before reporting complete: the embedded `SubagentStop` hook above runs the full repo verification suite (`uv run --frozen python3 -m pytest --no-cov -q` plus `uv run --frozen python3 -.github/scripts/gitapex_gate_local_preflight.py`) inside this worktree and -denies stopping until both pass (design doc Decision 20, issue #1476) -- -this is a deterministic backstop, not only this paragraph's own -instruction, but fixing a verification failure it reports is still this -task's own responsibility to act on, the same as any other blocked stop. +.github/scripts/gitapex_gate_local_preflight.py`) inside this dispatch's +own working checkout (a worktree at Step 6's call site; this dispatch's +own checkout directly at Step 8's, which runs without worktree isolation +-- see the "Sanctioned call sites" section above) and denies stopping +until both pass (design doc Decision 20, issue #1476) -- this is a +deterministic backstop, not only this paragraph's own instruction, but +fixing a verification failure it reports is still this dispatch's own +responsibility to act on, the same as any other blocked stop. This hook's own `timeout: 3900` above must stay comfortably above 2x `gitapex_check_task_full_verification.py`'s own `DEFAULT_TIMEOUT_SECONDS` diff --git a/agents/branch-plan-task.md b/agents/branch-plan-task.md index c890bf4f..85d4080a 100644 --- a/agents/branch-plan-task.md +++ b/agents/branch-plan-task.md @@ -1,22 +1,51 @@ --- name: branch-plan-task -description: Task-level subagent type for executing-a-branch-plan's Decision 4/16 execution step (one agent() call per Decision 3/15 task, dispatched per wave). Never invoke directly for anything else. Plugin-distributed variant -- see skills/executing-a-branch-plan/references/threat-model-and-authorization.md for why this variant carries no embedded hook and what that means for the Decision 17 backstop's actual strength in a plugin-installed deployment, and (Decision 20, issue #1476) the identical weaker-strength accounting for the full-verification-suite exit condition below. +description: Task-level, plugin-distributed subagent type for a fixed, enumerated set of call sites -- see this file's own "Sanctioned call sites" section for the exact, current list (executing-a-branch-plan Step 6's per-task dispatch, Step 8's refactor/simplify pass). Never invoke directly for anything else, and never add a new call site without updating that section first. See skills/executing-a-branch-plan/references/threat-model-and-authorization.md for why this variant carries no embedded hook and what that means for the Decision 17 backstop's actual strength in a plugin-installed deployment, and (Decision 20, issue #1476) the identical weaker-strength accounting for the full-verification-suite exit condition below. disallowedTools: mcp__github --- -Task-level dispatch target for `executing-a-branch-plan`. Do all Decision 3 -task work (Red-Green per Decision 14, screened per Decision 6) using Edit, -Write, Read, Grep, Glob, and Bash for non-excluded commands (git add, git -commit, running tests). Never attempt a GitHub write, the gh CLI, git push, -or a package-manager install -- those are main-thread-only per design doc -Decision 7. +Dispatch target for `executing-a-branch-plan`, scoped to the two call +sites the "Sanctioned call sites" section below enumerates. At Step 6's +own call site, do all Decision 3 task work (Red-Green per Decision 14, +screened per Decision 6); at Step 8's own call site, apply the same tool +set as a behavior-preserving refactor/simplify pass over the full +accumulated diff instead (no per-task Red-Green there -- see +`skills/executing-a-branch-plan/references/refactor-and-review-gate.md`'s +own sub-step 1). Both use Edit, Write, Read, Grep, Glob, and Bash for +non-excluded commands (git add, git commit, running tests). Never attempt +a GitHub write, the gh CLI, git push, or a package-manager install -- +those are main-thread-only per design doc Decision 7. -**Before reporting complete, run the full repo verification suite inside -your own worktree** (design doc Decision 20, issue #1476): `uv run +## Sanctioned call sites + +Only these. A caller outside this list should not name this +`agentType`/`subagent_type` -- propose adding it here first, in the same +change that adds the new call site, rather than reusing this definition +silently. + +1. `executing-a-branch-plan` Step 6's per-task dispatch (Decision 4/16) + -- one `agent()` call per Decision 3/15 task, dispatched per wave. The + existing, primary role this agent type was originally defined for. +2. `executing-a-branch-plan` Step 8's refactor/simplify pass (Decision + 12) -- a single dispatch over the full accumulated diff, after all + Step 6 tasks complete, behavior-preserving edits only. Any + behavior-affecting finding is out of this dispatch's own scope and + routes to the separate adversarial review pass instead + (`subagent_type: 'review-persona'`, see `agents/review-persona.md`'s + own "Sanctioned call sites" section) -- see + `skills/executing-a-branch-plan/references/refactor-and-review-gate.md`'s + own sub-step 1/2 split for why. + +**Before reporting complete, run the full repo verification suite in +your own dispatch's working checkout** (a worktree at Step 6's call +site; this dispatch's own checkout directly at Step 8's, which runs +without worktree isolation -- see the "Sanctioned call sites" section +above) (design doc Decision 20, issue #1476): `uv run --frozen python3 -m pytest --no-cov -q --ignore=tests/test_gitapex_check_bash_safety_oracle_pins.py --ignore=tests/test_gitapex_check_task_bash_safety_oracle_pins.py --ignore=tests/test_gitapex_check_bash_safety_differential.py --ignore=tests/test_gitapex_check_task_bash_safety_differential.py` then `uv run --frozen python3 .github/scripts/gitapex_gate_local_preflight.py`. -Do not report this task done while either fails -- fix the failure first, -the same as any other Red-Green check this task's own work requires. +Do not report this dispatch done while either fails -- fix the failure +first, the same as any other Red-Green check this dispatch's own work +requires. This tool restriction (`disallowedTools: mcp__github`) is structurally enforced and portable to a plugin-installed deployment. The Bash-level @@ -48,5 +77,8 @@ safety.sh`); this variant has no equivalent backstop of any kind for it, the identical asymmetry as the two mechanisms above -- this paragraph's own instruction is the only thing enforcing it here. Skip this check entirely if you are not running inside a worktree at all (the sequential- -fallback dispatch, no wave) -- there is no shared-branch fork point to +fallback dispatch, no wave, or Step 8's own single dispatch -- also no +wave, no worktree isolation, per +`skills/executing-a-branch-plan/references/execution-and-dispatch.md`'s +own Step 8 subsection) -- there is no shared-branch fork point to compare against in that case. diff --git a/agents/review-persona.md b/agents/review-persona.md index 95f6addf..094e1f5f 100644 --- a/agents/review-persona.md +++ b/agents/review-persona.md @@ -36,6 +36,14 @@ adds the new call site, rather than reusing this definition silently. respectively, neither performable by this dispatch's read-only, file-scoped tool set) -- while the rest of checks 2-8, check 5 included, still dispatch normally. +4. `executing-a-branch-plan` Step 8's adversarial code review pass + (Decision 12) -- a single dispatch over the full accumulated diff + after all Step 6 tasks complete, reviewing for correctness bugs. + Read-only: this dispatch returns findings only; it does NOT verify, + fix, or validate them -- see + `skills/executing-a-branch-plan/references/refactor-and-review-gate.md`'s + own sub-step 2 text for where that verify/fix/validate work actually + happens. ## What this dispatch does and does not do diff --git a/skills/executing-a-branch-plan/SKILL.md b/skills/executing-a-branch-plan/SKILL.md index 46334704..493551a7 100644 --- a/skills/executing-a-branch-plan/SKILL.md +++ b/skills/executing-a-branch-plan/SKILL.md @@ -234,12 +234,13 @@ first, not skimmed. dispatch is a deadlock, not a harmless leftover. Full dispatch table: [domain events and failure-handling reference](references/domain-events-and-failure-handling.md#failure-dispatch-step-7). -8. **Refactor and adversarially review the accumulated diff** (Decision - 12, mandatory, non-skippable). Two separate fresh subagent dispatches - over the full diff -- a refactor/simplify pass (behavior-preserving - only), then an independent adversarial code review -- findings - verified and fixed before proceeding. The independent adversarial code - review also specifically re-checks [Migrate Callers Then Delete Legacy +8. **Refactor and adversarially review the accumulated diff** (Decision 12, mandatory, + non-skippable). Two separate fresh subagent dispatches over the full diff -- a + refactor/simplify pass (behavior-preserving only, `agentType: 'branch-plan-task'`), then + an independent adversarial code review (`subagent_type: 'review-persona'`) -- findings + verified and fixed outside it (main thread or the refactor pass's own subagent), not + inside it, before proceeding. The independent adversarial code review also specifically + re-checks [Migrate Callers Then Delete Legacy APIs](references/code-quality-principles.md#4-migrate-callers-then-delete-legacy-apis), confirming a caller migration begun by one task actually completed cleanly rather than being left half-done by another -- a diff --git a/skills/executing-a-branch-plan/references/execution-and-dispatch.md b/skills/executing-a-branch-plan/references/execution-and-dispatch.md index 78a56fc3..5fecffd1 100644 --- a/skills/executing-a-branch-plan/references/execution-and-dispatch.md +++ b/skills/executing-a-branch-plan/references/execution-and-dispatch.md @@ -15,6 +15,7 @@ violation of it. ## Contents - [Primary path: one Workflow run per wave](#primary-path-one-workflow-run-per-wave) +- [Step 8's two dispatches](#step-8s-two-dispatches) - [Git worktree isolation for parallel task execution](#git-worktree-isolation-for-parallel-task-execution) - [Worktree-base precondition backstop](#worktree-base-precondition-backstop) - [Sequential fallback](#sequential-fallback) @@ -69,6 +70,25 @@ consent-friction control, not only a parallelism-maximizing one. The count is acceptable in practice, is unverified -- flagged for the first real run to measure, not assumed low-friction. +## Step 8's two dispatches + +Step 8 (`refactor-and-review-gate.md`) reuses this same agent type and +its read-only sibling, not a per-wave dispatch: the refactor/simplify +pass dispatches with `agentType: 'branch-plan-task'` (that agent type's +own second sanctioned call site, per `agents/branch-plan-task.md`'s own +"Sanctioned call sites" section), and the adversarial code review +dispatches with `subagent_type: 'review-persona'` (`agents/review- +persona.md`'s own "Sanctioned call sites" section, entry 4). Step 8 is +"Not itself parallelized" (`refactor-and-review-gate.md`'s own term) -- +a single dispatch each, run once after all waves complete, not per-wave +-- so neither needs `isolation: 'worktree'`, the same omission this +file's own primary-path section above already makes for a single-task +wave: no concurrent write exists for either dispatch to guard against. +What each dispatch does with its own turn, and how findings from the +review pass get fixed, stays `refactor-and-review-gate.md`'s own detail, +not duplicated here -- this section covers only the dispatch mechanics +this file's own subject matter already owns. + ## Git worktree isolation for parallel task execution A file-ownership map prevents two parallel tasks from touching the same diff --git a/skills/executing-a-branch-plan/references/refactor-and-review-gate.md b/skills/executing-a-branch-plan/references/refactor-and-review-gate.md index e10a43b3..29bea0ea 100644 --- a/skills/executing-a-branch-plan/references/refactor-and-review-gate.md +++ b/skills/executing-a-branch-plan/references/refactor-and-review-gate.md @@ -55,7 +55,10 @@ missed defeat-case here ships a checker script that looks tested but silently does not catch what it claims to. 1. **Refactor/simplify pass**, over the full accumulated diff (every - task's own diff combined), not per-task. A fresh subagent dispatch, + task's own diff combined), not per-task. A fresh subagent dispatch + (`agentType: 'branch-plan-task'` -- this agent type's own second + sanctioned call site, alongside Step 6's per-task dispatch; see + `agents/branch-plan-task.md`'s own "Sanctioned call sites" section), distinct from the task agents that wrote the code -- the same agent grading its own homework is a weaker check than an independent one. This pass finds and fixes reuse, redundancy, and dead code that @@ -63,9 +66,19 @@ silently does not catch what it claims to. behavior -- any behavior-affecting finding is out of this sub-step's scope and routes to sub-step 2 instead. 2. **Adversarial code review**, a separate fresh subagent dispatch (not - the refactor pass's own subagent, same independence reason) reviewing - the full accumulated diff for correctness bugs. Findings -> verify - each -> fix confirmed ones -> validate the fix. + the refactor pass's own subagent, same independence reason; + `subagent_type: 'review-persona'` -- that agent's own 4th sanctioned + call site, see `agents/review-persona.md`'s own "Sanctioned call + sites" section) reviewing the full accumulated diff for correctness + bugs, and returning findings only. `review-persona`'s own `tools: + Read, Grep, Glob` allow-list means this dispatch cannot itself verify + a finding against a live check, apply a fix, or validate one -- it is + read-only by construction, not merely by convention. That work + happens outside this dispatch: verify each returned finding, fix the + confirmed ones, and validate the fix, either in the calling main + thread or dispatched to the refactor pass's own subagent (sub-step 1 + above) for the actual file edit -- never inside the review dispatch + itself, which has no tool that could perform any of the three. **Deterministic gate/check script scrutiny.** When the diff adds, extends, *or narrows* a deterministic gate or check script -- a CI From 7672cb0d1f690a13b59ed48e32d9c20e396ef6b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 23:31:59 +0000 Subject: [PATCH 6/6] docs(executing-a-branch-plan): fix Step 8's self-contradicting fix-routing text Step 8's own adversarial review pass (dispatched as part of this same Branch Plan execution) confirmed a self-contradiction the prior commit introduced: refactor-and-review-gate.md's sub-step 2 and SKILL.md's Step 8 both allowed a confirmed, behavior-affecting correctness-bug fix to be applied by "the refactor pass's own subagent" -- but that same commit's own new "Sanctioned call sites" entry for that subagent restricts it to "behavior-preserving edits only." Drop that option; the calling main thread is now the sole place a confirmed finding's fix is applied, consistent with the subagent's own stated scope. Refs #1560 Co-Authored-By: Claude Sonnet 5 --- skills/executing-a-branch-plan/SKILL.md | 6 +++--- .../references/refactor-and-review-gate.md | 13 ++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/skills/executing-a-branch-plan/SKILL.md b/skills/executing-a-branch-plan/SKILL.md index 493551a7..eb8b9e9f 100644 --- a/skills/executing-a-branch-plan/SKILL.md +++ b/skills/executing-a-branch-plan/SKILL.md @@ -238,9 +238,9 @@ first, not skimmed. non-skippable). Two separate fresh subagent dispatches over the full diff -- a refactor/simplify pass (behavior-preserving only, `agentType: 'branch-plan-task'`), then an independent adversarial code review (`subagent_type: 'review-persona'`) -- findings - verified and fixed outside it (main thread or the refactor pass's own subagent), not - inside it, before proceeding. The independent adversarial code review also specifically - re-checks [Migrate Callers Then Delete Legacy + verified and fixed outside it, in the calling main thread only (never by the refactor + pass's own behavior-preserving-only subagent), before proceeding. The independent + adversarial code review also specifically re-checks [Migrate Callers Then Delete Legacy APIs](references/code-quality-principles.md#4-migrate-callers-then-delete-legacy-apis), confirming a caller migration begun by one task actually completed cleanly rather than being left half-done by another -- a diff --git a/skills/executing-a-branch-plan/references/refactor-and-review-gate.md b/skills/executing-a-branch-plan/references/refactor-and-review-gate.md index 29bea0ea..99368eb1 100644 --- a/skills/executing-a-branch-plan/references/refactor-and-review-gate.md +++ b/skills/executing-a-branch-plan/references/refactor-and-review-gate.md @@ -74,11 +74,14 @@ silently does not catch what it claims to. Read, Grep, Glob` allow-list means this dispatch cannot itself verify a finding against a live check, apply a fix, or validate one -- it is read-only by construction, not merely by convention. That work - happens outside this dispatch: verify each returned finding, fix the - confirmed ones, and validate the fix, either in the calling main - thread or dispatched to the refactor pass's own subagent (sub-step 1 - above) for the actual file edit -- never inside the review dispatch - itself, which has no tool that could perform any of the three. + happens outside this dispatch, in the calling main thread: verify + each returned finding, fix the confirmed ones, and validate the fix + -- never inside the review dispatch itself, which has no tool that + could perform any of the three, and never dispatched to the refactor + pass's own subagent (sub-step 1 above), whose own "Sanctioned call + sites" entry restricts it to behavior-preserving edits only -- a + confirmed correctness-bug fix is behavior-affecting by definition and + so falls outside that entry's own scope. **Deterministic gate/check script scrutiny.** When the diff adds, extends, *or narrows* a deterministic gate or check script -- a CI