Skip to content

jobs: toStepRecord casts an unvalidated status column into StepStatus — a value outside the union reaches every consumer typed as if it were inside it #213

Closed

Description

@sebyx07

Found while typechecking packages/jobs' tests for the first time. The type error was in a test; the mechanism it exposed is in the mapper.

What the test was doing

packages/jobs/src/driver-pg-rows.test.ts built a step row with status: 'suspended' and asserted it came back out as 'suspended'. But StepStatus is completed | sleeping | waiting | failed (packages/jobs/src/steps.ts:15) — 'suspended' is a JobRecord['state'], never a step status.

The test passed. That is the finding.

Why it passed

packages/jobs/src/driver-pg-rows.ts:

status: row.status as StepRecord['status'],

The column is cast, not validated. So the test's impossible value travelled straight through the mapper and came back out, and the assertion compared it against itself.

Why it matters beyond the test

The mapper will do exactly the same thing for a real row. A step row written by an older deploy — or by a future one that adds a status this build does not know — carries a value outside the union, and the cast launders it into every consumer as if it were inside. Nothing narrows it, nothing logs it, and every downstream switch on StepStatus silently takes no branch.

This is the same shape as the isCookie lying-predicate bug (fixed in the scraping slice): a type assertion standing in for a check, on data that crosses a process boundary.

Why it was not fixed in the typecheck slice

Deciding what to do with an unknown status is a behaviour decision, not a type fix:

  • refuse the row with an UltimateError carrying a stable code and a fix:?
  • map it to failed and log?
  • keep it and widen StepStatus to | (string & {}), which pushes the decision to every consumer?

The first is most consistent with the house rule that an absent or unrecognised fact is never a satisfied one, and it is what X_*-coded errors exist for. But it changes what a rolling deploy does when two versions share a queue, which needs its own test and its own thinking about the upgrade path.

The test itself is fixed ('suspended' → 'waiting', and the mutation toStepRecord stops passing the status through now reddens it). This issue is the mapper.

Activity

  1. developerz-ai commented on Aug 20, 2026

    @developerz-ai
    Contributor

    I couldn't reliably classify or scope this report right now, so I haven't guessed or handed it off. A maintainer will review it.

    🤖 Posted by developerz.ai — the maintainer agent, not a human.

  2. sebyx07 commented on Aug 20, 2026

    @sebyx07
    ContributorAuthor

    Fixed in #219 — and it was three casts, not one, with a worse consequence than filed.

    toStepRecord (status), toJobRecord (state) and toBackfillRun (status) all cast. The sharpest form: isBackfillStatus already existed at backfill-ledger.ts:30 with the doc "Never a cast — the list decides", and toBackfillRun two files over was the one caller ignoring it.

    The severity this issue understated. steps.ts:263 is if (existing?.status === 'completed') → return the memoized output. Every other value re-executes the step. So a laundered status turns "this step already ran" into "run it again" — a second step.run('charge', …) — in a file whose header promises the welcome email is not sent twice.

    Now STEP_STATUSES/isStepStatus, JOB_STATES/isJobState, and one statusIn() narrowing used by all three decoders. New code X_JOB_ROW_STATUS_UNKNOWN (500).

    What a mixed-version deploy does, since the two directions are not symmetric — a status string only reaches the column because some build of this framework wrote it:

    • N+1 reading N's rows — the normal direction — cannot refuse. A release that only adds a status still knows every older one. Pinned by decoding the whole vocabulary.
    • N reading N+1's row fails that one job, retries, dead-letters, and the rest of the queue stays readable. The fix: says to drain the older workers. Previously that same row silently re-ran its steps.

    One honest gap recorded rather than papered over: "no as onto a closed union in a decoder" is not a build error today. A cast always typechecks, so this is runtime-enforced only. A gate for it would look like bun run error-render and belongs in scripts/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions