Skip to content

Lifecycle status enum: task-pr-land/finish write invalid statuses (merged_to_main, shipped) #565

@psdjungpulzze

Description

@psdjungpulzze

The /i/task-pr-land and /i/task-finish runbooks set status: merged_to_main and status: shipped, but the tasks file-sync API rejects them (HTTP 500) because they are not valid Task status enum values (valid: no_status, spec_drafted, todo, in_progress, pr_open, in_review, staging, blocked, done, cancelled).

Effect: post-land bookkeeping silently fails — the task stays in_progress after a successful merge until manually corrected to done. Surfaced while landing T164 (PR #556).

Fix options: (a) add merged_to_main/shipped to the status enum + lifecycle mapping, or (b) change the runbooks to write valid terminal statuses (done). Decide one and align runbooks + enum + any UI status rendering.

Problem statement

The lifecycle runbooks write task statuses (merged_to_main, shipped) that aren't in the valid status set, and the file-sync route forwards them unvalidated to the DB write — so a successful PR land silently fails its status bookkeeping with an opaque 500 and the task is stranded at in_progress.

Acceptance criteria

  • POST /api/v1/projects/:id/tasks/file-sync with an unknown status returns 422 invalid_status (with the offending value + the valid set in the message) instead of a 500 — and does not write the task.
  • A valid status continues to update the task as before (no regression on the existing happy-path + implementation-gate behavior).
  • task-pr-land, task-deploy-prod, and task-finish runbooks only ever write statuses in the canonical set (no merged_to_main / shipped).
  • Running the land→finish lifecycle on a real task leaves it at a valid terminal status (done) with no manual correction needed.
  • There is a single exported canonical status set that the file-sync validation imports (no new hardcoded copy of the list).

In scope

  • src/lib/tasks/status-utils.ts — export a canonical VALID_TASK_STATUSES set/array derived from DEFAULT_TASK_STATUSES (single source of truth for validation)
  • src/app/api/v1/projects/[projectId]/tasks/file-sync/route.ts — validate status against the canonical set; return 422 invalid_status before the DB write
  • .claude/commands/i/task-pr-land.md — write a valid status (see Decisions) instead of merged_to_main
  • .claude/commands/i/task-deploy-prod.md — write a valid status instead of shipped
  • .claude/commands/i/task-finish.md — confirm it writes done (already correct; adjust only if the chosen mapping changes it)
  • Unit tests for the file-sync status validation

Out of scope

  • Adding merged_to_main / shipped as new first-class statuses (option (a) — rejected; see Decisions)
  • Consolidating the other 4+ hardcoded status lists (phase-board.tsx, sprints-client.tsx, lifecycle-prompts.ts, the TASK_STATUS_GUIDE string) into the canonical export — tracked separately; this task only wires the file-sync validator to one source
  • Fixing the PR-merge webhook auto-close reliability (it sets done on merge but didn't fire for T164) — separate concern
  • UI status label/colour maps (plan-client.tsx, sprints-client.tsx) — unaffected since no new status is introduced
  • Promoting Task.status to a Prisma/Postgres enum (larger migration; the 422 guard covers the gap)

Implementation Plan

Goal: Make invalid task statuses fail fast and clearly at the file-sync boundary (422, not 500), and stop the lifecycle runbooks from writing statuses that aren't in the valid set.

Decisions:

  • Confirm the 500 root cause first. Task.status is a plain String in the local Prisma schema (schema.prisma:951), which alone would not 500 on an arbitrary value — production returned 500, so the first todo reproduces and pins the cause (a Postgres-level CHECK/enum constraint on the deployed DB, or a downstream throw in the route after the write). The 422 guard is the right fix regardless, but the implementer must verify it actually pre-empts the observed 500.
  • Option (b), not (a). The merge webhook already maps merged PRs → done (integration-sync.ts:taskStatusFromPRAction); merged_to_main/shipped carry no lifecycle or UI semantics (deriveLifecycleStage defaults them to not_started; no status-label map renders them). Adding them everywhere is pure cost with no benefit.
  • Runbook status mapping (operator-approved): task-pr-landstaging (it triggers the staging deploy and staging is a valid status), task-deploy-proddone, task-finishdone. These are idempotent backstops for when the merge webhook doesn't fire (as happened for T164).
  • Validation source of truth: derive VALID_TASK_STATUSES from the existing DEFAULT_TASK_STATUSES in status-utils.ts so the validator can never drift from the seeded statuses.

Phase 1: Root-cause + backend guard (sequential)

  • [agent:backend] Reproduce the 500 against file-sync with status: "merged_to_main"; confirm whether it's a DB constraint or a downstream throw; note the finding on the task
  • [agent:backend] Export VALID_TASK_STATUSES (Set/readonly array) from src/lib/tasks/status-utils.ts, derived from DEFAULT_TASK_STATUSES
  • [agent:backend] In the file-sync route, reject unknown status with 422 invalid_status (message includes the bad value + valid set) before the db.task.update write; valid statuses unchanged
  • [agent:test] Unit tests: unknown status → 422 + no DB write; each canonical status → passes validation; gate-required statuses still gated

Phase 2: Runbook alignment (parallel with Phase 1)

  • [agent:docs] task-pr-land.md: replace status: merged_to_main with status: staging
  • [agent:docs] task-deploy-prod.md: replace status: shipped with status: done
  • [agent:docs] task-finish.md: confirm/keep status: done

Test plan

  • Unit (file-sync route): mock authorizeSyncToken + db; POST a task .md with status: "merged_to_main" → assert 422 invalid_status and db.task.update not called.
  • Unit (positive): each value in VALID_TASK_STATUSES passes the new guard (parametrised); in_progress/pr_open still hit the implementation gate when git context is missing (no regression).
  • Unit (status-utils): VALID_TASK_STATUSES equals the slugs of DEFAULT_TASK_STATUSES (guards against drift).
  • Manual: run land→finish on a throwaway task; confirm it ends at a valid status with no 500 and no manual fix.

Security checklist

  • Touches auth? No — same authorizeSyncToken path; unchanged.
  • Touches secrets? No.
  • Accepts untrusted input? Yes — status from frontmatter. This task adds validation (allowlist against the canonical set), strictly tightening input handling. Covered by the 422 test.
  • Handles file upload? No.
  • Calls external APIs? No.

Rollback plan

Revert the file-sync validation + status-utils export and the three runbook edits; behavior returns to the prior pass-through (invalid statuses 500 again) — no migration or data change involved.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions