fix(execute): accept phase-prefixed plan: frontmatter in routing resolver

[halindrome]

Linked Issue

Fixes #659

What

Fix both sides of the phase-prefixed plan: frontmatter failure:

1. Effect (resolver tolerance): the Execute routing resolver now accepts a phase-prefixed plan: value (NN-MM) in addition to the canonical plan-number-only form (MM); both normalize to the same canonical id.
2. Cause (Lead/template): the Lead and the plan templates now make clear that plan: carries the plan number MM only — not the NN-MM file id — so the non-canonical value stops being emitted in the first place.

Why

resolve_plan_path() rebuilt the canonical plan id by concatenating the phase and plan frontmatter fields:

norm_fm_id=$(normalize_plan_ref "$(pad_number "$fm_phase")-$(pad_number "$fm_plan")" ...)

When plan: is already phase-qualified (plan: "66-01"), this double-prefixes the phase → 66-66-01 != 66-01 → frontmatter_mismatch → the resolver exits invalid_dependency_graph and the entire phase fails before any agent is spawned.

Two root causes converged:

• The resolver shouldn't reject an equal-but-verbose id. The NN-MM form is already first-class in cross_phase_deps (templates/PLAN.md), so rejecting it here was inconsistent.
• The Lead shouldn't have emitted plan: "66-01" at all. Its Stage 4 prominently showed the {NN}-{MM} plan filename but never stated that the plan: frontmatter field is {MM} alone, so it mirrored the file id into the field.

The defect shipped in deb36058 (2026-05-01) and stayed latent on main until a Lead emitted the phase-prefixed form. It was hit live during a /vbw:vibe run building phase 66 in a real consumer repo.

How

Effect — scripts/resolve-execute-delegation-mode.sh, resolve_plan_path(): normalize an already-NN-MM fm_plan directly instead of re-prepending the phase. A bare plan number still combines with the phase. Genuine mismatches (wrong plan number / wrong phase, bare or phase-qualified) still fail closed.

case "$fm_plan" in
  *-*) norm_fm_id=$(normalize_plan_ref "$fm_plan" 2>/dev/null || true) ;;
  *)   norm_fm_id=$(normalize_plan_ref "$(pad_number "$fm_phase")-$(pad_number "$fm_plan")" 2>/dev/null || true) ;;
esac

Cause — self-document the plan: field in templates/PLAN.md and templates/SUMMARY.md (matching the existing phase: comment), and add an explicit Stage 4 instruction in agents/vbw-lead.md that plan: carries {MM} only while the phase lives in phase:.

Affected: scripts/resolve-execute-delegation-mode.sh, agents/vbw-lead.md, templates/PLAN.md, templates/SUMMARY.md (+ regression tests). The resolver is invoked from references/execute-protocol.md, driven by /vbw:vibe. No command, hook, or version-file changes.

Testing

