Skip to content

refactor(errors): represent domain failures as Schema.TaggedError - #53

Open
FreshlyBrewedCode wants to merge 5 commits into
33-parse-cli-with-effect-clifrom
34-domain-errors-as-tagged-errors
Open

FreshlyBrewedCode wants to merge 5 commits into
33-parse-cli-with-effect-clifrom
34-domain-errors-as-tagged-errors

Conversation

@FreshlyBrewedCode

@FreshlyBrewedCode FreshlyBrewedCode commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Part of #32 · Closes #34

server/scheduler.ts decided whether a missed cron window reads as skipped-concurrency or
fire-failed by instanceof-checking a caught unknown against ConcurrencyLimitError. That is
a semantic decision resting on an untyped catch: a future error type thrown from the same path
would silently land in the wrong bucket, and a suppressed window would look like a failed one.
Schema.TaggedError is already used twice in the codebase (SchedulerError,
AgentStepChunkError), so this finishes that pattern for the daemon's four domain failures rather
than introducing a new one.

What changed

  • ConcurrencyLimitError, DispatchCapError, DedupeKeyError (src/lib/dedupe.ts), and
    RunCancelledSignal (src/runtime/run.ts) are Schema.TaggedError classes instead of thrown
    Error subclasses. Each keeps its existing data: DedupeKeyError keeps key and
    holderRunId; ConcurrencyLimitError carries maxConcurrentRuns as a real field instead of
    only baking it into the message string; DispatchCapError keeps its formatted message as an
    explicit field; RunCancelledSignal stays field-less.
  • server/scheduler.ts's skip-vs-fail catch in tickOnce is now genuinely exhaustive and
    compiler-checked: an instanceof-narrowed ScheduleFireError union (the four domain classes,
    value-imported) feeds a switch over the resulting literal _tag type, whose default arm is a
    satisfies never check — a tag added to the union without a matching case fails to typecheck
    rather than silently dropping the schedule's result. Anything outside the union (a non-Error
    throw, or a genuinely unrecognized future error) still produces exactly one fire-failed entry,
    never zero.
  • ConcurrencyLimitError and DedupeKeyError now carry an overridden message getter, the same
    pattern DispatchCapError already used for its field — the collision/limit strings are written
    once, on the class, instead of being hand-duplicated at every catch site.
    domainErrorMessage() in runtime/run.ts collapses to the generic Error fallback now that all
    three domain errors format their own message; every call site (http.ts's three 409 handlers,
    run.ts) reads err.message instead of rebuilding the string inline. Output is verified
    byte-for-byte identical to the strings it replaces.
  • Single-tag call sites (ready-sweep.ts, run.ts, http.ts) use instanceof DedupeKeyError /
    instanceof RunCancelledSignal instead of the _tag-cast pattern — only the scheduler's
    multi-tag exhaustive match needs to dispatch on _tag. http.ts no longer mixes both styles in
    the same catch block.
  • sample/workflows/ready-sweep.ts regained the comment explaining why a dedupe collision
    continues the loop instead of aborting it, dropped in an earlier pass.
  • Tests: construction/field/_tag coverage for all four errors and a scheduler test per known
    outcome (skipped-concurrency, fire-failed for DedupeKeyError and DispatchCapError), plus
    a new regression test that throws an error whose _tag the switch doesn't recognize and asserts
    it lands in fire-failed with exactly one result entry — the case the earlier version of this
    branch silently dropped.

Notes for reviewers

  • The scheduler's exhaustiveness is enforced two ways: the switch lists all four known tags by
    name, and the default arm does return err satisfies never, which only typechecks once every
    member of ScheduleFireError has a preceding case. If a fifth tagged error is ever added to
    the union without a case, this fails to compile instead of reproducing the original bug.
  • HTTP 409 response bodies (error, dedupeKey, holderRunId fields and status code) are
    byte-for-byte unchanged from a client's perspective — checked by comparing the literal template
    strings the getters replaced, character for character, and confirmed by the existing http.test.ts
    assertions on dedupeKey/holderRunId/error continuing to pass unmodified.
  • RunCancelledSignal is still only checked in two places in runtime/run.ts (a re-throw guard in
    the write-back catch, and the terminal catch that emits RunCancelled), matching the pre-existing
    structure.

Verification

  • bun run check (format:check + lint + typecheck + bun test) passes: 293 tests across 35
    files, 0 failures.
  • Manually instantiated ConcurrencyLimitError/DedupeKeyError and diffed .message against the
    literal strings previously hand-written at each call site — identical.
  • Did not additionally run bun run test:e2e.

