Skip to content

feat(wrangler): introspect workflow with test harness#14438

Open
edmundhung wants to merge 5 commits into
mainfrom
edmundhung/workflow-harness-introspection
Open

feat(wrangler): introspect workflow with test harness#14438
edmundhung wants to merge 5 commits into
mainfrom
edmundhung/workflow-harness-introspection

Conversation

@edmundhung

@edmundhung edmundhung commented Jun 26, 2026

Copy link
Copy Markdown
Member

Fixes n/a.

This adds Workflow introspection to createTestHarness()


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: it will be covered with the rest of the test harness docs changes.

A picture of a cute animal (not mandatory, but encouraged)


Open in Devin Review

@changeset-bot

changeset-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4b12b7f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@cloudflare/vitest-pool-workers Minor
miniflare Patch
wrangler Minor
@cloudflare/deploy-helpers Patch
@cloudflare/pages-shared Patch
@cloudflare/vite-plugin Patch

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

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Jun 26, 2026
@workers-devprod workers-devprod requested review from a team and jamesopstad and removed request for a team June 26, 2026 13:07
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/workflows
  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/sharp-workflows-wait.md: [@cloudflare/wrangler]
  • .changeset/soft-workflows-smile.md: [@cloudflare/wrangler]
  • fixtures/vitest-pool-workers-examples/workflows/test/integration.test.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/workers/workflows/wrapped-binding.worker.ts: [@cloudflare/wrangler]
  • packages/vitest-pool-workers/src/worker/workflows.ts: [@cloudflare/wrangler]
  • packages/vitest-pool-workers/types/cloudflare-test.d.ts: [@cloudflare/wrangler]
  • packages/workflows-shared/src/binding.ts: [@cloudflare/workflows @cloudflare/wrangler]
  • packages/workflows-shared/src/introspection.ts: [@cloudflare/workflows @cloudflare/wrangler]
  • packages/workflows-shared/src/types.ts: [@cloudflare/workflows @cloudflare/wrangler]
  • packages/wrangler/e2e/createTestHarness.test.ts: [@cloudflare/wrangler]
  • packages/wrangler/src/api/test-harness.ts: [@cloudflare/wrangler]
  • packages/wrangler/tsup.config.ts: [@cloudflare/wrangler]

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review posted successfully to PR #14438.

Summary of my review:

  • Verified the PR consolidates Workflow introspection into @cloudflare/workflows-shared and exposes it on createTestHarness() worker handles, replacing the old env-binding Proxy approach with a module-scoped session map in WorkflowBinding.
  • Confirmed correct: the get() sync→async breaking change is properly documented in a minor changeset with migration guidance; the "queued" early-return matches instanceStatusName(InstanceStatus.Queued); createBatch tracking correctly flows through create().
  • One actionable (low-severity) issue: posted a suggestion on introspection.ts:142 for a potential unhandled promise rejection in WorkflowInstanceIntrospectorHandle — the constructor's modifier promise is only awaited inside modify(), so the new introspectWorkflowInstance flow (which may never call modify()) can leak an unhandled rejection on RPC failure.

github run

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@14438

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@14438

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@14438

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@14438

miniflare

npm i https://pkg.pr.new/miniflare@14438

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@14438

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@14438

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@14438

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@14438

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@14438

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@14438

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@14438

wrangler

npm i https://pkg.pr.new/wrangler@14438

commit: 4b12b7f

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +150 to +157
async modifyAll(fn: ModifierCallback): Promise<void> {
const sessionId = this.getSessionId();
await fn(new WorkflowInstanceModificationRecorder(this.#operations));
await this.workflow.unsafeSetIntrospectionOperations(
sessionId,
this.#operations
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Operations accumulate across multiple modifyAll calls rather than replacing

Each modifyAll call creates a new WorkflowInstanceModificationRecorder wrapping the same #operations array (packages/workflows-shared/src/introspection.ts:152). Operations pushed by the callback accumulate, and the full array is sent to the remote via unsafeSetIntrospectionOperations, which replaces (session.operations = operations at packages/workflows-shared/src/binding.ts:272). This means a second modifyAll call includes all operations from the first call. Current test usage only calls modifyAll once per introspector, so the accumulation pattern isn't exercised. If a user calls modifyAll twice expecting replacement semantics, they'd get unexpected duplication of operations like disableSleeps. This is a design choice rather than a bug, but the behavior isn't documented.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is the same behavior before this change.

@edmundhung edmundhung requested review from a team, penalosa and pombosilva and removed request for a team and jamesopstad June 26, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants