Skip to content

refactor(utils): consolidate duplicated helpers onto @sim/utils#5509

Merged
waleedlatif1 merged 4 commits into
stagingfrom
utils-consolidation-audit
Jul 8, 2026
Merged

refactor(utils): consolidate duplicated helpers onto @sim/utils#5509
waleedlatif1 merged 4 commits into
stagingfrom
utils-consolidation-audit

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Comprehensive audit of the repo for reimplementations of @sim/utils helpers (errors, id/random, retry, object, string, formatting) and fixed every genuine duplicate found.
  • Wired check:utils (existing enforcement script, previously unused in CI) into test-build.yml so these patterns can't silently regress.

What changed

  • Errors: error instanceof Error ? error.message : fallbackgetErrorMessage(); manual .code === '23505' checks → getPostgresErrorCode() (19 files)
  • Sleep / Math.random / retry: inline setTimeout promises → sleep(); Math.random()randomFloat/randomInt/generateShortId(); apps/sim/tools/index.ts's local calculateBackoff/parseRetryAfterHeaderbackoffWithJitter/parseRetryAfter (already importing sibling @sim/utils helpers in the same file)
  • noop / object filtering: 8 duplicate const noop = () => {} → shared noop; Object.fromEntries(Object.entries(...).filter(...))filterUndefined/omit in packages/workflow-persistence, packages/logger (added as a new workspace dependency, zero circularity risk), and apps/sim/providers/utils.ts
  • String truncation: 17 hand-rolled slice+ellipsis sites → truncate(), preserving exact visible output (two sites were unconditional before and are now correctly conditional — flagged as intentional behavior fixes)
  • Date/time formatting: local getTimezoneAbbreviation/relative-time ladder/toLocaleDateString reimplementations (10 files, including two files with an identical copy-pasted helper) → getTimezoneAbbreviation/formatRelativeTime/formatDate/formatDateTime. A few of these are minor visible format changes (e.g. teammates/billing dates move from locale-dependent M/D/YYYY to fixed MMM D, YYYY) in exchange for one consistent format across the app.
  • Email normalization: .trim().toLowerCase()normalizeEmail() at 13+ call sites across auth/invite/billing flows
  • Plain-object guards: local isRecord/isPlainObject reimplementations → isRecordLike() (7 sites)

Intentionally skipped (documented in code/PR, not silent): packages/ts-sdk and packages/cli (standalone published packages, already exempt in check-utils-enforcement.ts), packages/emcn's email handler (kept dependency-free by design), and a handful of lower-confidence tool-output/chart-axis date formatters that aren't generic UI dates.

Test plan

  • bun run check:utils — passes (was previously failing with ~30 violations, never run in CI)
  • bunx tsc --noEmit -p apps/sim/tsconfig.json — clean
  • bunx biome check — clean
  • bun run check:api-validation — passes
  • All 13 directly-touched test files + tools/index.test.ts + providers/utils.test.ts + 5 more files with dedicated coverage — 443 tests passing

Replaces ~90 hand-rolled reimplementations of error-message extraction,
postgres error-code checks, sleep, Math.random, retry/backoff, object
filtering/omission, noop, string truncation, date/time formatting, email
normalization, and plain-object type guards with the shared @sim/utils
exports. Wires check:utils into CI (test-build.yml) so these patterns
don't regress.
@waleedlatif1 waleedlatif1 requested a review from a team as a code owner July 8, 2026 17:43
@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jul 8, 2026 6:14pm

Request Review

@cursor

cursor Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches auth/email normalization, invite flows, tool HTTP retries, and schedule jitter—behavior should stay equivalent but spans many critical paths; minor visible date-format changes in some settings/billing UI.

Overview
Replaces scattered inline helpers across the monorepo with @sim/utils exports and adds bun run check:utils to the CI test-build workflow so duplicate patterns cannot regress.

Errors & DB: getErrorMessage, getPostgresErrorCode, and toError replace ad-hoc instanceof Error and .code === '23505' checks in API routes, blocks, copilot, and tooling.

Auth & email: Login, public file OTP/SSO, org invites, billing credits, deployment allow-lists, and related UI now use normalizeEmail() instead of repeated trim().toLowerCase().

Formatting & strings: Shared formatDate, formatDateTime, formatRelativeTime, and getTimezoneAbbreviation replace local date helpers; truncate() replaces hand-rolled slice+ellipsis in many UI and log paths (some invoice/teammate labels may shift from locale-specific to a fixed format).

