diff --git a/agents/vbw-lead.md b/agents/vbw-lead.md index ff2cc0d0..5f19ccb1 100644 --- a/agents/vbw-lead.md +++ b/agents/vbw-lead.md @@ -66,6 +66,7 @@ Display: `✓ Lead: All plans written to disk` PLAN_NAME=$(bash "$RESOLVE_SCRIPT" plan "{phase-dir}" --plan-number {MM}) ``` Write the plan to `{phase-dir}/${PLAN_NAME}`. If the orchestrator did not provide `RESOLVE_SCRIPT`, fall back to `{NN}-{MM}-PLAN.md` where `{NN}` is the phase number from the directory basename and `{MM}` is the zero-padded plan number. Do NOT use `PLAN-{NN}.md` — this format is rejected by file-guard. +**Frontmatter `plan:` value:** Set the `plan:` field to the plan number `{MM}` only (zero-padded, e.g. `01`). Do NOT write the `{NN}-{MM}` file id (e.g. `66-01`) into `plan:` — the phase is already carried by the separate `phase:` field. The execute router reconstructs the canonical `{NN}-{MM}` id from `phase:` + `plan:`, so a phase-prefixed `plan:` value misrepresents the plan id. Report: `Phase {NN}: {name}\nPlans: {N}\n {plan}: {title} (wave {W}, {N} tasks)` ## Goal-Backward Methodology diff --git a/scripts/resolve-execute-delegation-mode.sh b/scripts/resolve-execute-delegation-mode.sh index 9e3c55ed..3f767198 100755 --- a/scripts/resolve-execute-delegation-mode.sh +++ b/scripts/resolve-execute-delegation-mode.sh @@ -295,7 +295,15 @@ resolve_plan_path() { if [ -n "$fm_phase" ] || [ -n "$fm_plan" ]; then fm_phase=${fm_phase:-$phase_id} fm_plan=${fm_plan:-$plan_part} - norm_fm_id=$(normalize_plan_ref "$(pad_number "$fm_phase")-$(pad_number "$fm_plan")" 2>/dev/null || true) + # A plan: frontmatter value may already be phase-qualified (NN-MM), the + # same form cross_phase_deps uses. Normalizing it directly avoids + # re-prepending fm_phase (which would double-prefix to NN-NN-MM and + # raise a spurious frontmatter_mismatch). A bare plan-number is still + # combined with fm_phase to build the canonical id. + 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 if [ -n "$norm_fm_id" ] && [ "$norm_fm_id" != "$id" ]; then printf 'frontmatter_mismatch:%s:declares:%s\n' "$(basename "$candidate")" "$norm_fm_id" >&2 return 3 diff --git a/scripts/worktree-cleanup.sh b/scripts/worktree-cleanup.sh index 4479f71a..e433031b 100755 --- a/scripts/worktree-cleanup.sh +++ b/scripts/worktree-cleanup.sh @@ -12,10 +12,20 @@ fi PHASE="$1" PLAN="$2" +# Normalize the plan id into the canonical "-" slug so the worktree +# dir, branch, and agent-map glob match what worktree-create.sh produced. A +# caller may pass a bare plan number (MM) or an already phase-qualified id +# (NN-MM); prepending PHASE to the latter would double-prefix to NN-NN-MM. Use +# an already-qualified plan as-is, otherwise combine it with PHASE. +case "$PLAN" in + *-*) SLUG="$PLAN" ;; + *) SLUG="${PHASE}-${PLAN}" ;; +esac + PLANNING_DIR="${VBW_PLANNING_DIR:-.vbw-planning}" WORKTREES_PARENT=".vbw-worktrees" -WORKTREE_DIR="${WORKTREES_PARENT}/${PHASE}-${PLAN}" -BRANCH="vbw/${PHASE}-${PLAN}" +WORKTREE_DIR="${WORKTREES_PARENT}/${SLUG}" +BRANCH="vbw/${SLUG}" AGENT_WORKTREES_DIR="$PLANNING_DIR/.agent-worktrees" # Unlock the worktree if locked (locked worktrees resist single --force) @@ -67,7 +77,7 @@ git branch -d "$BRANCH" 2>/dev/null || true # Clear the agent-worktree mapping entry if the directory exists if [ -d "$AGENT_WORKTREES_DIR" ]; then # Remove any JSON file matching the phase-plan pattern - for f in "$AGENT_WORKTREES_DIR"/*"${PHASE}-${PLAN}"*.json; do + for f in "$AGENT_WORKTREES_DIR"/*"${SLUG}"*.json; do [ -f "$f" ] && rm -f "$f" 2>/dev/null || true done fi diff --git a/scripts/worktree-create.sh b/scripts/worktree-create.sh index 1ae79fce..c27f30eb 100755 --- a/scripts/worktree-create.sh +++ b/scripts/worktree-create.sh @@ -15,8 +15,19 @@ if [ -z "$PHASE" ] || [ -z "$PLAN" ]; then exit 0 fi -WORKTREE_DIR=".vbw-worktrees/${PHASE}-${PLAN}" -BRANCH="vbw/${PHASE}-${PLAN}" +# Normalize the plan id into the canonical "-" slug. A caller may +# pass either a bare plan number (MM) or an already phase-qualified id (NN-MM, +# the form .execution-state.json and plan filenames use). Prepending PHASE to an +# already-qualified id would double-prefix to NN-NN-MM (e.g. 43-43-03). Mirror +# resolve-execute-delegation-mode.sh: use an already-qualified plan as-is, +# otherwise combine it with PHASE. +case "$PLAN" in + *-*) SLUG="$PLAN" ;; + *) SLUG="${PHASE}-${PLAN}" ;; +esac + +WORKTREE_DIR=".vbw-worktrees/${SLUG}" +BRANCH="vbw/${SLUG}" # Idempotent: if worktree already exists, return its absolute path if [ -d "$WORKTREE_DIR" ]; then diff --git a/scripts/worktree-merge.sh b/scripts/worktree-merge.sh index 77326c19..3f9b1da1 100755 --- a/scripts/worktree-merge.sh +++ b/scripts/worktree-merge.sh @@ -14,10 +14,23 @@ if [ -z "$PHASE" ] || [ -z "$PLAN" ]; then exit 0 fi -BRANCH="vbw/${PHASE}-${PLAN}" +# Normalize the plan id into the canonical "-" slug so the branch +# matches the one worktree-create.sh produced. A caller may pass a bare plan +# number (MM) or an already phase-qualified id (NN-MM); prepending PHASE to the +# latter would double-prefix to NN-NN-MM. Use an already-qualified plan as-is, +# otherwise combine it with PHASE. +case "$PLAN" in + *-*) SLUG="$PLAN" ;; + *) SLUG="${PHASE}-${PLAN}" ;; +esac +PLAN_NUM="${SLUG#*-}" -# Attempt the merge -git merge --no-ff "$BRANCH" -m "merge: phase ${PHASE} plan ${PLAN}" 2>/dev/null +BRANCH="vbw/${SLUG}" + +# Attempt the merge. Suppress git's own stdout/stderr so the script honors its +# contract of emitting exactly "clean" or "conflict" (git prints a porcelain +# summary to stdout on success and conflict details on failure). +git merge --no-ff "$BRANCH" -m "merge: phase ${PHASE} plan ${PLAN_NUM}" >/dev/null 2>&1 MERGE_STATUS=$? if [ "$MERGE_STATUS" -eq 0 ]; then diff --git a/templates/PLAN.md b/templates/PLAN.md index a4f07144..68d58a57 100644 --- a/templates/PLAN.md +++ b/templates/PLAN.md @@ -1,6 +1,6 @@ --- phase: {NN} # bare integer, no quotes -plan: {plan-number} +plan: {plan-number} # plan number only (MM, zero-padded e.g. 01) — NOT the NN-MM file id title: {plan-title} type: execute wave: {wave-number} diff --git a/templates/SUMMARY.md b/templates/SUMMARY.md index 1febb999..e14ac59e 100644 --- a/templates/SUMMARY.md +++ b/templates/SUMMARY.md @@ -1,6 +1,6 @@ --- phase: {NN} # bare integer, no quotes -plan: {plan-number} +plan: {plan-number} # plan number only (MM, zero-padded e.g. 01) — NOT the NN-MM file id title: {plan-title} status: {complete|partial|failed} completed: {YYYY-MM-DD} diff --git a/testing/verify-execute-delegation-routing.sh b/testing/verify-execute-delegation-routing.sh index d2bd208c..09ca03e2 100755 --- a/testing/verify-execute-delegation-routing.sh +++ b/testing/verify-execute-delegation-routing.sh @@ -384,6 +384,54 @@ write_state '{"plans":[{"id":"01-02","status":"pending"}],"effort":"balanced","p write_plan_inline 01-02-PLAN.md 01 03 '[]' expect_helper_failure "frontmatter plan mismatch fails closed with invalid_dependency_graph" +# issue #659: a phase-prefixed plan: frontmatter value (NN-MM, the same form +# cross_phase_deps uses) must resolve to the canonical id instead of being +# double-prefixed to NN-NN-MM and rejected as a spurious frontmatter_mismatch. +make_fixture fm-phase-prefixed '"auto"' balanced +write_state '{"plans":[{"id":"01-01","status":"pending"}],"effort":"balanced","phase_effort":"balanced"}' +write_plan_inline 01-01-PLAN.md 01 01-01 '[]' +out=$(run_helper) +assert_eq "$(json_field "$out" '.delegation_mode')" "subagent" "phase-prefixed plan: frontmatter (unquoted NN-MM) resolves without frontmatter_mismatch" +assert_json_array_eq "$(jq -c '.dependency_waves' <<< "$out")" '[["01-01"]]' "phase-prefixed plan: frontmatter maps to canonical id 01-01" + +make_fixture fm-phase-prefixed-quoted '"auto"' balanced +write_state '{"plans":[{"id":"01-01","status":"pending"}],"effort":"balanced","phase_effort":"balanced"}' +write_plan_inline 01-01-PLAN.md 01 '"01-01"' '[]' +out=$(run_helper) +assert_eq "$(json_field "$out" '.delegation_mode')" "subagent" "phase-prefixed plan: frontmatter (quoted \"NN-MM\") resolves without frontmatter_mismatch" + +# the fix must not blunt genuine-mismatch detection: a phase-prefixed value that +# disagrees with the canonical id is still rejected. +make_fixture fm-phase-prefixed-mismatch '"auto"' balanced +write_state '{"plans":[{"id":"01-01","status":"pending"}],"effort":"balanced","phase_effort":"balanced"}' +write_plan_inline 01-01-PLAN.md 01 01-02 '[]' +expect_helper_failure "phase-prefixed frontmatter mismatch (NN-MM disagrees with id) still fails closed" + +# multi-digit phase symmetry (QA round 2): a 3-digit phase with a phase-prefixed +# plan: value (100-01) must still normalize to the canonical id, not 100-100-01. +# Guards the pad_number/normalize_plan_ref symmetry against future edits. Uses a +# bespoke fixture because make_fixture/run_helper assume a 2-digit phase dir. +MD_FIXTURE="$TMPDIR_BASE/fm-multidigit-phase" +MD_PHASE_DIR="$MD_FIXTURE/.vbw-planning/phases/100-test" +mkdir -p "$MD_PHASE_DIR" "$MD_FIXTURE/.vbw-planning/.cache" +printf '{"prefer_teams":"auto","effort":"balanced"}\n' > "$MD_FIXTURE/.vbw-planning/config.json" +printf '{"plans":[{"id":"100-01","status":"pending"}],"effort":"balanced","phase_effort":"balanced"}\n' > "$MD_FIXTURE/.vbw-planning/.execution-state.json" +cat > "$MD_PHASE_DIR/100-01-PLAN.md" <<'MDPLAN' +--- +phase: 100 +plan: "100-01" +title: Plan 100-01 +depends_on: [] +--- +# Plan 100-01 + +### Task 1: Work +- **Files:** `src/file.txt` +MDPLAN +md_out=$(cd "$MD_FIXTURE" && "$HELPER" --phase-dir .vbw-planning/phases/100-test) +assert_eq "$(json_field "$md_out" '.delegation_mode')" "subagent" "multi-digit phase with phase-prefixed plan: (100-01) resolves without frontmatter_mismatch" +assert_json_array_eq "$(jq -c '.dependency_waves' <<< "$md_out")" '[["100-01"]]' "multi-digit phase-prefixed plan: maps to canonical id 100-01 (no 100-100-01 double-prefix)" + # malformed execution-state / route-map schemas fail closed before spawning make_fixture valid-empty-plans '"auto"' balanced write_state '{"plans":[],"effort":"balanced","phase_effort":"balanced"}' diff --git a/tests/worktree.bats b/tests/worktree.bats index 7e41e0fc..f2508b4e 100644 --- a/tests/worktree.bats +++ b/tests/worktree.bats @@ -56,6 +56,28 @@ teardown() { [ "$output" = "vbw/02-03" ] } +@test "worktree-create: phase-qualified plan id does not double-prefix (regression #659)" { + cd "$TEST_TEMP_DIR" + git init -q + git config user.name "VBW Test" + git config user.email "vbw-test@example.com" + echo "seed" > README.md + git add README.md + git commit -q -m "chore(init): seed" + + # Callers pass the already phase-qualified plan id (the form + # .execution-state.json and plan filenames use). It must not become 43-43-03. + run bash "$SCRIPTS_DIR/worktree-create.sh" 43 43-03 + [ "$status" -eq 0 ] + [[ "$output" == *".vbw-worktrees/43-03" ]] + [ -d ".vbw-worktrees/43-03" ] + [ ! -d ".vbw-worktrees/43-43-03" ] + + run git -C ".vbw-worktrees/43-03" rev-parse --abbrev-ref HEAD + [ "$status" -eq 0 ] + [ "$output" = "vbw/43-03" ] +} + # --------------------------------------------------------------------------- # worktree-merge.sh tests # --------------------------------------------------------------------------- @@ -82,6 +104,28 @@ teardown() { [ "$output" = "conflict" ] } +@test "worktree-merge: phase-qualified plan id targets the un-doubled branch (regression #659)" { + cd "$TEST_TEMP_DIR" + git init -q + git config user.name "VBW Test" + git config user.email "vbw-test@example.com" + echo "seed" > README.md + git add README.md + git commit -q -m "chore(init): seed" + + # Work to be merged lives on vbw/43-03, not vbw/43-43-03. + git checkout -q -b vbw/43-03 + echo "feature" > feature.txt + git add feature.txt + git commit -q -m "feat: work" + git checkout -q - + + run bash "$SCRIPTS_DIR/worktree-merge.sh" 43 43-03 + [ "$status" -eq 0 ] + [ "$output" = "clean" ] + [ -f feature.txt ] +} + # --------------------------------------------------------------------------- # worktree-cleanup.sh tests # --------------------------------------------------------------------------- @@ -222,6 +266,23 @@ teardown() { [ ! -f ".vbw-planning/.agent-worktrees/agent-01-01.json" ] } +@test "worktree-cleanup: phase-qualified plan id removes the un-doubled worktree (regression #659)" { + cd "$TEST_TEMP_DIR" + mkdir -p .vbw-worktrees/43-03/.vbw-planning + run bash "$SCRIPTS_DIR/worktree-cleanup.sh" 43 43-03 + [ "$status" -eq 0 ] + [ ! -d ".vbw-worktrees/43-03" ] +} + +@test "worktree-cleanup: phase-qualified plan id clears agent-worktree JSON (regression #659)" { + cd "$TEST_TEMP_DIR" + mkdir -p .vbw-planning/.agent-worktrees + echo '{}' > .vbw-planning/.agent-worktrees/dev-43-03.json + run bash "$SCRIPTS_DIR/worktree-cleanup.sh" 43 43-03 + [ "$status" -eq 0 ] + [ ! -f ".vbw-planning/.agent-worktrees/dev-43-03.json" ] +} + # --------------------------------------------------------------------------- # worktree-status.sh tests # ---------------------------------------------------------------------------