Skip to content

Refactor/extra engine types - #40

Merged
SokratisVidros merged 12 commits into
SokratisVidros:mainfrom
johnretsas:refactor/extra-engine-types
Jun 16, 2026
Merged

Refactor/extra engine types#40
SokratisVidros merged 12 commits into
SokratisVidros:mainfrom
johnretsas:refactor/extra-engine-types

Conversation

@johnretsas

Copy link
Copy Markdown
Contributor

Parser detection driven by the type (types.ts + ast-parser.ts)

The AST parser's list of recognized step.* methods was maintained by hand,
disconnected from the actual step API (StepBaseContext). That let the two drift,
and the name→StepType mapping relied on an unchecked as StepType cast plus a
special-case sleep branch.

  • Added STEP_BASE_METHOD_TYPES, derived from
    _STEP_BASE_METHOD_TO_TYPE: Record<keyof StepBaseContext, StepType>. The
    Record<keyof StepBaseContext, …> annotation makes the compiler enforce two
    things: every step method is mapped (add a method to StepBaseContext
    compile error until it's in the map), and every value is a real StepType.
  • The parser's || chain of method-name comparisons + as StepType cast +
    sleep-alias ternary collapse into a single STEP_BASE_METHOD_TYPES.get(methodName)
    lookup. undefined doubles as the "not a step method" check, and the
    sleep → DELAY alias is now data in the map rather than a branch in the parser.

Engine type-safety & structure (engine.ts)

  • Typed StepTypeToIcon as Record<StepType, string> so a new StepType forces
    a matching icon entry.
  • Removed a redundant as WorkflowInternalDefinition cast on the
    workflows.set(...) literal. The spread already produces the correct type, and
    the cast was suppressing excess-property checking (stray keys are now caught).
  • Extracted the ~110-line baseStep object literal out of handleWorkflowRun
    into a private createBaseStep(run, workflowId, runId) method.
  • Added a private createChildWorkflowRun(...) that wraps createWorkflowRun
    with parentRunId/parentStepId required at the type level, so the
    child-invocation path can't be wired up without parent linkage.

Fail loudly on a foreign workflow_runs table (migration.ts)

Previously, if a consumer already had a workflow_runs table in the target
schema, migrations could ALTER/index a table pg-workflows didn't create. The
migration now checks (only on a fresh install, i.e. no workflow_schema_version)
whether a workflow_runs table already exists and throws a clear error
pointing the user at a dedicated schema/rename — before any DDL runs.

This is the one behavioral change in the PR; it only fires for genuine collisions
and is otherwise inert (healthy installs are already past version 0).

Test coverage

  • Parser (ast-parser.test.ts): added real expect(...).toEqual(...)
    assertions to the previously no-assert switch, nested conditional/loop, and
    mixed-step tests; added a test documenting the known limitation that
    externally-defined handlers aren't parsed (yields [] steps).
  • Migration (migration.test.ts, new): fresh DB migrates to the current
    version; migrations are idempotent across repeated runs; a foreign
    workflow_runs table makes runMigrations throw without touching it.
  • Idempotency (engine.test.ts): reusing an idempotencyKey enqueues no
    second job and leaves the first run untouched (completed case); and the same
    holds while the first run is still mid-execution (paused) — the duplicate is a
    no-op and the in-flight run is undisturbed.

Testing

  • tsc --noEmit clean
  • Full unit suite passes (parser, engine, definition, migration)

@SokratisVidros
SokratisVidros merged commit 9802333 into SokratisVidros:main Jun 16, 2026
3 checks passed
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.

2 participants