Repository navigation
Add package runtime workflow support - #335
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds package-owned durable workflows: manifest schema ( ChangesPackage-Owned Durable Workflows
Sequence DiagramsequenceDiagram
participant CodeMode as CodeMode
participant WorkflowTools as createWorkflowTools
participant CreateWorkflow as createPackageWorkflow
participant WorkflowBinding as PACKAGE_WORKFLOWS
participant Entrypoint as PackageWorkflowEntrypoint
participant PackageExport as Package Export
CodeMode->>WorkflowTools: workflows.create(input)
WorkflowTools->>WorkflowTools: validate userId & sourceId
WorkflowTools->>CreateWorkflow: createPackageWorkflow(env, ids, body)
CreateWorkflow->>CreateWorkflow: normalize payload, compute deterministic id
CreateWorkflow->>WorkflowBinding: get(id) -> check existing
alt exists
WorkflowBinding->>CreateWorkflow: return existing instance
else
CreateWorkflow->>WorkflowBinding: create(id, payload) -> schedule runAt
end
Note over Entrypoint: at runAt
Entrypoint->>Entrypoint: sleepUntil(runAt)
Entrypoint->>PackageExport: invokePackageExport(token, params)
PackageExport->>Entrypoint: return {status, body}
Entrypoint->>WorkflowBinding: complete with result
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
marked this pull request as ready for review
May 3, 2026 01:16
Contributor
|
🔎 Preview deployed: https://kody-pr-335.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/run-codemode-registry.ts`:
- Around line 660-667: The code in runModuleWithRegistry currently always sets
workflowTools to createWorkflowTools(...) which overwrites any caller-provided
options.workflowTools; change the assignment so it preserves a provided
workflowTools by using the caller's options.workflowTools when present (e.g.,
set workflowTools to options?.workflowTools ?? createWorkflowTools({...})), and
ensure packageContext is still passed into createWorkflowTools when it's used;
update the object construction around runModuleWithRegistry to reference
options.workflowTools rather than unconditionally calling createWorkflowTools.
In `@packages/worker/src/package-runtime/package-app.ts`:
- Around line 256-259: createWorkflowsProxy currently masks missing input by
coercing to {} and lets undefined required fields cause cryptic errors
downstream; remove the "?? {}" fallback in createWorkflowsProxy and add upfront
schema validation (e.g., check that input is an object and that workflowName,
exportName, runAt, and idempotencyKey are present and are non-empty
strings/dates as appropriate) before calling runtimeBridge.workflowCreate; when
validation fails, throw a clear Error mentioning which field is missing or
invalid (use the same normalizeNonEmptyString or similar validators but catch
and rethrow with a descriptive message at the proxy boundary).
In `@packages/worker/src/package-runtime/package-workflows.ts`:
- Around line 284-304: The current check-then-create in
getExistingWorkflowInstance + input.workflow.create can race; wrap the create
call (input.workflow.create) in a try/catch and if the create throws an "already
exists" / duplicate error, call getExistingWorkflowInstance(id) to fetch the
existing instance and use readWorkflowInstanceSummary(existing) (or return the
same summary shape you build after successful create) so callers get the
existing instance info; otherwise rethrow unexpected errors. Ensure you still
use the same id, payload, and return fields (ok, id, workflow_name, export_name,
run_at, plan_date, status) as when an existing instance was found.
- Around line 140-157: The function normalizePackageWorkflowParams currently
returns the original params object which may contain non-JSON values; change it
to return a JSON-normalized clone by serializing and deserializing the validated
params (use the existing paramsJson and return JSON.parse(paramsJson)) so the
caller always receives a plain JSON-compatible object; keep the null/undefined
check, the object/array validation, and the byte-length check as-is, but replace
the final "return params" with "return JSON.parse(paramsJson)" inside
normalizePackageWorkflowParams.
In `@tools/ci/resource-utils.ts`:
- Around line 319-334: The current patch silently no-ops when env.workflows is
missing or contains no PACKAGE_WORKFLOWS entry; update the logic around
resolvedPackageWorkflowName so it fails fast: after computing
resolvedPackageWorkflowName, assert that (targetEnv as
Record<string,unknown>).workflows exists and is an array and that at least one
workflow has workflowRecord.binding === 'PACKAGE_WORKFLOWS'; if any of those
checks fail throw a clear Error mentioning PACKAGE_WORKFLOWS and the env name;
otherwise proceed to set workflowRecord.name = resolvedPackageWorkflowName (use
the existing variables resolvedPackageWorkflowName, packageWorkflowName,
workerName, workflows and workflowRecord to find and update the entry).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce4fe6ee-d71f-44a9-8859-8b2159881948
📒 Files selected for processing (19)
docs/contributing/packages-and-manifests.mdpackages/worker/src/index.tspackages/worker/src/mcp/run-codemode-registry.node.test.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/package-registry/manifest.node.test.tspackages/worker/src/package-registry/manifest.tspackages/worker/src/package-registry/types.tspackages/worker/src/package-runtime/module-graph.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/package-runtime/package-workflows.node.test.tspackages/worker/src/package-runtime/package-workflows.tspackages/worker/src/test-support/cloudflare-workers-stub.tspackages/worker/src/test-support/sentry-cloudflare-stub.tspackages/worker/worker-configuration.d.tspackages/worker/wrangler.jsonctools/ci/preview-resources.tstools/ci/production-resources.tstools/ci/resource-utils.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/package-runtime/package-app.ts`:
- Around line 266-277: normalizeRunAt currently accepts locale-dependent date
strings but claims to require an ISO string; tighten validation by enforcing
strict ISO-8601 input and return a Date instance. In normalizeRunAt validate
string inputs against an ISO-8601 timestamp pattern (e.g. full-date T time with
optional timezone) before calling new Date(), throw a clear error mentioning
"ISO-8601" when validation fails, and return the parsed Date; also remove or
reconcile duplicate normalization in package-workflows.ts so ISO parsing logic
is centralized rather than duplicated.
In `@packages/worker/src/package-runtime/package-workflows.node.test.ts`:
- Around line 1-10: The test file imports normalizePackageWorkflowParams but
never uses it; remove the unused import from the import list in
package-workflows.node.test.ts (the import statement that currently lists
normalizePackageWorkflowParams alongside createPackageWorkflow,
createPackageWorkflowInstance, etc.) so the lint warning is resolved, or
alternatively add a minimal assertion exercising normalizePackageWorkflowParams
if you prefer keeping it — but the simplest fix is to delete the
normalizePackageWorkflowParams token from the import.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dee3bbf1-089b-4c24-8ece-c7eae17e8777
📒 Files selected for processing (8)
packages/worker/src/mcp/run-codemode-registry.node.test.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/package-runtime/package-app.node.test.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/package-runtime/package-workflows.node.test.tspackages/worker/src/package-runtime/package-workflows.tstools/ci/resource-utils.node.test.tstools/ci/resource-utils.ts
✅ Files skipped from review due to trivial changes (1)
- packages/worker/src/mcp/run-codemode-registry.node.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- tools/ci/resource-utils.ts
- packages/worker/src/package-runtime/package-workflows.ts
- packages/worker/src/mcp/run-codemode-registry.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/mcp/run-codemode-registry.node.test.ts (1)
82-137: ⚡ Quick win“Preserves caller-provided workflow tools” test should exercise the provider call path.
Right now the test only asserts generated wrapper content and that
customWorkflowTools.createwas not called. It can still pass even if provider forwarding breaks. Capture provider fns and invokepackage_workflow_createto prove delegation to the custom tool.Suggested test tightening
test('runModuleWithRegistry preserves caller-provided workflow tools', async () => { const env = {} as Env @@ const customWorkflowTools = { create: vi.fn(async () => ({ ok: true, id: 'custom-workflow' })), } + let providerFns: Record<string, (args: unknown) => Promise<unknown>> | null = + null const createExecuteExecutorSpy = vi .spyOn(await import('#mcp/executor.ts'), 'createExecuteExecutor') .mockReturnValue({ - async execute(wrapped) { + async execute(wrapped, providers) { expect(wrapped).toContain( 'codemode.package_workflow_create(input ?? {})', ) + providerFns = ( + providers[0] as { + fns: Record<string, (args: unknown) => Promise<unknown>> + } + ).fns return { result: 'ok', logs: [], } }, } as never) @@ - expect(customWorkflowTools.create).not.toHaveBeenCalled() + await expect( + providerFns?.package_workflow_create({ workflowName: 'custom' }), + ).resolves.toEqual({ ok: true, id: 'custom-workflow' }) + expect(customWorkflowTools.create).toHaveBeenCalledWith({ + workflowName: 'custom', + }) } finally { createExecuteExecutorSpy.mockRestore() } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/run-codemode-registry.node.test.ts` around lines 82 - 137, The test currently only checks the generated wrapper text and that customWorkflowTools.create was not called; update the mocked createExecuteExecutor.execute to simulate the provider call path by extracting or invoking the provider function passed into the generated wrapper (the package_workflow_create provider) so it delegates to the supplied workflowTools; specifically, in the spy returned by createExecuteExecutor().execute, after asserting wrapped contains 'codemode.package_workflow_create', execute the provider path by calling the provider's package_workflow_create (or invoking the wrapped module/provider function) with a sample input so that customWorkflowTools.create is actually invoked, then change the test assertion to expect(customWorkflowTools.create).toHaveBeenCalled() to prove delegation from runModuleWithRegistry -> workflows.create -> package_workflow_create -> customWorkflowTools.create.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/worker/src/mcp/run-codemode-registry.node.test.ts`:
- Around line 82-137: The test currently only checks the generated wrapper text
and that customWorkflowTools.create was not called; update the mocked
createExecuteExecutor.execute to simulate the provider call path by extracting
or invoking the provider function passed into the generated wrapper (the
package_workflow_create provider) so it delegates to the supplied workflowTools;
specifically, in the spy returned by createExecuteExecutor().execute, after
asserting wrapped contains 'codemode.package_workflow_create', execute the
provider path by calling the provider's package_workflow_create (or invoking the
wrapped module/provider function) with a sample input so that
customWorkflowTools.create is actually invoked, then change the test assertion
to expect(customWorkflowTools.create).toHaveBeenCalled() to prove delegation
from runModuleWithRegistry -> workflows.create -> package_workflow_create ->
customWorkflowTools.create.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05085268-e841-430e-8d6c-14982a1fd374
📒 Files selected for processing (6)
packages/worker/src/mcp/run-codemode-registry.node.test.tspackages/worker/src/mcp/run-codemode-registry.tspackages/worker/src/package-runtime/package-app.node.test.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/package-runtime/package-workflows.node.test.tspackages/worker/src/package-runtime/package-workflows.ts
✅ Files skipped from review due to trivial changes (1)
- packages/worker/src/package-runtime/package-workflows.node.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/worker/src/mcp/run-codemode-registry.ts
- packages/worker/src/package-runtime/package-workflows.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 61c66ee. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This was referenced May 8, 2026
Expose dynamic Workflows runtime in all execution contexts (hub Workflow + inline
code support)
#401
Merged
5 tasks done
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
2 participants
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
workflows.create(...)through saved package runtime contexts without adding a top-level saved workflow primitive.runAtvalidation at the package-app boundary, matching workflow helper prelude generation to actual workflow tool availability, normalized whitespace handling for all workflow ID inputs, tighter custom workflow tool delegation coverage, and keeping generated package-app worker source private.Testing
npx vitest --config vitest.node.config.ts packages/worker/src/package-runtime/package-workflows.node.test.ts packages/worker/src/package-registry/manifest.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/mcp/tools/search.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/package-runtime/package-workflows.node.test.ts packages/worker/src/package-runtime/package-app.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts tools/ci/resource-utils.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/package-runtime/package-app.node.test.ts packages/worker/src/package-runtime/package-workflows.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/package-runtime/package-workflows.node.test.ts --runnpx vitest --config vitest.node.config.ts packages/worker/src/package-runtime/package-app.node.test.ts --runnpm run typechecknpm run lint(warnings only, all pre-existing/outside this change set)npm run test:pushSummary by CodeRabbit
New Features
Documentation
Tests
Chores