feat(goal): align goal status enum with task statuses

[bborbe]

Summary

• Expand `AvailableGoalStatuses` to include `next, in_progress, backlog, hold, aborted` (task-aligned set) alongside existing `active, completed, on_hold` (kept as legacy aliases).
• Unblocks task-orchestrator drag-and-drop on the Goals view (PR feat(goals): drag-and-drop status changes on the Goals view task-orchestrator#19), which writes `in_progress` etc. from the same UI control set used for tasks.

Why

Personal vault has 184 goals using task-like statuses on disk (59 completed, 45 backlog, 30 next, 7 in_progress, 4 aborted, 1 hold, 1 active). Reads via `goal list` are lax (any string in frontmatter), but writes via `goal set status` validated against the original 3-value enum and rejected `in_progress` etc. This PR aligns the write validator with actual practice.

Test plan

• `make precommit` green (all 8 packages, 0 issues)
• Manual: `vault-cli goal set status in_progress --vault personal` succeeds after install

[ben-s-pull-request-reviewer]

I have all the data needed. Here is the consolidated review:

---

PR Review: feat(goal): align goal status enum with task statuses

Files changed: pkg/domain/goal.go (1 file, +27/-4 lines)

---

Must Fix (Critical)

None — no security vulnerabilities, data correctness bugs, or critical issues found.

---

Should Fix (Important)

1. Asymmetric normalization vs TaskStatus — goal_frontmatter.go:29-30

GoalFrontmatter.Status() returns the raw frontmatter string without normalization:

func (f GoalFrontmatter) Status() GoalStatus {
    return GoalStatus(f.GetString("status"))  // no normalization
}

In contrast, TaskFrontmatter.Status() at task_frontmatter.go:29-33 applies NormalizeTaskStatus before returning, mapping legacy aliases to canonical values. The PR's backward-compatibility strategy (include legacy constants in AvailableGoalStatuses) fixes the validation layer — legacy values like "active" now validate — but the read path remains asymmetric. A vault goal file with "status: active" will have Status() return GoalStatusActive, while a task file with "status: todo" normalizes to TaskStatusNext. Code that switches on Status() to drive UI logic (Kanban columns, filters) will see two distinct enum values for semantically identical states.

The fix would be adding a NormalizeGoalStatus function (mirroring NormalizeTaskStatus) and calling it in GoalFrontmatter.Status(). This is the same pattern already established for TaskStatus.

2. No test coverage for goal status enum — pkg/domain/

task_status_test.go has 8+ test cases covering canonical values, validation, normalization, and alias mapping. No equivalent goal_status_test.go exists. The new enum values (GoalStatusNext, GoalStatusInProgress, GoalStatusBacklog, GoalStatusHold, GoalStatusAborted) have zero dedicated unit tests. Legacy constants (GoalStatusActive, GoalStatusOnHold) also lack test coverage. The existing goal_frontmatter_test.go only tests GoalStatusActive via the legacy path — it does not exercise the new canonical statuses.

Recommend adding pkg/domain/goal_status_test.go covering: Validate() for all canonical values, Validate() rejection of unknown values, and backward-compatibility reads of legacy values.

---

Nice to Have (Optional)

3. Imprecise comment at goal.go:63

The comment says // AvailableGoalStatuses lists all valid canonical goal status values. — but the slice includes legacy constants (GoalStatusActive, GoalStatusOnHold) which are not canonical. Either update the comment to clarify it includes legacy values, or move the legacy values to a separate LegacyGoalStatuses slice.

---

Precommit / CI

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

---

{
  "verdict": "request-changes",
  "summary": "The enum expansion is correct and backward-compatible at the validation layer, but GoalFrontmatter.Status() lacks normalization — unlike TaskFrontmatter.Status() — creating an asymmetric read path where legacy goal statuses don't canonicalize to task-aligned values. Additionally, the new goal status enum values have no dedicated test coverage.",
  "comments": [
    {
      "file": "pkg/domain/goal_frontmatter.go",
      "line": 29,
      "severity": "major",
      "message": "Should Fix (Important): GoalFrontmatter.Status() returns raw values without normalization, unlike TaskFrontmatter.Status() which applies NormalizeTaskStatus. A goal file with \"status: active\" returns GoalStatusActive rather than GoalStatusInProgress, breaking semantic alignment with TaskStatus at the read layer. Add NormalizeGoalStatus mirroring NormalizeTaskStatus and apply it in Status()."
    },
    {
      "file": "pkg/domain/",
      "line": 0,
      "severity": "major",
      "message": "Should Fix (Important): No test file for goal status enum. task_status_test.go covers all canonical values, normalization, and aliases; goal_status_test.go does not exist. New values (GoalStatusNext, GoalStatusInProgress, GoalStatusBacklog, GoalStatusHold, GoalStatusAborted) and legacy backward-compatibility paths lack any unit tests."
    },
    {
      "file": "pkg/domain/goal.go",
      "line": 63,
      "severity": "nit",
      "message": "Nice to Have (Optional): Comment says 'all valid canonical goal status values' but AvailableGoalStatuses includes legacy non-canonical constants GoalStatusActive and GoalStatusOnHold. Clarify the comment or split into canonical/legacy slices."
    }
  ],
  "concerns_addressed": [
    "correctness: AvailableGoalStatuses validates both canonical and legacy values — validation logic is correct, legacy values pass validation. However, the normalization layer (NormalizeGoalStatus) is missing, creating asymmetry with how TaskStatus handles this.",
    "correctness: Backward compatibility for existing goal files is preserved — files with legacy status values deserialize and validate correctly. But the lack of normalization means GoalStatusActive is a distinct value from GoalStatusInProgress at the type level, not a true alias.",
    "tests: No test coverage for goal status enum — confirmed. goal_status_test.go does not exist, and no tests exercise the new canonical values."
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

I now have everything needed for the in-session adjudication. Let me compile the findings.

---

PR Review: feat(goal): align goal status enum with task statuses

Review basis: git diff origin/master...HEAD — 2 files changed (CHANGELOG.md, pkg/domain/goal.go)

---

Findings

Correctness

1. GoalStatus has no NormalizeGoalStatus() — alias semantics not enforced

TaskStatus has NormalizeTaskStatus() which converts legacy aliases (todo → next, current → in_progress, done → completed, deferred → hold) to their canonical form at write time. GoalStatus has no equivalent. The PR comment says active ↔ in_progress and on_hold ↔ hold are semantic aliases, but:

• goal.SetStatus("active") stores "active" as-is (no normalization)
• task.SetStatus("active") would normalize to TaskStatusInProgress

This means files written with legacy goal statuses never get migrated to canonical values. The alignment is at the enum-level only, not at the normalization level. The concern from the PR plan is partially valid — the backward compatibility works (reads and writes both tolerate legacy values), but the alias→canonical migration that NormalizeTaskStatus provides for tasks is absent for goals.

2. GoalStatus.Validate() doesn't normalize — it only accepts

GoalStatus.Validate() checks membership in AvailableGoalStatuses (which includes both canonical and legacy values). It passes "active" as a valid goal status but does not convert it to "in_progress". This is by design (backward compatibility), but means the alias semantics are tolerated, not normalized.

3. AvailableGoalStatuses duplicates are not problematic
The concern about "duplicate validation entries" is not valid — AvailableGoalStatuses is a flat list used only for Contains() checks. Linear scan over 8 items is O(n) with n=8, trivially fast. No duplicate map entries or unexpected behavior results.

Tests

4. No automated test coverage for new GoalStatus values

pkg/domain/task_status_test.go tests all 6 canonical task statuses + normalization. pkg/domain/ has no goal_status_test.go. The existing goal_frontmatter_test.go tests goal creation and basic status field behavior but does not test:

• GoalStatusNext, GoalStatusInProgress, GoalStatusBacklog, GoalStatusHold, GoalStatusAborted validation
• Legacy aliases (GoalStatusActive, GoalStatusOnHold) validation

The PR body confirms manual test required: vault-cli goal set <name> status in_progress --vault personal. This is a test gap — Should Fix.

Documentation

5. CHANGELOG entry is clear and accurate
The v0.90.0 entry correctly describes: the new canonical values, the backward-compat aliases, the behavioral effect, and the unblocking rationale with issue reference.

---

Must Fix (Critical)

None.

Should Fix (Important)

1. Test coverage gap — pkg/domain/goal_status.go has no dedicated test file. The new canonical statuses (next, in_progress, backlog, hold, aborted) and legacy aliases (active, on_hold) lack automated validation tests. Consider adding a goal_status_test.go mirroring the pattern in task_status_test.go.

Nice to Have (Optional)

1. Consider NormalizeGoalStatus() — If the goal is true behavioral alignment with TaskStatus (not just enum-value alignment), a NormalizeGoalStatus("active") → GoalStatusInProgress function would enable migration of legacy vault files to canonical values at write time. Currently legacy goal statuses persist indefinitely in vault files.

---

Traceability (Selector Mode)

Concern	Rule Trigger	File	Status
Semantic alias normalization	None triggered (ast-grep unavailable)	pkg/domain/goal.go:47-91	Raised as Should Fix
AvailableGoalStatuses expansion	None triggered	pkg/domain/goal.go:66-75	Not a bug
Missing test coverage	None triggered	pkg/domain/goal.go	Raised as Should Fix

Mechanicals: ast-grep-runner unavailable (toolchain gap), skipped per workflow §4.0.
Judgment rules: none applicable (no relevant rules match the changed files).

---

{
  "verdict": "approve",
  "summary": "The goal status enum expansion is well-designed and correctly adds the 6 canonical statuses aligned with TaskStatus, preserving backward compatibility with legacy values. The two concerns from the PR plan are either not bugs (duplicate validation entries) or are by-design behavior (legacy values tolerated but not normalized). One should-fix remains: no automated test coverage for the new GoalStatus values.",
  "comments": [
    {
      "file": "pkg/domain/goal.go",
      "line": 86,
      "severity": "major",
      "message": "Should Fix: GoalStatus.Validate() accepts legacy aliases (active, on_hold) but never normalizes them to canonical forms (in_progress, hold). Compare with TaskStatus.NormalizeTaskStatus() which converts legacy→canonical at write time. If behavioral alignment with TaskStatus is the goal, consider adding a NormalizeGoalStatus() function so legacy goal statuses get migrated to canonical values when written."
    },
    {
      "file": "pkg/domain/goal.go",
      "line": 1,
      "severity": "nit",
      "message": "Nice to Have: No dedicated goal_status_test.go exists. The new canonical statuses (next, in_progress, backlog, hold, aborted) and legacy aliases (active, on_hold) lack automated validation tests. Consider adding tests mirroring the pattern in task_status_test.go."
    }
  ],
  "concerns_addressed": [
    "correctness: semantic aliases are string-different but tolerated (not normalized) — Validate() accepts legacy values but does not convert them. This is by design but falls short of full TaskStatus alignment.",
    "correctness: AvailableGoalStatuses expansion does not produce duplicate validation entries — Contains() is a simple linear scan over a flat list, no issue.",
    "tests: no automated test coverage for new status values — manual test vault-cli goal set ... status in_progress confirmed needed by PR body. Automated tests are missing."
  ]
}

superseded by new automated review

[ben-s-pull-request-reviewer]

{
  "verdict": "request-changes",
  "summary": "The goal status enum expansion is well-designed and backward-compatible, but the version alignment check fails: CHANGELOG.md has been reverted to `## Unreleased` while the three plugin JSON files (`.claude-plugin/plugin.json`, `.claude-plugin/marketplace.json`) still carry `v0.89.0`. All four version strings must match per CLAUDE.md § Version Alignment. The code itself is correct.",
  "comments": [
    {
      "file": ".claude-plugin/plugin.json",
      "line": 1,
      "severity": "major",
      "message": "Should Fix: version field is `\"0.89.0\"` but the feature is a minor release (new goal statuses aligned with task statuses). Per CLAUDE.md § Version Alignment, all four version strings must equal each other. Update to `\"0.90.0\"` to match the changelog entry in CHANGELOG.md."
    },
    {
      "file": ".claude-plugin/marketplace.json",
      "line": 1,
      "severity": "major",
      "message": "Should Fix: both `metadata.version` and `plugins[0].version` fields are `\"0.89.0\"`. Per CLAUDE.md § Version Alignment, all four version strings must equal each other. Update both to `\"0.90.0\"` to match CHANGELOG.md."
    }
  ],
  "concerns_addressed": [
    "correctness: GoalStatusActive kept as legacy alias — Verified: AvailableGoalStatuses includes all 8 values (6 canonical + 2 legacy), SetStatus validates against AvailableGoalStatuses, so callers using string comparison 'active' still work correctly",
    "correctness: Semantic aliasing (active ↔ in_progress, on_hold ↔ hold) documented in comments — Confirmed: pkg/domain/goal.go:38-61 documents this clearly. No business logic beyond validation depends on these equivalences"
  ]
}