feat: promote goal to in_progress in work-on-goal-assistant

[bborbe]

What

work-on-goal-assistant now promotes a goal's status to in_progress when work starts — mirroring how work-on-task-assistant does it for tasks.

Why

Previously /vault-cli:work-on-goal was fully read-only: starting work on a goal left its status untouched, so goals never auto-advanced to in_progress the way tasks do. This closes that asymmetry.

How

• Added a <critical_writes> block (placed before <constraints>, matching the task-assistant layout) describing the mandatory mutation.
• Phase 1 runs vault-cli goal set "{goal}" status in_progress immediately after reading the goal, before guide search, so it can't be skipped.
• Terminal goals (completed / aborted) are not auto-reopened; already-in_progress goals are a silent no-op.
• Non-zero exit → ⚠️ Could not set status (never a false ✅).
• Uses goal set because vault-cli goal work-on does not exist (documented inline to prevent future confusion).

Notes

• Audited with coding:agent-auditor (7/10 → all findings addressed: tag ordering, failure branch, CLI-asymmetry note, Status (before): clarification, color: blue).
• Agent-definition markdown change → Direct edit per vault-cli CLAUDE.md (no dark-factory pipeline).

🤖 Generated with Claude Code

[ben-s-pull-request-reviewer]

Based on the diff analysis, I have enough information to provide the review. The changed file is a Claude Code agent definition (.md), not Go/Python code, so the mechanical funnel (ast-grep) is not applicable — it only scans code files. Let me analyze the changes directly.

Analysis of the PR:

The PR adds goal status promotion to in_progress when starting work on a goal, mirroring the existing behavior in work-on-task-assistant. Key changes:

1. New <critical_writes> section — MANDATORY mutation that must succeed or report ⚠️
2. Phase 1 now promotes status — after reading goal, before guide search
3. Updated output format — shows status transition
4. Updated constraints — READ-ONLY except for the status mutation
5. Updated success_criteria — includes status promotion as criterion #2

Concern review from the plan:

• Command syntax "{goal_name}": Double-quoting the goal name handles spaces correctly. For special shell characters ($, `, ), this could be problematic, but goal names in a task management vault rarely contain such characters. This is acceptable.
• Terminal goals (completed/aborted): Correctly skipped — never auto-reopened.
• Error handling: Non-zero exit correctly reports ⚠️ and continues without claiming ✅.
• No tests: This is an agent definition file, not code — tested via scenarios, not unit tests.

Issues found:

1. Minor: The output format shows Status (before): <status> AND ✅ Goal status: ... on two separate lines. This is redundant — Status (before) is only meaningful if followed by Status (after), but the after-status is implicit in the transition message. However, this is a cosmetic issue, not a bug.

{
  "verdict": "approve",
  "summary": "PR adds goal-to-in_progress promotion to work-on-goal-assistant, correctly mirroring work-on-task-assistant behavior. Terminal goals (completed/aborted) are properly skipped, error handling correctly reports ⚠️ on failure without claiming success, and status promotion runs before guide search so it cannot be forgotten mid-workflow.",
  "comments": [],
  "concerns_addressed": [
    "correctness: command syntax \"{goal_name}\" is correct for goal names with spaces; special shell chars are unlikely in vault goal names",
    "correctness: terminal goals (completed/aborted) are not auto-reopened — confirmed correct per critical_writes skip rules and error_handling block",
    "correctness: non-zero exit from vault-cli goal set correctly reports ⚠️ and continues, does not claim ✅",
    "tests: no test files modified — agent .md files are tested via scenarios, not unit tests"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
  "verdict": "approve",
  "summary": "The PR adds goal-to-in_progress status promotion to work-on-goal-assistant, mirroring the existing work-on-task-assistant behavior. The implementation is correct: status is recorded in Phase 1 and displayed in the context block (addressing concern #2). Shell interpolation risk for goal_name is theoretical — constrained by glob-based discovery but worth noting. No blocking issues found.",
  "comments": [
    {
      "file": "agents/work-on-goal-assistant.md",
      "line": 21,
      "severity": "major",
      "message": "Should Fix: goal_name is interpolated into a double-quoted bash command without explicit shell escaping. While constrained by glob discovery (*{name}*.md), consider validating goal_name contains no double-quotes or dollar signs before interpolation, or useprintf '%q' for safe shell interpolation."
    }
  ],
  "concerns_addressed": [
    "correctness: context propagation — status transition IS recorded in Phase 1 (lines 74-78) and displayed in context block (line 146) ✅",
    "correctness: terminal state reopen — user must explicitly choose to reopen; critical_writes skip is respected ✅",
    "security: shell injection — theoretical risk exists but constrained by glob discovery; no untrusted input path"
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
  "verdict": "approve",
  "summary": "PR correctly implements goal status promotion to in_progress, mirroring work-on-task-assistant. All six concerns are addressed: goal_name variable correctly populated from Phase 1 read, terminal states (completed/aborted) properly guarded, non-zero exits show ⚠️ not ✅, already-in_progress goals show ℹ️, shell quoting is correct, and color change (purple→blue) is consistent with goal-auditor and work-on-task-assistant (both blue).",
  "comments": [],
  "concerns_addressed": [
    "correctness: goal_name variable populated from Phase 1 goal read and consistently used in vault-cli goal set command",
    "correctness: terminal-state guard (completed/aborted) properly implemented - do NOT auto-reopen under any ordering or error condition",
    "correctness: non-zero exit from vault-cli goal set produces ⚠️ warning, never a false ✅",
    "correctness: already-in_progress goals show ℹ️ (not misleading ✅ or ⚠️)",
    "security: goal name properly quoted in vault-cli goal set \"{goal_name}\" command to prevent injection",
    "correctness: color changed from purple to blue - intentional and consistent with work-on-task-assistant.md (blue) and goal-auditor.md (blue)"
  ]
}