Skip to content

JITSU-158: explicit typing + unsafe-any lint gate on config exports - #1441

Open
absorbb wants to merge 5 commits into
fix/jitsu-139-export-resiliencefrom
fix/jitsu-158-export-typing
Open

JITSU-158: explicit typing + unsafe-any lint gate on config exports#1441
absorbb wants to merge 5 commits into
fix/jitsu-139-export-resiliencefrom
fix/jitsu-158-export-typing

Conversation

@absorbb

@absorbb absorbb commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Action items 2 and 3 of JITSU-158 (2026-07-30 blank-options postmortem).

Stacked on #1433 (fix/jitsu-139-export-resilience) — both rewrite the same export file; merge #1433 first, then this retargets to newjitsu automatically.

Item 2 — break the silent-any (commit 1)

Every paginated findMany in pages/api/admin/export/[name]/index.ts now carries an explicit Prisma.…GetPayload annotation. The cursor back-edge (cursor:lastIdobjects[last].id) made inference self-referential, and the checker silently collapsed the result to any — which is how { data_ } (for { data: data_ }) compiled on July 30.

Verified: re-introducing the exact incident typo now fails with TS2339: Property 'data_' does not exist on type '{ from: …; to: …; workspace: … }'.

Along the way:

  • All Prisma Json column reads (link.data, object.config, pb.connectionOptions, intermediateStorageCredentials) go through tolerant zod parsers — field-level catch drops only the offending field, non-object roots normalize to {}, so a junk row still can't fail the export (consistent with fix: harden config export/distribution pipeline (JITSU-139 postmortem) #1433's per-entity resilience).
  • $queryRaw / billing pgPool queries get typed results; rpc() backup-connections result is validated as an array instead of trusted as any.
  • object-hash / stable-hash imports pinned to explicit signatures (no shipped types / exports map without a types condition — their inferred type differs between tsc and the type-aware lint program).
  • getCoreDestinationTypeNonStrict accepts string | undefined — call sites always could pass undefined; the signature just couldn't say so.

Behavior preserved, except three crash→skip fixes on malformed rows (domain config without string name, classic-mapping without string value, canceled subscription with null period_end — each previously threw mid-export).

Item 3 — lint gate (commit 2)

