Skip to content

refactor(sim): consolidate record guards + pure utils into @sim/utils - #5061

Merged
waleedlatif1 merged 2 commits into
stagingfrom
refactor/consolidate-record-utils
Jun 15, 2026
Merged

refactor(sim): consolidate record guards + pure utils into @sim/utils#5061
waleedlatif1 merged 2 commits into
stagingfrom
refactor/consolidate-record-utils

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Add canonical record guards + pure helpers to @sim/utils: isRecordLike (loose), sortObjectKeysDeep; relocate isPlainRecord (strict) and normalizeEmail so they're reusable across apps and packages.
  • Replace ~55 re-implemented loose record guards (typeof === 'object' && !== null && !Array.isArray) across tools/providers/lib/workflows/copilot/triggers/contracts with the single isRecordLike; one strict site → isPlainRecord.
  • Dedupe three true-duplicate normalize* clusters: sortObjectKeysDeep (sanitization + copilot builders), normalizeToken (salesforce + servicenow triggers), normalizeEmail.
  • Intentionally left untouched: array-allowing guards (different predicate) and all domain-specific normalizers (comparison/normalize.ts, copilot persisted-message, vanta, SSO/domain) — migrating those would change behavior.

Type of Change

  • Refactor (no functional change)

Testing

  • New @sim/utils unit tests: loose-vs-strict guard distinction (Date/class-instance → loose true, strict false), deep key sort, email normalization.
  • Typecheck clean (apps + package), biome clean, check:api-validation:strict + monorepo-boundary checks pass.
  • Behavior audited line-by-line by independent adversarial reviewers against origin/staging — every migrated predicate/transform is an exact semantic no-op; the one risky union-narrowing site (lib/table/sql.ts) was deliberately skipped.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Add isRecordLike (loose, non-prototype-checked record guard) and
sortObjectKeysDeep; relocate isPlainRecord (strict) and normalizeEmail
into @sim/utils so they are reusable across apps and packages. Unit tests
cover the loose-vs-strict distinction, deep key sorting, and email
normalization.
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 15, 2026 6:55pm

Request Review

@cursor

cursor Bot commented Jun 15, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Mechanical refactor to shared utilities with intended semantic parity; touches auth/invitation email normalization but behavior matches prior trim+lowercase.

Overview
Centralizes shared pure helpers in @sim/utils and rewires apps/sim to import them instead of redefining the same logic locally.

@sim/utils/object: isRecordLike (loose non-array object guard), isPlainRecord (strict plain-object guard), and sortObjectKeysDeep replace dozens of copy-pasted isRecord / isJsonRecord / isPlainObject checks across API tool routes, Zod contracts, copilot stream parsing, executor start-block handling, webhooks, and UI hooks. lib/core/utils/records now delegates strict checks to isPlainRecord and drops its local isPlainRecord implementation (and related unit tests).

@sim/utils/string: normalizeEmail replaces email.trim().toLowerCase() and invitation-local normalizers in auth verification, access control, and invitation/credential-set API routes.

Other: Copilot workflow JSON formatting uses shared sortObjectKeysDeep instead of an inline recursive sorter; stream/index no longer re-exports a local isRecord helper.

Domain-specific normalizers and array-allowing guards called out in the PR description are intentionally unchanged.

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

@waleedlatif1 waleedlatif1 changed the title refactor(sim): consolidate record guards + pure utils into @sim/utils (no behavior change) refactor(sim): consolidate record guards + pure utils into @sim/utils Jun 15, 2026
@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates a widespread pattern of locally-duplicated record guard predicates and pure utility functions into the @sim/utils package, replacing ~55 inline isRecord/isPlainObject/isJsonRecord checks across tools, providers, copilot, and trigger code with canonical isRecordLike and isPlainRecord helpers exported from @sim/utils/object.

  • Record guards: isRecordLike (loose — accepts class instances, Date) and isPlainRecord (strict — plain-object only) are now shared via packages/utils/src/object.ts; all local variants were verified to be semantically equivalent before replacement.
  • Key-sort dedup: Two independent sortKeysRecursively/sortKeys implementations in json-sanitizer.ts and copilot builders are replaced by the new sortObjectKeysDeep export.
  • Token/email normalization: normalizeToken (salesforce + servicenow triggers) and normalizeEmail (auth + invitations) are consolidated into triggers/utils/tokens.ts and @sim/utils/string respectively.

