feat(workflow): add wait/resolve support for human-in-the-loop workflows#987
Conversation
🦋 Changeset detectedLatest commit: 0ac30fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
⚡ pkg.pr.new@tailor-platform/sdk@tailor-platform/create-sdk
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1b4db4d to
91bde2f
Compare
This comment has been minimized.
This comment has been minimized.
91bde2f to
9b1e1e6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
📖 Docs Consistency Check
|
| File | Issue | Suggested Fix |
|---|---|---|
packages/sdk/docs/services/workflow.md |
Wait point Payload type description does not clarify top-level null restriction |
Add explicit note similar to workflow job inputs (line 63) |
packages/sdk/docs/services/workflow.md |
Result type cannot be undefined but this is not documented |
Add note that Result must be a concrete value (platform constraint) |
Details
1. Top-level null restriction for Payload (workflow.md:241)
Current text:
Payload— Data sent when the job suspends (passed to.wait()). Must be a pure JSON value (string,number,boolean,null, arrays, plain objects). Useundefinedif no payload is needed.
This lists null as a valid primitive, but the implementation rejects top-level null (wait-point.test.ts:31-36):
// @ts-expect-error - null is not allowed at top level (even in union)
check: define<{ id: string } | null, { ok: boolean }>()The workflow job input documentation (line 63) handles this clearly:
Top-level
nullis also rejected because the platform normalizes top-levelnull/undefinedargs to{}(nestednullinside objects or arrays is preserved).
Recommendation: Add the same clarification to the wait point Payload description.
2. Result cannot be undefined (workflow.md:242)
Current text:
Result— Data returned when the wait point is resolved (returned from.wait(), produced by the.resolve()callback). Must be a pure JSON value.
The implementation explicitly rejects undefined as a Result type (wait-point.test.ts:97-102):
// @ts-expect-error - Result cannot be undefined; platform throws if callback returns undefined
check: define<undefined, undefined>()Unlike Payload (where undefined alone is allowed as a no-payload convention), Result cannot be undefined at all because the resolve callback must return a concrete value.
Recommendation: Clarify that Result must be a concrete JSON value (cannot be or include undefined), and briefly note this is a platform constraint.
Checked areas
✅ Fully consistent:
- New APIs documented:
defineWaitPoint,defineWaitPoints,WaitPointInstance - Test utility documented:
setupWaitPointMockwithonWait/onResolvehandlers - Workflow job I/O type changes properly documented (JsonValue-compatible, no Date/toJSON)
- Examples match implementation (
example/workflows/approval.ts,example/resolvers/resolveApproval.ts) - Template files follow documented patterns
- CLAUDE.md updated with wait point usage patterns
- All example code updated to comply with new type constraints (Date → ISO strings)
These are minor wording issues that could cause confusion, but the core functionality is well-documented and all code examples are correct.
📖 Docs Consistency Check✅ No inconsistencies found between documentation and implementation. Checked areas: 1. Public API Documentation - defineWaitPoints signature, WaitPointInstance interface, type parameters, Jsonify transformation, code examples, multiple wait points usage - all match implementation 2. Testing Documentation - In-memory coordination, setupWaitPointMock signature, return values, cleanup instructions, test examples - all accurate 3. Developer Guide (CLAUDE.md) - Wait point creation, delegation behavior, access patterns - all correct 4. Example Code - approval.ts workflow, resolveApproval.ts resolver, template files, test files - all valid 5. Module Exports - defineWaitPoints and setupWaitPointMock exports and import paths - all correct Note: Payload must be JSON-compatible is a runtime/platform requirement (not compile-time constraint), which is intentional. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fe74625 to
3f3c0d9
Compare
This comment has been minimized.
This comment has been minimized.
3f3c0d9 to
aef4c3e
Compare
This comment has been minimized.
This comment has been minimized.
aef4c3e to
c0b5a12
Compare
This comment has been minimized.
This comment has been minimized.
…eturn type - createWorkflowJob: Input/Output must be pure JSON (no Date, no toJSON); reject top-level null in Input - defineWaitPoint / defineWaitPoints: reject top-level null in Payload; Result keeps top-level null - Replace `..._guard` rest-param guard with `JobBody<I, O>` / `WaitPointDef<P, R>` conditional types (error surfaces at body/define call site) - example: serialize Date fields to ISO strings in workflow job outputs Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c0b5a12 to
fa470ac
Compare
This comment has been minimized.
This comment has been minimized.
- WaitPointDef now rejects Result = undefined and top-level undefined unions in Result. The platform throws `resolve: callback must return a value (got undefined)` at runtime, so surfacing it at compile time matches the platform's contract. - getPlatformWorkflow() also verifies that workflow.wait and workflow.resolve are functions. Without this, calling setupWorkflowMock() without setupWaitPointMock() threw a confusing TypeError instead of the intended "Use setupWaitPointMock()" message. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Previously `setupWorkflowMock` only set `triggerJobFunction`, and `setupWaitPointMock` only set `wait`/`resolve`. Users had to remember to call both; calling only one and then hitting the unmocked method produced a confusing TypeError (or silent undefined for triggerJobFunction). Each helper now stubs the other's methods with a throw-on-call handler that directs the user to the missing mock. This removes the need for the extra runtime guard in `getPlatformWorkflow` (reverted to the simple `!workflow` check) while still surfacing a clear error when a test forgets a mock. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
| : [Result] extends [undefined] | ||
| ? "ERROR: Result cannot be undefined (resolve callback must return a value)" | ||
| : [undefined] extends [Result] |
There was a problem hiding this comment.
[q]
I don't quite understand why both need to be checked — is this branching simply to make the error messages clearer?
There was a problem hiding this comment.
I originally intended to keep the messages separate, but I decided that much detail wasn't necessary, so I merged them.
| * @returns The same object returned by the builder (with correct keys set on each instance) | ||
| * @example | ||
| * export const waitPoints = defineWaitPoints(define => ({ | ||
| * /** Approval for order processing */ |
There was a problem hiding this comment.
[nits]
At least in my environment, * does not render as intended in the IDE.
| /** | ||
| * Allowed output types for workflow job body functions. | ||
| * Includes Jsonifiable (JSON-serializable values including objects with toJSON like Date), | ||
| * undefined, and void. | ||
| * Must be JsonValue-compatible, undefined, or void. | ||
| */ | ||
| export type WorkflowJobOutput = Jsonifiable | undefined | void; | ||
| export type WorkflowJobOutput = JsonCompatible<unknown> | undefined | void; | ||
|
|
||
| /** | ||
| * Convert output type to what trigger returns after JSON serialization. | ||
| * - Jsonifiable values are converted via Jsonify (Date -> string, etc.) | ||
| * - undefined remains undefined | ||
| * - void becomes void | ||
| * Input type constraint for workflow jobs. | ||
| * Must be JsonValue-compatible or undefined. | ||
| */ | ||
| type JsonifyOutput<T> = T extends Jsonifiable ? Jsonify<T> : T; | ||
| export type WorkflowJobInput = JsonCompatible<unknown> | undefined; |
…SDoc - Consolidate `[Result] extends [undefined]` and `[undefined] extends [Result]` into a single `[undefined] extends [Result]` branch. Both cases are errors (unlike Payload, where `Payload = undefined` is the no-payload convention), so one check with a combined message suffices. - Replace the nested `/** */` comment inside a `@example` block (written as `/** */` HTML entities) with a plain `//` comment. Some IDEs do not render the entities, leaving garbled text in autocompletion hover. - Remove unused `WorkflowJobInput` and `WorkflowJobOutput` type exports from `configure/services/workflow/job.ts`; `JobBody` references `JsonCompatible` directly and nothing else imports these aliases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
| - Use `workflow.mainJob.trigger()` to execute the full workflow chain and get the result | ||
| - **Best for:** Testing workflow orchestration and job dependencies | ||
|
|
||
| ## Testing Wait Points |
There was a problem hiding this comment.
[nits]
Would it be better for this paragraph to be inside the workflow tests section?
There was a problem hiding this comment.
Having three sections—Unit Tests, Workflow Tests, and E2E Tests—felt unnatural, so I have merged Workflow Tests into Unit Tests for now to make it two sections. I plan to revise the specific content of each section separately later.
Code Metrics Report (packages/sdk)
Details | | main (e9e0568) | #987 (90cc524) | +/- |
|--------------------|----------------|----------------|-------|
+ | Coverage | 59.2% | 59.3% | +0.1% |
| Files | 350 | 351 | +1 |
| Lines | 11894 | 11928 | +34 |
+ | Covered | 7046 | 7083 | +37 |
+ | Code to Test Ratio | 1:0.4 | 1:0.4 | +0.0 |
| Code | 77547 | 77802 | +255 |
+ | Test | 31368 | 31474 | +106 |Code coverage of files in pull request scope (80.5% → 84.7%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Summary
Implements SDK support for
tailor.workflow.wait()andtailor.workflow.resolve()(platform-planning#995, function#154).defineWaitPoint<P, R>(key)anddefineWaitPoints(define => ({ key: define<P, R>() }))for typed wait/resolve pointsWaitPointInstance.wait()and.resolve()runtime-delegate toglobalThis.tailor.workflowon the platformsetupWaitPointMocktest utility for local and bundled workflow testsexample/workflows/approval.ts,example/resolvers/resolveApproval.ts, and template test patterns