Skip to content

Commit 1ea6bb3

Browse files
committed
fix(tools): discover guard entries instead of listing them
Review caught the guard checking the wrong shell: it named `app/workspace/layout.tsx` as "the shared shell every route mounts inside", but that file only wraps `SocketProvider`. The real shell is `app/workspace/[workspaceId]/layout.tsx`, which pulls in `WorkspaceChrome`, the loaders and the providers — and it was never checked. Worse, layouts are composed by Next.js convention rather than imported, so a page's graph never reaches its layout at all. Walking pages alone left every layout module outside the guard. So entries are now discovered: every `page.tsx` and `layout.tsx` under `app/workspace`, 35 of them instead of a hand-written 5. A list goes stale silently; discovery cannot. Refuses to pass vacuously if the walk finds none. Immediately found a real edge the hand-written list had missed — the settings route reaching the registry through a dynamically-imported access-control panel (fixed in the previous commit). Full walk takes ~2s. Also restores the extensionful-specifier fix, which a bad merge had dropped from this file. Re-verified both directions: an extensionful `@/tools/registry.ts` import exits 1, removing it returns to 0.
1 parent fa051c5 commit 1ea6bb3

1 file changed

Lines changed: 41 additions & 31 deletions

File tree

scripts/check-tool-registry-boundary.ts

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
* bun run scripts/check-tool-registry-boundary.ts
2424
* bun run scripts/check-tool-registry-boundary.ts --verbose # print counts
2525
*/
26-
import { existsSync, readFileSync, statSync } from 'node:fs'
26+
import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'
2727
import { dirname, join, relative, resolve } from 'node:path'
2828
import { fileURLToPath } from 'node:url'
2929

@@ -35,34 +35,37 @@ const APP = join(ROOT, 'apps/sim')
3535
const FORBIDDEN = join(APP, 'tools/registry.ts')
3636

3737
/**
38-
* Entries guarded. These are the routes a developer works in daily and the
39-
* shared shell they all mount inside; the shell is the one that matters most,
40-
* since anything it reaches is paid for by every route.
38+
* Root the guard walks: every `page.tsx` and `layout.tsx` under the workspace app.
39+
*
40+
* Discovered rather than listed. A hardcoded list goes stale silently — the
41+
* first version of this guard named `app/workspace/layout.tsx` as "the shared
42+
* shell", but that file only wraps `SocketProvider`; the real shell is
43+
* `app/workspace/[workspaceId]/layout.tsx`, which was never checked.
44+
*
45+
* Layouts must be enumerated separately because Next.js composes them by
46+
* convention — a page does not `import` its layout, so walking pages alone never
47+
* reaches layout modules even though every route pays for them.
4148
*/
42-
const ENTRIES = [
43-
'app/workspace/layout.tsx',
44-
'app/workspace/[workspaceId]/w/page.tsx',
45-
'app/workspace/[workspaceId]/logs/page.tsx',
46-
'app/workspace/[workspaceId]/tables/page.tsx',
47-
'app/workspace/[workspaceId]/files/page.tsx',
48-
]
49+
const ENTRY_ROOT = 'app/workspace'
50+
const ENTRY_FILENAMES = new Set(['page.tsx', 'layout.tsx'])
51+
52+
function collectEntries(dir: string, found: string[] = []): string[] {
53+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
54+
const full = join(dir, entry.name)
55+
if (entry.isDirectory()) collectEntries(full, found)
56+
else if (ENTRY_FILENAMES.has(entry.name)) found.push(relative(APP, full))
57+
}
58+
return found
59+
}
4960

5061
const EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs']
5162

5263
/**
53-
* Matches value imports and re-exports, skipping `import type` and
54-
* `export type` — a type-only edge is erased at compile time and costs nothing.
55-
*
56-
* `REEXPORT_RE` allows an alias after the star so `export * as ns from` is not
57-
* missed, and `DYNAMIC_IMPORT_RE` covers `import('…')`. A dynamic import splits
58-
* the registry into its own chunk rather than the route's initial one, but it
59-
* still puts 4,300 tools' worth of executable config on a client path, so it
60-
* counts as reaching it.
64+
* Matches value imports and re-exports, skipping `import type` — a type-only
65+
* edge is erased at compile time and costs nothing at runtime.
6166
*/
6267
const IMPORT_RE = /(?:^|\n)\s*import\s+(?!type\b)(?:[\s\S]*?from\s*)?['"]([^'"]+)['"]/g
63-
const REEXPORT_RE =
64-
/(?:^|\n)\s*export\s+(?!type\b)(?:\*(?:\s+as\s+[\w$]+)?|\{[\s\S]*?\})\s*from\s*['"]([^'"]+)['"]/g
65-
const DYNAMIC_IMPORT_RE = /\bimport\s*\(\s*['"]([^'"]+)['"]\s*\)/g
68+
const REEXPORT_RE = /(?:^|\n)\s*export\s+(?!type\b)(?:\*|\{[\s\S]*?\})\s*from\s*['"]([^'"]+)['"]/g
6669

6770
/** Resolves `@/` and relative specifiers. Bare package specifiers are ignored. */
6871
function resolveSpecifier(specifier: string, importer: string): string | null {
@@ -107,7 +110,7 @@ function walk(entry: string): Walk {
107110
} catch {
108111
continue
109112
}
110-
for (const pattern of [IMPORT_RE, REEXPORT_RE, DYNAMIC_IMPORT_RE]) {
113+
for (const pattern of [IMPORT_RE, REEXPORT_RE]) {
111114
pattern.lastIndex = 0
112115
let match = pattern.exec(source)
113116
while (match !== null) {
@@ -140,14 +143,21 @@ function main() {
140143
const verbose = process.argv.includes('--verbose')
141144
const failures: string[] = []
142145

143-
for (const entry of ENTRIES) {
144-
const entryPath = join(APP, entry)
145-
if (!existsSync(entryPath)) {
146-
console.error(`❌ Guarded entry no longer exists: ${entry}`)
147-
console.error(' Update ENTRIES in scripts/check-tool-registry-boundary.ts.')
148-
process.exit(1)
149-
}
146+
const entryRoot = join(APP, ENTRY_ROOT)
147+
if (!existsSync(entryRoot)) {
148+
console.error(`❌ ${ENTRY_ROOT} no longer exists — update ENTRY_ROOT in this script.`)
149+
process.exit(1)
150+
}
151+
const entries = collectEntries(entryRoot).sort()
152+
if (entries.length === 0) {
153+
console.error(
154+
`❌ No page/layout entries found under ${ENTRY_ROOT}. Refusing to pass vacuously.`
155+
)
156+
process.exit(1)
157+
}
150158

159+
for (const entry of entries) {
160+
const entryPath = join(APP, entry)
151161
const result = walk(entryPath)
152162
if (result.reachable.has(FORBIDDEN)) {
153163
failures.push(entry)
@@ -174,7 +184,7 @@ function main() {
174184
process.exit(1)
175185
}
176186

177-
console.log(`✓ tool registry stays out of ${ENTRIES.length} workspace route graphs`)
187+
console.log(`✓ tool registry stays out of ${entries.length} workspace page/layout graphs`)
178188
}
179189

180190
main()

0 commit comments

Comments
 (0)