Skip to content

Commit 3cc9ffa

Browse files
committed
perf(tools): guard the tool-registry client boundary in CI
The registry was 71-82% of every workspace route's module graph, and the two edges that put it there were invisible at the call site: `providers/utils.ts` imported `mergeToolParameters`, and `mcp-dynamic-args.tsx` imported `formatParameterLabel`. Neither import looks remotely like "pull in 4,700 modules of SDK clients", which is why this needs a lint rather than a convention. `check-tool-registry-boundary.ts` walks the value-import graph (skipping `import type`, which is erased) from the workspace layout and the four routes that mount inside it, and fails if `@/tools/registry` is reachable — printing the exact chain that reintroduced it. Verified it fails: reintroducing a `getTool` import in `serializer/index.ts` exits 1 and names the chain through `stores/workflow-diff/store.ts`; removing it returns to 0. There is deliberately no allowlist. The fix for a failure is always to move the symbol the file actually needs into a registry-free module, not to exempt the route. Documents the guard in the tool-registry-boundary skill.
1 parent c926582 commit 3cc9ffa

6 files changed

Lines changed: 195 additions & 0 deletions

File tree

.agents/skills/tool-registry-boundary/SKILL.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ Three non-obvious properties, each of which was measured and is easy to undo by
4444
- **The generator refuses to emit function values.** If you add a field to `METADATA_FIELDS` that contains a closure, generation fails loudly rather than shipping executable config to the client. `hosting` and `schemaEnrichment` are excluded for exactly this reason (`hosting.enabled`, `pricing`, and `enrichSchema` are functions) — they are server-only.
4545
- **Empty param entries are stripped.** The registry contains one (`stt_deepgram_v2`), which crashes callers that read `param.type` while iterating.
4646

47+
## The guard
48+
49+
`bun run check:tool-registry-boundary` (CI: "Tool registry client-boundary audit") walks the module graph from each workspace route and fails if `@/tools/registry` is reachable, printing the exact import chain that reintroduced it.
50+
51+
If it fails, do not add the entry to an allowlist — there isn't one. Find the symbol the offending file actually needs and move it to a registry-free module, exactly as `mergeToolParameters` and `formatParameterLabel` were.
52+
53+
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
54+
4755
## How to verify an edge actually got cut
4856

4957
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

.claude/commands/tool-registry-boundary.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ Three non-obvious properties, each of which was measured and is easy to undo by
4343
- **The generator refuses to emit function values.** If you add a field to `METADATA_FIELDS` that contains a closure, generation fails loudly rather than shipping executable config to the client. `hosting` and `schemaEnrichment` are excluded for exactly this reason (`hosting.enabled`, `pricing`, and `enrichSchema` are functions) — they are server-only.
4444
- **Empty param entries are stripped.** The registry contains one (`stt_deepgram_v2`), which crashes callers that read `param.type` while iterating.
4545

46+
## The guard
47+
48+
`bun run check:tool-registry-boundary` (CI: "Tool registry client-boundary audit") walks the module graph from each workspace route and fails if `@/tools/registry` is reachable, printing the exact import chain that reintroduced it.
49+
50+
If it fails, do not add the entry to an allowlist — there isn't one. Find the symbol the offending file actually needs and move it to a registry-free module, exactly as `mergeToolParameters` and `formatParameterLabel` were.
51+
52+
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
53+
4654
## How to verify an edge actually got cut
4755

4856
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

.cursor/commands/tool-registry-boundary.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ Three non-obvious properties, each of which was measured and is easy to undo by
3939
- **The generator refuses to emit function values.** If you add a field to `METADATA_FIELDS` that contains a closure, generation fails loudly rather than shipping executable config to the client. `hosting` and `schemaEnrichment` are excluded for exactly this reason (`hosting.enabled`, `pricing`, and `enrichSchema` are functions) — they are server-only.
4040
- **Empty param entries are stripped.** The registry contains one (`stt_deepgram_v2`), which crashes callers that read `param.type` while iterating.
4141

42+
## The guard
43+
44+
`bun run check:tool-registry-boundary` (CI: "Tool registry client-boundary audit") walks the module graph from each workspace route and fails if `@/tools/registry` is reachable, printing the exact import chain that reintroduced it.
45+
46+
If it fails, do not add the entry to an allowlist — there isn't one. Find the symbol the offending file actually needs and move it to a registry-free module, exactly as `mergeToolParameters` and `formatParameterLabel` were.
47+
48+
Run it with `--verbose` to print per-route module counts, which is also the quickest way to see whether a change moved the graph.
49+
4250
## How to verify an edge actually got cut
4351

4452
Do not eyeball imports — the registry is reached through several redundant paths, so cutting one buys nothing while another survives. Walk the graph:

.github/workflows/test-build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ jobs:
153153
- name: Verify realtime prune graph
154154
run: bun run check:realtime-prune
155155

156+
- name: Tool registry client-boundary audit
157+
run: bun run check:tool-registry-boundary
158+
156159
- name: Verify generated tool metadata is in sync
157160
run: bun run tool-metadata:check
158161

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
"check:api-validation": "bun run scripts/check-api-validation-contracts.ts --check",
3131
"check:api-validation:strict": "bun run scripts/check-api-validation-contracts.ts --check --enforce-boundary-baseline",
3232
"check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts",
33+
"check:tool-registry-boundary": "bun run scripts/check-tool-registry-boundary.ts",
3334
"check:zustand-v5": "bun run scripts/check-zustand-v5-selectors.ts",
3435
"check:react-query": "bun run scripts/check-react-query-patterns.ts --check",
3536
"check:client-boundary": "bun run scripts/check-client-boundary-imports.ts --check",
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
#!/usr/bin/env bun
2+
/**
3+
* Fails if a workspace route can reach the executable tool registry.
4+
*
5+
* `@/tools/registry` is a barrel over 4,300+ tools whose `ToolConfig`s hold
6+
* closures (`request.headers`, `transformResponse`, `directExecution`). Those
7+
* closures reach every integration's SDK client and parser, so reaching the
8+
* barrel costs ~4,700 modules — it was 71-82% of every workspace route's module
9+
* graph until those edges were cut.
10+
*
11+
* Client-reachable code reads `@/tools/metadata`, `@/tools/metadata-outputs` or
12+
* `@/tools/tool-ids` instead. See
13+
* `.agents/skills/tool-registry-boundary/SKILL.md`.
14+
*
15+
* This regresses silently and cheaply: any file under a route can import one
16+
* helper from a module that happens to import `getTool`, and the whole registry
17+
* comes back. That is exactly how it got there — `providers/utils.ts` pulled it
18+
* in through `mergeToolParameters`, and `mcp-dynamic-args.tsx` through
19+
* `formatParameterLabel`. Neither import looks remotely suspicious at the call
20+
* site, which is why this is a lint and not a convention.
21+
*
22+
* Usage:
23+
* bun run scripts/check-tool-registry-boundary.ts
24+
* bun run scripts/check-tool-registry-boundary.ts --verbose # print counts
25+
*/
26+
import { existsSync, readFileSync, statSync } from 'node:fs'
27+
import { dirname, join, relative, resolve } from 'node:path'
28+
import { fileURLToPath } from 'node:url'
29+
30+
const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url))
31+
const ROOT = resolve(SCRIPT_DIR, '..')
32+
const APP = join(ROOT, 'apps/sim')
33+
34+
/** Module no client-reachable entry may reach. */
35+
const FORBIDDEN = join(APP, 'tools/registry.ts')
36+
37+
/**
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.
41+
*/
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+
50+
const EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs']
51+
52+
/**
53+
* Matches value imports and re-exports, skipping `import type` — a type-only
54+
* edge is erased at compile time and costs nothing at runtime.
55+
*/
56+
const IMPORT_RE = /(?:^|\n)\s*import\s+(?!type\b)(?:[\s\S]*?from\s*)?['"]([^'"]+)['"]/g
57+
const REEXPORT_RE = /(?:^|\n)\s*export\s+(?!type\b)(?:\*|\{[\s\S]*?\})\s*from\s*['"]([^'"]+)['"]/g
58+
59+
/** Resolves `@/` and relative specifiers. Bare package specifiers are ignored. */
60+
function resolveSpecifier(specifier: string, importer: string): string | null {
61+
let base: string
62+
if (specifier.startsWith('@/')) base = join(APP, specifier.slice(2))
63+
else if (specifier.startsWith('.')) base = resolve(dirname(importer), specifier)
64+
else return null
65+
66+
for (const ext of EXTENSIONS) {
67+
if (existsSync(base + ext)) return base + ext
68+
}
69+
if (existsSync(base) && statSync(base).isDirectory()) {
70+
for (const ext of EXTENSIONS) {
71+
const indexPath = join(base, `index${ext}`)
72+
if (existsSync(indexPath)) return indexPath
73+
}
74+
}
75+
return null
76+
}
77+
78+
interface Walk {
79+
reachable: Set<string>
80+
importedBy: Map<string, string>
81+
}
82+
83+
function walk(entry: string): Walk {
84+
const reachable = new Set<string>()
85+
const importedBy = new Map<string, string>()
86+
const queue = [entry]
87+
reachable.add(entry)
88+
89+
while (queue.length > 0) {
90+
const file = queue.pop() as string
91+
let source: string
92+
try {
93+
source = readFileSync(file, 'utf8')
94+
} catch {
95+
continue
96+
}
97+
for (const pattern of [IMPORT_RE, REEXPORT_RE]) {
98+
pattern.lastIndex = 0
99+
let match = pattern.exec(source)
100+
while (match !== null) {
101+
const resolved = resolveSpecifier(match[1], file)
102+
if (resolved && !reachable.has(resolved)) {
103+
reachable.add(resolved)
104+
importedBy.set(resolved, file)
105+
queue.push(resolved)
106+
}
107+
match = pattern.exec(source)
108+
}
109+
}
110+
}
111+
112+
return { reachable, importedBy }
113+
}
114+
115+
/** Walks parent links back to the entry so the offending edge is obvious. */
116+
function explainChain({ importedBy }: Walk, target: string): string[] {
117+
const chain: string[] = []
118+
let current: string | undefined = target
119+
while (current) {
120+
chain.push(relative(ROOT, current))
121+
current = importedBy.get(current)
122+
}
123+
return chain.reverse()
124+
}
125+
126+
function main() {
127+
const verbose = process.argv.includes('--verbose')
128+
const failures: string[] = []
129+
130+
for (const entry of ENTRIES) {
131+
const entryPath = join(APP, entry)
132+
if (!existsSync(entryPath)) {
133+
console.error(`❌ Guarded entry no longer exists: ${entry}`)
134+
console.error(' Update ENTRIES in scripts/check-tool-registry-boundary.ts.')
135+
process.exit(1)
136+
}
137+
138+
const result = walk(entryPath)
139+
if (result.reachable.has(FORBIDDEN)) {
140+
failures.push(entry)
141+
console.error(`\n❌ ${entry} can reach @/tools/registry via:`)
142+
for (const step of explainChain(result, FORBIDDEN)) {
143+
console.error(` ${step}`)
144+
}
145+
} else if (verbose) {
146+
console.log(`✓ ${entry}${result.reachable.size} modules, registry unreachable`)
147+
}
148+
}
149+
150+
if (failures.length > 0) {
151+
console.error(
152+
`\n${failures.length} route(s) reach the executable tool registry, which adds ~4,700 modules to each.`
153+
)
154+
console.error(
155+
'Read the metadata instead: `@/tools/metadata` (params), `@/tools/metadata-outputs`'
156+
)
157+
console.error(
158+
'(outputs), or `@/tools/tool-ids` (existence/resolution). Only code that executes a tool'
159+
)
160+
console.error('may import `getTool`. See .agents/skills/tool-registry-boundary/SKILL.md.')
161+
process.exit(1)
162+
}
163+
164+
console.log(`✓ tool registry stays out of ${ENTRIES.length} workspace route graphs`)
165+
}
166+
167+
main()

0 commit comments

Comments
 (0)