Skip to content

Commit 10878fb

Browse files
authored
fix(utils): drop the .js specifiers Turbopack cannot resolve (#6351)
* fix(utils): drop the .js specifiers Turbopack cannot resolve Every dev server on staging is currently returning 500 from any route whose module graph reaches the `@sim/utils` barrel: Module not found: Can't resolve './errors.js' > 1 | export { getErrorMessage, getPostgresErrorCode, toError } from './errors.js' Import trace: ./packages/utils/src/index.ts ./apps/sim/lib/embeddings/client.ts ./apps/sim/lib/knowledge/embeddings.ts ./apps/sim/app/api/knowledge/route.ts `packages/utils/src/index.ts` addresses its siblings as `./errors.js` while the files are `./errors.ts`. webpack rewrites that through `resolve.extensionAlias`; Turbopack has no equivalent (vercel/next.js#82945). `next build` is webpack and `next dev` is Turbopack, so this passes CI and breaks every local dev server — #6317 went green. Nothing required the extensions: the repo is on `moduleResolution: "bundler"`, and no other package barrel uses them. Two changes, either of which fixes the symptom; both are here because they fail differently: - `packages/utils/src/index.ts` drops all 12 `.js` specifiers. Fixes the barrel for every current and future consumer. - `apps/sim/lib/embeddings/client.ts` imports `chunkArray` from `@sim/utils/helpers` rather than the barrel. #6317 added the only bare-barrel `@sim/utils` import in the monorepo; the subpath form is the documented convention (CLAUDE.md, "Common Utilities") and resolves to one module instead of pulling twelve. `scripts/check-import-specifiers.ts` fails the build on either shape and runs in CI. Verified it goes red by restoring both halves of the bug. It scans only bundler-compiled source — vitest and standalone `bun run` scripts resolve `.js` -> `.ts` themselves, so flagging their specifiers would be noise. Verified against a real dev server with production env: `/api/knowledge`, `/api/tools/embeddings` and `/api/workflows/[id]/deploy` all go 500 -> 401, `/workspace` renders, and the Turbopack log is free of resolution errors. `tsc --noEmit` clean, `packages/utils` 147/147. * refactor(scripts): resolve specifiers instead of pattern-matching one mistake The first version banned `.js` specifiers by regex, which catches the bug that happened and nothing adjacent to it. This runs the actual resolution algorithm with Turbopack's rules — extensionAlias deliberately absent — and fails on anything that does not land on a real file. That covers the whole "Module not found" class rather than one shape of it: `.js` specifiers, typo'd paths, files moved or deleted with a stale importer left behind, `@/` aliases pointing nowhere, and `@sim/*` subpaths a package does not export. Verified against three synthetic breakages the regex version passed clean: '@/lib/webhooks/providerz' — '@/' alias matches a tsconfig path but nothing is there './does-not-exist' — no file at that path '@sim/utils/chunking' — @sim/utils does not export './chunking' Getting to zero false positives on 37,307 specifiers needed three things the naive version got wrong: - tsconfig `paths` are per-workspace. `@/*` is `apps/sim/*` inside apps/sim but `apps/realtime/src/*` inside apps/realtime, and apps/sim maps `@sim/db/*` straight at the package directory, legitimately bypassing that package's exports map. One hardcoded alias produced ~30 false positives in apps/realtime alone. - `exports` maps have wildcards. `@sim/emcn` publishes `"./*": "./src/*"`, so `@sim/emcn/components/code/code.css` is valid despite no literal entry. - TSDoc contains example imports. `packages/db/triggers.ts` documents `import { ensureRowCountTriggers } from '@sim/db/triggers'` — a subpath the package deliberately does not export. Comments are now blanked in place, preserving byte offsets so reported line numbers stay exact. * fix(scripts): close three coverage gaps in the specifier audit Review round 1 on #6351. All three findings were real and all three let the exact regression this guard exists for slip through. - Reported line numbers were one early. `SPECIFIER_RE` opens with `(?:^|\n)`, so `m.index` is the newline ENDING the previous line, not the start of the statement. `./helpers.js` on line 13 was reported as line 12. Anchoring to the specifier's own offset is exact, and for a multi-line import it points at the `from '...'` line — where the reader needs to look anyway. - `require()` was not scanned. This repo uses lazy requires deliberately to break import cycles: `tools/params.ts` reaches `@/blocks` that way and `blocks/blocks/agent.ts` reaches `@/blocks/registry`, 22 first-party call sites in total. Those edges resolve exactly like static ones, so a bad specifier in one fails identically. Verified by pointing `tools/params.ts` at a non-existent module and watching the audit catch it. - `apps/docs` was not scanned, despite being a second Next.js app with its own `next.config.ts` — so it carries identical Turbopack exposure. Now covered, and clean. Side-effect imports and dynamic `import()` were called out in the same round but are already covered: the optional `from` group in `SPECIFIER_RE` matches bare `import '...'`, and `DYNAMIC_RE` handles `import('...')`. That review ran against 1c6073e, before the resolver rewrite. Coverage goes from 37,307 specifiers across 11,182 files to 37,438 across 11,243, still with zero violations. * chore(tools): regenerate the stale tool metadata `bun run tool-metadata:check` has been failing on staging since #6317, so every PR branched off it inherits a red CI regardless of its own contents. Reproduced against a clean `origin/staging` to confirm it is not this branch's doing. #6317 rewrote the embeddings tools' `apiKey` descriptions from provider-specific strings to one generic string in `tools/embeddings/factory.ts`, but did not regenerate `tools/generated/tool-metadata.ts`. The whole delta is 89 bytes of description text — the tool set is unchanged at 4380 ids, none added, none removed: - "description":"Cohere Embeddings API key" + "description":"API key for the selected embedding provider" The old strings no longer exist anywhere in source, so the generated file was the stale side. `tool-metadata:check` passes after regenerating, and the generator's own resolver cross-check agrees. `mship:check` and `mship-tools:check` also fail locally, but neither is a CI gate and both fail only because they read contracts from the sibling copilot repo, which is not checked out here. Left alone. * fix(scripts): substitute every wildcard in a resolved target CodeQL js/incomplete-sanitization, two instances, both correct. `String.replace('*', x)` fills only the first occurrence. Node's `exports` resolver uses a global regex, so a target carrying more than one `*` — e.g. `"./src/*/index-*.ts"` — gets every occurrence substituted. Replacing only the first leaves a literal `*` in the path, so `probe()` finds nothing and the audit reports a perfectly valid subpath as missing. TypeScript `paths` allows at most one `*`, so the tsconfig branch was already correct in practice; it changes for consistency and because nothing enforces that assumption. Not a suppression — the resolver now matches Node's behaviour. 37,438 specifiers still resolve clean. * fix(scripts): do not assert on generated output in the specifier audit CI red on a fresh checkout, green locally — the tell that the audit was depending on build state rather than on source. apps/docs/lib/source.ts imports '@/.source/server'. apps/docs maps '@/.source/*' at './.source/*', which fumadocs-mdx generates and apps/docs/.gitignore excludes. It exists on any machine that has built the docs and is absent from CI's checkout, so the audit reported a valid import as unresolvable. A path landing in output the scanner itself refuses to read as source — node_modules, a build directory, any dot-directory — is now treated as unverifiable rather than missing. That is the consistent rule: if we do not scan it as source, we cannot assert on its presence, and asserting anyway makes the verdict depend on build order. Applied to all three resolution paths (relative, tsconfig paths, exports map), with a GENERATED sentinel keeping 'matched but generated' distinct from 'matched and genuinely missing'. Only the repo-relative portion is inspected. Checking the absolute path would match the '.claude/worktrees/...' a git worktree lives under and silently skip every specifier in the repo. Verified both directions: passes with apps/docs/.source moved away (CI's state), and still catches a require('@/blocks/still-not-real') planted in tools/params.ts. * refactor(scripts): trim the specifier audit's comments The audit shipped at 24% comment lines — the header alone retold the whole incident. Cut to 15% (452 -> 401 lines) by collapsing the narrative and keeping only what the code cannot say: the webpack/Turbopack extensionAlias divergence, why '.js' is a probed extension but not a fallback, why paths resolve per-workspace, why targets substitute with replaceAll, why generated output is unverifiable, and the '.claude/' worktree trap in the relative-path check. No behaviour change: 37,437 specifiers still resolve clean.
1 parent 2b35a3c commit 10878fb

6 files changed

Lines changed: 420 additions & 14 deletions

File tree

.github/workflows/test-build.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ jobs:
162162
- name: Trigger/block initialization cycle audit
163163
run: bun run check:trigger-block-cycle
164164

165+
# This job builds with webpack; devs run Turbopack. A specifier only webpack
166+
# resolves passes here and breaks every dev server.
167+
- name: Import specifier hygiene audit
168+
run: bun run check:import-specifiers
169+
165170
- name: SQL Date binding audit
166171
run: bun run check:sql-date-binding
167172

apps/sim/lib/embeddings/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { chunkArray } from '@sim/utils'
2+
import { chunkArray } from '@sim/utils/helpers'
33
import { env, envNumber } from '@/lib/core/config/env'
44
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
55
import {

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
"check:tool-request-boundary": "bun run scripts/check-tool-request-boundary.ts",
3535
"check:tool-registry-boundary": "bun run scripts/check-tool-registry-boundary.ts",
3636
"check:trigger-block-cycle": "bun run scripts/check-trigger-block-cycle.ts",
37+
"check:import-specifiers": "bun run scripts/check-import-specifiers.ts",
3738
"check:sql-date-binding": "bun run scripts/check-sql-date-binding.ts",
3839
"check:zustand-v5": "bun run scripts/check-zustand-v5-selectors.ts",
3940
"check:react-query": "bun run scripts/check-react-query-patterns.ts --check",

packages/utils/src/index.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export { getErrorMessage, getPostgresErrorCode, toError } from './errors.js'
1+
export { getErrorMessage, getPostgresErrorCode, toError } from './errors'
22
export {
33
formatAbsoluteDate,
44
formatCompactTimestamp,
@@ -9,18 +9,18 @@ export {
99
formatTime,
1010
formatTimeWithSeconds,
1111
getTimezoneAbbreviation,
12-
} from './formatting.js'
13-
export { chunkArray, noop, sleep } from './helpers.js'
14-
export { generateId, generateShortId, isValidUuid } from './id.js'
15-
export type { EmbedInfo } from './media-embed.js'
16-
export { getEmbedInfo } from './media-embed.js'
12+
} from './formatting'
13+
export { chunkArray, noop, sleep } from './helpers'
14+
export { generateId, generateShortId, isValidUuid } from './id'
15+
export type { EmbedInfo } from './media-embed'
16+
export { getEmbedInfo } from './media-embed'
1717
export {
1818
filterUndefined,
1919
isPlainRecord,
2020
isRecordLike,
2121
omit,
2222
sortObjectKeysDeep,
23-
} from './object.js'
23+
} from './object'
2424
export {
2525
generateRandomBytes,
2626
generateRandomHex,
@@ -29,14 +29,14 @@ export {
2929
randomFloat,
3030
randomInt,
3131
randomItem,
32-
} from './random.js'
33-
export type { BackoffOptions } from './retry.js'
34-
export { backoffWithJitter, parseRetryAfter } from './retry.js'
35-
export { normalizeSSODomain } from './sso-domain.js'
32+
} from './random'
33+
export type { BackoffOptions } from './retry'
34+
export { backoffWithJitter, parseRetryAfter } from './retry'
35+
export { normalizeSSODomain } from './sso-domain'
3636
export {
3737
isValidEmailSyntax,
3838
normalizeEmail,
3939
sanitizeForJsonb,
4040
sanitizeValueForJsonb,
4141
truncate,
42-
} from './string.js'
42+
} from './string'

0 commit comments

Comments
 (0)