feat: enforce one-task-per-session contract (closer panel + session-close anchor check)

[bborbe]

Summary

Three slash commands updated to enforce the one-task-per-session contract end-to-end:

• /vault-cli:sync-progress — new Phase 6 appends a ⚪ DONE state-closer panel below the report when Phase 4 actually completed a task (auto via 4a or confirmed via 4b). PR-only and progress-only sync paths skip it.
• /vault-cli:complete-task — MODE=interactive step 2e appends the same panel after ✅ Task completed. MODE=tool step 3e explicitly forbids it — JSON output stays clean.
• /vault-cli:session-close — new Phase 4.5 scans the session's touched vault tasks; any task still status: in_progress surfaces as outstanding with concrete next-actions (/complete-task, /defer-task, or set status hold/aborted). Scoped to TOUCHED tasks only — untouched [/] items on today's daily note belong to other sessions or the orchestrator and are intentionally NOT flagged.

Plus a related cleanup driven by /coding:pr-review (commit 3):

• complete-task frontmatter gets the missing allowed-tools block (Bash(vault-cli:*), Read).
• complete-task interactive incomplete-items gate replaces AskUserQuestion with abort + --force hint — less friction on the common path, same safety on partial completions.
• CHANGELOG nested sub-bullets flattened to top-level entries (every bullet under ## Unreleased now has a conventional prefix so the auto-release watcher parses cleanly).

Why

Two related drift modes were observed:

1. After /vault-cli:sync-progress and /vault-cli:complete-task, Claude kept inventing closer panels recommending /vault-cli:next-task — wrong for one-task-per-session, where queued daily-note items get fresh sessions via the orchestrator, never appended to the current one.
2. /vault-cli:session-close happily printed ✅ Nothing to do — safe to close Claude even when the session's anchor task was still in_progress — undermining the same contract from the opposite side.

The closer panel tells you to close the session; the session-close anchor check refuses to call it clean if the task isn't actually done. Both rationales are documented inline so future readers (human + LLM) don't re-derive them.

Closer shape (verbatim)

⚪ DONE
👤 You: approve: /vault-cli:session-close
⏰ Next: your reply

Review-loop dismissals (documented)

• command-thin × 3 files — pre-existing condition (all three commands are over the 100-line guideline ceiling). Each command's own docs explicitly state it must stay inline because it analyzes the parent conversation. Out of scope for this PR; not introduced by this diff.
• imperative-mood on commit 2 subject (feat: session-close warns when touched task still in_progress — warns is 3rd-person, not bare imperative). Reword would require amend + force-push; declined. Master uses --merge commits, so the merge commit subject is the user-visible record on master; this commit subject only lives in the merge commit's parent.

Test plan

• make precommit green locally (after each commit)
• Post-merge live E2E: /vault-cli:sync-progress on a completed task → ⚪ DONE panel with approve: /vault-cli:session-close
• Post-merge live E2E: /vault-cli:complete-task interactively on a complete task → same closer after ✅ Task completed
• Post-merge live E2E: /vault-cli:complete-task --tool → JSON only, no closer
• Post-merge live E2E: /vault-cli:complete-task on an incomplete task → abort with --force hint, no prompt
• Post-merge live E2E: /vault-cli:session-close on a session that touched a still-in_progress task → Phase 4.5 surfaces it; on a session with a completed touched task → no false positive
• Post-merge: git fetch origin --tags && git tag --list | tail -1 shows new vX.Y.Z (auto-release watcher); .claude-plugin/plugin.json version matches

🤖 Generated with Claude Code

[ben-s-pull-request-reviewer]

Sandbox is blocking. Let me proceed with the in-session review since I have all the changed files.

---

PR Review: feat: enforce one-task-per-session contract (closer panel + session-close anchor check)

Changed Files Reviewed

• commands/complete-task.md
• commands/session-close.md
• commands/sync-progress.md
• CHANGELOG.md

---

Must Fix (Critical)

None. No security vulnerabilities, no correctness bugs, no architectural violations in the diff.

---

Should Fix (Important)

1. session-close.md — Phase 4.5: No error handling for vault-cli task get failure

vault-cli task get "$T" status --output json

If vault-cli is unavailable, the binary errors, or the JSON is malformed, the phase silently continues without surfacing any warning. The phase iterates touched tasks and calls this for each — a single failure should at minimum produce a warning rather than silently skipping the check.

File: commands/session-close.md:136-138

---

Nice to Have (Optional)

2. sync-progress.md — Phase 6 closer panel: skip-condition rationale could be more explicit

The Phase 6 skip rule says: "Skip the closer entirely when Phase 4 did NOT complete a task — i.e. PR-only sync, progress-only sync, or any path where the active task is still in_progress."

The PR-only and progress-only paths are clear. However, the in_progress path is ambiguous: if Phase 4 runs but doesn't complete a task (e.g., criteria 1–4 not all met, falls through to AskUserQuestion, user says "Hold"), the closer should indeed be skipped. The current language covers this but could distinguish "Phase 4 skipped" from "Phase 4 ran but chose not to complete" to prevent future misinterpretation.

File: commands/sync-progress.md:244

3. complete-task.md — Step 2e: cross-ref rationale is correct but fragile

The step 2e closer panel cross-references sync-progress.md Phase 6 for the rationale ("same contract applies"). If one doc is updated and the other isn't, they'll drift. The PR itself addresses this for this change, but there's no mechanical enforcement (e.g., a lint rule or test that asserts both panels are identical). Consider extracting the panel text into a shared include or asserting parity in make precommit.

File: commands/complete-task.md:50

4. CHANGELOG.md — Unreleased section correctly captures all four changes

The changelog accurately describes: (1) Phase 6 + step 2e ⚪ DONE closer, (2) session-close Phase 4.5 anchor check, (3) complete-task AskUserQuestion→abort refactor, (4) --force flag. Well-structured.

---

Precommit Note

precommit skipped (selector mode) — CI covers lint+test

The project has make precommit which runs lint+test+checks. No Go code changed — all changes are markdown command docs + CHANGELOG. The precommit check for version alignment (check-versions) would pass since no version files were modified.

---

Selector Mode Traceability

Rule	Owner	Triggered?	Finding
incomplete-items-gate	go-quality-assistant	commands/complete-task.md	✅ The refactor from AskUserQuestion to abort-with-hint is correctly implemented
closer-panel-contract	go-architecture-assistant	sync-progress.md, complete-task.md	✅ Both emit ⚪ DONE panel; MODE=tool correctly suppressed
session-anchor-check	go-architecture-assistant	session-close.md	⚠️ Phase 4.5 has no error handling for vault-cli task get failure

---

Summary

The PR correctly implements the one-task-per-session contract: sync-progress and complete-task now emit a ⚪ DONE closer pointing at session-close, and session-close Phase 4.5 verifies the anchor task was actually completed before calling it clean. The complete-task refactor from interactive prompt to abort-with-hint reduces friction on the common path. One actionable gap: Phase 4.5 should handle vault-cli task get failures gracefully rather than silently continuing.

{
  "verdict": "request-changes",
  "summary": "The one-task-per-session contract is correctly implemented across all three commands, but Phase 4.5 in session-close lacks error handling when vault-cli task get fails, silently skipping the anchor-task check on error.",
  "comments": [
    {
      "file": "commands/session-close.md",
      "line": 136,
      "severity": "major",
      "message": "Should Fix: Phase 4.5 calls `vault-cli task get \"$T\" status --output json` with no error handling. If vault-cli fails or returns unexpected output, the phase silently continues without warning. Consider adding: capture stderr, check exit code, and surface a warning if the anchor-task check cannot be performed."
    },
    {
      "file": "commands/sync-progress.md",
      "line": 244,
      "severity": "nit",
      "message": "Nice to Have: Phase 6 skip rule conflates 'Phase 4 skipped' and 'Phase 4 ran but chose not to complete' — both skip the closer but have different root causes. Adding a brief distinction would prevent future misinterpretation."
    },
    {
      "file": "commands/complete-task.md",
      "line": 50,
      "severity": "nit",
      "message": "Nice to Have: The closer panel cross-references sync-progress.md Phase 6 for rationale. No mechanical enforcement ensures both stay in sync. Consider a precommit check that asserts panel text parity between the two files."
    }
  ],
  "concerns_addressed": [
    "correctness: Phase 6 closer panel skip logic is correctly implemented — PR-only and progress-only paths skip, in_progress path skips, task-complete path emits",
    "correctness: Phase 4.5 anchor check correctly iterates touched tasks and interprets status values — but error handling gap identified",
    "correctness: Incomplete-items gate correctly refactored from AskUserQuestion to abort-with-hint; --force flag bypasses gate in interactive mode; MODE=tool always sets phase=human_review",
    "tests: precommit uses shell/assertion checks; no Go test files changed; all changes are markdown docs — precommit CI covers what would be testable"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
  "verdict": "approve",
  "summary": "PR #18 enforces the one-task-per-session contract via three coordinated changes: (1) `complete-task` and `sync-progress` emit a `⚪ DONE` state-closer pointing at `session-close` after a task completes, replacing the prior drift where Claude invented `next-task`; (2) `session-close` Phase 4.5 now scans touched vault tasks and surfaces any still `in_progress` as outstanding before close, closing the loop opposite to the closer; (3) the `AskUserQuestion` gate in `complete-task` is replaced with a non-interactive abort hint plus a `--force` bypass. All three concerns from the plan are correctly addressed: Phase 4.5 is scoped to TOUCHED tasks only and surfaces failed lookups; Phase 6 panel only fires when Phase 4 actually completed a task; MODE=tool path is explicitly guarded from emitting the closer.",
  "comments": [],
  "concerns_addressed": [
    "correctness: session-close Phase 4.5 correctly scopes to touched tasks only, does not check daily-note [/] items, and surfaces failed status lookups as outstanding (8b25dc1)",
    "correctness: sync-progress Phase 6 panel only fires when Phase 4 completed a task — PR-only and progress-only sync paths skip the closer per explicit skip logic",
    "correctness: complete-task MODE=tool step 3e explicitly suppresses the ⚪ DONE closer; vault-cli task complete call in MODE=tool is JSON-only"
  ]
}