Skip to content

fix: enforce task state machine on task status writes - #97

Merged
uberware merged 2 commits into
mainfrom
fix/task-state-machine-enforcement
Jul 21, 2026
Merged

fix: enforce task state machine on task status writes#97
uberware merged 2 commits into
mainfrom
fix/task-state-machine-enforcement

Conversation

@uberware

Copy link
Copy Markdown
Owner

The task state machine was documented but never enforced. openjd.ValidateTaskTransition
encoded the legal arrows and was well tested, but had zero non-test callers
store.UpdateTaskStatus wrote whatever status it was handed.

What changed

  • The task machine moves to internal/store (openjd imports store, so the store could not
    import it back). openjd keeps the step machine; its dead task half is removed rather than
    left as a second table that can drift.
  • UpdateTaskStatus now validates before writing, in both implementations — SQLite reads and
    writes in one transaction (serialized by the single-connection pool, mirroring TryClaimSlots),
    the fake under its mutex. An illegal arrow returns store.ErrInvalidTransition and leaves the
    row untouched.

Two rules that make enforcement safe under at-least-once delivery

  • Writing a task's current status is a no-op, not an error — duplicate running messages are
    routine on JetStream.
  • The status consumer acks an invalid transition instead of Nak'ing. handleTaskStatusMessage
    Naks on error, so without this a single stale message would redeliver forever.

Deliberately legal: assignedsucceeded/failed

This looks like a skipped state but is reachable in normal operation: the worker publishes
running first, but status.Publisher.publishWithRetry gives up after MaxRetries and returns,
so that message can be lost while the task still completes. Rejecting the terminal message would
strand finished work until the heartbeat sweep reclaimed it.

Cancellation

CancelTask's terminal guard is a separate read from its write, so a task can complete in
between and the state machine then rejects the cancel. That is treated as the no-op it would have
been had the guard seen the newer value. It is narrow by construction: every non-terminal status
has a legal arrow to canceled, so ErrInvalidTransition on that write can only mean "already
terminal". Other store errors still propagate — there is a test pinning that.

Notes for review

  • No production call site was writing an illegal transition; the scheduler already upheld the
    machine by construction. The fixture churn is test-only.
  • Two test doubles were modelling transitions production never performs: the API retry double
    faked failed → ready via UpdateTaskStatus, where production's RetryTasks writes
    failed → pending with its own SQL; and a scheduler harness drove ready → failed, skipping
    assignment. Both now match production.
  • Bulk paths (RetryTasks, TransitionStepPendingTasks, CancelJobTasks, reclaim sweeps) keep
    their own guarded SQL and do not route through UpdateTaskStatus.
  • Test fixtures walk legal arrows via a new walkTaskTo helper.

Developed test-first; make ci and make test-integration green.

@uberware
uberware merged commit 9c386d4 into main Jul 21, 2026
27 checks passed
@uberware
uberware deleted the fix/task-state-machine-enforcement branch July 25, 2026 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant