fix(ci): walk every route entry the workspace app composes - #7026
Conversation
…pages and layouts The tool-registry guard collected `page.tsx` and `layout.tsx`, and Next composes three more entries by convention: `error.tsx`, `loading.tsx`, `not-found.tsx`. Twenty-six exist under `app/workspace` and none was walked. `error.tsx` is always a Client Component — Next requires it — so a registry edge there reaches the browser bundle exactly as one from a page does. Coverage goes from 34 entry graphs to 60. Nothing new is reported: the hole was unexploited, and closing it costs nothing. The root deliberately stays at `app/workspace`. Widening it to `app` reports `(interfaces)/resume/[workflowId]/[executionId]/page.tsx`, a Server Component (`runtime = 'nodejs'`, `force-dynamic`) whose `PauseResumeManager` import resolves server-side and never reaches a client bundle. The guard cannot distinguish server from client entries, so it stays where its premise holds.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview The walk root stays at Reviewed by Cursor Bugbot for commit f2aa705. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort 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 f2aa705. Configure here.
| 'error.tsx', | ||
| 'loading.tsx', | ||
| 'not-found.tsx', | ||
| ]) |
There was a problem hiding this comment.
Non-route error.tsx treated as entry
Medium Severity
Adding error.tsx to ENTRY_FILENAMES makes collectEntries pick up app/workspace/[workspaceId]/components/error/error.tsx, a shared ErrorState helper with no default export, not a Next-composed route boundary. That file is one of the claimed 26 new roots and already sits on real error.tsx graphs via the components barrel, so the guard now walks a non-route entry and will baseline it as one.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f2aa705. Configure here.
Greptile SummaryThis PR expands the tool-registry boundary check from workspace pages and layouts to error, loading, and not-found route entries.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking coverage gap for future convention-composed workspace entries. The added filenames cover all such entries currently present under the workspace tree, but future template or parallel-route default entries would silently bypass the guard. Files Needing Attention: scripts/check-tool-registry-boundary.ts
|
| Filename | Overview |
|---|---|
| scripts/check-tool-registry-boundary.ts | Expands CI route-entry coverage correctly for currently present workspace files, but its exhaustive claim and hardcoded list can diverge when other Next.js entry conventions are introduced. |
Reviews (1): Last reviewed commit: "fix(ci): walk every route entry the work..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const ENTRY_FILENAMES = new Set([ | ||
| 'page.tsx', | ||
| 'layout.tsx', | ||
| 'error.tsx', | ||
| 'loading.tsx', | ||
| 'not-found.tsx', | ||
| ]) |
There was a problem hiding this comment.
The expanded allowlist still excludes convention-composed entries such as template.tsx and parallel-route default.tsx. Adding either beneath app/workspace would leave its dependency graph outside this guard, allowing a future registry edge to bypass the CI check despite the new exhaustive-coverage claim.
…ntry (#7028) * fix(ci): require a default export before treating a file as a route entry Follow-up to #7026, which added `error.tsx` to the entry filenames and with it picked up `[workspaceId]/components/error/error.tsx` — named like a boundary, and not one. It exports `ErrorShell` and `ErrorState` for the thirteen real boundaries to use; Next would reject it as a boundary for having no default export. Counting it inflated the coverage number and would have recorded a shared component in the graph-weight baseline as though it were a route. The filename was never the right test. Every convention-composed entry must default-export the thing Next renders, so that is the discriminator now. Entry count goes 60 → 59, and all thirteen real `error.tsx` boundaries still walk. Also adds `template.tsx` and `default.tsx`. Neither exists under `app/workspace` today, so this changes nothing now — but the enumeration claims to cover what Next composes, and leaving two out makes that claim false the day someone adds one. Both raised in review on #7026 (Cursor and Greptile respectively); I merged before reading them, so this lands separately. * fix(ci): count every form that declares a default export `export { default } from './page'` is a valid Next entry and the regex required `as default`, so such an entry would have dropped out of the walk and skipped both the registry gate and the graph-weight ratchet — silently, which is the dangerous direction for a discriminator to fail in. Latent rather than live: the form appears once under `app/workspace`, in a barrel, not in an entry filename. Four forms now count — `export default …`, `export { default } from`, `export { default, … } from`, and `export { X as default }`. `export { default as X }` still does not: it re-exports another module's default under a name and leaves this one without one. Verified all ten variants, including that last distinction. Raised by both Cursor and Greptile on #7028.
1 participant
The gap
check-tool-registry-boundary.tscollected two entry filenames:Next composes three more by convention —
error.tsx,loading.tsx,not-found.tsx. 26 exist underapp/workspaceand none was walked.The guard's own TSDoc already makes the argument for including them: layouts are enumerated separately "because Next.js composes them by convention — a page does not
importits layout". That reasoning covers error boundaries and loading states identically.error.tsxmatters most: Next requires it to be a Client Component, so a registry edge there lands in the browser bundle exactly as one from a page does.Result
Coverage goes from 34 entry graphs to 60. Nothing new is reported — the hole was unexploited. This closes it before it costs something, at no price.
The root stays at
app/workspaceWidening
ENTRY_ROOTtoapplooks tempting and is wrong. It reports:That page is a Server Component (
runtime = 'nodejs',dynamic = 'force-dynamic',asyncwithawait params). ItsPauseResumeManagerimport resolves server-side and never reaches a client bundle, so the registry costs nothing there.The guard cannot distinguish a server entry from a client one — its premise is "everything a route entry imports ships to the browser", which holds across
app/workspaceand not acrossapp. Recorded in the TSDoc so the next person to consider widening it does not have to rediscover this.Testing
bun run scripts/check-tool-registry-boundary.ts→✓ tool registry stays out of 60 workspace page/layout graphs(was 34).Provenance
From an audit of the repo's custom gates for blind spots. Two sibling claims in the same report were false positives and are not fixed here: the walker was said to follow no re-exports (it applies
REEXPORT_REalongside three other patterns at line 151, coveringexport * from,export * as ns from, andexport { … } from), and the root was said to need widening (above).