chore(utils): consolidate record guards and remove dead code - #6715
Conversation
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.
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.
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.
…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.
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`.
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview A large dead-code pass removes unused landing hero/preview components, stranded React Query hooks, admin API contract definitions, and hundreds of unreachable exports/helpers (credentials lookup wrappers, file authorization helpers, executor constants, copilot mention utilities, etc.). Smaller behavior tweaks include accepting array-shaped Reviewed by Cursor Bugbot for commit 27ff773. Configure here. |
Greptile SummaryThis PR consolidates duplicated object guards into shared
Confidence Score: 5/5The PR appears safe to merge because no concrete changed-code failure or actionable non-blocking issue remained after review. The shared helpers match the intended non-null, non-array record semantics at the investigated call sites, the GitLab change prevents malformed refs from crashing normalization, and targeted checks found no surviving references to the investigated deleted exports.
|
| Filename | Overview |
|---|---|
| packages/utils/src/object.ts | Adds shared record coercion helpers using the existing non-null, non-array object predicate. |
| packages/utils/src/object.test.ts | Covers accepted records, rejected arrays and primitives, identity preservation, and fallback behavior. |
| apps/sim/lib/webhooks/providers/gitlab.ts | Consolidates record normalization and safely defaults malformed non-string refs before branch extraction. |
| apps/sim/lib/webhooks/providers/whatsapp.ts | Replaces array-permissive local object narrowing with the shared record guard; no valid array-shaped provider field was established. |
| apps/desktop/src/main/browser-agent/driver.ts | Reuses shared coercion after existing result validation without changing accepted browser-action result shapes. |
| apps/sim/executor/handlers/pi/core/events.ts | Consolidates event payload record checks and updates corresponding test coverage. |
| apps/sim/tools/index.ts | Removes unreachable exports and consolidates object normalization without an established surviving dependency. |
| apps/sim/lib/api/contracts/admin.ts | Removes stranded admin contract declarations with no concrete remaining importer identified. |
Reviews (1): Last reviewed commit: "refactor: delete 434 stranded exported s..." | Re-trigger Greptile
Summary
@sim/utils/objecthelpers. AddstoRecord/toRecordOrNullfor the two coercion shapes 14 local helpers were each re-deriving; both preserve identity on a hit.const ref = (b.ref as string) || ''only catches falsy, so a truthy non-string ({"ref": 12345}) reached.replaceand threw. That runs in the background worker after the delivery is already 200-ACKed, and GitLab does not auto-retry, so the delivery was lost silently.findWebhookAndWorkflow,calculateBillingProjection, unused React Query hooks and admin contracts).The local copies were not interchangeable — several omitted the array exclusion, and three webhook helpers (
pagerduty/zendesk/gitlab) did(value as Record) || {}, which type-checks nothing. Each call site was reviewed individually rather than swept; two sites were reverted whereisRecordLikenarrowing broke a downstreamobject as Interfacecast.edit-workflow/validation.tsis deliberately left array-permissive: itsObject.entrieswalk mirrors the equally unguarded walk inoperations.ts, so tightening only the validation side would stop platform-managed API keys from being stripped while the apply side still creates those child blocks. That pair needs to change together.Type of Change
ref)Testing
bunx turbo run type-check(23 workspaces), fullapps/simsuite (25,290 tests), all 26 audits, and a productionnext build. Dead-code candidates were verified against unfiltered repo-wide searches plus registry, dynamic-import, barrel, and.mdxreachability; 29 candidates were confirmed live and kept.Checklist