Skip to content

feat(ts): let FailureHandler declare its own inputMapping and outputId - #204

Merged
sksizer merged 1 commit into
mainfrom
feat/recovery-input-output-mapping
Apr 26, 2026
Merged

sksizer merged 1 commit into
mainfrom
feat/recovery-input-output-mapping

Conversation

@sksizer

@sksizer sksizer commented Apr 26, 2026

Copy link
Copy Markdown
Owner

Summary

The runner was hardcoding the recovery task's WrappedTask as { inputMapping: undefined, outputId: undefined }, so recovery code could only resolve(cls) the default slot. That meant:

  • a recovery couldn't read a named upstream output (.named("scan", scanTask) writes ScanResult:scan, but the recovery's resolve(ScanResult) only sees ScanResult:default and throws);
  • a recovery couldn't direct its own writes to a named slot.

This PR extends FailureHandler with optional inputMapping and outputId fields and propagates them in the runner. Both default to undefined, preserving prior behavior at every existing call site.

Why this shape (vs auto-inheritance)

An alternative was to "auto-inherit" the failing parent's outputId so the recovery's resolve(ParentWritesType) lands on the parent's named slot. Rejected — it would carve out a special-case rule that silently diverges from the documented resolver contract:

tasks that want to read a non-default output must supply an explicit id or an inputMapping entry.

Explicit inputMapping on FailureHandler keeps the rule uniform: every task — recovery or not — declares what it reads.

Files

  • engine/types.ts — extend FailureHandler with inputMapping? and outputId?
  • engine/runner.ts — propagate them onto recoveryWrapped (3 lines)

Test plan

  • runner.test.ts "recovery uses FailureHandler.inputMapping to read named upstream outputs" — recovery reads Counter:upstream-id via its declared mapping
  • runner.test.ts "recovery's output lands at FailureHandler.outputId when set" — recovery writes to Counter:recovery-slot, downstream task reads it via its own mapping
  • bun run lint clean
  • bun run typecheck clean
  • bun test — 184/184 pass (+2 new)

Out of scope

  • Type-level checking that handler.task.reads is satisfied by the current Ctx. No onFailure recovery gets that check today; closing it requires more involved generic plumbing on the builder.

Addresses Copilot review thread on PR #195 (runner.ts:107).

The runner was hardcoding the recovery task's WrappedTask as
{ inputMapping: undefined, outputId: undefined }, so recovery code could
only resolve(cls) the 'default' slot — there was no way for a recovery
to read a named upstream output (or the parent's own named output once
.named() supports onFailure), and no way to direct the recovery's writes
to a named slot.

Extends FailureHandler with optional inputMapping and outputId fields
and propagates them in the runner. Both default to undefined, preserving
prior behavior at every existing call site.

This keeps the model uniform — recoveries declare their inputs the same
way every other task does — instead of carving out a special
'auto-inherit parent's outputId' rule that would silently diverge from
the documented resolver contract:

  > tasks that want to read a non-default output must supply an explicit
  > id or an inputMapping entry.

Two new tests cover (a) recovery reading a named upstream output via
FailureHandler.inputMapping, and (b) recovery's output landing at
FailureHandler.outputId so a downstream task can address it.
@sksizer
sksizer merged commit 7d409e6 into main Apr 26, 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.

1 participant