feat(ts): let FailureHandler declare its own inputMapping and outputId - #204
Merged
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The runner was hardcoding the recovery task's
WrappedTaskas{ inputMapping: undefined, outputId: undefined }, so recovery code could onlyresolve(cls)thedefaultslot. That meant:.named("scan", scanTask)writesScanResult:scan, but the recovery'sresolve(ScanResult)only seesScanResult:defaultand throws);This PR extends
FailureHandlerwith optionalinputMappingandoutputIdfields 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
outputIdso the recovery'sresolve(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:Explicit
inputMappingonFailureHandlerkeeps the rule uniform: every task — recovery or not — declares what it reads.Files
engine/types.ts— extendFailureHandlerwithinputMapping?andoutputId?engine/runner.ts— propagate them ontorecoveryWrapped(3 lines)Test plan
runner.test.ts"recovery uses FailureHandler.inputMapping to read named upstream outputs" — recovery readsCounter:upstream-idvia its declared mappingrunner.test.ts"recovery's output lands at FailureHandler.outputId when set" — recovery writes toCounter:recovery-slot, downstream task reads it via its own mappingbun run lintcleanbun run typecheckcleanbun test— 184/184 pass (+2 new)Out of scope
handler.task.readsis satisfied by the currentCtx. NoonFailurerecovery gets that check today; closing it requires more involved generic plumbing on the builder.Addresses Copilot review thread on PR #195 (runner.ts:107).