Confidence Score: 5/5

This is a mechanical deduplication refactor with no functional changes — every replaced predicate and transform is semantically identical to its canonical equivalent.

All ~55 inline record-guard substitutions replace predicates logically equivalent to isRecordLike. Boolean(v) and v !== null produce identical results for objects. isPlainRecord is a straight move with unchanged prototype logic. sortObjectKeysDeep is byte-for-byte identical to the two local implementations it replaces. normalizeToken and normalizeEmail consolidations are exact copies. The package.json subpath exports for @sim/utils/object and @sim/utils/string are already in place.

No files require special attention — the most complex migration sites (copilot/request/session/contract.ts and executor/utils/start-block.ts) were audited and found to be clean substitutions.

Important Files Changed

Filename Overview
packages/utils/src/object.ts Adds isPlainRecord (strict prototype check), isRecordLike (loose non-null non-array check), and sortObjectKeysDeep (recursive key-sort). All implementations are correct and well-documented.
packages/utils/src/string.ts Adds normalizeEmail as a pure trim+lowercase helper. Identical to the function removed from invitations/core.ts.
packages/utils/src/index.ts Re-exports new canonical symbols (isPlainRecord, isRecordLike, sortObjectKeysDeep, normalizeEmail) from the package root barrel.
packages/utils/src/object.test.ts New test suite covering isPlainRecord, isRecordLike, and sortObjectKeysDeep with Date/class-instance/array/primitive cases. Coverage is complete for the public contract.
apps/sim/lib/core/utils/records.ts isPlainRecord removed from local definition; now imported from @sim/utils/object for internal use. All callers updated to import directly from @sim/utils/object rather than via this file.
apps/sim/triggers/utils/tokens.ts New file consolidating the duplicated normalizeToken function from salesforce and servicenow triggers. Implementation is identical to both originals.
apps/sim/lib/copilot/request/session/contract.ts Local isRecord (Boolean(value) && typeof === 'object' && !Array.isArray) replaced by isRecordLike across 13 call sites. Boolean(value) and value !== null are semantically equivalent for non-null objects, so no behavior change.
apps/sim/executor/utils/start-block.ts Local isPlainObject replaced by isRecordLike across 13 call sites; predicate is semantically identical.
apps/sim/lib/workflows/sanitization/json-sanitizer.ts Local sortKeysRecursively and isRecord replaced by sortObjectKeysDeep and isRecordLike from @sim/utils/object. Implementations are identical.
apps/sim/lib/uploads/utils/user-file-base64.server.ts Local isPlainObject (checked !value
apps/sim/tools/vanta/utils.ts Local isJsonRecord replaced by isRecordLike across 18 call sites. Predicate is identical.
apps/sim/lib/invitations/core.ts normalizeEmail removed from this file; callers updated to import from @sim/utils/string. normalizeEmail imported here from @sim/utils/string for continued local use.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["@sim/utils/object"] -->|isRecordLike| B["55+ call sites across\ntools / copilot / triggers / lib"]
    A -->|isPlainRecord| C["records.ts\nexecution-core.ts\nsubblocks.ts\nsubblock-migrations.ts\nuser-file-base64.server.ts"]
    A -->|sortObjectKeysDeep| D["json-sanitizer.ts\nbuilders.ts"]

    E["@sim/utils/string"] -->|normalizeEmail| F["invitations/core.ts\ninvitations/send.ts\nworkspace-invitations.ts\nroute handlers"]

    G["triggers/utils/tokens.ts"] -->|normalizeToken| H["salesforce/utils.ts\nservicenow/utils.ts"]

    style A fill:#4ade80,stroke:#16a34a
    style E fill:#4ade80,stroke:#16a34a
    style G fill:#86efac,stroke:#16a34a
Loading

Reviews (2): Last reviewed commit: "refactor(sim): consolidate record guards..." | Re-trigger Greptile

…sim/utils

Replace ~55 re-implemented loose record guards with the canonical
@sim/utils isRecordLike (and one strict site with isPlainRecord), and
dedupe three normalize clusters: sortObjectKeysDeep (sanitization +
copilot builders), normalizeToken (salesforce + servicenow triggers),
and normalizeEmail. Array-allowing guards and domain-specific normalizers
are intentionally left untouched. Pure refactor — identical predicates
and transforms, no behavior change.
@waleedlatif1
waleedlatif1 force-pushed the refactor/consolidate-record-utils branch from 69ad368 to 3cbf4c7 Compare June 15, 2026 18:55
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

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

@waleedlatif1
waleedlatif1 merged commit 0673e3c into staging Jun 15, 2026
15 checks passed
@waleedlatif1
waleedlatif1 deleted the refactor/consolidate-record-utils branch June 15, 2026 19:32
waleedlatif1 added a commit that referenced this pull request Aug 15, 2026
* refactor: consolidate local isRecord guards onto shared isRecordLike

Nineteen files had re-declared a local `isRecord` guard rather than using the
shared one from `@sim/utils/object`, drift that reappeared after #5061 first
consolidated them. Two more imported the shared guard under an `isRecordLike as
isRecord` alias.

The copies were not interchangeable. Nine matched `isRecordLike` exactly. The
rest omitted the array exclusion (`typeof x === 'object' && x !== null`, or
`Boolean(x) && typeof x === 'object'`), so arrays passed the guard. Each of
those call sites was reviewed individually: in every case the guard is followed
by string/number field checks that an array fails anyway, so the outcome is
unchanged.

The one exception is `isOptionsTagData`, where `Object.values` on an array of
option items really did make an array-form `<options>` tag render. It now
accepts arrays explicitly rather than by accident.

`executor/handlers/pi/search/extension-source.ts` keeps its own copy: it is
source text written into an E2B/Daytona sandbox at runtime and cannot import.

* refactor: replace inline record guards with isRecordLike

103 inline `typeof x === 'object' && x !== null && !Array.isArray(x)` guards
(and the `x &&` / `Boolean(x)` spellings of the same conjunction) now call the
shared guard. Inside a conjunction that already asserts `typeof x === 'object'`,
`x &&` and `x !== null` are interchangeable, so all three orderings are the same
predicate at runtime.

Only exactly-equivalent conjunctions were converted. Matching required the three
clauses to be one adjacent conjunction over the same operand, so a nearby but
unrelated clause cannot be absorbed — generic-handler.ts, where the array case is
handled inside the block rather than excluded by the guard, is correctly left
alone.

Two sites were reverted after type-check rejected them: instagram/server-utils.ts
and workflows/[id]/log/route.ts both cast straight to a specific interface, which
is legal from `object` but not from `Record<string, unknown>`. Their narrowing is
genuinely not identical, so they keep the inline form rather than acquiring a
double cast.

Left as-is: `packages/ts-sdk` (published with no runtime dependencies) and the two
sandbox sources written into E2B/Daytona as text, which cannot import.

* refactor: consolidate duplicate record coercion helpers onto @sim/utils

A second sweep found 28 more local record helpers hiding under names the
previous `isRecord` grep never matched — `asRecord`, `toRecord`,
`toRecordOrNull`, `asObject`, `isJsonObject`. rabbitmq defined the same
`asRecord` twice within one service; dynatrace had three variants in one file.

Fourteen of them were re-deriving the same two shapes, so those shapes now live
in `@sim/utils/object` beside the guards they wrap:

  toRecord(value)        // isRecordLike(value) ? value : {}
  toRecordOrNull(value)  // isRecordLike(value) ? value : null

Both preserve identity on a hit, so no call site starts copying.

Eighteen local definitions are gone. `tools/instantly/utils.ts` keeps its
exported `asRecord` because its return type is the local `JsonRecord` alias, but
its body now delegates. `app/api/mcp/serve/[serverId]/route.ts` had an
`isJsonObject` with zero call sites — deleted outright.

Six helpers were deliberately left alone because they are NOT equivalent:
`pagerduty`/`zendesk`/`gitlab` do `(value as Record) || {}`, which type-checks
nothing at all, and `copilot/resources/extraction.ts`,
`edit-workflow/validation.ts`, `pi/core/events.ts` omit the array exclusion.
Those sit on webhook ingress and copilot paths where tightening is a behavior
change, not a cleanup; they are audited separately.

Two guards were removed rather than substituted, each proven dominated by an
earlier check: the bedrock streaming `toolUse.input` guard was unreachable
(`parseToolInput` already throws on non-objects before the loop builds
`assembledToolUses`), and four `driver.ts` re-narrows follow an
`if (!isRecordLike(x) || ...) throw` that dominates the later use.

* fix(webhooks): guard non-string GitLab ref, and consolidate the last record helpers

Two audits covered the six helpers held back from the previous commit for not
being equivalent to isRecordLike. Five are now migrated; one is deliberately not.

`gitlab.ts` carried a real crash path, independent of the guard work:

    const ref = (b.ref as string) || ''
    const branch = ref.replace('refs/heads/', '')

The cast is unchecked and `|| ''` only catches falsy values, so a truthy
non-string body field — `{"ref": 12345}` — reaches `.replace` and throws
`TypeError: ref.replace is not a function` inside `formatInput`. That runs in the
background worker after the webhook is already 200-ACKed, and GitLab does not
auto-retry, so the delivery is lost silently. Now checks `typeof`.

`pagerduty`/`zendesk`/`gitlab` each defined `asRecord` as
`(value as Record<string, unknown>) || {}`, which type-checks nothing — a string
or array passed through and was then spread into the workflow trigger payload
(`gitlab.ts:114`) as character- or index-keyed garbage. All three now use the
shared `toRecord`. These sit behind `verifyProviderAuth`, so reaching them
requires the shared secret; this is robustness, not authorization.

`copilot/resources/extraction.ts` and `pi/core/events.ts` were array-permissive
but provably inert — extraction.ts has no key enumeration or spread anywhere,
and events.ts only diverges by returning `null` instead of `{type:'other'}` for
an array, which every consumer already no-ops on. Pinned with a test.

`edit-workflow/validation.ts` is left permissive ON PURPOSE. Its `Object.entries`
walk mirrors the unguarded walk in `operations.ts:86,191`, so an array-shaped
`nestedNodes` from the model is currently visited by both. Tightening only the
validation side would stop `collectHostedApiKeyInput` from stripping
platform-managed API keys while the apply side still creates those child blocks.
Both paths have to change together, with tests, in their own PR.

* refactor: delete 31 unreachable module-local functions

Removes 815 lines of provably dead code: module-local (non-exported)
declarations whose identifier appears exactly once in their own file — the
declaration itself. A non-exported symbol cannot be reached by an import, a
barrel, a dynamic import, or a framework convention, so "unreferenced in its own
file" is a complete proof of deadness rather than a heuristic.

Notable removals include whole abandoned code paths: `findWebhookAndWorkflow`
(78 lines), `calculateBillingProjection` and `initializeUserUsageLimit`
(80 lines), `removeCredits` + `deductFromCredits` (54), `sendBatchSMS`,
`executeToolBatch`, and four unused `async-runs/repository.ts` queries.

Spans come from the TypeScript AST, not a regex. A regex cannot find a
declaration's extent — a first-brace scan cuts inside a return-type annotation
such as `Promise<{ canCreate: boolean }>` and silently corrupts the file. The
AST pass also re-derives deadness from identifier nodes, which caught two
wealthbox helpers that a regex export-check had wrongly reported as local.

Note the runtime TypeScript API here is `@typescript/typescript6`; the bare
`typescript` specifier resolves to the native compiler, which exposes no
`createSourceFile`.

* refactor: delete 434 stranded exported symbols

Removes ~5,930 lines of unreachable code across 166 files: exported symbols that
no other file in the repo mentions and that are unused inside their own file.
Whole abandoned surfaces go with them — unused React Query hooks
(useOrganizations, useOrganizationMembers, useUpgradeSubscription, ...), unused
admin route contracts, dead executor constants and reference builders, and the
landing-page StageWorkflow/LandingPreviewMount components. Three files left with
no remaining code were removed outright.

Candidates came from an AST pass; each was then verified individually against an
UNFILTERED repo-wide search plus the reachability paths a name search misses:
string-keyed tool/block registries, dynamic imports, `export *` barrels, and the
docs generator's source-text parsing of tool files.

29 candidates were verified LIVE and kept. Those exposed a flaw in the candidate
generator: it indexed only .ts/.tsx, while apps/docs/content/**/*.mdx imports
React components directly — ActionImage appears in ~190 MDX pages and ActionVideo
in ~115, and both scanned as dead. `app/global-error.tsx` was likewise kept,
since Next.js reaches it by filename and its default export can never have a name
reference. All 434 deleted names were afterwards cross-checked against every
.mdx/.md/.json/.yaml in the repo: no hits.

Verified with turbo type-check (23 workspaces), the full apps/sim suite
(25,219 tests), all 26 audits, and a production `next build` — the last of these
being what actually exercises route- and component-level reachability.
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