Skip to content

Commit fb3c08b

Browse files
perf: batch of low-risk fixes for hangs and unnecessary FS work (#187)
* fix(staleness): bound npm registry fetch with a timeout fetchNpmVersion had no timeout/AbortSignal, so an unreachable or slow npm registry could block `intent stale` indefinitely. Bound it to 5s; timeout/error still falls through to the existing null fallback. * fix(shared): bound global package-manager detection with a timeout * perf(setup): early-exit skill-file check in getWorkspaceInfo * perf(bench): add process-level cold-start benchmark * perf: fix potential hangs on slow or large environments * add timeout to node process execution in cold start benchmark
1 parent b7920e9 commit fb3c08b

5 files changed

Lines changed: 83 additions & 2 deletions

File tree

.changeset/nine-pigs-laugh.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@tanstack/intent': patch
3+
---
4+
5+
Fix potential hangs on slow or large environments:
6+
7+
- Bound the npm registry staleness check with a request timeout so `intent list` and other staleness checks can't hang indefinitely on a slow or unreachable registry.
8+
- Bound global package-manager detection with a command timeout so it can't hang indefinitely when the environment's global `node_modules` is slow to resolve.
9+
- Avoid enumerating a workspace package's entire skill tree just to check whether it has any skills, reducing filesystem work in large monorepos.

benchmarks/intent/startup.bench.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { spawnSync } from 'node:child_process'
2+
import { fileURLToPath } from 'node:url'
3+
import { bench, describe } from 'vitest'
4+
5+
const cliPath = fileURLToPath(
6+
new URL('../../packages/intent/dist/cli.mjs', import.meta.url),
7+
)
8+
9+
const coldStartBenchOptions = {
10+
warmupIterations: 20,
11+
time: 3_000,
12+
}
13+
14+
function runNode(args: Array<string>): void {
15+
const result = spawnSync(process.execPath, args, {
16+
stdio: 'ignore',
17+
timeout: 10_000,
18+
})
19+
if (result.status !== 0) {
20+
throw new Error(
21+
`spawn ${[process.execPath, ...args].join(' ')} exited with code ${result.status}`,
22+
)
23+
}
24+
}
25+
26+
describe('cold start', () => {
27+
bench(
28+
'empty node process (baseline)',
29+
() => {
30+
runNode(['-e', ''])
31+
},
32+
coldStartBenchOptions,
33+
)
34+
35+
bench(
36+
'intent --help',
37+
() => {
38+
runNode([cliPath, '--help'])
39+
},
40+
coldStartBenchOptions,
41+
)
42+
})

packages/intent/src/setup/workspace-patterns.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'
22
import { dirname, join } from 'node:path'
33
import { parse as parseJsonc } from 'jsonc-parser'
44
import { parse as parseYaml } from 'yaml'
5-
import { findSkillFiles } from '../shared/utils.js'
5+
import { hasAnySkillFile } from '../shared/utils.js'
66
import type { ParseError } from 'jsonc-parser'
77

88
function normalizeWorkspacePattern(pattern: string): string {
@@ -227,7 +227,7 @@ export function getWorkspaceInfo(root: string): WorkspaceInfo | null {
227227
const packageDirs = readWorkspacePackageDirs(root) ?? []
228228
const packageDirsWithSkills = packageDirs.filter((dir) => {
229229
const skillsDir = join(dir, 'skills')
230-
return existsSync(skillsDir) && findSkillFiles(skillsDir).length > 0
230+
return existsSync(skillsDir) && hasAnySkillFile(skillsDir)
231231
})
232232
const info = {
233233
root,

packages/intent/src/shared/utils.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,30 @@ function collectSkillFiles(
114114
}
115115
}
116116

117+
/**
118+
* Like `findSkillFiles`, but stops at the first SKILL.md found instead of
119+
* enumerating the whole tree.
120+
*/
121+
export function hasAnySkillFile(dir: string, fs: ReadFs = nodeReadFs): boolean {
122+
let entries: Array<Dirent<string>>
123+
try {
124+
entries = fs.readdirSync(dir, { withFileTypes: true, encoding: 'utf8' })
125+
} catch {
126+
return false
127+
}
128+
129+
for (const entry of entries) {
130+
const fullPath = join(dir, entry.name)
131+
if (entry.isDirectory()) {
132+
if (hasAnySkillFile(fullPath, fs)) return true
133+
} else if (entry.name === 'SKILL.md') {
134+
return true
135+
}
136+
}
137+
138+
return false
139+
}
140+
117141
/**
118142
* Read dependencies and peerDependencies (and optionally devDependencies) from
119143
* a parsed package.json object.
@@ -232,6 +256,8 @@ export function listNestedNodeModulesPackageDirs(
232256
return packageDirs
233257
}
234258

259+
const GLOBAL_NODE_MODULES_COMMAND_TIMEOUT_MS = 5_000
260+
235261
export function detectGlobalNodeModules(packageManager: string): {
236262
path: string | null
237263
source?: string
@@ -267,6 +293,7 @@ export function detectGlobalNodeModules(packageManager: string): {
267293
const output = execFileSync(candidate.command, candidate.args, {
268294
encoding: 'utf8',
269295
stdio: ['ignore', 'pipe', 'ignore'],
296+
timeout: GLOBAL_NODE_MODULES_COMMAND_TIMEOUT_MS,
270297
}).trim()
271298
if (!output) continue
272299

packages/intent/src/staleness/check.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,13 @@ function readLocalVersion(packageDir: string): string | null {
9090
}
9191
}
9292

93+
const NPM_REGISTRY_FETCH_TIMEOUT_MS = 5_000
94+
9395
async function fetchNpmVersion(packageName: string): Promise<string | null> {
9496
try {
9597
const res = await fetch(
9698
`https://registry.npmjs.org/${encodeURIComponent(packageName)}/latest`,
99+
{ signal: AbortSignal.timeout(NPM_REGISTRY_FETCH_TIMEOUT_MS) },
97100
)
98101
if (!res.ok) return null
99102
const data = (await res.json()) as Record<string, unknown>

0 commit comments

Comments
 (0)