• Loaded plugin locally (claude-vbw or claude --plugin-dir "<path-to-vbw-clone>")
• Tested affected commands against a real project (not the VBW repo)
• No errors on plugin load
• Existing commands still work (BATS + contract suite green; see below)
• Ran QA review (3 separate QA rounds acting as devil's advocate on the diff)

Automated testing performed:

• testing/verify-execute-delegation-routing.sh: 93/93 PASS — adds the 4 execute resolver: phase-prefixed plan: frontmatter double-prefixes id, fails with frontmatter_mismatch/invalid_dependency_graph #659 cases (phase-prefixed unquoted/quoted accepted, canonical-id mapping, mismatch still fails closed) plus a multi-digit-phase case (phase: 100, plan: "100-01" → 100-01, no 100-100-01).
• Full testing/run-all.sh: BATS 3565 passed / 0 failed, Lint 1/1. (A few contract checks flag only under heavy parallel load — shared-TMPDIR flakes; all pass in isolation and none reference the changed paths.)
• Template/cause edits re-validated in isolation: template-parser BATS, verify-lsp-first-policy.sh, verify-commands-contract.sh, verify-permission-mode-contract.sh all green.
• shellcheck -S warning + bash -n clean on the changed script and test.
• Deterministic repro of the original 66-66-01 diagnostic confirmed before, and fixed after, the change.

QA Review Evidence

Three independent read-only devil's-advocate rounds on Claude Opus 4.8, each as a fresh session. Reports posted as separate PR comments. Rounds 1 and 3 found no actionable issues; round 2 surfaced one minor suggestion (multi-digit-phase coverage), which was addressed with a new regression case.

• Rounds completed: 3
• Model used: Claude Opus 4.8 (claude-opus-4-8)
• Fix/evidence commits: a45ebeb8 (round 1), ed4b75c0 (round 2 — added multi-digit test), 7fa9add6 (round 3)
• QA-relevant changes have 3 QA evidence commits using fix(scope): address QA round N
• Clean rounds still have an evidence commit

Notes

• Two functional commits: 6b768ab8 (resolver tolerance — effect) and 5d78e8cd (Lead/template canonical form — cause), plus the three QA evidence commits.
• Unrelated pre-existing flake: tests/agent-shutdown-integration.bats (test_prune_recovers_from_stale_lock_directory_with_no_pid_file) hangs intermittently; it is byte-identical to main and independent of this change.

🤖 Generated with Claude Code

[halindrome]

QA Review — Round 1

Model used: Claude Opus 4.8 (claude-opus-4-8)
Method: fresh read-only reviewer session, devil's-advocate. Reviewed both commits, traced normalize_plan_ref/pad_number/strip_quotes/resolve_plan_path, ran the affected suite with AND without the fix, searched for other call sites, and inspected the SUMMARY plan: consumer.

What was tested

• Both commits in full: resolver fix (scripts/resolve-execute-delegation-mode.sh + 3 new test cases) and cause-side fix (templates/PLAN.md, templates/SUMMARY.md, agents/vbw-lead.md).
• Suite with the fix: 91 PASS / 0 FAIL (exit 0).
• Suite without the fix (resolver reverted to origin/main, then restored): aborts under set -euo pipefail at the first new accepted-case test (exit 2) — confirming the regression genuinely fails without the fix.
• Searched scripts/ for other phase+plan id reconstruction sites (CMM + grep).
• Inspected state-updater.sh SUMMARY plan: consumer for the same double-prefix risk.

Edge cases (all CONFIRMED safe — funnel through the same guarded normalize_plan_ref path)

• Quoting/whitespace: stripped by frontmatter_value + strip_quotes; quoted "01-01" test passes.
• Leading zeros / single-digit / 3+ digit: re-padded via pad_number (10#$raw, so no octal trap on 08/09); 1-1→01-01, 100-2→100-02.
• empty / null: empty can't reach the *-*) branch (no dash); null|none|[] caught by normalize_plan_ref guard → empty id skipped by [ -n ... ].
• *-* glob on a non-pair (e.g. foo-bar): inner numeric check fails → value printed verbatim → real frontmatter_mismatch (correct).

Regression-test validity

• Fails without the fix (suite aborts, exit 2). Passes with it (91/91). The "still fails closed" mismatch case (plan: 01-02 vs id 01-01) specifically drives the new *-*) branch → return 3 → invalid_dependency_graph. Guard is not blunted.

Other call sites / consumers

• No other script reconstructs a plan id by concatenating phase:+plan: frontmatter the buggy way. Other id reconstructions parse from filenames (worktree-status.sh, session-stop.sh, auto-repair.sh, generate-contract.sh) — not vulnerable.
• state-updater.sh SUMMARY plan: matcher is tolerant (.id == $plan OR numeric last-segment compare, null-guarded) — a stray phase-prefixed value still matches; no double-prefix bug.

Findings

1. [minor / confirmed] Asymmetric padding between the two branches (the bare branch's outer pad_number is now redundant since normalize_plan_ref re-pads). Cosmetic only — identical output; out of scope for a bug fix. No action required.
2. [minor / hypothetical] Cause-side fix doesn't migrate pre-existing on-disk plans/summaries with the old phase-prefixed value — but those are now tolerated by the resolver (and the SUMMARY consumer), which is the intended design. Not actionable.

Verdict

No blocking or major issues. Fix is correct on both the resolver (tolerance) and authoring (templates + Lead) sides; regression test genuinely fails without the change and passes with it. Ship it.

— Clean round. Evidence commit: fix(execute): address QA round 1.

[halindrome]

QA Review — Round 2

Model used: Claude Opus 4.8 (claude-opus-4-8)
Method: independent fresh read-only reviewer. Re-implemented strip_quotes/pad_number/normalize_plan_ref/the new *-*) branch in a sandbox and drove adversarial inputs; probed the candidate-filename loop and the mismatch-comparison key; re-read the cause-side files end-to-end; ran the routing suite.

What was tested

• Full diff (ignoring the empty round-1 commit).
• Whole resolver: pad_number, normalize_plan_ref, frontmatter_value, resolve_plan_path, and the caller where id originates.
• Adversarial plan: inputs: 01-01, 1-2-3, -01, 01-, 0x10-1, 01-1e2, bare 1/01, 07-01, 100-01, 7-1, wrong-phase 02-01, remediation R01-02/R01.
• Candidate-loop / mismatch-key interaction; multi-digit & zero-padded phase symmetry.
• Cause-side: agents/vbw-lead.md, templates/PLAN.md, templates/SUMMARY.md, templates/REMEDIATION-PLAN.md, commands/vibe.md, references/execute-protocol.md.
• testing/verify-execute-delegation-routing.sh: 91 PASS / 0 FAIL.

Findings (per angle, all CONFIRMED)

1. Root cause fixed. plan: "01-01" → 01-01 (was 01-01-01); bare 1 → 01-01. Correct.
2. No wrong-candidate selection / masked mismatch. The comparison key id is the pre-normalized canonical value; plan_part="${id#*-}"; all three candidate filenames derive from it. A wrong-phase 02-01 vs id 01-01 still normalizes to 02-01 != 01-01 → frontmatter_mismatch. Tolerance did not widen too far.
3. Multi-digit / zero-padded phase safe. phase_id is 2-padded; both branches stay symmetric (7-1→07-01, 100-01→100-01). Verified against the live helper.
4. Malformed values safe. 1-2-3, -01, 01-, 0x10-1, 01-1e2 all hit normalize_plan_ref's literal-passthrough leg → differ from canonical id → fail closed. No crash, no spurious accept.
5. Cause-side clean. No remaining example writes a phase-prefixed plan: scalar. The source_plan: 01-01-PLAN.md hits are filename references (correctly NN-MM-PLAN.md); REMEDIATION-PLAN.md's plan: R01-02 carries an R prefix that passes through literally — not consumed by resolve_plan_path.

Severity

• No critical, no major findings (CONFIRMED).
• Minor / hypothetical: add a multi-digit-phase regression assertion to lock the angle-3 symmetry against future edits. (Current behavior already correct.)

Action taken this round

Addressed the minor suggestion: added a self-contained 3-digit-phase regression case (phase: 100, plan: "100-01") asserting it resolves to canonical 100-01 (no 100-100-01 double-prefix). Suite now 93 PASS / 0 FAIL.

Evidence commit: fix(testing): address QA round 2.

[halindrome]

QA Review — Round 3 (final)

Model used: Claude Opus 4.8 (claude-opus-4-8)
Method: independent fresh read-only reviewer, final pre-merge pass. Re-ran the suite, audited the new tests for tautology and global-state leakage, re-examined the resolver for fail-closed gaps, checked cause-side wording for contradictions, and lint/quoting-checked the new test block.

What was tested

• Full diff origin/main..HEAD (resolver fix, Lead cause-side, round-1 + round-2 evidence commits).
• Re-ran testing/verify-execute-delegation-routing.sh → 93 PASS / 0 FAIL.
• Resolver vs normalize_plan_ref/pad_number/frontmatter_value; route-map path, cross_phase_deps, completed/partial statuses, and the silent-accept concern on the *-*) branch.
• Cause-side wording across templates/, agents/, commands/, references/.
• shellcheck -S warning on the test and resolver; quoting/heredoc/subshell-cd safety of the new fixture.

Findings (all CONFIRMED, none actionable)

1. Tests are meaningful, not tautological. The round-1 cases pin canonical id (dependency_waves), the mismatch case proves the guard isn't blunted, and the round-2 multi-digit case asserts [["100-01"]] — exactly the value that would read 100-100-01 under the old code.
2. Multi-digit fixture is self-contained. It uses its own MD_* locals and an inline subshell cd; it never reassigns the global FIXTURE/PHASE_DIR and doesn't call run_helper. The next make_fixture resets globals anyway. 93/0 with all post-block tests passing confirms no leak.
3. No fail-closed gap. The comparison key $id is derived from the directory phase + plan_part, never from frontmatter — so a plan declaring a divergent phase (02-01 in an 01-01 slot) normalizes to 02-01 != 01-01 → frontmatter_mismatch. Non-circular, cannot silently accept. Route-map / status logic is downstream and unaffected.
4. Cause-side accurate and consistent. No surviving instruction tells the Lead to put the NN-MM file id in plan:. The template comment, the Lead instruction, and the resolver's reconstruction all agree; the {NN}-{MM}-PLAN.md guidance is about the filename, not the field.
5. Lint/quoting clean. shellcheck -S warning exit 0 on both files; single-quoted heredoc delimiter prevents interpolation/execution; all expansions quoted; cd isolated in a command-substitution subshell.

Severity

No critical, major, or minor actionable findings. All five confirmed safe.

Verdict

Correctly-scoped, fail-closed-preserving change. Tests assert canonical-id values (not just absence of error); the genuine-mismatch and multi-digit guards prevent silent acceptance and lock the pad symmetry; the new fixture is isolated; cause-side docs are consistent. Safe to merge.

— Clean round. Evidence commit: fix(execute): address QA round 3.

[halindrome]

QA Round 4 — review of commit ec66eb8d (worktree double-prefix fix)

Evidence commit: 22f0f82b (fix(worktree): address QA round 4).

This round covers the worktree-script additions made after rounds 1–3 (those rounds predated this code). Independent read-only review.

Model used: Claude Opus 4.8

WHAT WAS TESTED
1. SLUG normalization (case "$PLAN" in *-*) across create/merge/cleanup — all
   three use byte-identical case blocks; SLUG drives worktree dir, branch, and
   (cleanup) the agent-map glob.
2. Regression-test integrity: reverted ONLY the 3 scripts to origin/main, kept the
   new tests, re-ran tests/worktree.bats. The 4 new #659 tests FAIL on old code
   (not ok 5/9/20/21) and PASS on new code — genuine regression tests.
3. PLAN_NUM="${SLUG#*-}" for bare/qualified/multi-digit inputs.
4. merge stdout suppression (>/dev/null 2>&1) vs the "exactly clean|conflict"
   contract, and MERGE_STATUS capture.
5. cleanup agent-map glob *"${SLUG}"*.json vs old *"${PHASE}-${PLAN}"*.json.
6. Caller (references/execute-protocol.md) arg shapes for create/merge/cleanup/
   agent-map, plus session-stop.sh and doctor-cleanup.sh.
7. shellcheck -S warning + bash -n on all 3 changed scripts (clean).
8. Full tests/worktree.bats (28/28 pass).

CONTRACT VERIFICATION
| Criterion                                                   | Status    |
|-------------------------------------------------------------|-----------|
| Qualified plan id (NN-MM) must not double-prefix to NN-NN-MM | satisfied |
| Bare plan number (MM) still combines with PHASE             | satisfied |
| merge.sh honors "exactly clean|conflict" output contract    | satisfied |
| Multi-digit phase handled                                   | satisfied |
| Regression coverage added (proven to fail vs origin/main)   | satisfied |

FINDINGS
Contract: none.  Regression: none.

Pre-existing observations (NON-BLOCKING, not introduced by this commit):
- F-01 (minor, confirmed): cleanup agent-map glob *"${SLUG}"*.json is a substring
  match, so *43-03*.json also matches dev-43-031.json or qa-143-03.json. Identical
  behavior already existed in the old bare-input path; this commit only makes the
  qualified path behave the same. It is a belt-and-suspenders secondary to the
  exact-name clear (worktree-agent-map.sh clear "dev-{plan}"). Unreachable under
  2-digit id conventions. scripts/worktree-cleanup.sh:80.
- F-02 (minor, confirmed): merge.sh commit-message text can mix the arg PHASE with
  the qualified plan's own number (e.g. phase=3 plan=03-01 -> "merge: phase 3 plan
  01"). Cosmetic only; the merge always targets the correct branch (vbw/03-01).
  Unreachable on the happy path (the resolver guards PHASE/plan agreement).
  scripts/worktree-merge.sh:24-26,33.

SUMMARY: 0 contract, 0 regression, 2 observation. No blocking findings.

VERDICT: Commit ec66eb8d correctly fixes the double-prefix family (#659) across
create/merge/cleanup with genuine, proven regression tests. The merge
stdout-contract fix is correct. Both observations are pre-existing and unreachable
under current id conventions.

Decision: round closed clean per the CONTRIBUTING stopping rule (clean / only minor findings). F-01 and F-02 are pre-existing and unreachable, so they were documented rather than fixed, to keep this commit minimal and the round genuinely clean.