chore(hygiene): add structural lint enforcement script and noConsole rule by waleedlatif1 · Pull Request #4627 · simstudioai/sim

waleedlatif1 · 2026-05-16T01:08:55Z

Summary

Add scripts/check-hygiene.ts — catches three classes of violations Biome can't: @ts-ignore/@ts-expect-error without explanation, bare Next.js route exports missing withRouteHandler, and banned Vitest anti-patterns (vi.doMock, vi.resetModules, vi.importActual)
Add check:hygiene to root package.json scripts
Add noConsole Biome rule at error level with overrides for scripts, vitest setup files, and the hydration error handler (which intentionally patches console.error)
Wrap bare GET export in apps/sim/app/api/copilot/chat/route.ts with withRouteHandler; allowlist health and mothership stream routes which are intentionally bare
Fix vi.importActual in route-helpers.test.ts — replaced with explicit mock (no real module needed)
Add // hygiene-suppress comments to legitimate vi.resetModules uses in redis.test.ts and isolated-vm.test.ts (both test singleton module initialization behavior)

Type of Change

Chore / maintenance

Testing

bun run check:hygiene passes clean
bun run check:utils passes clean
bun run lint passes with no errors
All 397 test files / 6404 tests passing

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)

…rule

vercel · 2026-05-16T01:09:00Z

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

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	May 16, 2026 1:29am

cursor · 2026-05-16T01:09:04Z

PR Summary

Low Risk
Low risk maintenance work: adds repo-wide linting/CI enforcement and small route/test hygiene tweaks, with minimal runtime impact limited to one API route wrapper.

Overview
Adds a new scripts/check-hygiene.ts structural linter (wired in via check:hygiene) to enforce comment-backed @ts-ignore/@ts-expect-error, require withRouteHandler wrapping for Next.js route exports (with a small allowlist), and flag Vitest mocking anti-patterns with an opt-out // hygiene-suppress comment.

Tightens Biome linting by making noConsole an error by default, with targeted overrides for scripts/setup and a known console-patching handler, and updates a Copilot chat API GET handler plus a few tests to comply (wrap route handler, simplify a mock, and add explicit suppressions where module re-evaluation is intentional).

^{Reviewed by Cursor Bugbot for commit 3193cac. Configure here.}

greptile-apps · 2026-05-16T01:13:06Z

Greptile Summary

This PR introduces structural hygiene enforcement: a new scripts/check-hygiene.ts script that catches @ts-ignore without explanation, bare Next.js route exports, and banned Vitest anti-patterns; a noConsole Biome rule at error level; and targeted fixes to bring the existing codebase into compliance.

check-hygiene.ts: Walks apps/ and packages/ for three violation classes, using a one-line-back // hygiene-suppress: mechanism; violations are suppressed by placing the comment on the line immediately preceding the offending line.
biome.json: Adds noConsole: \"error\" globally with path-level overrides for scripts, Vitest setup files, and the hydration error handler.
Compliance fixes: route-helpers.test.ts replaces vi.importActual with an explicit mock; redis.test.ts and isolated-vm.test.ts get correctly-placed suppression comments; the copilot chat GET export is wrapped with withRouteHandler.

Confidence Score: 3/5

The route fix and most suppressions are correct, but the misaligned suppression comment in stream.test.ts means the hygiene script would still report a violation on that file rather than passing clean as claimed.

The hygiene-suppress comment in stream.test.ts sits before vi.mock (line 14), but vi.importActual is on line 16. Because isSuppressed only looks one line back, the script evaluates the vi.mock line as the predecessor and emits the violation, contradicting the PR claim that check:hygiene passes clean.

apps/sim/lib/copilot/request/go/stream.test.ts — the hygiene-suppress comment needs to move inside the vi.mock callback, directly above the vi.importActual call.

Important Files Changed

Filename	Overview
scripts/check-hygiene.ts	New lint-enforcement script; logic is sound, but the one-line-back suppression model cannot handle anti-patterns that appear inside callback bodies, which causes the stream.test.ts suppression to be misaligned.
apps/sim/lib/copilot/request/go/stream.test.ts	hygiene-suppress comment placed before vi.mock (line 14), but vi.importActual is on line 16 — the checker only looks one line back, so the suppression does not cover the violation.
apps/sim/app/api/copilot/chat/route.ts	Bare GET export correctly wrapped with withRouteHandler; no issues.
apps/sim/lib/core/config/redis.test.ts	Two hygiene-suppress comments correctly placed immediately before vi.resetModules and vi.doMock respectively; no issues.
apps/sim/lib/core/rate-limiter/route-helpers.test.ts	vi.importActual removed and replaced with an explicit mock of only the needed export; clean fix.
biome.json	noConsole added at error level with per-path overrides for scripts, Vitest setup files, and the hydration error handler; configuration looks correct.
apps/sim/lib/execution/isolated-vm.test.ts	hygiene-suppress comment correctly placed directly before vi.resetModules; no issues.
package.json	Adds check:hygiene script entry; no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[bun run check:hygiene] --> B[checkTsIgnore]
    A --> C[checkBareRoutes]
    A --> D[checkVitestAntiPatterns]

    B --> B1[Scan apps and packages ts/tsx files]
    C --> C1[Scan apps/sim/app/api route.ts files]
    D --> D1[Scan apps and packages test.ts files]

    B1 --> SUP{isSuppressed - lines i-1 has hygiene-suppress?}
    C1 --> ALLOW{In BARE_ROUTE_ALLOWLIST?}
    D1 --> SUP

    ALLOW -- Yes --> SKIP[Skip]
    ALLOW -- No --> SUP

    SUP -- Yes --> SKIP
    SUP -- No --> V[Add violation]

    V --> EXIT1[process.exit 1]
    SKIP --> EXIT0[process.exit 0]

_{Reviews (1): Last reviewed commit: "chore(hygiene): add structural lint enfo..." | Re-trigger Greptile}

greptile-apps · 2026-05-16T01:13:09Z

+// hygiene-suppress: session re-exports many real domain functions (eventToStreamEvent, parsePersistedStreamEventEnvelope) used by stream.ts under test — must spread real implementations and override only hasAbortMarker
 vi.mock('@/lib/copilot/request/session', async () => {
  const actual = await vi.importActual<typeof import('@/lib/copilot/request/session')>(


Misaligned hygiene-suppress comment — vi.importActual is still flagged

isSuppressed(lines, lineIndex) checks only lines[lineIndex - 1] (the line immediately before the violation). The suppression comment is on line 14 (before vi.mock), but the anti-pattern vi.importActual fires on line 16. When the checker evaluates line 16 it looks at line 15 (vi.mock(…) — which is not a suppression comment — so the violation is emitted and bun run check:hygiene would exit 1 on this file. The comment must be moved to sit directly above line 16, inside the mock callback.

Real issue — fixed in 7663ab3. The regex now handles generics with (?:<[^>]*>)? and the suppression comment is moved inside the mock callback to sit directly above the vi.importActual call.

cursor

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

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

^{Reviewed by Cursor Bugbot for commit 3193cac. Configure here.}

cursor · 2026-05-16T01:14:25Z

+      'vi.resetModules() — use vi.hoisted() + vi.mock() + static imports instead (exception: singleton modules that cache state)',
+  },
+  {
+    pattern: /\bvi\.importActual\s*\(/,


Regex misses vi.importActual with TypeScript generics

Medium Severity

The vi.importActual anti-pattern regex /\bvi\.importActual\s*\(/ requires ( to immediately follow (with optional whitespace), but the common TypeScript form vi.importActual<T>( has a generic type parameter between the method name and (. The only real-world usage in the codebase (stream.test.ts:16) uses exactly this form, so the lint rule silently misses it. Additionally, the hygiene-suppress comment in stream.test.ts is positioned to suppress line 15 (vi.mock), not line 16 where vi.importActual actually appears—so even a fixed regex would still flag it.

Additional Locations (1)

apps/sim/lib/copilot/request/go/stream.test.ts#L13-L14

^{Reviewed by Cursor Bugbot for commit 3193cac. Configure here.}

Real issue — both halves fixed in 7663ab3. Regex updated to /\bvi.importActual\s*(?:<[^>]>)?\s(/ to handle TypeScript generics, and the suppression comment repositioned directly above the vi.importActual line.

…ct base64 size gate

…x suppression placement

waleedlatif1 · 2026-05-16T01:39:09Z

Closing — the hygiene enforcement script adds more complexity than value right now. Pulling out the only real fix (MCP status codes) into a separate PR.

chore(hygiene): add structural lint enforcement script and noConsole …

3193cac

…rule

vercel Bot deployed to Preview May 16, 2026 01:12 View deployment

greptile-apps Bot reviewed May 16, 2026

View reviewed changes

cursor Bot reviewed May 16, 2026

View reviewed changes

fix(mcp,attachments): add missing orchestration error codes and corre…

fad60b3

…ct base64 size gate

vercel Bot temporarily deployed to Preview May 16, 2026 01:22 Inactive

fix(attachments): clean up size limit error message

df33c79

vercel Bot temporarily deployed to Preview May 16, 2026 01:23 Inactive

fix(hygiene): handle TypeScript generics in importActual regex and fi…

7663ab3

…x suppression placement

vercel Bot deployed to Preview May 16, 2026 01:29 View deployment

waleedlatif1 closed this May 16, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(hygiene): add structural lint enforcement script and noConsole rule#4627

chore(hygiene): add structural lint enforcement script and noConsole rule#4627
waleedlatif1 wants to merge 4 commits into
stagingfrom
chore/hygiene

waleedlatif1 commented May 16, 2026

Uh oh!

vercel Bot commented May 16, 2026 •

edited

Loading

Uh oh!

cursor Bot commented May 16, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented May 16, 2026

Uh oh!

greptile-apps Bot May 16, 2026

Uh oh!

waleedlatif1 May 16, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot May 16, 2026

Uh oh!

waleedlatif1 May 16, 2026

Uh oh!

waleedlatif1 commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

waleedlatif1 commented May 16, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

greptile-apps Bot commented May 16, 2026

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot May 16, 2026

Choose a reason for hiding this comment

Uh oh!

waleedlatif1 May 16, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot May 16, 2026

Choose a reason for hiding this comment

Regex misses vi.importActual with TypeScript generics

Uh oh!

waleedlatif1 May 16, 2026

Choose a reason for hiding this comment

Uh oh!

waleedlatif1 commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented May 16, 2026 •

edited

Loading

cursor Bot commented May 16, 2026 •

edited

Loading

Regex misses `vi.importActual` with TypeScript generics