feat(workflow): opt-in settled result API - #109
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/dynamic-workflows/SKILL.md`:
- Around line 90-92: Update the journaling statement in the parallel()
documentation to scope it specifically to underlying agent calls that fail after
execution begins. Do not claim that settle() itself journals failures; retain
that settle() preserves branch failures as data and clarify that only failed
agent-call outcomes are journaled and excluded from successful cached-result
replay.
In `@src/workflow-sandbox.test.ts`:
- Around line 15-24: Normalize caught errors like production in both test
doubles: update the settle implementations in src/workflow-sandbox.test.ts lines
15-24 and src/workflow-script.test.ts lines 90-99 to use error.message for Error
instances and String(error) otherwise, while preserving the existing failure
result structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f725217c-3185-4e56-9ef6-7333918c192c
📒 Files selected for processing (11)
skills/dynamic-workflows/SKILL.mdsrc/workflow-api.tssrc/workflow-contracts.tssrc/workflow-engine.test.tssrc/workflow-engine.tssrc/workflow-sandbox.test.tssrc/workflow-sandbox.tssrc/workflow-script.test.tssrc/workflow-script.tssrc/workflow-tools.tssrc/workflow-types.ts
| Inside `parallel()`, wrap each branch with `settle()` to preserve failures as | ||
| data instead of `null`. Failed settled outcomes are journaled failures and are | ||
| not replayed as successful cached agent results. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scope the journaling claim to agent calls.
settle() does not journal itself: settle(() => { throw new Error("x") }) returns failure data without an agent-call record. Limit the statement to underlying agent calls that fail after execution begins.
Proposed wording
-Inside `parallel()`, wrap each branch with `settle()` to preserve failures as
-data instead of `null`. Failed settled outcomes are journaled failures and are
-not replayed as successful cached agent results.
+Inside `parallel()`, wrap each agent branch with `settle()` to preserve failures
+as data instead of `null`. Failed agent calls are journaled as failures and are
+not replayed as successful cached agent results.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Inside `parallel()`, wrap each branch with `settle()` to preserve failures as | |
| data instead of `null`. Failed settled outcomes are journaled failures and are | |
| not replayed as successful cached agent results. | |
| Inside `parallel()`, wrap each agent branch with `settle()` to preserve failures | |
| as data instead of `null`. Failed agent calls are journaled as failures and are | |
| not replayed as successful cached agent results. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/dynamic-workflows/SKILL.md` around lines 90 - 92, Update the
journaling statement in the parallel() documentation to scope it specifically to
underlying agent calls that fail after execution begins. Do not claim that
settle() itself journals failures; retain that settle() preserves branch
failures as data and clarify that only failed agent-call outcomes are journaled
and excluded from successful cached-result replay.
| settle: async <T>(task: () => T | Promise<T>) => { | ||
| try { | ||
| return { ok: true, value: await task() }; | ||
| } catch (error) { | ||
| return { | ||
| ok: false, | ||
| error: { kind: "internal", message: String(error), retryable: false }, | ||
| }; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match production Error-message normalization in both test doubles.
Production returns error.message for Error instances; these stubs return String(error) (for example, "Error: boom"). Tests inspecting settled failures can therefore validate behavior scripts never receive.
src/workflow-sandbox.test.ts#L15-L24: useerror instanceof Error ? error.message : String(error).src/workflow-script.test.ts#L90-L99: use the same normalization.
📍 Affects 2 files
src/workflow-sandbox.test.ts#L15-L24(this comment)src/workflow-script.test.ts#L90-L99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-sandbox.test.ts` around lines 15 - 24, Normalize caught errors
like production in both test doubles: update the settle implementations in
src/workflow-sandbox.test.ts lines 15-24 and src/workflow-script.test.ts lines
90-99 to use error.message for Error instances and String(error) otherwise,
while preserving the existing failure result structure.
1 participant
Summary
settle(() => operation)workflow primitiveagent()throws,parallel()/pipeline()map failures tonull{ ok, value } | { ok, error: { kind, message, retryable } }agent()and nested workflow operations without caching failed outcomes as successesStack
pr/dw-9-resume-iteration-inspection(PR feat(workflow): script iteration, replay provenance, and call inspection #108)pr/dw-10-settled-resultsTest plan
npm run typechecknpm testnpm run buildgit diff --checkparallel()settle()Summary by CodeRabbit
New Features
settle()for handling workflow operations without throwing.Documentation
settle()syntax.Tests