refactor: centralize lifecycle policy

[nnennandukwe]

Summary

• define lifecycle states in one immutable canonical owner
• derive structural transition and deterministic-next behavior from one topology
• derive proof-guard requirements from that topology instead of repeating transition edges
• use sourceState and targetState consistently for lifecycle direction across domain, persistence, service, and test code
• pass the frozen TASK_STATUS_VALUES array to state-schema z.enum() boundaries
• update CLI parsing, Zod validation, SQLite persistence checks, and session services to consume shared lifecycle values
• retain literal expectations in tests so serialized values are verified independently

Why

Follow-up to #38.

Lifecycle values such as verifying and reviewing were repeated across runtime layers. That made a vocabulary change vulnerable to drift and forced transition policy to be maintained in more than one place. This refactor centralizes the domain vocabulary and topology while keeping the existing CLI, wire, database, and migration contracts unchanged. The directional names sourceState and targetState make transition logic explicit without changing persisted from_state and to_state fields. State schemas now consume the canonical values array explicitly, avoiding ambiguity at the Zod boundary without an unsafe tuple assertion.

Impact

There is no intended user-facing or persisted-contract change. Existing lifecycle strings, ordering, transition behavior, JSON output, CLI errors, legacy migration behavior, and packaged execution remain compatible.

Validation

• npm run check — 16 test files and 159 tests passed, followed by build and packaged-install smoke verification
• npm run security:dependencies — passed the high-severity threshold; the existing low-severity esbuild advisory remains tracked by Upgrade esbuild when tsup supports the fixed release #46
• CLI, transition, proof-gate, concurrency, idempotency, rollback, migration, corruption, and connection-reset suites passed three consecutive runs of 100 tests
• schema regression covers module construction, current lifecycle values, legacy active normalization, and invalid status rejection
• packaged CLI exercised start, capture, block/recovery, proof-plan transitions, a passing gate, status, artifact generation, help, and protocol output
• git diff --check origin/main...HEAD
• independent Codex Standards review: 0 actionable findings
• independent Codex Spec review: 0 actionable findings
• focused AST naming check: no variables or parameters named from or to under src/ or tests/

Review focus

• src/domain/types.ts for lifecycle vocabulary ownership
• src/domain/lifecycle.ts for structural topology ownership and directional naming
• src/domain/session-transition.ts for guard-requirement derivation and directional naming
• src/schemas/state.ts for canonical values-array validation
• persistence and CLI callers for contract-preserving consumption of shared values

[qodo-code-review]

PR Summary by Qodo

Centralize lifecycle states and transition policy across layers

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Define immutable lifecycle states in one canonical domain owner.
• Derive forward transitions, deterministic next state, and guard requirements from shared topology.
• Update CLI, SQLite persistence, services, schemas, and tests to consume shared values.

Diagram

graph TD
  CLI["CLI parsing"] --> Types["Domain types"]
  Schema["Zod state schema"] --> Types
  Types --> Lifecycle["Lifecycle topology"]
  Lifecycle --> Guards["Transition guard policy"]
  SessionSvc(["Session service"]) --> Guards
  SqliteStore(["SQLite store"]) --> Lifecycle
  SqliteStore --> Types

  subgraph Legend
    direction LR
    _mod["Module"] ~~~ _svc(["Service"]) ~~~ _db[("Database")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a TypeScript enum for TaskStatus

• ➕ Strongly named constants without manual object keys
• ➕ Easy interop with some tooling
• ➖ Runtime enum shape differs (reverse mapping for numeric enums; extra runtime output)
• ➖ Harder to keep an ordered values list as the canonical contract

2. Keep `TASK_STATUS` as an ordered `as const` array and derive helpers

• ➕ Natural ordering is inherent
• ➕ Directly usable with z.enum([...])
• ➖ Weaker ergonomics for named constants across the codebase
• ➖ Call sites tend to reintroduce string literals and .includes(...) casts

3. Represent transition topology as an edge list and derive adjacency/requirements

• ➕ Single source of truth for both adjacency and guard ownership
• ➕ Can generate transition matrix and diagrams easily
• ➖ More indirection than a simple adjacency map
• ➖ Requires careful derivation to keep deterministic-next semantics obvious

Recommendation: The PR’s approach (frozen TASK_STATUS object + TASK_STATUS_VALUES list + topology-driven helpers) is the best fit: it preserves the existing serialized string contract, provides named constants to eliminate drift, and enables guard/next-state derivation from a single structural workflow without introducing enum runtime quirks.

Files changed (9) +230 / -163

Enhancement (2) +61 / -42

lifecycle.tsDefine lifecycle topology once and derive forward/next-state behavior +42/-29

Define lifecycle topology once and derive forward/next-state behavior

• Rebuilds the forward-transition map using 'TASK_STATUS' constants and standardizes direction naming ('sourceState'/'targetState'). Exposes 'isForwardLifecycleTransition' and 'getDeterministicForwardTarget' to share topology-derived behavior with other layers.

src/domain/lifecycle.ts

types.tsMake TaskStatus vocabulary immutable and add runtime validator helpers +19/-13

Make TaskStatus vocabulary immutable and add runtime validator helpers

• Replaces the 'TASK_STATUS' string array with a frozen object of named constants, plus 'TASK_STATUS_VALUES' for ordered iteration. Adds 'isTaskStatus' to validate external inputs safely at runtime while keeping 'TaskStatus' as the same string-union contract.

src/domain/types.ts

Refactor (5) +127 / -108

sqlite-store.tsUse canonical TaskStatus constants and runtime validation in SQLite persistence +27/-21

Use canonical TaskStatus constants and runtime validation in SQLite persistence

• Replaces hard-coded lifecycle strings with 'TASK_STATUS' constants and parameterizes related SQL predicates. Renames transition direction parameters to 'sourceState'/'targetState' and hardens corruption detection with 'isTaskStatus'.

src/adapters/fs/sqlite-store.ts

cli-program.tsCentralize CLI task-status parsing via isTaskStatus and TASK_STATUS_VALUES +11/-4

Centralize CLI task-status parsing via isTaskStatus and TASK_STATUS_VALUES

• Switches CLI argument validation from array-casts to 'isTaskStatus' and uses 'TASK_STATUS_VALUES' for user-facing error messages. Keeps the CLI contract unchanged while reducing duplication and drift risk.

src/cli-program.ts

session-transition.tsDerive guard requirements from lifecycle topology and standardize direction names +79/-65

Derive guard requirements from lifecycle topology and standardize direction names

• Introduces 'TransitionGuardRequirement' and 'getTransitionGuardRequirement' to determine guard ownership/needs from the canonical workflow. Replaces bespoke deterministic/owner logic with lifecycle-derived helpers and exports 'requiresProofGuardContext' for callers.

src/domain/session-transition.ts

state.tsUse canonical queued status during legacy 'active' normalization +1/-1

Use canonical queued status during legacy 'active' normalization

• Updates the Zod transform that maps persisted legacy 'active' to 'queued' to use 'TASK_STATUS.QUEUED'. Keeps persistence/migration behavior identical while avoiding repeated literals.

src/schemas/state.ts

session-service.tsConsume shared lifecycle helpers when deciding proof-context loading +9/-17

Consume shared lifecycle helpers when deciding proof-context loading

• Uses 'TASK_STATUS' constants throughout and replaces the local 'isIssue40ProofTransition' edge list with 'requiresProofGuardContext'. Keeps transition behavior and contracts stable while centralizing policy in the domain layer.

src/services/session-service.ts

Tests (2) +42 / -13

lifecycle.test.tsValidate frozen lifecycle vocabulary and deterministic-next derivation +31/-13

Validate frozen lifecycle vocabulary and deterministic-next derivation

• Updates tests to assert against 'TASK_STATUS_VALUES' and adds coverage ensuring lifecycle constants are frozen. Adds explicit checks for 'getDeterministicForwardTarget' derived from the structural workflow.

tests/unit/lifecycle.test.ts

session-transition.test.tsAdd coverage for topology-derived guard requirements and proof-context needs +11/-0

Add coverage for topology-derived guard requirements and proof-context needs

• Adds a unit test validating 'getTransitionGuardRequirement' and 'requiresProofGuardContext' behavior against key transitions. Ensures guard ownership is derived from the canonical workflow rather than repeated transition edge lists.

tests/unit/session-transition.test.ts

[qodo-code-review]

Code Review by Qodo

Context used

✅ Compliance rules (platform): 15 rules

1. Zod enum uses object ✓ Resolved 🐞 Bug ≡ Correctness

Description

src/schemas/state.ts still calls z.enum(TASK_STATUS), but TASK_STATUS was refactored into a
frozen object map in src/domain/types.ts. This will throw during schema construction/evaluation,
breaking stateDataSchema.safeParse(...) callers (e.g., readState).

Code

src/domain/types.ts[R12-30]

+export const TASK_STATUS = Object.freeze({
+  QUEUED: 'queued',
+  FRAMED: 'framed',
+  PROOF_READY: 'proof_ready',
+  IMPLEMENTING: 'implementing',
+  VERIFYING: 'verifying',
+  REVIEWING: 'reviewing',
+  REPAIRING: 'repairing',
+  READY_FOR_HUMAN: 'ready_for_human',
+  BLOCKED: 'blocked',
+  COMPLETED: 'completed',
+} as const);
+export type TaskStatus = (typeof TASK_STATUS)[keyof typeof TASK_STATUS];
+export const TASK_STATUS_VALUES = Object.freeze(Object.values(TASK_STATUS));
+const TASK_STATUS_SET = new Set<string>(TASK_STATUS_VALUES);
+
+export function isTaskStatus(value: string): value is TaskStatus {
+  return TASK_STATUS_SET.has(value);
+}

Evidence

The PR changes TASK_STATUS into an object, but the state schema still passes it into z.enum,
which requires an array/tuple of allowed string values. stateDataSchema is imported and executed
by sqlite-store when reading state, so this becomes a runtime-breaking error path.

src/domain/types.ts[11-30]
src/schemas/state.ts[1-19]
src/adapters/fs/sqlite-store.ts[260-271]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TASK_STATUS` is now a frozen object (e.g. `{ QUEUED: 'queued', ... }`), but the state Zod schemas still pass it to `z.enum(...)`, which expects an array/tuple of string values. This causes schema creation to fail at runtime/module evaluation.

## Issue Context
- `TASK_STATUS` was changed from an array of literals to an object map in `src/domain/types.ts`.
- `src/schemas/state.ts` still uses `z.enum(TASK_STATUS)` in multiple places.
- `stateDataSchema` is used by sqlite-store when reading state; if the schema fails to construct, state reads break.

## Fix Focus Areas
- src/schemas/state.ts[1-20]

## Suggested fix
Replace `z.enum(TASK_STATUS)` usages with one of:
- `z.nativeEnum(TASK_STATUS)` (preferred for an enum-like object), or
- `z.enum(TASK_STATUS_VALUES as [TaskStatus, ...TaskStatus[]])` (if you want to keep `z.enum`, ensure it receives an array/tuple).

Update both:
- `persistedTaskStatusSchema` union branch
- `blockedFromState` field schema

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

PR approved by Qodo

All merge criteria satisfied — approved by default policy

[qodo-code-review]

Qodo Fixer

🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (1)

🔗 Fix PR: #63

This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR.

Prompt for coding agent

This is an automated fix prepared on a separate branch (#63). It is NOT applied to this PR.
To use it: review Fix PR #63 (https://github.com/nnennandukwe/threadloop/pull/63), evaluate each change critically against your local context, and cherry-pick the changes that are correct into this branch. Do not accept them blindly.

Process — 1 fixed

• ☑ Fixed: Zod enum uses object

[qodo-code-review]

Code review by qodo was updated up to the latest commit 465d72e