Stack

  1. refactor(cli): replace hand-rolled parsers with effect/unstable/cli #52 — 33-parse-cli-with-effect-cli (issue Parse CLI arguments with effect/unstable/cli #33)
  2. refactor(errors): represent domain failures as Schema.TaggedError #53 — 34-domain-errors-as-tagged-errors (issue Represent domain failures as Schema.TaggedError #34) ← you are here
  3. refactor(runtime): move chunk interpretation into the agent adapter #54 — 35-move-chunk-interpretation-into-adapter (issue Move chunk interpretation into the agent adapter #35)
  4. refactor(runtime): move headless permission setup into the agent adapter #55 — 37-move-headless-permissions (issue Move headless permission setup out of the workspace allocator #37)
  5. feat(runtime): add Effect composition root and agent runtime service #56 — 36-agent-runtime-service (issue Add an Effect composition root and make the agent runtime a service #36)
  6. refactor(daemon): move singletons into per-daemon layers #57 — 38-move-singletons-into-layers (issue Move the daemon's remaining singletons into layers #38)

Stack created with GitHub Stacks CLI • Give Feedback 💬

Convert the four domain failures from thrown Error subclasses matched by
instanceof to Schema.TaggedError with checked tag matching:

- DedupeKeyError: carries key and holderRunId fields
- ConcurrencyLimitError: carries maxConcurrentRuns field
- DispatchCapError: carries message field
- RunCancelledSignal: no fields (the run's own unwind signal)

Constructor signatures change from positional to struct args
(e.g. new DedupeKeyError({ key, holderRunId }) instead of
new DedupeKeyError(key, holderRunId)).

Add domainErrorMessage helper in run.ts to construct human-readable
messages for RunFailed events, since TaggedError.message is empty
in this Effect version when classes are loaded across module
boundaries.

Update runtime catch sites in run.ts to match on _tag instead of
instanceof, preserving the single-catch property of RunCancelledSignal.

Add tests verifying _tag, fields, and instanceof Error for all four
errors. Update existing tests to use new constructor signatures.
Update the HTTP layer, scheduler, and sample workflow to match domain
errors by _tag instead of instanceof:

- HTTP: 409 status mapping unchanged from a client's perspective;
  error messages now constructed from TaggedError fields
- Scheduler: skip-vs-fail branch is now an exhaustive switch over
  all four domain error tags (ConcurrencyLimitError → skipped-concurrency,
  DedupeKeyError/DispatchCapError/RunCancelledSignal/unknown → fire-failed)
- ready-sweep: dedupe collision check uses _tag matching
- Add exhaustive match test in scheduler.test.ts verifying each
  domain error type is explicitly classified
…zed error tags (#34)

The catch block matched on a plain, `as`-cast `_tag: string | undefined`
with no `default` arm: an error whose tag was none of the four known ones
matched nothing in the switch, so no `TickResult` was pushed for that
schedule at all — a silent drop, worse than the wrong-bucket bug issue
#34 set out to fix.

Replace the cast with a real `instanceof`-narrowed union
(`ScheduleFireError`) and a switch over the resulting literal `_tag`
type, whose `default` arm is a `satisfies never` compile-time
exhaustiveness check — so a tag added to the union without a
corresponding case fails to typecheck instead of silently vanishing at
runtime. Anything outside the union (a non-`Error` throw, or a future
error type) still lands in `fire-failed`, never nothing.

Add a regression test that throws an error with a tag the switch
doesn't recognize and asserts it produces exactly one `fire-failed`
result.
…classes (#34)

The "concurrency limit reached..." and "dedupe key held..." strings
were hand-written in up to six places (run.ts's domainErrorMessage
and three call sites in http.ts), free to drift apart.
DispatchCapError already avoided this by carrying its message as a
field; give ConcurrencyLimitError and DedupeKeyError an overridden
`message` getter (Schema.TaggedError's base only sets an own
`message` property when a `message` field is passed, so the getter is
free to take over) and have every call site read `err.message`
instead of rebuilding the string.

domainErrorMessage collapses to the generic Error fallback now that
all three domain errors carry their own message. Also replace the
remaining `_tag`-cast pattern with `instanceof` at the single-tag
call sites in run.ts and http.ts (http.ts mixed both styles two lines
apart) — only the scheduler's exhaustive multi-tag match needs
tag-based dispatch.

The produced strings are unchanged character-for-character (verified
against the literals they replace), so the HTTP 409 responses are
unchanged from a client's perspective.
…y-sweep (#34)

The tag-matching pass on issue #34 replaced this single-tag
`instanceof DedupeKeyError` check with an unsafe `_tag`-cast for
consistency with the scheduler, but dropped the comment explaining
why a dedupe collision here continues the loop rather than aborting
it, and the workflow only ever needs to distinguish one tag — restore
both.
@FreshlyBrewedCode
FreshlyBrewedCode force-pushed the 34-domain-errors-as-tagged-errors branch from 4a93e1c to dcaf696 Compare September 23, 2026 07:09

This branch has not been deployed

No deployments
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.

Represent domain failures as Schema.TaggedError

1 participant