Uh oh!
There was an error while loading. Please reload this page.
[errors] Replace chalk import with inline ANSI shim - #1915
Merged
Conversation
`@workflow/errors/ansi` is reachable from the workflow-VM bundle via
`@workflow/core/workflow` → `context-errors` → `context-violation-error`
→ here. The real `chalk` package pulls in `supports-color`, which calls
`require('os')` at module load — crashing every workflow with
`ReferenceError: require is not defined` in the sandboxed VM the moment
any user step is registered:
./src/workflows/v2/workflow:2131
var os = require("os");
^
ReferenceError: require is not defined
at .../node_modules/.pnpm/supports-color@7.2.0/.../index.js:2:11
at __require (./src/workflows/v2/workflow:11:50)
at .../node_modules/.pnpm/chalk@4.1.2/.../source/index.js:3:53
at __require (./src/workflows/v2/workflow:11:50)
at .../@workflow/errors/src/ansi.ts:1:18
at Script.runInContext (node:vm:149:12)
Replace `chalk` with a 25-line inline SGR helper exposing the same call
surface (`chalk.red`, `chalk.bold`, …). Color detection mirrors chalk's
default at a coarse level — `FORCE_COLOR` on, `NO_COLOR` off, otherwise
gated on `process.stdout.isTTY` — so non-TTY logs and the workflow VM
(no `process`) get plain text, matching previous behavior in tests and
in production log drains.
Drops the `chalk` runtime dependency from `@workflow/errors` and removes
the now-unused `__mocks__/chalk.ts` Vitest mock.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Contributor
🦋 Changeset detectedLatest commit: 47be549 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 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 |
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
VaguelySerious
enabled auto-merge (squash)
May 4, 2026 10:41
Move the inline ANSI shim out of `ansi.ts` into its own
`./internal-chalk.ts` so tests can swap it for a tag-emitting mock via
`vi.mock('./internal-chalk.js', …)`. Snapshots for `Ansi.code` /
`Ansi.hint` / `Ansi.note` / `Ansi.help` / `Ansi.docs` go back to the
readable HTML-like form (`<blue><b>hint:</b> try reloading</blue>`),
making it obvious from the test file which fragments are colored and
how — the same review affordance the previous `__mocks__/chalk.ts`
mock provided before the chalk dep was dropped.
`Ansi.frame` and `Ansi.inline` remain plain in snapshots because they
don't apply any styling themselves; that matches the pre-fix behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>TooTallNate
approved these changes
May 4, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
LGTM. The diagnosis is correct, the fix is appropriately surgical, and the shim is well-written.
Verified
Reachability claim — confirmed: packages/core/src/workflow/index.ts → context-errors.ts → context-violation-error.ts → import * as Ansi from '@workflow/errors/ansi'. So any consumer that uses workflow-context-checked APIs (createHook, createWebhook, getWorkflowMetadata, etc.) ends up dragging the entire ansi/chalk transitive graph into the workflow VM bundle.
Output parity — for the only nested call in ansi.ts (code(): chalk.italic(\${chalk.dim('`')}${str}${chalk.dim('`')}`)), the shim produces byte-for-byte the same SGR sequence as chalk@5.6.2`:
\x1b[3m\x1b[2m`\x1b[22mfoo\x1b[2m`\x1b[22m\x1b[23m
The 22 (normal intensity) close after each dim correctly leaves italic (3) on, since 22 only resets bold/dim — same as chalk's behavior.
Bundle effect — built workbench/example from this branch and grepped the workflow VM bundle (the string passed to workflowEntrypoint):
$ grep -cE "supports-color|require\(['\"](node:)?os['\"]|chalk|internal_chalk" /tmp/vm-bundle.txt
0
No chalk, no supports-color, no os anywhere. (The example workbench's VM bundle didn't actually pull in context-violation-error due to tree-shaking in this particular workflow set, so the chalk references were already absent on main for this specific bundle — but the change is still correct, and downstream consumers like vade that exercise the workflow-context paths will get the fix.)
Tests — pnpm --filter @workflow/errors test passes (4 files, 26 tests including all 11 ansi.test.ts snapshots), pnpm --filter @workflow/core test passes (820 tests).
CI failures are pre-existing flakes
The two red checks (Unit Tests (ubuntu-latest), E2E Required Check) are both from the same flaky world-postgres test:
FAIL test/spec.test.ts > sequential steps with stream complete in a single flow invocation
AssertionError: expected +0 to be 1 // Object.is equality
This same test fails on origin/main HEAD (run 25343680860 job 74307194029) and is unrelated to this change.
Minor nits (non-blocking)
colorEnabledis evaluated once at module-load time, whereaschalkre-evaluates lazily on each call. If a host process programmatically flipsFORCE_COLOR/NO_COLORafter@workflow/errorsis imported, the shim won't pick up the change. In practice no one in this codebase does that, and the JSDoc oninternal-chalk.tsis implicit about it — but if you wanted to be belt-and-braces, wrapping the env-var check insidesgr()would keep parity. Totally fine to leave as-is.The
__mocks__/chalk.tsdeletion is correct, but worth noting that the module-graph entrypoint for the test mock changed from "thechalkpackage as resolved by node" to "this sibling file at a literal relative path". That's fine for this codebase but means the mock now relies onvi.mock('./internal-chalk.js', ...)paths rather than package identity — slightly more fragile ifansi.tsis ever moved to a different directory. Fine for now.
Approving.
Uh oh!
There was an error while loading. Please reload this page.
pranaygp added a commit
that referenced
this pull request
May 5, 2026
* origin/main: [core] Skip inline step execution when suspension also has a wait (#1924) [errors] Replace chalk import in @workfow/errors with inline ANSI shim (#1915) Fix compatibility with Zod 4.4.x (#1902) Serialize `run_failed`/`step_failed` errors through serialization pipeline (#1851) tarballs: redesign preview tarballs index page (#1911) Remove extra changeset (#1922) Add stable Next.js eager and lazy test coverage (#1747) Enforce per-(run, correlation) uniqueness for entity-creating events in world-postgres (#1878) fix(world-vercel): add default request timeout to workflow-server HTTP calls (#1807)
This was referenced May 4, 2026
pranaygp added a commit
that referenced
this pull request
May 5, 2026
…ignal * origin/main: [core] Skip inline step execution when suspension also has a wait (#1924) [errors] Replace chalk import in @workfow/errors with inline ANSI shim (#1915) Fix compatibility with Zod 4.4.x (#1902) Serialize `run_failed`/`step_failed` errors through serialization pipeline (#1851) tarballs: redesign preview tarballs index page (#1911) Remove extra changeset (#1922) Add stable Next.js eager and lazy test coverage (#1747) Enforce per-(run, correlation) uniqueness for entity-creating events in world-postgres (#1878) fix(world-vercel): add default request timeout to workflow-server HTTP calls (#1807) Allow disabling step sourcemap with new `sourcemap` option in builders (#1842) [ci] Enable Vercel-prod e2e for tanstack-start (#1904) web: configure vercelPreset() for Vercel deployments (#1815) [core] Combine flow+step bundle and process steps eagerly (#1338) [world-vercel] Revert stream close control framing (#1891) [tarballs] Use turbo to build workspace deps before packing (#1908) # Conflicts: # packages/core/src/runtime/step-handler.test.ts # packages/core/src/runtime/step-handler.ts # packages/core/src/runtime/suspension-handler.ts # packages/core/src/step.test.ts # packages/world-local/src/storage/events-storage.ts # packages/world-postgres/src/drizzle/migrations/meta/_journal.json
This was referenced May 5, 2026
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 freeto 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
@workflow/errors/ansiimportschalk, which transitivelyrequire('os')s viasupports-color. That module is reachable from the workflow-VM bundle through:The workflow VM has no
require(), so importing chalk crashes every workflow at registration time:Fix
Replace
chalkwith a 25-line inline SGR helper that exposes the same call surface (chalk.red,chalk.bold, …). Color detection mirrors chalk's defaults at a coarse level —FORCE_COLORon,NO_COLORoff, otherwise gated onprocess.stdout.isTTY— so non-TTY logs and the workflow VM (noprocess) get plain text, matching the prior behavior in tests and in production log drains.Drops the
chalkruntime dependency from@workflow/errorsand removes the now-unused__mocks__/chalk.tsVitest mock. All 820@workflow/coreunit tests + 11@workflow/errorssnapshot tests pass.Test plan
pnpm --filter @workflow/errors test(26 tests pass)pnpm --filter @workflow/core test(820 unit tests pass; e2e excluded)ReferenceError: require is not defined🤖 Generated with Claude Code