feat(workflow): stable script-thrown error contract - #110
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/workflow-engine.test.ts (1)
594-594: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the cancellation error fields.
This only verifies the runtime class; assert
kind === "cancelled"andretryable === falseso the script-visible contract is covered.🤖 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-engine.test.ts` at line 594, Extend the rejection assertion for api.agent("x") to validate the WorkflowScriptRuntimeError fields, asserting kind is "cancelled" and retryable is false in addition to the runtime class.
🤖 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.
Nitpick comments:
In `@src/workflow-engine.test.ts`:
- Line 594: Extend the rejection assertion for api.agent("x") to validate the
WorkflowScriptRuntimeError fields, asserting kind is "cancelled" and retryable
is false in addition to the runtime class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50575161-a375-4678-a74b-4c04745c4107
📒 Files selected for processing (8)
skills/dynamic-workflows/SKILL.mdsrc/workflow-api.tssrc/workflow-contracts.tssrc/workflow-engine.test.tssrc/workflow-engine.tssrc/workflow-errors.tssrc/workflow-tools.tssrc/workflow-types.ts
| } catch (error) { | ||
| throw toWorkflowScriptRuntimeError(error); |
There was a problem hiding this comment.
Nested failure kinds become internal
When a nested workflow has a parsing, metadata, script-size, or determinism failure, this catch passes the original WorkflowScriptError or WorkflowDeterminismError through a converter that does not recognize those classes and assigns kind: "internal", preventing parent scripts from branching on the established kind and recording an incorrect run failure classification.
Greptile SummaryThis PR introduces a stable script-visible runtime error contract for agent and nested-workflow failures.
Confidence Score: 3/5The nested-error classification defect should be fixed before merging because valid workflow failure kinds are currently exposed and recorded as internal failures. The new catch around nested workflow execution normalizes error classes that the converter does not recognize, losing syntax, metadata, script-size, and determinism classifications that the engine otherwise preserves. Files Needing Attention: src/workflow-api.ts
|
| Filename | Overview |
|---|---|
| src/workflow-api.ts | Adds host-to-script error normalization, but nested script and determinism errors are incorrectly collapsed to internal. |
| src/workflow-errors.ts | Defines the stable script-visible runtime error class and its typed fields. |
| src/workflow-engine.ts | Preserves WorkflowScriptRuntimeError.kind when classifying uncaught workflow failures. |
| src/workflow-engine.test.ts | Covers provider fallback, retryability, journaling, nesting limits, and cancellation, but not nested parse or determinism classification. |
| src/workflow-contracts.ts | Adds the public TypeScript interface for script-thrown workflow errors. |
| skills/dynamic-workflows/SKILL.md | Documents typed error handling, fallback, fan-out retention, and resume retry behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[agent or nested workflow] --> B{Operation succeeds?}
B -->|Yes| C[Return result]
B -->|No| D[toWorkflowScriptRuntimeError]
D --> E[WorkflowScriptRuntimeError]
E --> F{Script catches error?}
F -->|Yes| G[Fallback or retained fan-out result]
F -->|No| H[Engine maps failure kind]
Reviews (1): Last reviewed commit: "docs(workflow): teach typed error handli..." | Re-trigger Greptile
Summary
agent()and nestedworkflow()intoWorkflowScriptRuntimeErrorkind,message, andretryablefields to normal JavaScripttry/catchparallel()/pipeline()still map uncaught branch failures tonullattempt()helpers for failure-aware fan-out without adding a new primitiveStack
pr/dw-9-resume-iteration-inspection(PR feat(workflow): script iteration, replay provenance, and call inspection #108)pr/dw-10-script-error-contractCommit structure
feat(workflow): define script error contractfeat(workflow): normalize script-thrown errorstest(workflow): cover script error handlingdocs(workflow): teach typed error handlingTest plan
npm run typechecknpm testnpm run buildgit diff --checktry/catchSummary by CodeRabbit