@typescript-eslint/no-unsafe-assignment / -member-access / -argument as errors on pages/api/admin/**, type-aware via the project tsconfig. Runs inside pnpm lint, already part of the required ✨ Lint & Test check — no CI changes needed.

The export endpoints pass clean. Eight admin files with pre-existing implicit-any debt (261 findings) carry a file-top exemption header scoped to exactly the rules they violate — the ratchet: remove the header when fixing a file, never add new ones.

Verification

  • tsc --noEmit — clean
  • eslint . — 0 errors (2 pre-existing react-hooks warnings, untouched files)
  • vitest --project unit — 30/30
  • incident-typo repro — fails to compile (see above)

🤖 Generated with Claude Code

absorbb and others added 2 commits August 4, 2026 16:06
…U-158 #2)

Annotate every paginated findMany in the export endpoints with explicit
Prisma GetPayload types. The cursor back-edge previously made inference
self-referential and TypeScript silently collapsed the results to any,
which is how the 2026-07-30 blank-options typo ({ data_ } for
{ data: data_ }) compiled. With the annotations the same typo now fails
with TS2339 (verified).

All Prisma Json column reads now go through tolerant zod parsers
(field-level catch drops only the offending field) instead of implicit
any. Raw queries and the billing pg query get typed results. object-hash
and stable-hash imports are pinned to explicit signatures - their
inferred types differ between tsc and the type-aware lint program.

getCoreDestinationTypeNonStrict accepts string | undefined - call sites
were always able to pass undefined; the signature just could not say so.

Behavior deliberately preserved except three crash-to-skip fixes on
malformed rows: a domain config without a string name, a classic-mapping
without a string value, and a canceled subscription with null period_end
no longer throw mid-export.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…TSU-158 #3)

Enable @typescript-eslint/no-unsafe-assignment/-member-access/-argument
as errors on pages/api/admin/**, type-aware via project tsconfig. The
export endpoints pass clean; the eight admin files with pre-existing
implicit-any debt carry an explicit file-top exemption header scoped to
the rules they actually violate - remove the header when fixing a file,
never add new ones. Runs as part of pnpm lint, which is already in the
required Lint & Test check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@jitsu-code-review jitsu-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the diff from e307cd5 to 6507f93, focusing on admin export serialization paths and the new type-aware lint guard on pages/api/admin/**.

I did not find actionable bugs, security issues, or correctness regressions in this changeset. The refactor appears to preserve existing behavior while hardening unsafe JSON/object handling and null cases.

Connection options in the three connection exports now parse through the
destination type's own connectionOptions schema (destinations.tsx) with
.passthrough(), falling back to the generic tolerant parse only when the
type is unknown or the stored data does not conform (logged at warn).

Absent fields therefore materialize to the console defaults - the export
emits explicit deduplicate: true, mode: batch, frequency: 60,
primaryKey: message_id, ... instead of leaving consumers to re-default
absent options themselves, differently and unsafely (JITSU-136). A
blank-data row - the 2026-07-30 failure shape - now exports safe
defaults rather than nothing. Stored values always win over defaults;
unknown keys pass through untouched.

Note: one-time optionsHash churn on deploy, and connections that never
persisted frequency move from bulker's fast absent-default to the 60m
the UI has always shown.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@absorbb

absorbb commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Added a third commit per discussion: connection options now parse through the destination type's own connectionOptions schema (destinations.tsx) with .passthrough(), generic tolerant parse only as the corner-case fallback (unknown type / non-conforming stored data, logged at warn).

Consequence (deliberate): absent fields materialize to console defaults — the export emits explicit deduplicate: true, mode: batch, frequency: 60, …, so consumers never re-default absent options themselves (JITSU-136, JITSU-158 item 8 console-side). A blank-data row — the exact 2026-07-30 failure shape — now exports safe defaults instead of nothing. Stored values win over defaults; unknown keys flow through.

Heads-up for rollout: one-time optionsHash churn across all connections, and connections that never persisted frequency move from bulker's fast absent-default (~1 min) to the 60 min the UI has always claimed.

The console default (60m) differs from bulker's absent-option default,
so materializing it would silently change the batch cadence of every
connection that never persisted a frequency. All other defaults still
materialize; frequency exports only when stored (explicit null included).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@absorbb

absorbb commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Back-compat amendment: frequency is now excluded from default materialization — it exports only when actually stored (explicit null included), so connections that never persisted a frequency keep bulker's current cadence. All other defaults (deduplicate, mode, primaryKey, dataLayout, batchSize, …) still materialize.

@jitsu-code-review jitsu-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the admin export/type-safety changes across the PR diff, with focus on correctness, runtime behavior, and potential regressions in exported payloads.\n\nI did not find actionable bugs or security/correctness regressions in this changeset. The refactor mostly hardens typing/parsing paths and keeps export behavior aligned while improving failure handling around malformed data.

@jitsu-code-review jitsu-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the admin export/type-safety changes in webapps/console/pages/api/admin/export/[name]/index.ts, the destination type helper update, and the new admin ESLint unsafe-any gate.

I focused on correctness and regression risk in export payload generation, per-entity error handling, and defaulting behavior. I did not find actionable bugs or security regressions in this changeset.

.passthrough() only affects the object it is called on - unknown keys
inside nested declared objects were still stripped by the per-type
parse. Prod data has real instances (an enabled flag on functions[]
entries of 3 live connections) that the old spread-raw-data export
preserved. deepPassthrough rebuilds the connectionOptions schema with
passthrough on every nested object; defaults and validation semantics
are unchanged (verified).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@absorbb

absorbb commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

One more hardening after checking prod data: .passthrough() only applies to the object it's called on, and 3 live connections carry an enabled flag inside functions[] entries that the nested schema would have stripped. Added deepPassthrough — rebuilds the per-type schema with passthrough at every object level, so stored fields survive at any depth. Defaults and validation semantics unchanged (tested: nested extras kept, blank-{} still materializes defaults, invalid entries still fall back).

@jitsu-code-review jitsu-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the range e307cd57864f53933dab2dbd39f7b68fccb04be9..461c867a4363ed2e3a0c253645978f64b44649f4, focusing on admin export behavior, sync export correctness, and typed-any hardening changes.

I did not find actionable bugs, security issues, or user-visible regressions in this diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant