feat(execute-task): hard-gate slash command for planning → execution

[bborbe]

Summary

• New /vault-cli:execute-task slash command — the blocking gate between planning and execution (sibling of /work-on-task's informational readiness nudge)
• Re-runs /plan-task's 4 hard non-negotiables (SC defined, subtasks reach goal, e2e verify subtask, subtask-goal alignment); on pass, flips phase: planning → execution and prints first unchecked subtask + DoD reminder
• Idempotent on phase: execution — re-prints work block + DoD without mutation (useful as session-start "where was I?")
• Refusal cases enumerated: closed tasks, missing planning gate, hard-check failures — each points the operator at the right recovery command

Why

Closes the lifecycle's last operational gap. After this ships, every phase transition has an enforced command:

Transition	Command	Gate type
→ planning	/plan-task	5 hard checks + audit loop
→ execution	/execute-task (this PR)	re-checks 4 hard non-negotiables
→ ai_review / human_review	agent-driven (out of scope)	—
→ completed	/complete-task	4-criteria auto-complete

Advances [[Phase-Gated Task Flow]] SC #2: "Each phase transition has a slash command that gates it with concrete checks (not subjective approval)" for the planning → execution transition (the only un-gated one remaining after /plan-task shipped in v0.81.0).

Test plan

• make precommit clean
• Manual: invoke on phase: planning task with empty # Success Criteria → refuses with ❌ Plan not ready + lists failed check
• Manual: invoke after /plan-task flips gates green → ✅ Phase: planning → execution + prints 🎯 Start with: <subtask> + 📋 When done, verify: <DoD>
• Manual: re-invoke on same task (now phase: execution) → idempotent, no flip, re-prints work block
• Manual: invoke on status: completed task → refuses with ❌ Task closed
• Manual: invoke on phase: todo task → refuses with ❌ Planning gate not run. Run /vault-cli:plan-task first.

Closes vault task [[Planning-To-Execution Transition Lacks Enforcement Gate]] (formerly "Build vault-cli execute-task slash command", renamed problem-framed during plan-task gate).

[ben-s-pull-request-reviewer]

{
  "verdict": "request-changes",
  "summary": "The execute-task slash command is well-designed with clear idempotency, refusal hierarchy, and self-documented trade-offs. However, two important gaps require fixes before merge: (1) the frontmatter is missing the required `name` field, and (2) the CHANGELOG entry omits three material behaviors — the status entry contract (step 4 mutates `next/backlog/hold → in_progress`), the shipping-class scope carve-out for hard check #3, and the 4-priority task-detection chain — making the entry misleading for anyone reading it to understand what the command actually does to a task file.",
  "comments": [
    {
      "file": "commands/execute-task.md",
      "line": 1,
      "severity": "major",
      "message": "Should Fix: Missing `name` field in YAML frontmatter. Per the Skill Writing Guide, a `name: execute-task` field is required. The file's location in `commands/` makes the filename the de-facto identifier, but the frontmatter should still declare it explicitly."
    },
    {
      "file": "commands/execute-task.md",
      "line": 3,
      "severity": "major",
      "message": "Should Fix: Unquoted `argument-hint` contains `<task-file-path-or-name>`. The `<` character is a YAML tag indicator — while other vault-cli commands share this pattern (plan-task.md line 3), sibling commands like `complete-task.md` and `create-task.md` correctly quote theirs. Fix: `argument-hint: \"<task-file-path-or-name> (or detects from conversation)\"`."
    },
    {
      "file": "CHANGELOG.md",
      "line": 13,
      "severity": "major",
      "message": "Should Fix: CHANGELOG omits the status entry contract (step 4) — flipping `next/backlog/hold → in_progress` before the hard checks run. This is a real file mutation that silently changes task state; operators deserve to know. Add to the entry: \"also promotes `status: next/backlog/hold → in_progress` as a resume signal before gating\"."
    },
    {
      "file": "CHANGELOG.md",
      "line": 13,
      "severity": "major",
      "message": "Should Fix: CHANGELOG implies hard check #3 (e2e verify subtask) applies to all tasks. It does not — the command explicitly skips it for non-shipping-class tasks (pure research, decision, doc-only). A reader would incorrectly believe every task without an e2e subtask is rejected. Add: \"(shipping-class tasks only — PR/release/deploy/plugin/agent/library publish)\"."
    },
    {
      "file": "CHANGELOG.md",
      "line": 13,
      "severity": "major",
      "message": "Should Fix: CHANGELOG omits the 4-priority task-detection chain (step 1) — conversation scan, wikilink, daily-note checkbox, MRU file. The command's input contract is a primary user-facing behavior not captured in the description."
    },
    {
      "file": "commands/execute-task.md",
      "line": 65,
      "severity": "nit",
      "message": "Nice to Have: The 4 hard non-negotiables are copy-pasted inline from plan-task.md rather than shared. The inline note documents the design rationale and refactor trigger correctly — acceptable as-is, but be aware that any future drift between the two implementations will not be caught mechanically. The `vault-cli task verify-plan` CLI verb lift path is the correct solution when a third caller appears."
    },
    {
      "file": "commands/execute-task.md",
      "line": 41,
      "severity": "nit",
      "message": "Nice to Have: No `vault-cli` availability check — if the binary is absent or broken, the command's behavior is undefined. Consider a lightweight preflight (e.g., `vault-cli --version`) with a clear error before any state mutation."
    },
    {
      "file": "CHANGELOG.md",
      "line": 13,
      "severity": "nit",
      "message": "Nice to Have: The changelog doesn't mention that `phase: ai_review` and `phase: human_review` are also treated as idempotent re-entry phases (same as `phase: execution`). Minor — only matters if someone reads the command source."
    },
    {
      "file": "CHANGELOG.md",
      "line": 13,
      "severity": "nit",
      "message": "Nice to Have: The changelog doesn't mention that when all `# Tasks` checkboxes are already checked, the command prints \"All subtasks complete — run /vault-cli:complete-task\" instead of the \"Start with\" line. Minor terminal branch."
    }
  ],
  "concerns_addressed": [
    "correctness: Status entry contract (step 4) — confirmed behavior; verified present in command (line 55-59). CHANGELOG gap: not documented (raised as Should Fix).",
    "correctness: Hard checks duplicated inline — acknowledged as intentional design (line 65). Refactor trigger documented. Not a bug.",
    "correctness: Idempotency on phase:execution — confirmed at lines 99 and 123. Re-runs re-surface context without mutation.",
    "correctness: phase:empty handling — confirmed at line 51: refuses `phase` empty with `status: in_progress`.",
    "security: vault-cli task set commands — reviewed. Command only flips phase and status flags; no arbitrary command injection, no cross-task contamination vectors.",
    "security: The command reads ~/.claude/plugins/marketplaces/vault-cli/docs/task-writing.md as the canonical rule source — an external file. If that file is absent, the behavior is undefined (graceful degradation not specified). Not a blocking finding but worth hardening.",
    "tests: No automated test coverage — confirmed. Command is entirely manual tested. No automated tests for the 4 hard checks, phase transitions, or idempotency. Raised as concern in plan but not actionable in PR review."
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
  "verdict": "approve",
  "summary": "The `/vault-cli:execute-task` slash command is well-designed and correctly implements the hard gate between planning and execution. The logic is sound across all refusal cases, idempotent re-entry, and status/phase transition matrix. The code correctly stays inline, frontmatter is consistent with sibling commands, and the 4 hard checks are appropriately marked as intentional duplication (with a shared-source comment and a lift trigger). One Should Fix (documentation of the duplication maintenance risk) and one Nice to Have (race condition caveat), but neither is a blocker.",
  "comments": [
    {
      "file": "commands/execute-task.md",
      "line": 66,
      "severity": "major",
      "message": "Should Fix: The 4 hard checks are copy-pasted from plan-task rather than shared. The note says 'lift to a vault-cli task verify-plan CLI verb if a third caller appears' — consider tracking this in a docs/decision-records/ or as a FIXME comment so the duplication doesn't silently drift when task-writing.md is updated."
    },
    {
      "file": "commands/execute-task.md",
      "line": 56,
      "severity": "nit",
      "message": "Nice to Have: The status flip (`next/backlog/hold → in_progress`) is not atomic with the subsequent hard-check / phase flip. If two concurrent invocations run on the same task, both could flip status before either reaches phase transition. A comment noting this as a known trade-off (intentional for ergonomics) would help future maintainers."
    }
  ],
  "concerns_addressed": [
    "correctness: Complex state machine (4 refusal + idempotent re-entry + status/phase flips) — reviewed, logic is correct. Refusal cases are exhaustive and mutually exclusive, phase transitions are gated correctly, idempotent paths do not mutate.",
    "correctness: 4 hard checks duplicated verbatim from plan-task — acknowledged in command docs with explicit rationale and lift trigger. Not a blocker but noted as Should Fix for maintenance.",
    "correctness: Task path detection (4-source priority chain) — correct priority order, multiple matches ask via AskUserQuestion, zero matches is terminal error. Logic is sound.",
    "security: Task name used in Glob patterns and vault-cli invocations — task names come from vault-cli task resolution (not raw user input), and the command escapes arguments appropriately via vault-cli's own argument handling. No injection risk identified.",
    "correctness: Status flip race condition — acknowledged as Nice to Have. Not a blocking issue.",
    "tests: No automated test file changes — this is a markdown-only slash command definition; no Go unit tests are appropriate for this change."
  ]
}