Async & random: sleep(), noop, randomInt/randomFloat, generateShortId, and retry backoffWithJitter/parseRetryAfter replace local setTimeout promises, no-op lambdas, Math.random, and duplicate backoff logic in tools/index.ts and schedule jitter.

Object guards: isRecordLike, filterUndefined, and omit replace local plain-object checks and manual entry filtering; packages/logger gains a workspace dependency on @sim/utils for filterUndefined.

Reviewed by Cursor Bugbot for commit 3a06e25. Configure here.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3a06e25. Configure here.

Comment thread apps/sim/tools/index.ts
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR is a comprehensive deduplication refactor across 93 files, consolidating hand-rolled helpers (error extraction, sleep/retry/backoff, string truncation, date/time formatting, email normalization, plain-object guards) onto the shared @sim/utils package, and wires the existing check:utils enforcement script into CI so these patterns can't silently reappear.

  • Retry/backoff (tools/index.ts): local calculateBackoff/parseRetryAfterHeader replaced by backoffWithJitter/parseRetryAfter; the return-type change (numbernumber | null) is correctly handled at all three call sites with ?? 0 and explicit null checks.
  • Date/time formatting: 10+ local relative-time and date formatters replaced by formatRelativeTime/formatDate/formatDateTime; a few visible format changes (e.g. "yesterday" → "1d ago", locale-dependent dates → fixed en-US MMM D, YYYY) are intentional and documented.
  • Email, truncation, object filtering, record guards: normalizeEmail, truncate, filterUndefined/omit, and isRecordLike replace their respective inline reimplementations across auth, billing, logging, and persistence code.

Confidence Score: 4/5

Safe to merge; the one display-quality regression in schedule timezone labels for non-hardcoded IANA zones is cosmetic and does not affect data or workflow execution.

The vast majority of substitutions are mechanically correct and the retry/backoff rework in tools/index.ts handles the changed null-return contract properly at every call site. The only notable regression is in schedules/utils.ts: the deleted local getTimezoneAbbreviation resolved abbreviations for any IANA timezone via Intl.DateTimeFormat, while the replacement from @sim/utils hardcodes 9 zones and returns the raw IANA string for everything else — so users with timezones like Europe/Berlin or America/Toronto will see the full IANA name in schedule descriptions instead of a short abbreviation.

apps/sim/lib/workflows/schedules/utils.ts — the getTimezoneAbbreviation substitution degrades display quality for any IANA timezone not in the shared util's 9-entry hardcoded map

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Replaces local calculateBackoff/parseRetryAfterHeader with shared backoffWithJitter/parseRetryAfter; null-return difference from parseRetryAfter correctly handled at all three call sites
apps/sim/lib/workflows/schedules/utils.ts Removes local getTimezoneAbbreviation in favour of the shared util, which only covers 9 IANA timezones and falls back to raw IANA strings for all others — regression in schedule display quality
apps/sim/app/workspace/[workspaceId]/logs/utils.ts Replaces hand-rolled relative-time ladder with formatRelativeTime; intentional visible changes: 'yesterday' → '1d ago', dates ≥7 days change from absolute 'Jan 5' to relative 'Xw/Xmo/Xy ago'
packages/logger/src/index.ts Replaces inline Object.entries filter with filterUndefined; adds @sim/utils as a new dependency — zero-circularity confirmed
packages/workflow-persistence/src/subblocks.ts Replaces combined null+undefined filter with filterUndefined + separate null-check; semantically equivalent at runtime
apps/sim/providers/utils.ts Replaces forEach delete with omit(result, sourceIds); result is typed Record<string, any> so string[] keys are valid
apps/sim/lib/table/sql.ts Replaces inline typeof/null/Array.isArray guard with isRecordLike; adds explicit as JsonValue cast in else branch
.github/workflows/test-build.yml Wires bun run check:utils enforcement script into CI — previously unused in CI

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["apps/sim (93 files)"] --> B["@sim/utils"]
    C["packages/logger"] --> B
    D["packages/workflow-persistence"] --> B
    B --> E["errors\ngetErrorMessage / toError / getPostgresErrorCode"]
    B --> F["string\nnormalizeEmail / truncate"]
    B --> G["retry\nbackoffWithJitter / parseRetryAfter"]
    B --> H["formatting\nformatDate / formatDateTime\nformatRelativeTime / getTimezoneAbbreviation"]
    B --> I["object\nfilterUndefined / omit / isRecordLike"]
    B --> J["helpers\nsleep / noop"]
    K[".github/workflows/test-build.yml"] --> L["bun run check:utils\n(CI enforcement)"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["apps/sim (93 files)"] --> B["@sim/utils"]
    C["packages/logger"] --> B
    D["packages/workflow-persistence"] --> B
    B --> E["errors\ngetErrorMessage / toError / getPostgresErrorCode"]
    B --> F["string\nnormalizeEmail / truncate"]
    B --> G["retry\nbackoffWithJitter / parseRetryAfter"]
    B --> H["formatting\nformatDate / formatDateTime\nformatRelativeTime / getTimezoneAbbreviation"]
    B --> I["object\nfilterUndefined / omit / isRecordLike"]
    B --> J["helpers\nsleep / noop"]
    K[".github/workflows/test-build.yml"] --> L["bun run check:utils\n(CI enforcement)"]
Loading

Comments Outside Diff (1)

  1. apps/sim/lib/workflows/schedules/utils.ts, line 503 (link)

    P2 getTimezoneAbbreviation falls back to raw IANA names for unlisted zones

    The shared getTimezoneAbbreviation from @sim/utils/formatting only covers 9 hardcoded IANA timezones, returning the raw timezone string (e.g. "Europe/Berlin") for everything else. The deleted local function used Intl.DateTimeFormat with timeZoneName: 'short' to resolve a proper abbreviation (e.g. "CET" or "CEST") for any valid IANA timezone. Users with timezones outside those 9 — Europe/Berlin, America/Toronto, Asia/Hong_Kong, Pacific/Auckland, etc. — will now see the full IANA string inside the parenthetical: "Every day at 9:00 AM (Europe/Berlin)" instead of "Every day at 9:00 AM (CET)".

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "refactor(utils): consolidate duplicated ..." | Re-trigger Greptile

…execute tests

The schedules/execute route previously used Math.random() for jitter delay;
this consolidation PR switched it to randomInt() from @sim/utils/random,
which is backed by crypto.getRandomValues() rather than Math.random(). The
route.test.ts spies on Math.random() no longer had any effect, so jitter
became real random delay instead of the deterministic 0ms the tests expect,
causing intermittent 10s timeouts in CI.
parseRetryAfter() caps its return value at 30s by default. tools/index.ts
compares the parsed Retry-After against a caller-configured maxDelayMs to
decide whether to skip a retry entirely -- capping before that comparison
silently defeats the skip check whenever maxDelayMs is configured above
30s, since a Retry-After between 30s and maxDelayMs would incorrectly look
"within limits" and get retried instead of skipped (caught by Cursor
Bugbot). Added an optional maxMs param (default unchanged) so tools/index.ts
can request the raw, uncapped value for its own comparison while
backoffWithJitter still clamps the actual sleep duration to maxDelayMs.
Added a regression test covering maxDelayMs > 30s.
…zones

getTimezoneAbbreviation only covered 9 hardcoded IANA zones and returned
the raw IANA string for everything else, degrading schedule descriptions
for zones like Europe/Berlin or America/Toronto (caught by Greptile). The
deleted local implementation in schedules/utils.ts resolved any valid IANA
timezone generically via Intl.DateTimeFormat's short timeZoneName. Restore
that as a fallback so only genuinely invalid timezone strings return
themselves unchanged.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Addressed both review findings on this branch:

  • Cursor (tools/index.ts Retry-After cap): fixed in 1b4c9b8feparseRetryAfter now takes an optional maxMs so the skip-vs-retry comparison against retryConfig.maxDelayMs uses the raw uncapped value. Regression test added.
  • Greptile (schedules/utils.ts timezone abbreviation regression): fixed in ef55cdaf6getTimezoneAbbreviation in @sim/utils/formatting now falls back to Intl.DateTimeFormat's generic short timeZoneName resolution for any valid IANA zone outside the 9 hardcoded entries, restoring the deleted local implementation's behavior. Regression test added.

Also fixed an unrelated CI test failure in app/api/schedules/execute/route.test.ts (c440d1ac9) — the test spied on Math.random(), which no longer has any effect now that the route uses randomInt() from @sim/utils/random (backed by crypto.getRandomValues()); switched the test to mock @sim/utils/random directly.

@waleedlatif1 waleedlatif1 merged commit 9d34fbe into staging Jul 8, 2026
16 checks passed
@waleedlatif1 waleedlatif1 deleted the utils-consolidation-audit branch July 8, 2026 18:26
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