From 485dbc024fa6ef248f71dbea593b55b94068df1a Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 15:10:28 -0400 Subject: [PATCH 01/11] perf(tools): bound foreground file reads and cancellable searches --- main/services/coding-tool-matcher.ts | 89 +++++++ main/services/coding-tools.ts | 353 ++++++++++++++++++++------- 2 files changed, 352 insertions(+), 90 deletions(-) create mode 100644 main/services/coding-tool-matcher.ts diff --git a/main/services/coding-tool-matcher.ts b/main/services/coding-tool-matcher.ts new file mode 100644 index 000000000..cfb286e47 --- /dev/null +++ b/main/services/coding-tool-matcher.ts @@ -0,0 +1,89 @@ +import { Worker } from "node:worker_threads"; + +// Fixed code, with all model input passed as data. Keeping this self-contained +// also works in the packaged Electron main bundle without a worker asset loader. +const SOURCE = ` +const { parentPort, workerData } = require('node:worker_threads'); +const { matchesGlob } = require('node:path'); +try { + const regex = workerData.kind === 'grep' ? new RegExp(workerData.pattern) : null; + parentPort.on('message', ({ values, limit }) => { + const indices = []; + for (let i = 0; i < values.length && indices.length < limit; i++) { + if (regex ? regex.test(values[i]) : matchesGlob(values[i], workerData.pattern)) indices.push(i); + } + parentPort.postMessage({ indices }); + }); + parentPort.postMessage({ indices: [] }); +} catch (error) { parentPort.postMessage({ error: error.message }); } +`; + +export class CodingToolMatchTimeout extends Error {} +let activeMatchers = 0; + +/** One sequential matcher per search; terminated before its capacity is released. */ +export async function withCodingToolMatcher( + kind: "grep" | "glob", + pattern: string, + deadline: number, + signal: AbortSignal | undefined, + run: (match: (values: string[], limit: number) => Promise) => Promise, +): Promise { + signal?.throwIfAborted(); + if (activeMatchers >= 4) throw new Error("File searches are busy; try again shortly."); + const worker = new Worker(SOURCE, { + eval: true, + execArgv: [], + workerData: { kind, pattern }, + resourceLimits: { maxOldGenerationSizeMb: 32 }, + }); + activeMatchers++; + let failure: Error | undefined; + let rejectPending: ((error: Error) => void) | undefined; + const failed = (error: Error) => { + failure = error; + rejectPending?.(error); + }; + worker.on("error", failed); + worker.on("exit", () => failed(new Error("File search matcher exited."))); + const receive = (values?: string[], limit = 0) => + new Promise((resolve, reject) => { + if (failure) return reject(failure); + if (signal?.aborted) return reject(signal.reason ?? new Error("File search cancelled.")); + const remaining = deadline - Date.now(); + if (remaining <= 0) return reject(new CodingToolMatchTimeout()); + const finish = (error?: Error, indices?: number[]) => { + clearTimeout(timer); + worker.off("message", message); + signal?.removeEventListener("abort", abort); + rejectPending = undefined; + if (error) reject(error); + else resolve(indices!); + }; + const abort = () => finish(signal?.reason ?? new Error("File search cancelled.")); + const message = (reply: { error?: string; indices: number[] }) => + finish( + reply.error + ? new Error( + `Invalid ${kind === "grep" ? "regular expression" : "glob"}: ${reply.error}`, + ) + : undefined, + reply.indices, + ); + const timer = setTimeout(() => finish(new CodingToolMatchTimeout()), remaining); + rejectPending = (error) => finish(error); + worker.once("message", message); + signal?.addEventListener("abort", abort, { once: true }); + if (values) worker.postMessage({ values, limit }); + }); + try { + await receive(); + return await run(receive); + } finally { + try { + await worker.terminate(); + } finally { + activeMatchers--; + } + } +} diff --git a/main/services/coding-tools.ts b/main/services/coding-tools.ts index 105fee247..6955f9903 100644 --- a/main/services/coding-tools.ts +++ b/main/services/coding-tools.ts @@ -6,6 +6,8 @@ // Tool inputs use typebox schemas (pi's AgentTool.parameters), matching tools.ts. import { spawn } from "node:child_process"; +import { StringDecoder } from "node:string_decoder"; +import { CodingToolMatchTimeout, withCodingToolMatcher } from "./coding-tool-matcher.js"; import { agentCommandEnvironment } from "./agent-command-environment.js"; import { constants as fsConstants, @@ -261,6 +263,9 @@ interface WorkspaceRootGuard { beforeDirectoryOpen?(directoryPath: string): void | Promise; afterDirectoryOpen?(directoryPath: string): void | Promise; beforeEntryAccess?(entryPath: string): void | Promise; + afterFileStat?(entryPath: string): void | Promise; + onFileRead?(bytes: number): void; + onDirectoryEntry?(entryPath: string): void; beforeWriteCommit?(entryPath: string): void | Promise; }; } @@ -903,6 +908,113 @@ function makeSubagentReadFile(workspace: WorkspaceRootGuard): AgentTool { }; } +/** Parent policy deliberately permits metadata/credential names other than .env. */ +async function readParentFile( + workspace: WorkspaceRootGuard, + root: string, + full: string, + maxBytes: number, + signal?: AbortSignal, + onRead?: (bytes: number) => void, +): Promise<{ buffer: Buffer; truncated: boolean; bytesRead: number }> { + throwIfAborted(signal, "File read cancelled."); + await workspace.testObserver?.beforeEntryAccess?.(full); + await verifyWorkspaceRoot(workspace, signal); + const canonical = assertRealPathInRoot(root, await fs.realpath(full), full); + rejectEnvironmentSecret(root, canonical); + const handle = await fs.open(canonical, fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) | (fsConstants.O_NONBLOCK ?? 0)); + try { + const stat = await handle.stat(); + if (!stat.isFile()) throw new Error("The requested path is not a regular file."); + const verified = assertRealPathInRoot(root, await fs.realpath(full), full); + rejectEnvironmentSecret(root, verified); + if (!sameFile(stat, await fs.stat(verified))) throw new Error("File changed while opening."); + await workspace.testObserver?.afterFileStat?.(full); + const buffer = Buffer.allocUnsafe(maxBytes + 1); + let bytesRead = 0; + while (bytesRead < buffer.length) { + throwIfAborted(signal, "File read cancelled."); + const result = await handle.read(buffer, bytesRead, Math.min(64 * 1024, buffer.length - bytesRead), bytesRead); + onRead?.(result.bytesRead); + workspace.testObserver?.onFileRead?.(result.bytesRead); + if (!result.bytesRead) break; + bytesRead += result.bytesRead; + } + await verifyWorkspaceRoot(workspace, signal); + return { buffer: buffer.subarray(0, Math.min(bytesRead, maxBytes)), truncated: bytesRead > maxBytes, bytesRead }; + } finally { + await handle.close(); + } +} + +interface ParentScanBudget { + entries: number; + bytes: number; + deadline: number; + stopped: boolean; + warnings: Set; +} + +function parentScanBudget(): ParentScanBudget { + return { entries: 0, bytes: 0, deadline: Date.now() + MAX_GREP_DURATION_MS, stopped: false, warnings: new Set() }; +} + +function parentScanStopped(budget: ParentScanBudget, signal?: AbortSignal): boolean { + throwIfAborted(signal, "File search cancelled."); + if (Date.now() >= budget.deadline) { + budget.stopped = true; + budget.warnings.add(`… [search stopped after ${MAX_GREP_DURATION_MS} ms]`); + } + return budget.stopped; +} + +/** opendir bounds enumeration even when a directory has no matching entries. */ +async function parentDirectoryEntries( + workspace: WorkspaceRootGuard, + full: string, + budget: ParentScanBudget, + signal?: AbortSignal, +): Promise { + if (parentScanStopped(budget, signal)) return []; + const root = await verifyWorkspaceRoot(workspace, signal); + await workspace.testObserver?.beforeDirectoryOpen?.(full); + const canonical = assertRealPathInRoot(root, await fs.realpath(full), full); + const before = await fs.stat(canonical); + const directory = await fs.opendir(canonical, { bufferSize: 1 }); + try { + await workspace.testObserver?.afterDirectoryOpen?.(full); + await verifyWorkspaceRoot(workspace, signal); + const after = assertRealPathInRoot(root, await fs.realpath(full), full); + if (canonical !== after || !sameFile(before, await fs.stat(after))) throw new Error("Directory changed while opening."); + const entries: Dirent[] = []; + while (!parentScanStopped(budget, signal)) { + // No lookahead beyond the work budget: reaching it is conservatively incomplete. + if (budget.entries >= MAX_GLOB_ENTRIES) { + budget.stopped = true; + budget.warnings.add(`… [scan stopped after ${MAX_GLOB_ENTRIES} entries]`); + break; + } + const entry = await directory.read(); + if (!entry) break; + budget.entries++; + workspace.testObserver?.onDirectoryEntry?.(path.join(full, entry.name)); + throwIfAborted(signal, "Directory traversal cancelled."); + entries.push(entry); + } + // Omit an over-budget directory instead of allowing filesystem enumeration + // order to select the returned subset. Earlier complete directories survive. + return budget.stopped ? [] : entries.sort(compareEntryNames); + } finally { + await directory.close(); + } +} + +function validateParentPattern(pattern: string): void { + if (typeof pattern !== "string" || pattern.length > MAX_SEARCH_PATTERN_CHARS) { + throw new Error(`Search pattern must be at most ${MAX_SEARCH_PATTERN_CHARS} characters.`); + } +} + function makeParentReadFile(workspace: WorkspaceRootGuard): AgentTool { return { name: "read_file", @@ -912,14 +1024,15 @@ function makeParentReadFile(workspace: WorkspaceRootGuard): AgentTool { parameters: Type.Object({ path: Type.String({ description: "File path relative to the workspace folder." }), }), - execute: async (_id, params): Promise> => { + execute: async (_id, params, signal): Promise> => { const { path: p } = params as { path: string }; - const { root: realRoot, full } = await resolveExistingInRoot(workspace, p); + const { root: realRoot, full } = await resolveExistingInRoot(workspace, p, signal); rejectEnvironmentSecret(realRoot, full); - const buffer = await fs.readFile(full); - const text = buffer.subarray(0, MAX_READ_BYTES).toString("utf-8"); + const { buffer, truncated } = await readParentFile(workspace, realRoot, full, MAX_READ_BYTES, signal); + const decoder = new StringDecoder("utf8"); + const text = decoder.write(buffer) + (truncated ? "" : decoder.end()); return textResult( - buffer.length > MAX_READ_BYTES ? `${text}\n… [truncated]` : text || "[empty file]", + truncated ? `${text}\n… [truncated]` : text || "[empty file]", ); }, }; @@ -1107,15 +1220,21 @@ function makeParentListDir(workspace: WorkspaceRootGuard): AgentTool { Type.String({ description: "Directory relative to the workspace folder (default root)." }), ), }), - execute: async (_id, params): Promise> => { + execute: async (_id, params, signal): Promise> => { const { path: p } = params as { path?: string }; - const { full } = await resolveExistingInRoot(workspace, p ?? "."); - const entries = await fs.readdir(full, { withFileTypes: true }); + const { full } = await resolveExistingInRoot(workspace, p ?? ".", signal); + const budget = parentScanBudget(); + const entries = await parentDirectoryEntries(workspace, full, budget, signal); + const truncated = entries.length > MAX_LIST_ENTRIES; + entries.sort(compareEntryNames); + entries.length = Math.min(entries.length, MAX_LIST_ENTRIES); const lines = entries.map( (entry) => `${entry.isDirectory() ? "dir " : "file"} ${entry.name}`, ); lines.sort(); - return textResult(lines.length ? lines.join("\n") : "[empty directory]"); + if (truncated) budget.warnings.add(`… [truncated at ${MAX_LIST_ENTRIES} entries]`); + await verifyWorkspaceRoot(workspace, signal); + return textResult(formatBoundedSearchResult(lines, budget.stopped ? "[no entries returned]" : "[empty directory]", [...budget.warnings])); }, }; } @@ -1268,37 +1387,80 @@ function makeSubagentGlob(workspace: WorkspaceRootGuard): AgentTool { } function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { - const root = workspace.lexical; return { name: "glob", label: "Find Files", - description: 'Find files in the workspace folder matching a glob pattern, e.g. "src/**/*.ts".', - parameters: Type.Object({ - pattern: Type.String({ description: "Glob pattern relative to the workspace folder." }), - }), - execute: async (_id, params): Promise> => { + description: 'Find files in the workspace folder matching a glob pattern, e.g. "src/**/*.ts". Large searches return an incomplete-result notice.', + parameters: Type.Object({ pattern: Type.String({ maxLength: MAX_SEARCH_PATTERN_CHARS, description: "Glob pattern relative to the workspace folder." }) }), + execute: async (_id, params, signal): Promise> => { const { pattern } = params as { pattern: string }; + throwIfAborted(signal, "File search cancelled."); + validateParentPattern(pattern); + const budget = parentScanBudget(); const matches: string[] = []; - const realRoot = await verifyWorkspaceRoot(workspace); - for await (const entry of fs.glob(pattern, { cwd: root })) { - try { - const lexical = resolveInRoot(root, entry); - const realPath = await fs.realpath(lexical); - assertRealPathInRoot(realRoot, realPath, entry); - if ( - isEnvironmentSecretPath(entry) || - isEnvironmentSecretPath(path.relative(realRoot, realPath)) - ) { - continue; + const root = await verifyWorkspaceRoot(workspace, signal); + const normalized = path.isAbsolute(pattern) ? path.relative(workspace.lexical, pattern) : path.normalize(pattern); + const segments = normalized.split(path.sep); + const magicIndex = segments.findIndex((segment) => /[*?[\]{}()!+@]/.test(segment)); + const base = magicIndex < 0 ? path.dirname(normalized) : segments.slice(0, magicIndex).join(path.sep) || "."; + const pending = [{ relative: base, shallow: false }]; + const displayPath = (candidate: string) => path.isAbsolute(pattern) ? path.resolve(workspace.lexical, candidate) : candidate; + try { + await withCodingToolMatcher("glob", normalized, budget.deadline, signal, async (match) => { + if (magicIndex >= 0 && (await match([base === "." ? "" : `${base}${path.sep}`], 1)).length) { + const { full } = await resolveExistingInRoot(workspace, base, signal); + if (!isEnvironmentSecretPath(path.relative(root, full))) matches.push(displayPath(base)); } - } catch { - continue; - } - matches.push(entry); - if (matches.length >= 500) break; + while (pending.length && !parentScanStopped(budget, signal)) { + const { relative, shallow } = pending.pop()!; + let entries: Dirent[]; + try { + entries = await parentDirectoryEntries(workspace, resolveInRoot(workspace.lexical, relative), budget, signal); + } catch { + throwIfAborted(signal, "File search cancelled."); + await verifyWorkspaceRoot(workspace, signal); + budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); + continue; + } + const candidates = entries.map((entry) => path.join(relative, entry.name)); + const matchInputs = candidates.flatMap((candidate, index) => [candidate, entries[index]!.isDirectory() ? `${candidate}${path.sep}` : candidate]); + const indices = new Set((await match(matchInputs, matchInputs.length)).map((index) => Math.floor(index / 2))); + for (let index = 0; index < entries.length; index++) { + if (parentScanStopped(budget, signal)) break; + const entry = entries[index]!; + const candidate = candidates[index]!; + if (isEnvironmentSecretPath(candidate)) continue; + // Native globstar does not recurse through symlinks. Explicit linked + // prefixes are resolved above and remain confined to the workspace. + if (magicIndex >= 0 && (normalized.includes("**") || candidate.split(path.sep).length < segments.length) && entry.isDirectory() && !shallow) pending.push({ relative: candidate, shallow: false }); + if (!indices.has(index)) continue; + // Node glob allows one linked-directory level for a terminal **/*. + if (!shallow && entry.isSymbolicLink() && normalized.endsWith(`**${path.sep}*`)) { + pending.push({ relative: candidate, shallow: true }); + } + try { + const canonical = assertRealPathInRoot(root, await fs.realpath(resolveInRoot(workspace.lexical, candidate)), candidate); + if (isEnvironmentSecretPath(path.relative(root, canonical))) continue; + } catch { + throwIfAborted(signal, "File search cancelled."); + continue; + } + if (matches.length === MAX_GLOB_MATCHES) { + budget.warnings.add(`… [truncated at ${MAX_GLOB_MATCHES} matches]`); + budget.stopped = true; + break; + } + matches.push(displayPath(candidate)); + } + } + }); + } catch (error) { + if (!(error instanceof CodingToolMatchTimeout)) throw error; + budget.warnings.add(`… [search stopped after ${MAX_GREP_DURATION_MS} ms]`); } + await verifyWorkspaceRoot(workspace, signal); matches.sort(); - return textResult(matches.length ? matches.join("\n") : "[no matches]"); + return textResult(formatBoundedSearchResult(matches, "[no matches]", [...budget.warnings])); }, }; } @@ -1505,74 +1667,85 @@ function makeSubagentGrep(workspace: WorkspaceRootGuard): AgentTool { }; } -async function grepParentDir( - dir: string, - root: string, - regex: RegExp, - out: string[], -): Promise { - if (out.length >= MAX_GREP_MATCHES) return; - const entries = await fs.readdir(dir, { withFileTypes: true }); - for (const entry of entries) { - if (out.length >= MAX_GREP_MATCHES) return; - if (entry.name.startsWith(".") || entry.isSymbolicLink()) continue; - const full = path.join(dir, entry.name); - if (entry.isDirectory()) { - if (SKIP_DIRS.has(entry.name)) continue; - await grepParentDir(full, root, regex, out); - continue; - } - if (!entry.isFile()) continue; - let stat: Stats; - try { - stat = await fs.stat(full); - } catch { - continue; - } - if (stat.size > 512_000) continue; - let content: string; - try { - content = await fs.readFile(full, "utf-8"); - } catch { - continue; - } - const relativePath = path.relative(root, full); - const lines = content.split("\n"); - for (let index = 0; index < lines.length; index += 1) { - if (regex.test(lines[index] ?? "")) { - out.push(`${relativePath}:${index + 1}: ${(lines[index] ?? "").trim().slice(0, 200)}`); - if (out.length >= MAX_GREP_MATCHES) return; - } - } - } -} - function makeParentGrep(workspace: WorkspaceRootGuard): AgentTool { return { name: "grep", label: "Search Files", - description: - "Search the workspace folder for lines matching a regular expression. Returns file:line: match, capped at 200 hits.", + description: "Search the workspace folder for lines matching a JavaScript regular expression. Returns file:line: match, capped at 200 hits. Large searches return an incomplete-result notice.", parameters: Type.Object({ - pattern: Type.String({ description: "JavaScript regular expression to search for." }), - path: Type.Optional( - Type.String({ description: "Subdirectory to limit the search to (default root)." }), - ), + pattern: Type.String({ maxLength: MAX_SEARCH_PATTERN_CHARS, description: "JavaScript regular expression to search for." }), + path: Type.Optional(Type.String({ description: "Subdirectory to limit the search to (default root)." })), }), - execute: async (_id, params): Promise> => { + execute: async (_id, params, signal): Promise> => { const { pattern, path: p } = params as { pattern: string; path?: string }; - let regex: RegExp; + throwIfAborted(signal, "File search cancelled."); + validateParentPattern(pattern); + const budget = parentScanBudget(); + const { root, full: start } = await resolveExistingInRoot(workspace, p ?? ".", signal); + const out: string[] = []; try { - regex = new RegExp(pattern); + await withCodingToolMatcher("grep", pattern, budget.deadline, signal, async (match) => { + const pending = [start]; + while (pending.length && !parentScanStopped(budget, signal)) { + const full = pending.pop()!; + const stat = await fs.lstat(full).catch(() => null); + if (!stat) { budget.warnings.add("… [search incomplete: unreadable paths skipped]"); continue; } + if (stat.isDirectory()) { + let entries: Dirent[]; + try { entries = await parentDirectoryEntries(workspace, full, budget, signal); } + catch { + throwIfAborted(signal, "File search cancelled."); + await verifyWorkspaceRoot(workspace, signal); + budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); + continue; + } + // Reverse push preserves the original sorted depth-first order. + for (const entry of entries.reverse()) { + if (entry.name.startsWith(".") || entry.isSymbolicLink()) continue; + if (entry.isDirectory() && SKIP_DIRS.has(entry.name)) continue; + if (entry.isDirectory() || entry.isFile()) pending.push(path.join(full, entry.name)); + } + continue; + } + if (!stat.isFile()) continue; + if (stat.size > 512_000) { budget.warnings.add("… [search incomplete: oversized files skipped]"); continue; } + const remaining = MAX_GREP_BYTES - budget.bytes; + if (remaining <= 1) { + budget.warnings.add(`… [scan stopped after ${MAX_GREP_BYTES} bytes]`); + break; + } + let read: Awaited>; + try { + read = await readParentFile(workspace, root, full, Math.min(512_000, remaining - 1), signal, (bytes) => { budget.bytes += bytes; }); + } catch { + throwIfAborted(signal, "File search cancelled."); + await verifyWorkspaceRoot(workspace, signal); + budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); + continue; + } + if (read.truncated) { + budget.warnings.add(remaining <= 512_001 ? `… [scan stopped after ${MAX_GREP_BYTES} bytes]` : "… [search incomplete: oversized files skipped]"); + if (remaining <= 512_001) break; + continue; + } + const lines = read.buffer.toString("utf8").split("\n"); + const indices = await match(lines, MAX_GREP_MATCHES + 1 - out.length); + for (const index of indices) { + if (out.length === MAX_GREP_MATCHES) { + budget.warnings.add(`… [truncated at ${MAX_GREP_MATCHES} matches]`); + budget.stopped = true; + break; + } + out.push(`${path.relative(root, full)}:${index + 1}: ${lines[index]!.trim().slice(0, 200)}`); + } + } + }); } catch (error) { - throw new Error( - `Invalid regular expression: ${error instanceof Error ? error.message : String(error)}`, - ); + if (!(error instanceof CodingToolMatchTimeout)) throw error; + budget.warnings.add(`… [search stopped after ${MAX_GREP_DURATION_MS} ms]`); } - const { root: realRoot, full: start } = await resolveExistingInRoot(workspace, p ?? "."); - const out: string[] = []; - await grepParentDir(start, realRoot, regex, out); - return textResult(out.length ? out.join("\n") : "[no matches]"); + await verifyWorkspaceRoot(workspace, signal); + return textResult(formatBoundedSearchResult(out, "[no matches]", [...budget.warnings])); }, }; } From 8e97fe333a4a171e5356c6d02e26397861bf5862 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 15:10:38 -0400 Subject: [PATCH 02/11] test(tools): verify foreground scan budgets and Electron worker lifecycle --- .memory/foreground-file-tool-bounds.md | 36 ++++ .papercuts/troubleshooting.md | 5 + .../README.md | 71 ++++++++ .../after.json | 49 +++++ .../before.json | 49 +++++ .../electron.json | 26 +++ main/services/coding-tools.test.ts | 167 ++++++++++++++++++ scripts/benchmark-foreground-file-tools.ts | 133 ++++++++++++++ scripts/smoke-foreground-file-tools.ts | 93 ++++++++++ 9 files changed, 629 insertions(+) create mode 100644 .memory/foreground-file-tool-bounds.md create mode 100644 docs/performance/foreground-file-tools-2026-09-28/README.md create mode 100644 docs/performance/foreground-file-tools-2026-09-28/after.json create mode 100644 docs/performance/foreground-file-tools-2026-09-28/before.json create mode 100644 docs/performance/foreground-file-tools-2026-09-28/electron.json create mode 100644 scripts/benchmark-foreground-file-tools.ts create mode 100644 scripts/smoke-foreground-file-tools.ts diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md new file mode 100644 index 000000000..ba4ae49f6 --- /dev/null +++ b/.memory/foreground-file-tool-bounds.md @@ -0,0 +1,36 @@ +# Foreground file-tool bounds — 2026-09-28 + +Scope: audit X03/X04/X05, based on a9baa4aa3027893e5455043083465c34b4c8b4ac. +All 36 then-open PRs rechecked with paginated file inventories (#85: 295 files, +#37: 209); none touched coding-tools.ts or its suite. Workspace/Git lanes are separate. + +Parent read_file now opens/validates a regular descriptor, reads at most 200,001 +bytes (including overflow probe), closes in finally, observes cancellation, and +omits an incomplete UTF-8 trailing sequence. Parent policy intentionally remains +broader than child policy: hidden metadata and credential names remain readable +except .env secrets; in-root links resolve, outside-root links fail. No subagent +credential/RE2 rules were transplanted into parent tools. + +Parent list/glob/grep enumerate with opendir(bufferSize: 1), at most 10,000 entries +and 5 seconds per invocation. Over-budget directories are omitted rather than +allowing OS enumeration order to select a subset; prior complete-directory +results survive. List retains at most 500 results, glob 500, grep 200; collection +and output bounds have explicit notices (20,000 output chars). Grep skips hidden, +linked, dependency/build directories as before, reads at most 512,001 bytes per +file and 10 MiB total (including probes), and skips a growing oversized file. + +JavaScript RegExp remains native JS, including lookbehind and backreferences, +inside a fixed-source owned worker. Model patterns are workerData, never code. +Glob uses Node matchesGlob with native-compatible directory and explicit linked +prefix handling. Native-glob differential fixtures cover ordinary patterns, +braces, extglobs, hidden paths, absolute paths, directory roots and symlinks. +Patterns have a new explicit 1,000-character ceiling. At most four matchers can +run concurrently; excess searches report busy. Cancellation, errors and deadlines +terminate and await the worker before releasing capacity; no persistent worker. +The small-search cost of worker startup is intentional and measured. + +No Remote DTO, transcript/activity UI, native implementation or onboarding +capability changed. iOS/Android consumers were inspected: they use unchanged tool +names/labels, not filesystem scanning/matching internals. No plan status changed. +Evidence and repeatable commands: docs/performance/foreground-file-tools-2026-09-28/. +Independent Astra review and hosted exact-head CI/bot review are delivery gates. diff --git a/.papercuts/troubleshooting.md b/.papercuts/troubleshooting.md index ceeb89458..7a4f4fb6b 100644 --- a/.papercuts/troubleshooting.md +++ b/.papercuts/troubleshooting.md @@ -1404,3 +1404,8 @@ because their native file-mutator test binary had not been built. Run ## 2026-09-26 PR #121 merge of #251 (Remote contract revision 14) - A PR that adds to the Remote contract has to renumber when main bumps `contractRevision`. The conflicts show up in 7 files: both fixtures, the TS/iOS/Android fixture assertions and the iOS fixture CodingKeys. After resolving, `cmp` the Android copy against the shared fixture. Plan docs that name the revision also go stale. + +## 2026-09-28 — Foreground file-tool performance fixtures + +- Node built-in ESM namespaces retain their bindings when a benchmark wraps the default fs/promises export. Call `syncBuiltinESMExports()` after instrumentation and restoration; otherwise byte counters misleadingly report zero. The clean 0.87.1 runner now reproduces the original 16 MiB read. +- A standalone Electron ESM smoke must externalize `electron` explicitly (tsconfig path resolution otherwise bundles its npm launcher), and register `app.whenReady().then(...)` without top-level-awaiting readiness. Electron waits for module evaluation before ready; top-level await deadlocks the fixture. The resulting fixed-source worker is validated in pinned Electron 43.1.1 / Node 24.18.0. diff --git a/docs/performance/foreground-file-tools-2026-09-28/README.md b/docs/performance/foreground-file-tools-2026-09-28/README.md new file mode 100644 index 000000000..f2a7abc45 --- /dev/null +++ b/docs/performance/foreground-file-tools-2026-09-28/README.md @@ -0,0 +1,71 @@ +# Foreground file tools: X03/X04/X05 + +Baseline: `a9baa4aa3027893e5455043083465c34b4c8b4ac`. Same macOS arm64 host, +Node 22.22.3, and clean `npm ci --ignore-scripts` dependencies (Pi **0.87.1**) +for both source-counter runs. No provider traffic, catalog fetch or user profiles. +These are deterministic filesystem counters and synthetic timings, not energy or +packaged UI latency measurements. Raw receipts: [before](before.json), +[after](after.json), [Electron runtime](electron.json). + +| Fixture | Before | After | +| --- | ---: | ---: | +| 16 MiB read_file: actual bytes read | 16,777,216 | 200,001 | +| Same read: output characters | 200,014 | 200,014 | +| Already-aborted grep | Returns match; reads 7 bytes | Rejects; reads 0 bytes | +| 10,100-entry no-match scan: entries enumerated | 10,100 | 10,000, explicit incomplete notice | +| Wide list output | 200,989 characters | Bounded incomplete notice; over-cap directory omitted | + +Grep has a 10 MiB aggregate input ceiling, including each overflow probe; tests +exercise growth after stat and exact byte counts. List/glob/grep have 10,000-entry, +5-second work limits; an over-cap directory is omitted for deterministic results. +Collection limits are 500/500/200 and search/list output is at most 20,000 chars. +Read_file retains its existing 200,000-byte content cap and truncation suffix, +but no longer returns a broken trailing UTF-8 character. + +Native JavaScript regex semantics remain supported, including lookbehind and +backreferences. No RE2 fallback changes the accepted language. Patterns above +1,000 characters now fail explicitly; a pathological pattern stops at the search +deadline with an incomplete notice. A fixed-source worker receives model input as +data and is terminated/awaited on every exit. Four concurrent workers are allowed; +additional searches return a busy error. This adds roughly 17 ms per tiny grep on +the quiet fixture run (see raw samples), versus sub-millisecond baseline calls. +It buys cancellable matching and removes regex execution from Electron's main +thread. The worker is not retained between calls. + +Parent hidden-file/credential policy remains distinct from the stricter child +policy. Read_file still supports safe symlinks and metadata, excludes .env secrets, +and rejects outside-root targets. Grep retains hidden/symlink/dependency ignores. +Glob retains Node matching semantics for the tested normal patterns and explicit +in-root linked prefixes, while now limiting traversal. Differential tests include +absolute paths, braces/extglobs, `**`, directory roots and linked prefixes. + +Pinned Electron 43.1.1 / Node 24.18.0 smoke uses a bundled entry and real app main +process. It verifies native glob compatibility, JS lookbehind/backreferences, +six invalid-pattern failures and six catastrophic-pattern cancellations followed +by successful calls, deadline settlement, and zero retained worker message ports. +150 ms cancellation timers settled at 150–154 ms in the recorded run. + +Reproduce counters (the temporary baseline copy stays in this worktree): + +```sh +npm ci --ignore-scripts +git show a9baa4aa3027893e5455043083465c34b4c8b4ac:main/services/coding-tools.ts > main/services/.foreground-baseline.ts +npx tsx scripts/benchmark-foreground-file-tools.ts main/services/.foreground-baseline.ts +npx tsx scripts/benchmark-foreground-file-tools.ts +rm main/services/.foreground-baseline.ts +``` + +Run behavioral acceptance and the Electron smoke: + +```sh +npx tsx --test main/services/coding-tools.test.ts main/services/generation-runtime.test.ts +npm run type-check +npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts +node node_modules/electron/install.js +npx esbuild scripts/smoke-foreground-file-tools.ts --bundle --platform=node --format=esm --packages=external --external:electron --outfile=build/main/foreground-file-tools-smoke.mjs +node_modules/electron/dist/Electron.app/Contents/MacOS/Electron build/main/foreground-file-tools-smoke.mjs +``` + +The existing coding-tools test file is already in the root test chain and CI +registry. No shared server/native contract or transcript UI changed; no mobile +implementation or plan-status update is needed for these internal tools. diff --git a/docs/performance/foreground-file-tools-2026-09-28/after.json b/docs/performance/foreground-file-tools-2026-09-28/after.json new file mode 100644 index 000000000..4e7a5c746 --- /dev/null +++ b/docs/performance/foreground-file-tools-2026-09-28/after.json @@ -0,0 +1,49 @@ +{ + "modulePath": "main/services/coding-tools.ts", + "node": "v22.22.3", + "platform": "darwin", + "arch": "arm64", + "piVersion": "0.87.1", + "read": { + "inputBytes": 16777216, + "bytesRead": 200001, + "outputChars": 200014 + }, + "abortedGrep": { + "rejected": true, + "bytesRead": 0 + }, + "smallGrepMs": [ + 20.772999999999996, + 17.98208299999999, + 17.47387500000002, + 17.484375, + 17.382083000000023, + 17.51716700000003, + 16.555041000000017, + 16.932082999999977, + 17.844750000000033, + 18.124041999999974 + ], + "wideFixtureEntries": 10100, + "scans": [ + { + "name": "list_dir", + "enumeratedEntries": 10000, + "outputChars": 58, + "incompleteNotice": true + }, + { + "name": "glob", + "enumeratedEntries": 10000, + "outputChars": 49, + "incompleteNotice": true + }, + { + "name": "grep", + "enumeratedEntries": 10000, + "outputChars": 49, + "incompleteNotice": true + } + ] +} diff --git a/docs/performance/foreground-file-tools-2026-09-28/before.json b/docs/performance/foreground-file-tools-2026-09-28/before.json new file mode 100644 index 000000000..7027e6280 --- /dev/null +++ b/docs/performance/foreground-file-tools-2026-09-28/before.json @@ -0,0 +1,49 @@ +{ + "modulePath": "main/services/.foreground-baseline.ts", + "node": "v22.22.3", + "platform": "darwin", + "arch": "arm64", + "piVersion": "0.87.1", + "read": { + "inputBytes": 16777216, + "bytesRead": 16777216, + "outputChars": 200014 + }, + "abortedGrep": { + "rejected": false, + "bytesRead": 7 + }, + "smallGrepMs": [ + 0.2409579999999778, + 0.23141700000002174, + 0.4002079999999637, + 0.4630830000000401, + 0.4339589999999589, + 0.3725839999999607, + 0.3684170000000222, + 0.40104199999996126, + 0.31462499999997817, + 0.233333000000016 + ], + "wideFixtureEntries": 10100, + "scans": [ + { + "name": "list_dir", + "enumeratedEntries": 10100, + "outputChars": 200989, + "incompleteNotice": false + }, + { + "name": "glob", + "enumeratedEntries": 10100, + "outputChars": 12, + "incompleteNotice": false + }, + { + "name": "grep", + "enumeratedEntries": 10100, + "outputChars": 12, + "incompleteNotice": false + } + ] +} diff --git a/docs/performance/foreground-file-tools-2026-09-28/electron.json b/docs/performance/foreground-file-tools-2026-09-28/electron.json new file mode 100644 index 000000000..3fff7f668 --- /dev/null +++ b/docs/performance/foreground-file-tools-2026-09-28/electron.json @@ -0,0 +1,26 @@ +{ + "electron": "43.1.1", + "node": "v24.18.0", + "globs": [ + "*", + "**", + "**/*", + "**/*.ts", + "src/**", + "src", + "src/*.{ts,js}", + "link/*.ts", + "*/a.ts" + ], + "cancellationMs": [ + 150.44795899999997, + 152.30762500000003, + 151.294625, + 153.20070899999996, + 152.88079199999993, + 152.90762500000005 + ], + "deadline": "settled with incomplete notice", + "workerPortsAfter": 0, + "result": "passed" +} diff --git a/main/services/coding-tools.test.ts b/main/services/coding-tools.test.ts index d1f9efaea..749e58fea 100644 --- a/main/services/coding-tools.test.ts +++ b/main/services/coding-tools.test.ts @@ -2153,3 +2153,170 @@ test("POSIX colon filenames retain actual write and edit provenance", { skip: pr assert.deepEqual((edited.details as { producedFile: unknown }).producedFile, { relativePath: "foo:bar.txt", operation: "edited", bytes: 4 }); } finally { await fs.rm(root, { recursive: true, force: true }); } }); + +async function foregroundText(root: string, name: string, params: Record, signal?: AbortSignal, observer?: Parameters[1]): Promise { + const result = await buildCodingTools(root, observer).find((tool) => tool.name === name)!.execute("bounded-parent", params, signal); + return result.content.filter((block) => block.type === "text").map((block) => block.text).join("\n"); +} + +test("foreground read bounds actual bytes for sparse files and growth after stat, preserving UTF-8", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-read-")); + try { + const full = path.join(root, "large.txt"); + await fs.writeFile(full, "a".repeat(199_999) + "😀end"); + let bytes = 0; + const text = await foregroundText(root, "read_file", { path: "large.txt" }, undefined, { + afterFileStat: async () => { const h = await fs.open(full, "r+"); try { await h.truncate(32 * 1024 * 1024); } finally { await h.close(); } }, + onFileRead: (count) => { bytes += count; }, + }); + assert.equal(bytes, 200_001); + assert.equal(text, "a".repeat(199_999) + "\n… [truncated]"); + await fs.writeFile(full, "é😀\n"); + assert.equal(await foregroundText(root, "read_file", { path: "large.txt" }), "é😀\n"); + await fs.writeFile(full, ""); + assert.equal(await foregroundText(root, "read_file", { path: "large.txt" }), "[empty file]"); + await fs.symlink("large.txt", path.join(root, "alias")); + assert.equal(await foregroundText(root, "read_file", { path: "alias" }), "[empty file]"); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground tools reject cancellation before I/O and during reads or enumeration", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-cancel-")); + try { + await fs.writeFile(path.join(root, "a.txt"), "needle".repeat(40_000)); + for (const [name, params] of [["read_file", { path: "a.txt" }], ["list_dir", {}], ["glob", { pattern: "**/*" }], ["grep", { pattern: "needle" }]] as const) { + const pre = new AbortController(); pre.abort(new Error("test cancelled")); + let entries = 0; let bytes = 0; + await assert.rejects(foregroundText(root, name, params, pre.signal, { onFileRead: () => { bytes++; }, onDirectoryEntry: () => { entries++; } }), /test cancelled/); + assert.equal(bytes + entries, 0); + const during = new AbortController(); + await assert.rejects(foregroundText(root, name, params, during.signal, { + onFileRead: () => during.abort(new Error("mid-read cancelled")), + onDirectoryEntry: () => during.abort(new Error("mid-scan cancelled")), + }), /mid-(read|scan) cancelled/); + } + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground grep retains JS patterns and stays cancellable during catastrophic backtracking", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-regex-")); + try { + await fs.writeFile(path.join(root, "a.txt"), "foobar foofoo\nnope\n"); + assert.equal(await foregroundText(root, "grep", { pattern: "(?<=foo)bar|\\b(foo)\\1\\b" }), "a.txt:1: foobar foofoo"); + await assert.rejects(foregroundText(root, "grep", { pattern: "[" }), /Invalid regular expression/); + await fs.writeFile(path.join(root, "a.txt"), "a".repeat(100_000) + "!"); + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(new Error("regex cancelled")), 150); + try { + await assert.rejects(foregroundText(root, "grep", { pattern: "^(a+)+$" }, controller.signal), /regex cancelled/); + } finally { clearTimeout(timer); } + // A subsequent call must have both a live worker and released capacity. + assert.equal(await foregroundText(root, "grep", { pattern: "not-present" }), "[no matches]"); + assert.match(await foregroundText(root, "grep", { pattern: "^(a+)+$" }), /search stopped after 5000 ms/); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground glob agrees with native glob on normal patterns, hidden paths and safe links", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-glob-")); + try { + await fs.mkdir(path.join(root, "src")); await fs.mkdir(path.join(root, ".meta")); + for (const file of ["src/a.ts", "src/b.js", "root.txt", ".meta/config.json"]) await fs.writeFile(path.join(root, file), "safe"); + await fs.symlink("src", path.join(root, "link")); + for (const pattern of ["*", "**", "**/*", "src/**", "src/*/", "src", "src/*.{js,ts}", "src/@(a|b).*", "./src/*.ts", ".meta/*", "link/*.ts", "*/a.ts", path.join(root, "src/*.ts")]) { + const expected = (await (async () => { const values: string[] = []; for await (const value of fs.glob(pattern, { cwd: root })) values.push(value); return values; })()).sort().join("\n") || "[no matches]"; + assert.equal(await foregroundText(root, "glob", { pattern }), expected, pattern); + } + await fs.writeFile(path.join(root, ".env"), "SECRET"); + assert.equal(await foregroundText(root, "glob", { pattern: ".env" }), "[no matches]"); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground scans cap work on wide no-match directories and bound output", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-wide-")); + try { + for (let offset = 0; offset < 10_020; offset += 100) { + await Promise.all(Array.from({ length: 100 }, (_, index) => fs.writeFile(path.join(root, `file-${offset + index}.txt`), ""))); + } + for (const [name, params] of [["list_dir", {}], ["glob", { pattern: "**/*.absent" }], ["grep", { pattern: "absent" }]] as const) { + let entries = 0; + const text = await foregroundText(root, name, params, undefined, { onDirectoryEntry: () => { entries++; } }); + assert.equal(entries, 10_000, name); + assert.match(text, /scan stopped after 10000 entries/, name); + assert.ok(text.length <= 20_000, name); + } + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground grep caps bytes even when files grow, and skips ignored directories", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-grep-bytes-")); + try { + await fs.mkdir(path.join(root, "node_modules")); + await fs.writeFile(path.join(root, "node_modules", "ignored.txt"), "needle"); + await fs.mkdir(path.join(root, ".hidden")); + await fs.writeFile(path.join(root, ".hidden", "ignored.txt"), "needle"); + for (let index = 0; index < 24; index++) await fs.writeFile(path.join(root, `${index}.txt`), "x".repeat(500_000)); + let bytes = 0; + const result = await foregroundText(root, "grep", { pattern: "needle" }, undefined, { onFileRead: (count) => { bytes += count; } }); + assert.equal(bytes, 10 * 1024 * 1024); + assert.match(result, /scan stopped after 10485760 bytes/); + assert.doesNotMatch(result, /ignored.txt/); + const growing = await fs.realpath(path.join(root, "0.txt")); + let growthBytes = 0; + const text = await foregroundText(root, "grep", { pattern: "x" }, undefined, { + afterFileStat: async (full) => { if (full === growing) { const h = await fs.open(full, "r+"); try { await h.truncate(32 * 1024 * 1024); } finally { await h.close(); } } }, + onFileRead: (count) => { growthBytes += count; }, + }); + assert.match(text, /oversized files skipped/); + assert.doesNotMatch(text, /^0.txt:/m); + assert.ok(growthBytes <= 10 * 1024 * 1024); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground deep scans preserve normal results and ignore dependency and hidden subtrees", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-deep-")); + try { + const relative = Array.from({ length: 48 }, () => "nested").join(path.sep); + await fs.mkdir(path.join(root, relative), { recursive: true }); + await fs.writeFile(path.join(root, relative, "leaf.txt"), "needle\n"); + await fs.mkdir(path.join(root, "node_modules")); + await fs.writeFile(path.join(root, "node_modules/ignored.txt"), "needle\n"); + await fs.mkdir(path.join(root, ".hidden")); + await fs.writeFile(path.join(root, ".hidden/ignored.txt"), "needle\n"); + const opened: string[] = []; + const result = await foregroundText(root, "grep", { pattern: "needle" }, undefined, { beforeDirectoryOpen: (directory) => { opened.push(path.basename(directory)); } }); + assert.equal(result, `${relative}${path.sep}leaf.txt:1: needle`); + assert.ok(!opened.includes("node_modules") && !opened.includes(".hidden")); + assert.equal(await foregroundText(root, "grep", { pattern: "missing" }), "[no matches]"); + assert.equal(await foregroundText(root, "glob", { pattern: "nested/**/*.txt" }), path.join(relative, "leaf.txt")); + assert.equal(await foregroundText(root, "glob", { pattern: "nested/**/*.missing" }), "[no matches]"); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground collection limits distinguish exact caps and cap rendered output", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-caps-")); + try { + for (let i = 0; i < 500; i++) await fs.writeFile(path.join(root, `file-${String(i).padStart(3, "0")}.txt`), i < 200 ? "needle\n" : ""); + for (const [name, params] of [["list_dir", {}], ["glob", { pattern: "*.txt" }], ["grep", { pattern: "needle" }]] as const) { + assert.doesNotMatch(await foregroundText(root, name, params), /truncated|scan stopped/); + } + await fs.writeFile(path.join(root, "extra.txt"), "needle\n"); + for (const [name, params, cap] of [["list_dir", {}, 500], ["glob", { pattern: "*.txt" }, 500], ["grep", { pattern: "needle" }, 200]] as const) { + const text = await foregroundText(root, name, params); + assert.match(text, new RegExp(`truncated at ${cap}`)); + assert.ok(text.length <= 20_000); + } + await fs.writeFile(path.join(root, "file-000.txt"), `${"needle".padEnd(300, "x")}\n`.repeat(201)); + const output = await foregroundText(root, "grep", { pattern: "needle" }); + assert.ok(output.length <= 20_000); + assert.match(output, /output truncated/); + assert.match(output, /truncated at 200 matches/); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground reads reject non-regular FIFOs without waiting for a writer", { skip: process.platform === "win32", timeout: 2_000 }, async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-fifo-")); + try { + await execFileAsync("mkfifo", [path.join(root, "pipe")]); + await assert.rejects(foregroundText(root, "read_file", { path: "pipe" }), /not a regular file/); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); diff --git a/scripts/benchmark-foreground-file-tools.ts b/scripts/benchmark-foreground-file-tools.ts new file mode 100644 index 000000000..9939cdc4c --- /dev/null +++ b/scripts/benchmark-foreground-file-tools.ts @@ -0,0 +1,133 @@ +/** Run with tsx; optional module path permits the exact baseline source with this lockfile. */ +import fs from "node:fs/promises"; +import { syncBuiltinESMExports } from "node:module"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; + +const modulePath = process.argv[2] ?? "main/services/coding-tools.ts"; +const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-foreground-benchmark-")); +const originalReadFile = fs.readFile; +const originalOpen = fs.open; +const originalReaddir = fs.readdir; +const originalOpendir = fs.opendir; +let enumeratedEntries = 0; +let bytesRead = 0; +fs.readFile = (async (...args: Parameters) => { + const value = await originalReadFile(...args); + bytesRead += Buffer.byteLength(value); + return value; +}) as typeof fs.readFile; +fs.open = (async (...args: Parameters) => { + const handle = await originalOpen(...args); + const read = handle.read.bind(handle); + handle.read = (async (...readArgs: Parameters) => { + const result = await read(...readArgs); + bytesRead += result.bytesRead; + return result; + }) as typeof handle.read; + return handle; +}) as typeof fs.open; +fs.readdir = (async (...args: Parameters) => { + const entries = await originalReaddir(...args); + enumeratedEntries += entries.length; + return entries; +}) as typeof fs.readdir; +fs.opendir = (async (...args: Parameters) => { + const directory = await originalOpendir(...args); + const read = directory.read.bind(directory); + directory.read = (async () => { + const entry = await read(); + if (entry) enumeratedEntries++; + return entry; + }) as typeof directory.read; + return directory; +}) as typeof fs.opendir; +syncBuiltinESMExports(); +try { + const { buildCodingTools } = await import(pathToFileURL(path.resolve(modulePath)).href); + const full = path.join(root, "large.txt"); + await fs.writeFile(full, Buffer.alloc(16 * 1024 * 1024, 97)); + const tools = buildCodingTools(root); + const invoke = async (name: string, params: object, signal?: AbortSignal) => { + const result = await tools + .find((tool: { name: string }) => tool.name === name) + .execute("benchmark", params, signal); + return result.content[0].text as string; + }; + bytesRead = 0; + const readText = await invoke("read_file", { path: "large.txt" }); + const read = { inputBytes: 16 * 1024 * 1024, bytesRead, outputChars: readText.length }; + await fs.rm(full); + await fs.writeFile(path.join(root, "tiny.txt"), "needle\n"); + const controller = new AbortController(); + controller.abort(new Error("benchmark cancelled")); + let rejected = false; + bytesRead = 0; + try { + await invoke("grep", { pattern: "needle" }, controller.signal); + } catch { + rejected = true; + } + const abortedGrep = { rejected, bytesRead }; + const timings: number[] = []; + for (let i = 0; i < 10; i++) { + const start = performance.now(); + if ((await invoke("grep", { pattern: "needle" })) !== "tiny.txt:1: needle") + throw new Error("Incorrect normal result"); + timings.push(performance.now() - start); + } + await fs.rm(path.join(root, "tiny.txt")); + for (let offset = 0; offset < 10_100; offset += 100) { + await Promise.all( + Array.from({ length: 100 }, (_, index) => + fs.writeFile(path.join(root, `file-${offset + index}.txt`), ""), + ), + ); + } + const scans = []; + for (const [name, params] of [ + ["list_dir", {}], + ["glob", { pattern: "**/*.absent" }], + ["grep", { pattern: "absent" }], + ] as const) { + enumeratedEntries = 0; + const text = await invoke(name, params); + scans.push({ + name, + enumeratedEntries, + outputChars: text.length, + incompleteNotice: text.includes("scan stopped"), + }); + } + console.log( + JSON.stringify( + { + modulePath, + node: process.version, + platform: process.platform, + arch: process.arch, + piVersion: JSON.parse( + await originalReadFile( + new URL("../node_modules/@earendil-works/pi-ai/package.json", import.meta.url), + "utf8", + ), + ).version, + read, + abortedGrep, + smallGrepMs: timings, + wideFixtureEntries: 10_100, + scans, + }, + null, + 2, + ), + ); +} finally { + fs.readFile = originalReadFile; + fs.open = originalOpen; + fs.readdir = originalReaddir; + fs.opendir = originalOpendir; + syncBuiltinESMExports(); + await fs.rm(root, { recursive: true, force: true }); +} diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts new file mode 100644 index 000000000..8733f610b --- /dev/null +++ b/scripts/smoke-foreground-file-tools.ts @@ -0,0 +1,93 @@ +/** Bundle with esbuild and run using the pinned Electron executable (see evidence doc). */ +import { app } from "electron"; +import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { buildCodingTools } from "../main/services/coding-tools.js"; + +void app.whenReady().then(async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-electron-file-tools-")); + try { + const tools = buildCodingTools(root); + const invoke = async (name: string, params: object, signal?: AbortSignal) => { + const result = await tools + .find((tool) => tool.name === name)! + .execute("electron-smoke", params, signal); + const block = result.content[0]; + return block?.type === "text" ? block.text : ""; + }; + await fs.mkdir(path.join(root, "src")); + await fs.writeFile(path.join(root, "src/a.ts"), "foobar foofoo\n"); + await fs.symlink("src", path.join(root, "link")); + const globs = [ + "*", + "**", + "**/*", + "**/*.ts", + "src/**", + "src", + "src/*.{ts,js}", + "link/*.ts", + "*/a.ts", + ]; + for (const pattern of globs) { + const expected: string[] = []; + for await (const entry of fs.glob(pattern, { cwd: root })) expected.push(entry); + assert.equal( + await invoke("glob", { pattern }), + expected.sort().join("\n") || "[no matches]", + pattern, + ); + } + assert.equal( + await invoke("grep", { pattern: "(?<=foo)bar|\\b(foo)\\1\\b" }), + "src/a.ts:1: foobar foofoo", + ); + for (let i = 0; i < 6; i++) + await assert.rejects(invoke("grep", { pattern: "[" }), /Invalid regular expression/); + await fs.writeFile(path.join(root, "src/a.ts"), "a".repeat(100_000) + "!"); + const cancellationMs: number[] = []; + for (let i = 0; i < 6; i++) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(new Error("electron cancelled")), 150); + const start = performance.now(); + try { + await assert.rejects( + invoke("grep", { pattern: "^(a+)+$" }, controller.signal), + /electron cancelled/, + ); + } finally { + clearTimeout(timer); + } + cancellationMs.push(performance.now() - start); + } + assert.match(await invoke("grep", { pattern: "^(a+)+$" }), /search stopped after 5000 ms/); + assert.equal(await invoke("grep", { pattern: "missing" }), "[no matches]"); + const resources = process + .getActiveResourcesInfo() + .filter((resource) => resource === "MessagePort"); + assert.deepEqual(resources, []); + console.log( + JSON.stringify( + { + electron: process.versions.electron, + node: process.version, + globs, + cancellationMs, + deadline: "settled with incomplete notice", + workerPortsAfter: resources.length, + result: "passed", + }, + null, + 2, + ), + ); + } catch (error) { + console.error(error); + process.exitCode = 1; + } finally { + await fs.rm(root, { recursive: true, force: true }); + app.exit(process.exitCode ?? 0); + } +}); From b37b4b269c967bbff6647dbef6213654ce97a96b Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 15:20:49 -0400 Subject: [PATCH 03/11] fix(tools): preserve native glob segment and symlink traversal semantics --- .memory/foreground-file-tool-bounds.md | 11 +- THIRD_PARTY_NOTICES.md | 27 +++ .../README.md | 12 +- .../after.json | 20 +- .../before.json | 20 +- .../electron.json | 17 +- main/services/coding-tool-glob-worker.ts | 114 ++++++++++ main/services/coding-tool-matcher.ts | 97 +++++--- main/services/coding-tools.test.ts | 28 ++- main/services/coding-tools.ts | 98 ++++---- package-lock.json | 215 ++++++++---------- package.json | 1 + scripts/smoke-foreground-file-tools.ts | 6 +- 13 files changed, 437 insertions(+), 229 deletions(-) create mode 100644 main/services/coding-tool-glob-worker.ts diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index ba4ae49f6..94c397b50 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -21,8 +21,10 @@ file and 10 MiB total (including probes), and skips a growing oversized file. JavaScript RegExp remains native JS, including lookbehind and backreferences, inside a fixed-source owned worker. Model patterns are workerData, never code. -Glob uses Node matchesGlob with native-compatible directory and explicit linked -prefix handling. Native-glob differential fixtures cover ordinary patterns, +Glob parses bounded patterns with pinned minimatch 9.0.9 using Node fs.glob +options, then tracks glob segment positions and linked traversal states in the +worker. Host-side filesystem access remains bounded and confined. Traversal +rules are adapted from Node.js (MIT notice included). Native-glob differential fixtures cover ordinary patterns, braces, extglobs, hidden paths, absolute paths, directory roots and symlinks. Patterns have a new explicit 1,000-character ceiling. At most four matchers can run concurrently; excess searches report busy. Cancellation, errors and deadlines @@ -33,4 +35,7 @@ No Remote DTO, transcript/activity UI, native implementation or onboarding capability changed. iOS/Android consumers were inspected: they use unchanged tool names/labels, not filesystem scanning/matching internals. No plan status changed. Evidence and repeatable commands: docs/performance/foreground-file-tools-2026-09-28/. -Independent Astra review and hosted exact-head CI/bot review are delivery gates. +First independent Astra review found safe-link/brace and glob-dependent .. +compatibility gaps in the initial flattened-path matcher. Replaced that approach +with segment-state traversal and added differential fixtures before publication. +Re-review and hosted exact-head CI/bot review remain delivery gates. diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index 5b44b0d38..e3777a874 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -393,3 +393,30 @@ FluidAudio source: https://github.com/FluidInference/FluidAudio/tree/87a39dfe406 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + +## Node.js glob traversal rules + +The segment traversal rules in `main/services/coding-tool-glob-worker.ts` are +adapted from Node.js `lib/internal/fs/glob.js` (Node 22.22.3), with host-owned +bounded filesystem access. Node.js is licensed under the MIT license: + +Copyright Node.js contributors. All rights reserved. +Copyright Joyent, Inc. and other Node contributors. All rights reserved. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to +deal in the Software without restriction, including without limitation the +rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +sell copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +IN THE SOFTWARE. diff --git a/docs/performance/foreground-file-tools-2026-09-28/README.md b/docs/performance/foreground-file-tools-2026-09-28/README.md index f2a7abc45..9cec3bc4b 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/README.md +++ b/docs/performance/foreground-file-tools-2026-09-28/README.md @@ -35,9 +35,13 @@ thread. The worker is not retained between calls. Parent hidden-file/credential policy remains distinct from the stricter child policy. Read_file still supports safe symlinks and metadata, excludes .env secrets, and rejects outside-root targets. Grep retains hidden/symlink/dependency ignores. -Glob retains Node matching semantics for the tested normal patterns and explicit -in-root linked prefixes, while now limiting traversal. Differential tests include -absolute paths, braces/extglobs, `**`, directory roots and linked prefixes. +Glob uses pinned minimatch 9.0.9 with Node fs.glob parser options. It tracks +segment positions and link traversal states without prematurely normalizing +`**/..`. Filesystem access stays on the bounded host. The first independent review +found gaps in a flattened-path matcher; the replacement has differential tests +for absolute paths, braces/extglobs, globstars, linked prefixes, linked wildcard +paths, and glob-dependent parent segments. Node traversal attribution is in +THIRD_PARTY_NOTICES.md. Pinned Electron 43.1.1 / Node 24.18.0 smoke uses a bundled entry and real app main process. It verifies native glob compatibility, JS lookbehind/backreferences, @@ -60,7 +64,7 @@ Run behavioral acceptance and the Electron smoke: ```sh npx tsx --test main/services/coding-tools.test.ts main/services/generation-runtime.test.ts npm run type-check -npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts +npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tool-glob-worker.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts node node_modules/electron/install.js npx esbuild scripts/smoke-foreground-file-tools.ts --bundle --platform=node --format=esm --packages=external --external:electron --outfile=build/main/foreground-file-tools-smoke.mjs node_modules/electron/dist/Electron.app/Contents/MacOS/Electron build/main/foreground-file-tools-smoke.mjs diff --git a/docs/performance/foreground-file-tools-2026-09-28/after.json b/docs/performance/foreground-file-tools-2026-09-28/after.json index 4e7a5c746..8cd0b856c 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/after.json +++ b/docs/performance/foreground-file-tools-2026-09-28/after.json @@ -14,16 +14,16 @@ "bytesRead": 0 }, "smallGrepMs": [ - 20.772999999999996, - 17.98208299999999, - 17.47387500000002, - 17.484375, - 17.382083000000023, - 17.51716700000003, - 16.555041000000017, - 16.932082999999977, - 17.844750000000033, - 18.124041999999974 + 18.926792000000034, + 17.84620799999999, + 17.246708999999953, + 17.875292, + 17.257499999999993, + 17.86224999999996, + 17.177625000000035, + 17.380792000000042, + 17.254166999999995, + 17.109041999999988 ], "wideFixtureEntries": 10100, "scans": [ diff --git a/docs/performance/foreground-file-tools-2026-09-28/before.json b/docs/performance/foreground-file-tools-2026-09-28/before.json index 7027e6280..db026d9ee 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/before.json +++ b/docs/performance/foreground-file-tools-2026-09-28/before.json @@ -14,16 +14,16 @@ "bytesRead": 7 }, "smallGrepMs": [ - 0.2409579999999778, - 0.23141700000002174, - 0.4002079999999637, - 0.4630830000000401, - 0.4339589999999589, - 0.3725839999999607, - 0.3684170000000222, - 0.40104199999996126, - 0.31462499999997817, - 0.233333000000016 + 0.3235419999999749, + 0.2934579999999869, + 0.29383300000000645, + 0.3069160000000011, + 0.5061660000000074, + 0.239750000000015, + 0.26924999999999955, + 0.3658750000000168, + 0.192875000000015, + 0.17037499999997863 ], "wideFixtureEntries": 10100, "scans": [ diff --git a/docs/performance/foreground-file-tools-2026-09-28/electron.json b/docs/performance/foreground-file-tools-2026-09-28/electron.json index 3fff7f668..8081d49ed 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/electron.json +++ b/docs/performance/foreground-file-tools-2026-09-28/electron.json @@ -10,15 +10,18 @@ "src", "src/*.{ts,js}", "link/*.ts", - "*/a.ts" + "*/a.ts", + "**/*/*.ts", + "{src,link}/**/*", + "src/**/.." ], "cancellationMs": [ - 150.44795899999997, - 152.30762500000003, - 151.294625, - 153.20070899999996, - 152.88079199999993, - 152.90762500000005 + 152.4152499999999, + 153.43987500000003, + 152.18025000000011, + 152.60799999999995, + 153.09254099999998, + 152.68233299999997 ], "deadline": "settled with incomplete notice", "workerPortsAfter": 0, diff --git a/main/services/coding-tool-glob-worker.ts b/main/services/coding-tool-glob-worker.ts new file mode 100644 index 000000000..c06cd9ab4 --- /dev/null +++ b/main/services/coding-tool-glob-worker.ts @@ -0,0 +1,114 @@ +/** + * Traversal rules adapted from Node.js internal/fs/glob.js (MIT); see + * THIRD_PARTY_NOTICES.md. Fixed worker code using Minimatch's parsed segments with Node fs.glob options. + * Traversal tracks segment positions (including positions reached through links) + * rather than matching a flattened pathname. Filesystem access stays on the host. + */ +export const CODING_GLOB_WORKER_SOURCE = ` +const { join, isAbsolute } = require('node:path'); +const { Minimatch, GLOBSTAR } = require(workerData.minimatchPath); +const matcher = new Minimatch(workerData.pattern, { + nocase: process.platform === 'win32' || process.platform === 'darwin', + windowsPathsNoEscape: true, nonegate: true, nocomment: true, + optimizationLevel: 2, platform: process.platform, nocaseMagicOnly: true, +}); +if (matcher.set.length > 1000) throw new Error('Glob expands to too many alternatives.'); +const seen = new Set(); +const seeds = matcher.set.map((parts, pattern) => { + let current = isAbsolute(workerData.pattern) ? '/' : '.'; + let index = 0; + // Resolve literal prefixes only. A globstar followed by .. is never collapsed. + while (index < parts.length - 1 && typeof parts[index] === 'string') { + current = join(current, parts[index++]); + } + if (index === 0 && parts[0] === '.') index++; + return { path: current, pattern, indexes: [index], symlinks: [] }; +}); +function globStep({ task, entries, directory }) { + const parts = matcher.set[task.pattern]; + const last = parts.length - 1; + const indexes = task.indexes.filter(index => index <= last); + const links = new Set(task.symlinks); + const isDirectory = directory && indexes.some(index => !links.has(index)); + const isLast = indexes.includes(last) || + (parts[last] === '' && isDirectory && parts[last - 1] === GLOBSTAR && indexes.includes(last - 1)); + const matches = []; + const tasks = []; + const add = (path, positions, symlinks = []) => { + if (positions.length) tasks.push({ path, pattern: task.pattern, indexes: positions, symlinks }); + }; + const test = (index, name) => { + const token = parts[index]; + return token === GLOBSTAR || (typeof token === 'string' ? token === name : !!token?.test(name)); + }; + if (!entries) { + const fresh = indexes.filter(index => !seen.has(JSON.stringify([task.pattern, task.path, index]))); + if (!fresh.length) return { matches, tasks, done: true }; + for (const index of fresh) seen.add(JSON.stringify([task.pattern, task.path, index])); + if (isLast && typeof parts[last] === 'string') { + if (parts[last] || isDirectory) matches.push(join(task.path, parts[last])); + if (indexes.length === 1 && indexes[0] === last) return { matches, tasks, done: true }; + } else if (isLast && parts[last] === GLOBSTAR && + (task.path !== '.' || parts[0] === '.' || last === 0)) matches.push(task.path); + if (!isDirectory) return { matches, tasks, done: true }; + const token = indexes.length === 1 ? parts[indexes[0]] : null; + return { matches, tasks, literal: typeof token === 'string' ? token : undefined }; + } + for (const entry of entries) { + const child = join(task.path, entry.name); + const positions = new Set(); + const symlinks = new Set(); + for (const index of indexes) { + const token = parts[index]; + const next = index + 1; + if (token === GLOBSTAR) { + let afterStars = next; + while (parts[afterStars] === GLOBSTAR) afterStars++; + if (entry.name.startsWith('.') && !test(afterStars, entry.name)) continue; + const viaLink = links.has(index); + if (!viaLink && entry.directory) positions.add(index); + else if (!viaLink && index === last) matches.push(child); + const nextMatches = test(next, entry.name); + if (nextMatches && next === last && !isLast) matches.push(child); + else if (nextMatches && entry.directory) positions.add(index + 2); + if ((nextMatches || parts[0] === '.') && (entry.directory || entry.link) && !viaLink) positions.add(next); + if (entry.link) symlinks.add(index); + if (parts[next] === '..' && entry.directory) { + if (next === last) matches.push(task.path, join(task.path, '..')); + else { add(task.path, [next + 1]); add(join(task.path, '..'), [next + 1]); } + } + } else if (typeof token === 'string') { + if (test(index, entry.name) && index !== last) positions.add(next); + else if (token === '.' && test(next, entry.name)) { + if (next === last) matches.push(child); else positions.add(next + 1); + } + } else if (test(index, entry.name)) { + if (index === last) matches.push(child); + else if (entry.directory) positions.add(next); + } + } + add(child, [...positions], [...symlinks]); + } + return { matches, tasks }; +} +`; + +export interface CodingGlobTask { + path: string; + pattern: number; + indexes: number[]; + symlinks: number[]; +} + +export interface CodingGlobRequest { + task: CodingGlobTask; + directory: boolean; + entries?: { name: string; directory: boolean; link: boolean }[]; +} + +export interface CodingGlobReply { + matches: string[]; + tasks: CodingGlobTask[]; + done?: boolean; + literal?: string; +} diff --git a/main/services/coding-tool-matcher.ts b/main/services/coding-tool-matcher.ts index cfb286e47..715f57dcd 100644 --- a/main/services/coding-tool-matcher.ts +++ b/main/services/coding-tool-matcher.ts @@ -1,40 +1,84 @@ import { Worker } from "node:worker_threads"; +import { createRequire } from "node:module"; +import { + CODING_GLOB_WORKER_SOURCE, + type CodingGlobRequest, + type CodingGlobReply, + type CodingGlobTask, +} from "./coding-tool-glob-worker.js"; // Fixed code, with all model input passed as data. Keeping this self-contained // also works in the packaged Electron main bundle without a worker asset loader. -const SOURCE = ` +const SOURCE = (kind: "grep" | "glob") => ` const { parentPort, workerData } = require('node:worker_threads'); -const { matchesGlob } = require('node:path'); try { - const regex = workerData.kind === 'grep' ? new RegExp(workerData.pattern) : null; - parentPort.on('message', ({ values, limit }) => { - const indices = []; - for (let i = 0; i < values.length && indices.length < limit; i++) { - if (regex ? regex.test(values[i]) : matchesGlob(values[i], workerData.pattern)) indices.push(i); - } - parentPort.postMessage({ indices }); +${kind === "glob" ? CODING_GLOB_WORKER_SOURCE : "const regex = new RegExp(workerData.pattern);"} + parentPort.on('message', request => { + try { + ${ + kind === "glob" + ? "parentPort.postMessage({ result: globStep(request) });" + : ` + const indices = []; + for (let i = 0; i < request.values.length && indices.length < request.limit; i++) { + if (regex.test(request.values[i])) indices.push(i); + } + parentPort.postMessage({ result: indices });` + } + } catch(error) { parentPort.postMessage({ error: error.message }); } }); - parentPort.postMessage({ indices: [] }); + parentPort.postMessage({ result: ${kind === "glob" ? "seeds" : "[]"} }); } catch (error) { parentPort.postMessage({ error: error.message }); } `; +export function withCodingToolMatcher( + kind: "grep", + pattern: string, + deadline: number, + signal: AbortSignal | undefined, + run: (match: (values: string[], limit: number) => Promise) => Promise, +): Promise { + return withWorker(kind, pattern, deadline, signal, (send) => + run((values, limit) => send({ values, limit }) as Promise), + ); +} + +export function withCodingToolGlob( + pattern: string, + deadline: number, + signal: AbortSignal | undefined, + run: ( + seeds: CodingGlobTask[], + step: (request: CodingGlobRequest) => Promise, + ) => Promise, +): Promise { + return withWorker("glob", pattern, deadline, signal, (send, ready) => + run(ready as CodingGlobTask[], (request) => send(request) as Promise), + ); +} + export class CodingToolMatchTimeout extends Error {} let activeMatchers = 0; /** One sequential matcher per search; terminated before its capacity is released. */ -export async function withCodingToolMatcher( +async function withWorker( kind: "grep" | "glob", pattern: string, deadline: number, signal: AbortSignal | undefined, - run: (match: (values: string[], limit: number) => Promise) => Promise, + run: (send: (request?: unknown) => Promise, ready: unknown) => Promise, ): Promise { signal?.throwIfAborted(); if (activeMatchers >= 4) throw new Error("File searches are busy; try again shortly."); - const worker = new Worker(SOURCE, { + const worker = new Worker(SOURCE(kind), { eval: true, execArgv: [], - workerData: { kind, pattern }, + workerData: { + kind, + pattern, + minimatchPath: + kind === "glob" ? createRequire(import.meta.url).resolve("minimatch") : undefined, + }, resourceLimits: { maxOldGenerationSizeMb: 32 }, }); activeMatchers++; @@ -46,44 +90,41 @@ export async function withCodingToolMatcher( }; worker.on("error", failed); worker.on("exit", () => failed(new Error("File search matcher exited."))); - const receive = (values?: string[], limit = 0) => - new Promise((resolve, reject) => { + const receive = (request?: unknown) => + new Promise((resolve, reject) => { if (failure) return reject(failure); if (signal?.aborted) return reject(signal.reason ?? new Error("File search cancelled.")); const remaining = deadline - Date.now(); if (remaining <= 0) return reject(new CodingToolMatchTimeout()); - const finish = (error?: Error, indices?: number[]) => { + const finish = (error?: Error, result?: unknown) => { clearTimeout(timer); worker.off("message", message); signal?.removeEventListener("abort", abort); rejectPending = undefined; if (error) reject(error); - else resolve(indices!); + else resolve(result); }; const abort = () => finish(signal?.reason ?? new Error("File search cancelled.")); - const message = (reply: { error?: string; indices: number[] }) => + const message = (reply: { error?: string; result: unknown }) => finish( reply.error ? new Error( `Invalid ${kind === "grep" ? "regular expression" : "glob"}: ${reply.error}`, ) : undefined, - reply.indices, + reply.result, ); const timer = setTimeout(() => finish(new CodingToolMatchTimeout()), remaining); rejectPending = (error) => finish(error); worker.once("message", message); signal?.addEventListener("abort", abort, { once: true }); - if (values) worker.postMessage({ values, limit }); + if (request) worker.postMessage(request); }); try { - await receive(); - return await run(receive); + const ready = await receive(); + return await run(receive, ready); } finally { - try { - await worker.terminate(); - } finally { - activeMatchers--; - } + await worker.terminate(); + activeMatchers--; } } diff --git a/main/services/coding-tools.test.ts b/main/services/coding-tools.test.ts index 749e58fea..b012201db 100644 --- a/main/services/coding-tools.test.ts +++ b/main/services/coding-tools.test.ts @@ -12,6 +12,7 @@ import { runCommandEnv, summarizeToolCall, } from "./coding-tools.js"; +import { withCodingToolMatcher } from "./coding-tool-matcher.js"; import { createShareImageTool } from "./share-image-tool.js"; import { agentCommandEnvironment } from "./agent-command-environment.js"; @@ -2219,10 +2220,11 @@ test("foreground grep retains JS patterns and stays cancellable during catastrop test("foreground glob agrees with native glob on normal patterns, hidden paths and safe links", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-glob-")); try { - await fs.mkdir(path.join(root, "src")); await fs.mkdir(path.join(root, ".meta")); - for (const file of ["src/a.ts", "src/b.js", "root.txt", ".meta/config.json"]) await fs.writeFile(path.join(root, file), "safe"); + await fs.mkdir(path.join(root, "src/sub"), { recursive: true }); await fs.mkdir(path.join(root, ".meta")); + for (const file of ["src/a.ts", "src/b.js", "src/sub/c.ts", "root.txt", ".meta/config.json"]) await fs.writeFile(path.join(root, file), "safe"); await fs.symlink("src", path.join(root, "link")); - for (const pattern of ["*", "**", "**/*", "src/**", "src/*/", "src", "src/*.{js,ts}", "src/@(a|b).*", "./src/*.ts", ".meta/*", "link/*.ts", "*/a.ts", path.join(root, "src/*.ts")]) { + await fs.symlink("sub", path.join(root, "src/alias")); + for (const pattern of ["*", "**", "**/*", "src/**", "src/*/", "src", "src/*.{js,ts}", "src/@(a|b).*", "./src/*.ts", ".meta/*", "link/*.ts", "*/a.ts", "**/*/*.ts", "**/*/**/c.ts", "**/*/", "src/**/..", "{src,link}/**/*", "src/{sub,alias}/*", path.join(root, "src/*.ts")]) { const expected = (await (async () => { const values: string[] = []; for await (const value of fs.glob(pattern, { cwd: root })) values.push(value); return values; })()).sort().join("\n") || "[no matches]"; assert.equal(await foregroundText(root, "glob", { pattern }), expected, pattern); } @@ -2320,3 +2322,23 @@ test("foreground reads reject non-regular FIFOs without waiting for a writer", { await assert.rejects(foregroundText(root, "read_file", { path: "pipe" }), /not a regular file/); } finally { await fs.rm(root, { recursive: true, force: true }); } }); + + +test("foreground matcher capacity rejects excess work and recovers after concurrent cancellations", async () => { + const controllers = Array.from({ length: 4 }, () => new AbortController()); + const ready = controllers.map(() => (() => { let resolve!: () => void; const promise = new Promise((done) => { resolve = done; }); return { resolve, promise }; })()); + const calls = controllers.map((controller, index) => withCodingToolMatcher( + "grep", "^(a+)+$", Date.now() + 5_000, controller.signal, + async (match) => { ready[index]!.resolve(); return await match(["a".repeat(100_000) + "!"], 1); }, + )); + const settled = Promise.allSettled(calls); + try { + await Promise.all(ready.map((entry) => entry.promise)); + await assert.rejects(withCodingToolMatcher("grep", "safe", Date.now() + 5_000, undefined, async (match) => match(["safe"], 1)), /searches are busy/); + } finally { + for (const controller of controllers) controller.abort(new Error("capacity test cancelled")); + } + const outcomes = await settled; + assert.ok(outcomes.every((outcome) => outcome.status === "rejected" && outcome.reason.message === "capacity test cancelled")); + assert.deepEqual(await withCodingToolMatcher("grep", "safe", Date.now() + 5_000, undefined, async (match) => match(["other", "safe"], 1)), [1]); +}); diff --git a/main/services/coding-tools.ts b/main/services/coding-tools.ts index 6955f9903..9fe9ca8b6 100644 --- a/main/services/coding-tools.ts +++ b/main/services/coding-tools.ts @@ -7,7 +7,7 @@ import { spawn } from "node:child_process"; import { StringDecoder } from "node:string_decoder"; -import { CodingToolMatchTimeout, withCodingToolMatcher } from "./coding-tool-matcher.js"; +import { CodingToolMatchTimeout, withCodingToolGlob, withCodingToolMatcher } from "./coding-tool-matcher.js"; import { agentCommandEnvironment } from "./agent-command-environment.js"; import { constants as fsConstants, @@ -1397,61 +1397,78 @@ function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { throwIfAborted(signal, "File search cancelled."); validateParentPattern(pattern); const budget = parentScanBudget(); - const matches: string[] = []; + const matches = new Set(); const root = await verifyWorkspaceRoot(workspace, signal); - const normalized = path.isAbsolute(pattern) ? path.relative(workspace.lexical, pattern) : path.normalize(pattern); - const segments = normalized.split(path.sep); - const magicIndex = segments.findIndex((segment) => /[*?[\]{}()!+@]/.test(segment)); - const base = magicIndex < 0 ? path.dirname(normalized) : segments.slice(0, magicIndex).join(path.sep) || "."; - const pending = [{ relative: base, shallow: false }]; - const displayPath = (candidate: string) => path.isAbsolute(pattern) ? path.resolve(workspace.lexical, candidate) : candidate; try { - await withCodingToolMatcher("glob", normalized, budget.deadline, signal, async (match) => { - if (magicIndex >= 0 && (await match([base === "." ? "" : `${base}${path.sep}`], 1)).length) { - const { full } = await resolveExistingInRoot(workspace, base, signal); - if (!isEnvironmentSecretPath(path.relative(root, full))) matches.push(displayPath(base)); - } + await withCodingToolGlob(pattern, budget.deadline, signal, async (seeds, step) => { + const pending = [...seeds].reverse(); + const acceptMatches = async (candidates: string[]) => { + for (const candidate of candidates) { + if (parentScanStopped(budget, signal)) break; + if (matches.has(candidate)) continue; + try { + const canonical = assertRealPathInRoot(root, await fs.realpath(resolveInRoot(workspace.lexical, candidate)), candidate); + if (isEnvironmentSecretPath(candidate) || isEnvironmentSecretPath(path.relative(root, canonical))) continue; + } catch { + throwIfAborted(signal, "File search cancelled."); + continue; + } + if (matches.size === MAX_GLOB_MATCHES) { + budget.warnings.add(`… [truncated at ${MAX_GLOB_MATCHES} matches]`); + budget.stopped = true; + break; + } + matches.add(candidate); + } + }; while (pending.length && !parentScanStopped(budget, signal)) { - const { relative, shallow } = pending.pop()!; - let entries: Dirent[]; + const task = pending.pop()!; + let full: string; + let directory: boolean; try { - entries = await parentDirectoryEntries(workspace, resolveInRoot(workspace.lexical, relative), budget, signal); + const resolved = await resolveExistingInRoot(workspace, task.path, signal); + full = resolved.full; + if (isEnvironmentSecretPath(task.path) || isEnvironmentSecretPath(path.relative(root, full))) continue; + directory = (await fs.stat(full)).isDirectory(); } catch { throwIfAborted(signal, "File search cancelled."); await verifyWorkspaceRoot(workspace, signal); - budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); continue; } - const candidates = entries.map((entry) => path.join(relative, entry.name)); - const matchInputs = candidates.flatMap((candidate, index) => [candidate, entries[index]!.isDirectory() ? `${candidate}${path.sep}` : candidate]); - const indices = new Set((await match(matchInputs, matchInputs.length)).map((index) => Math.floor(index / 2))); - for (let index = 0; index < entries.length; index++) { - if (parentScanStopped(budget, signal)) break; - const entry = entries[index]!; - const candidate = candidates[index]!; - if (isEnvironmentSecretPath(candidate)) continue; - // Native globstar does not recurse through symlinks. Explicit linked - // prefixes are resolved above and remain confined to the workspace. - if (magicIndex >= 0 && (normalized.includes("**") || candidate.split(path.sep).length < segments.length) && entry.isDirectory() && !shallow) pending.push({ relative: candidate, shallow: false }); - if (!indices.has(index)) continue; - // Node glob allows one linked-directory level for a terminal **/*. - if (!shallow && entry.isSymbolicLink() && normalized.endsWith(`**${path.sep}*`)) { - pending.push({ relative: candidate, shallow: true }); + const prepared = await step({ task, directory }); + await acceptMatches(prepared.matches); + if (prepared.done || parentScanStopped(budget, signal)) continue; + let entries: { name: string; directory: boolean; link: boolean }[]; + if (prepared.literal !== undefined) { + if (budget.entries >= MAX_GLOB_ENTRIES) { + budget.stopped = true; + budget.warnings.add(`… [scan stopped after ${MAX_GLOB_ENTRIES} entries]`); + break; } + budget.entries++; try { - const canonical = assertRealPathInRoot(root, await fs.realpath(resolveInRoot(workspace.lexical, candidate)), candidate); - if (isEnvironmentSecretPath(path.relative(root, canonical))) continue; + const lexical = resolveInRoot(workspace.lexical, path.join(task.path, prepared.literal)); + await workspace.testObserver?.beforeEntryAccess?.(lexical); + const stat = await fs.lstat(lexical); + workspace.testObserver?.onDirectoryEntry?.(lexical); + entries = [{ name: prepared.literal, directory: stat.isDirectory(), link: stat.isSymbolicLink() }]; } catch { throwIfAborted(signal, "File search cancelled."); continue; } - if (matches.length === MAX_GLOB_MATCHES) { - budget.warnings.add(`… [truncated at ${MAX_GLOB_MATCHES} matches]`); - budget.stopped = true; - break; + } else { + try { + entries = (await parentDirectoryEntries(workspace, full, budget, signal)).map((entry) => ({ name: entry.name, directory: entry.isDirectory(), link: entry.isSymbolicLink() })); + } catch { + throwIfAborted(signal, "File search cancelled."); + await verifyWorkspaceRoot(workspace, signal); + budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); + continue; } - matches.push(displayPath(candidate)); } + const next = await step({ task, directory, entries }); + await acceptMatches(next.matches); + pending.push(...next.tasks.reverse()); } }); } catch (error) { @@ -1459,8 +1476,7 @@ function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { budget.warnings.add(`… [search stopped after ${MAX_GREP_DURATION_MS} ms]`); } await verifyWorkspaceRoot(workspace, signal); - matches.sort(); - return textResult(formatBoundedSearchResult(matches, "[no matches]", [...budget.warnings])); + return textResult(formatBoundedSearchResult([...matches].sort(), "[no matches]", [...budget.warnings])); }, }; } diff --git a/package-lock.json b/package-lock.json index 10a504601..21cce834c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32,6 +32,7 @@ "katex": "^0.16.47", "lucide-react": "^0.542.0", "mdast-util-to-string": "4.0.0", + "minimatch": "9.0.9", "node-pty": "^1.1.0", "parse5": "7.3.0", "plotly.js-dist-min": "^3.7.0", @@ -844,13 +845,6 @@ "node": ">=10.12.0" } }, - "node_modules/@electron/asar/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/@electron/asar/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -1025,23 +1019,6 @@ "node": ">=16.4" } }, - "node_modules/@electron/universal/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, - "node_modules/@electron/universal/node_modules/brace-expansion": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", - "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", - "dev": true, - "license": "MIT", - "dependencies": { - "balanced-match": "^1.0.0" - } - }, "node_modules/@electron/universal/node_modules/fs-extra": { "version": "11.3.6", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-11.3.6.tgz", @@ -1057,22 +1034,6 @@ "node": ">=14.14" } }, - "node_modules/@electron/universal/node_modules/minimatch": { - "version": "9.0.9", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.9.tgz", - "integrity": "sha512-OBwBN9AL4dqmETlpS2zasx+vTeWclWzkblfZk7KTA5j3jeOONz/tRCnZomUyvNg83wL5Zv9Ss6HMJXAgL8R2Yg==", - "dev": true, - "license": "ISC", - "dependencies": { - "brace-expansion": "^2.0.2" - }, - "engines": { - "node": ">=16 || 14 >=14.17" - }, - "funding": { - "url": "https://github.com/sponsors/isaacs" - } - }, "node_modules/@electron/windows-sign": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/@electron/windows-sign/-/windows-sign-1.2.2.tgz", @@ -1606,13 +1567,6 @@ "node": "^18.18.0 || ^20.9.0 || >=21.1.0" } }, - "node_modules/@eslint/config-array/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/@eslint/config-array/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -1704,13 +1658,6 @@ "url": "https://github.com/sponsors/epoberezkin" } }, - "node_modules/@eslint/eslintrc/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/@eslint/eslintrc/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -5174,6 +5121,45 @@ "typescript": ">=4.8.4 <6.1.0" } }, + "node_modules/@typescript-eslint/typescript-estree/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, + "node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": { + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "20 || >=22" + } + }, + "node_modules/@typescript-eslint/typescript-estree/node_modules/minimatch": { + "version": "10.2.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", + "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "brace-expansion": "^5.0.5" + }, + "engines": { + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/@typescript-eslint/utils": { "version": "8.64.0", "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.64.0.tgz", @@ -5544,6 +5530,29 @@ "semver": "bin/semver.js" } }, + "node_modules/app-builder-lib/node_modules/balanced-match": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", + "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "18 || 20 || >=22" + } + }, + "node_modules/app-builder-lib/node_modules/brace-expansion": { + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", + "dev": true, + "license": "MIT", + "dependencies": { + "balanced-match": "^4.0.2" + }, + "engines": { + "node": "20 || >=22" + } + }, "node_modules/app-builder-lib/node_modules/ci-info": { "version": "4.3.1", "resolved": "https://registry.npmjs.org/ci-info/-/ci-info-4.3.1.tgz", @@ -5590,6 +5599,22 @@ "graceful-fs": "^4.1.6" } }, + "node_modules/app-builder-lib/node_modules/minimatch": { + "version": "10.2.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", + "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", + "dev": true, + "license": "BlueOak-1.0.0", + "dependencies": { + "brace-expansion": "^5.0.5" + }, + "engines": { + "node": "18 || 20 || >=22" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/app-builder-lib/node_modules/semver": { "version": "7.7.4", "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz", @@ -5946,14 +5971,10 @@ } }, "node_modules/balanced-match": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", - "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", - "dev": true, - "license": "MIT", - "engines": { - "node": "18 || 20 || >=22" - } + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", + "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", + "license": "MIT" }, "node_modules/base64-js": { "version": "1.5.1", @@ -6068,16 +6089,12 @@ "license": "MIT" }, "node_modules/brace-expansion": { - "version": "5.0.9", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", - "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", - "dev": true, + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", + "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", "license": "MIT", "dependencies": { - "balanced-match": "^4.0.2" - }, - "engines": { - "node": "20 || >=22" + "balanced-match": "^1.0.0" } }, "node_modules/browserslist": { @@ -7000,13 +7017,6 @@ "p-limit": "^3.1.0 " } }, - "node_modules/dir-compare/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/dir-compare/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -7905,13 +7915,6 @@ "eslint": "^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8 || ^9" } }, - "node_modules/eslint-plugin-import/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/eslint-plugin-import/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -8003,13 +8006,6 @@ "url": "https://github.com/sponsors/epoberezkin" } }, - "node_modules/eslint/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/eslint/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -8368,23 +8364,6 @@ "minimatch": "^5.0.1" } }, - "node_modules/filelist/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, - "node_modules/filelist/node_modules/brace-expansion": { - "version": "2.1.4", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", - "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", - "dev": true, - "license": "MIT", - "dependencies": { - "balanced-match": "^1.0.0" - } - }, "node_modules/filelist/node_modules/minimatch": { "version": "5.1.9", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.1.9.tgz", @@ -8829,13 +8808,6 @@ "node": ">=10.13.0" } }, - "node_modules/glob/node_modules/balanced-match": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", - "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", - "dev": true, - "license": "MIT" - }, "node_modules/glob/node_modules/brace-expansion": { "version": "1.1.18", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz", @@ -11594,16 +11566,15 @@ } }, "node_modules/minimatch": { - "version": "10.2.5", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", - "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", - "dev": true, - "license": "BlueOak-1.0.0", + "version": "9.0.9", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.9.tgz", + "integrity": "sha512-OBwBN9AL4dqmETlpS2zasx+vTeWclWzkblfZk7KTA5j3jeOONz/tRCnZomUyvNg83wL5Zv9Ss6HMJXAgL8R2Yg==", + "license": "ISC", "dependencies": { - "brace-expansion": "^5.0.5" + "brace-expansion": "^2.0.2" }, "engines": { - "node": "18 || 20 || >=22" + "node": ">=16 || 14 >=14.17" }, "funding": { "url": "https://github.com/sponsors/isaacs" diff --git a/package.json b/package.json index fe9c8a2ff..2fa00bf65 100644 --- a/package.json +++ b/package.json @@ -214,6 +214,7 @@ "katex": "^0.16.47", "lucide-react": "^0.542.0", "mdast-util-to-string": "4.0.0", + "minimatch": "9.0.9", "node-pty": "^1.1.0", "parse5": "7.3.0", "plotly.js-dist-min": "^3.7.0", diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts index 8733f610b..51862ab14 100644 --- a/scripts/smoke-foreground-file-tools.ts +++ b/scripts/smoke-foreground-file-tools.ts @@ -17,7 +17,8 @@ void app.whenReady().then(async () => { const block = result.content[0]; return block?.type === "text" ? block.text : ""; }; - await fs.mkdir(path.join(root, "src")); + await fs.mkdir(path.join(root, "src/sub"), { recursive: true }); + await fs.writeFile(path.join(root, "src/sub/b.ts"), ""); await fs.writeFile(path.join(root, "src/a.ts"), "foobar foofoo\n"); await fs.symlink("src", path.join(root, "link")); const globs = [ @@ -30,6 +31,9 @@ void app.whenReady().then(async () => { "src/*.{ts,js}", "link/*.ts", "*/a.ts", + "**/*/*.ts", + "{src,link}/**/*", + "src/**/..", ]; for (const pattern of globs) { const expected: string[] = []; From b1bc608d179a641d6fd24c3a02eb6c80634f78ac Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 15:52:11 -0400 Subject: [PATCH 04/11] fix(tools): own pending filesystem work through cancellation --- .memory/foreground-file-tool-bounds.md | 12 ++ .papercuts/troubleshooting.md | 12 ++ .../README.md | 19 +- .../electron.json | 15 +- main/services/coding-tool-glob-worker.ts | 9 +- main/services/coding-tool-matcher.ts | 16 +- main/services/coding-tools.test.ts | 202 +++++++++++++++++- main/services/coding-tools.ts | 55 ++++- main/services/foreground-read-scope.ts | 70 ++++++ scripts/smoke-foreground-file-tools.ts | 1 + 10 files changed, 385 insertions(+), 26 deletions(-) create mode 100644 main/services/foreground-read-scope.ts diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 94c397b50..7179dc465 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -39,3 +39,15 @@ First independent Astra review found safe-link/brace and glob-dependent .. compatibility gaps in the initial flattened-path matcher. Replaced that approach with segment-state traversal and added differential fixtures before publication. Re-review and hosted exact-head CI/bot review remain delivery gates. + + +PR #288 Pullfrog follow-up: derive each brace-expanded glob arm's root independently; +native differential tests cover mixed absolute/relative alternatives and reject an +outside-root arm before opening its directory. Foreground read/list/glob/grep now +share four operation owners. Caller abort and search deadline settle independently +of pending filesystem I/O; the original operation retains its slot through late +I/O and cleanup. An issued syscall itself is not cancellable. Late handles close +without further reads; failed cleanup quarantines admission. Root verification +awaits both started metadata requests even if one fails. Worker termination starts +on lifetime abort/deadline while its traversal callback may still be pending. +The stalled-I/O deadline result is an explicit notice without partial output. diff --git a/.papercuts/troubleshooting.md b/.papercuts/troubleshooting.md index 7a4f4fb6b..2c7d42ee7 100644 --- a/.papercuts/troubleshooting.md +++ b/.papercuts/troubleshooting.md @@ -1409,3 +1409,15 @@ because their native file-mutator test binary had not been built. Run - Node built-in ESM namespaces retain their bindings when a benchmark wraps the default fs/promises export. Call `syncBuiltinESMExports()` after instrumentation and restoration; otherwise byte counters misleadingly report zero. The clean 0.87.1 runner now reproduces the original 16 MiB read. - A standalone Electron ESM smoke must externalize `electron` explicitly (tsconfig path resolution otherwise bundles its npm launcher), and register `app.whenReady().then(...)` without top-level-awaiting readiness. Electron waits for module evaluation before ready; top-level await deadlocks the fixture. The resulting fixed-source worker is validated in pinned Electron 43.1.1 / Node 24.18.0. + + +### Foreground pending-I/O review follow-up (2026-09-28, PR #288) + +Pullfrog found mixed-root brace glob arms and cancellation waiting on pending +filesystem reads. Cover syscall ownership as well as matcher termination: retain +admission through original I/O/cleanup, and fence late handle acquisitions. Worker +message ports are absent from Node `_getActiveHandles()` after its one-shot ready +listener is removed, even for a live idle worker. The initial lifecycle test failed +`0 !== 4`; corrected the oracle to await the actual Worker.terminate promises while +traversal stays deferred, then independently assert held admission and recovery. +This was a deterministic test-oracle failure, not a passing rerun or timeout change. diff --git a/docs/performance/foreground-file-tools-2026-09-28/README.md b/docs/performance/foreground-file-tools-2026-09-28/README.md index 9cec3bc4b..5ae595eab 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/README.md +++ b/docs/performance/foreground-file-tools-2026-09-28/README.md @@ -29,9 +29,22 @@ deadline with an incomplete notice. A fixed-source worker receives model input a data and is terminated/awaited on every exit. Four concurrent workers are allowed; additional searches return a busy error. This adds roughly 17 ms per tiny grep on the quiet fixture run (see raw samples), versus sub-millisecond baseline calls. -It buys cancellable matching and removes regex execution from Electron's main +It provides cancellable matching and removes regex execution from Electron's main thread. The worker is not retained between calls. +A foreground operation scope covers pending filesystem I/O as well as matching. +Cancellation rejects the caller promptly; the five-second search deadline returns +an explicit incomplete notice even if a syscall is still pending. An issued +kernel syscall cannot be cancelled in JavaScript: its owner keeps one of four +process-wide admission slots until the original operation and descriptor cleanup +settle. Late acquisitions close without another read, late errors remain observed, +and a failed close quarantines capacity rather than admitting unbounded work. +Worker termination begins on cancellation/deadline even while traversal is pending. +Deferred-I/O tests cover acquisition/read cancellation across all four tools, +concurrent and repeated cancellation, blocked cleanup, and capacity recovery. +A deadline while I/O is pending returns only the notice, without partial results. + + Parent hidden-file/credential policy remains distinct from the stricter child policy. Read_file still supports safe symlinks and metadata, excludes .env secrets, and rejects outside-root targets. Grep retains hidden/symlink/dependency ignores. @@ -39,7 +52,7 @@ Glob uses pinned minimatch 9.0.9 with Node fs.glob parser options. It tracks segment positions and link traversal states without prematurely normalizing `**/..`. Filesystem access stays on the bounded host. The first independent review found gaps in a flattened-path matcher; the replacement has differential tests -for absolute paths, braces/extglobs, globstars, linked prefixes, linked wildcard +for absolute paths, mixed absolute/relative brace arms, braces/extglobs, globstars, linked prefixes, linked wildcard paths, and glob-dependent parent segments. Node traversal attribution is in THIRD_PARTY_NOTICES.md. @@ -64,7 +77,7 @@ Run behavioral acceptance and the Electron smoke: ```sh npx tsx --test main/services/coding-tools.test.ts main/services/generation-runtime.test.ts npm run type-check -npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tool-glob-worker.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts +npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tool-glob-worker.ts main/services/foreground-read-scope.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts node node_modules/electron/install.js npx esbuild scripts/smoke-foreground-file-tools.ts --bundle --platform=node --format=esm --packages=external --external:electron --outfile=build/main/foreground-file-tools-smoke.mjs node_modules/electron/dist/Electron.app/Contents/MacOS/Electron build/main/foreground-file-tools-smoke.mjs diff --git a/docs/performance/foreground-file-tools-2026-09-28/electron.json b/docs/performance/foreground-file-tools-2026-09-28/electron.json index 8081d49ed..faf24bd52 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/electron.json +++ b/docs/performance/foreground-file-tools-2026-09-28/electron.json @@ -13,15 +13,16 @@ "*/a.ts", "**/*/*.ts", "{src,link}/**/*", - "src/**/.." + "src/**/..", + "{/var/folders/bb/wd_m6tl14y5c1wklxz3sj9840000gn/T/aiden-electron-file-tools-dzVPQW/src/*.ts,src/sub/*.ts}" ], "cancellationMs": [ - 152.4152499999999, - 153.43987500000003, - 152.18025000000011, - 152.60799999999995, - 153.09254099999998, - 152.68233299999997 + 152.08937500000002, + 152.27324999999996, + 152.46399999999994, + 152.52579100000003, + 151.98649999999998, + 151.38937499999997 ], "deadline": "settled with incomplete notice", "workerPortsAfter": 0, diff --git a/main/services/coding-tool-glob-worker.ts b/main/services/coding-tool-glob-worker.ts index c06cd9ab4..132b8e57d 100644 --- a/main/services/coding-tool-glob-worker.ts +++ b/main/services/coding-tool-glob-worker.ts @@ -5,7 +5,7 @@ * rather than matching a flattened pathname. Filesystem access stays on the host. */ export const CODING_GLOB_WORKER_SOURCE = ` -const { join, isAbsolute } = require('node:path'); +const { join, isAbsolute, parse, sep } = require('node:path'); const { Minimatch, GLOBSTAR } = require(workerData.minimatchPath); const matcher = new Minimatch(workerData.pattern, { nocase: process.platform === 'win32' || process.platform === 'darwin', @@ -15,8 +15,11 @@ const matcher = new Minimatch(workerData.pattern, { if (matcher.set.length > 1000) throw new Error('Glob expands to too many alternatives.'); const seen = new Set(); const seeds = matcher.set.map((parts, pattern) => { - let current = isAbsolute(workerData.pattern) ? '/' : '.'; - let index = 0; + // Brace expansion may mix roots; never infer an arm's root from the whole pattern. + const expanded = matcher.globParts[pattern].join('/'); + const root = isAbsolute(expanded) ? parse(expanded).root : ''; + let current = root || '.'; + let index = root ? root.split(sep).length - 1 : 0; // Resolve literal prefixes only. A globstar followed by .. is never collapsed. while (index < parts.length - 1 && typeof parts[index] === 'string') { current = join(current, parts[index++]); diff --git a/main/services/coding-tool-matcher.ts b/main/services/coding-tool-matcher.ts index 715f57dcd..be168586f 100644 --- a/main/services/coding-tool-matcher.ts +++ b/main/services/coding-tool-matcher.ts @@ -1,4 +1,5 @@ import { Worker } from "node:worker_threads"; +import { closeForegroundResource } from "./foreground-read-scope.js"; import { createRequire } from "node:module"; import { CODING_GLOB_WORKER_SOURCE, @@ -88,6 +89,17 @@ async function withWorker( failure = error; rejectPending?.(error); }; + let termination: Promise | undefined; + const terminate = () => termination ??= worker.terminate(); + const abortWorker = () => { + failed(signal?.reason ?? new Error("File search cancelled.")); + void terminate().catch(failed); + }; + signal?.addEventListener("abort", abortWorker, { once: true }); + const lifetimeTimer = setTimeout(() => { + failed(new CodingToolMatchTimeout()); + void terminate().catch(failed); + }, Math.max(0, deadline - Date.now())); worker.on("error", failed); worker.on("exit", () => failed(new Error("File search matcher exited."))); const receive = (request?: unknown) => @@ -124,7 +136,9 @@ async function withWorker( const ready = await receive(); return await run(receive, ready); } finally { - await worker.terminate(); + clearTimeout(lifetimeTimer); + signal?.removeEventListener("abort", abortWorker); + await closeForegroundResource({ close: async () => { await terminate(); } }); activeMatchers--; } } diff --git a/main/services/coding-tools.test.ts b/main/services/coding-tools.test.ts index b012201db..66b5d4751 100644 --- a/main/services/coding-tools.test.ts +++ b/main/services/coding-tools.test.ts @@ -1,6 +1,9 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; -import * as fs from "node:fs/promises"; +import fsPromises, * as fs from "node:fs/promises"; +import { Worker } from "node:worker_threads"; +import { syncBuiltinESMExports } from "node:module"; +import { ForegroundReadOperations, ForegroundReadCleanupError, closeForegroundResource } from "./foreground-read-scope.js"; import os from "node:os"; import * as path from "node:path"; import { promisify } from "node:util"; @@ -2224,12 +2227,22 @@ test("foreground glob agrees with native glob on normal patterns, hidden paths a for (const file of ["src/a.ts", "src/b.js", "src/sub/c.ts", "root.txt", ".meta/config.json"]) await fs.writeFile(path.join(root, file), "safe"); await fs.symlink("src", path.join(root, "link")); await fs.symlink("sub", path.join(root, "src/alias")); - for (const pattern of ["*", "**", "**/*", "src/**", "src/*/", "src", "src/*.{js,ts}", "src/@(a|b).*", "./src/*.ts", ".meta/*", "link/*.ts", "*/a.ts", "**/*/*.ts", "**/*/**/c.ts", "**/*/", "src/**/..", "{src,link}/**/*", "src/{sub,alias}/*", path.join(root, "src/*.ts")]) { + for (const pattern of ["*", "**", "**/*", "src/**", "src/*/", "src", "src/*.{js,ts}", "src/@(a|b).*", "./src/*.ts", ".meta/*", "link/*.ts", "*/a.ts", "**/*/*.ts", "**/*/**/c.ts", "**/*/", "src/**/..", "{src,link}/**/*", "src/{sub,alias}/*", path.join(root, "src/*.ts"), `{${path.join(root, "src/*.ts")},src/*.js}`, `{src/*.js,${path.join(root, "src/*.ts")}}`, `{${path.join(root, "src")},${path.join(root, "link")}}/*.ts`]) { const expected = (await (async () => { const values: string[] = []; for await (const value of fs.glob(pattern, { cwd: root })) values.push(value); return values; })()).sort().join("\n") || "[no matches]"; assert.equal(await foregroundText(root, "glob", { pattern }), expected, pattern); } await fs.writeFile(path.join(root, ".env"), "SECRET"); assert.equal(await foregroundText(root, "glob", { pattern: ".env" }), "[no matches]"); + const outside = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-glob-outside-")); + try { + await fs.writeFile(path.join(outside, "outside.txt"), "PRIVATE"); + const opened: string[] = []; + assert.equal(await foregroundText(root, "glob", { pattern: `{${path.join(outside, "*.txt")},src/*.js}` }, undefined, { + beforeDirectoryOpen: (directory) => { opened.push(directory); }, + }), "src/b.js"); + assert.ok(opened.every((directory) => !directory.includes(path.basename(outside)))); + } finally { await fs.rm(outside, { recursive: true, force: true }); } + } finally { await fs.rm(root, { recursive: true, force: true }); } }); @@ -2342,3 +2355,188 @@ test("foreground matcher capacity rejects excess work and recovers after concurr assert.ok(outcomes.every((outcome) => outcome.status === "rejected" && outcome.reason.message === "capacity test cancelled")); assert.deepEqual(await withCodingToolMatcher("grep", "safe", Date.now() + 5_000, undefined, async (match) => match(["other", "safe"], 1)), [1]); }); + +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); + return { promise, resolve, reject }; +} + +test("foreground callers cancel pending reads and late acquisitions while handles retain an owner", { timeout: 15_000 }, async (t) => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-pending-")); + await fs.writeFile(path.join(root, "a.txt"), "needle"); + try { + for (const phase of ["acquire", "read"] as const) { + for (const [name, params] of [["read_file", { path: "a.txt" }], ["list_dir", {}], ["glob", { pattern: "*" }], ["grep", { pattern: "needle" }]] as const) { + const entered = deferred(); const release = deferred(); const closed = deferred(); + const controller = new AbortController(); + let reads = 0; let closes = 0; + const method = name === "read_file" ? "open" : "opendir"; + const original = fsPromises[method]; + // Delay the actual descriptor acquisition/read promise, not just a tool hook. + t.mock.method(fsPromises, method, async (...args: unknown[]) => { + const handle = await Reflect.apply(original, fsPromises, args); + const read = handle.read.bind(handle); const close = handle.close.bind(handle); + handle.read = async (...readArgs: unknown[]) => { + reads++; + if (phase === "read") { entered.resolve(); await release.promise; } + return Reflect.apply(read, handle, readArgs); + }; + handle.close = async () => { closes++; await close(); closed.resolve(); }; + if (phase === "acquire") { entered.resolve(); await release.promise; } + return handle; + }); + syncBuiltinESMExports(); + const call = foregroundText(root, name, params, controller.signal); + try { + await entered.promise; + controller.abort(new Error("pending I/O cancelled")); + await assert.rejects(call, /pending I\/O cancelled/); + assert.equal(closes, 0, "pending I/O still owns its handle"); + release.resolve(); + await closed.promise; + assert.equal(closes, 1); + assert.equal(reads, phase === "acquire" ? 0 : 1, "no subsequent reads after cancellation"); + } finally { + release.resolve(); + t.mock.restoreAll(); syncBuiltinESMExports(); + } + } + } + assert.equal(await foregroundText(root, "read_file", { path: "a.txt" }), "needle"); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); + +test("foreground deadline returns while a directory read is pending and closes it when settled", { timeout: 12_000 }, async (t) => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-deadline-")); + const entered = deferred(); const release = deferred(); const closed = deferred(); + const original = fsPromises.opendir; + let reads = 0; let closes = 0; + t.mock.method(fsPromises, "opendir", async (...args: Parameters) => { + const directory = await original(...args); + const read = directory.read.bind(directory); const close = directory.close.bind(directory); + directory.read = (async () => { reads++; entered.resolve(); await release.promise; return read(); }) as typeof directory.read; + directory.close = (async () => { closes++; await close(); closed.resolve(); }) as typeof directory.close; + return directory; + }); + syncBuiltinESMExports(); + try { + const call = foregroundText(root, "list_dir", {}); + await entered.promise; + assert.match(await call, /search stopped after 5000 ms/); + assert.equal(closes, 0); + release.resolve(); await closed.promise; + assert.equal(reads, 1); assert.equal(closes, 1); + } finally { + release.resolve(); t.mock.restoreAll(); syncBuiltinESMExports(); + await fs.rm(root, { recursive: true, force: true }); + } +}); + +test("foreground operation admission remains bounded through repeated cancellation and late cleanup", { timeout: 5_000 }, async () => { + const scope = new ForegroundReadOperations(2); + const controllers = [new AbortController(), new AbortController()]; + const pending = controllers.map(() => deferred()); + const cleaning = controllers.map(() => deferred()); + const cleanup = controllers.map(() => deferred()); + const completed = controllers.map(() => deferred()); + const started = controllers.map(() => deferred()); + const calls = controllers.map((controller, i) => scope.run(controller.signal, undefined, async (signal) => { + started[i]!.resolve(); + try { await pending[i]!.promise; signal.throwIfAborted(); return "unexpected"; } + finally { cleaning[i]!.resolve(); await cleanup[i]!.promise; completed[i]!.resolve(); } + }, () => "deadline")); + const rejected = calls.map((call) => assert.rejects(call, /cancelled/)); + await Promise.all(started.map((gate) => gate.promise)); + for (let i = 0; i < 2; i++) controllers[i]!.abort(new Error("cancelled")); + await Promise.all(rejected); + const run = () => scope.run(undefined, undefined, async () => "recovered", () => "deadline"); + for (let i = 0; i < 8; i++) await assert.rejects(run(), /operations are busy/); + pending[0]!.reject(new Error("late I/O failure")); pending[1]!.resolve(); + await Promise.all(cleaning.map((gate) => gate.promise)); + await assert.rejects(run(), /operations are busy/, "cleanup retains admission"); + cleanup.forEach((gate) => gate.resolve()); + await Promise.all(completed.map((gate) => gate.promise)); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(await run(), "recovered"); +}); + +test("foreground cleanup failures quarantine admission and undefined failures still reject", async () => { + const scope = new ForegroundReadOperations(1); + await assert.rejects(scope.run(undefined, undefined, async () => { + await closeForegroundResource({ close: async () => { throw new Error("close failed"); } }); + }, () => undefined), ForegroundReadCleanupError); + await assert.rejects(scope.run(undefined, undefined, async () => "unused", () => "timeout"), /operations are busy/); + const fresh = new ForegroundReadOperations(1); + const result = await Promise.allSettled([fresh.run(undefined, undefined, async () => { throw undefined; }, () => undefined)]); + assert.equal(result[0]!.status, "rejected"); + assert.equal(await fresh.run(undefined, undefined, async () => "ok", () => "timeout"), "ok"); +}); + +test("foreground workers terminate on cancellation while traversal callbacks remain pending", { timeout: 8_000 }, async (t) => { + const terminated = deferred(); + let terminationCount = 0; + const originalTerminate = Worker.prototype.terminate; + t.mock.method(Worker.prototype, "terminate", async function (this: Worker) { + const code = await originalTerminate.call(this); + if (++terminationCount === 4) terminated.resolve(); + return code; + }); + const controllers = Array.from({ length: 4 }, () => new AbortController()); + const started = controllers.map(() => deferred()); + const release = deferred(); + const calls = controllers.map((controller, i) => withCodingToolMatcher("grep", "safe", Date.now() + 5_000, controller.signal, async () => { + started[i]!.resolve(); await release.promise; controller.signal.throwIfAborted(); + })); + const settled = Promise.allSettled(calls); + try { + await Promise.all(started.map((gate) => gate.promise)); + controllers.forEach((controller) => controller.abort(new Error("pending traversal cancelled"))); + await terminated.promise; + assert.equal(terminationCount, 4, "workers terminate without waiting for the traversal callback"); + await assert.rejects(withCodingToolMatcher("grep", "safe", Date.now() + 5_000, undefined, async () => undefined), /searches are busy/); + } finally { release.resolve(); await settled; t.mock.restoreAll(); } + assert.deepEqual(await withCodingToolMatcher("grep", "safe", Date.now() + 5_000, undefined, async (match) => match(["safe"], 1)), [0]); +}); + +test("foreground tools retain global admission until both root metadata requests settle", { timeout: 8_000 }, async (t) => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-parent-metadata-")); + await fs.writeFile(path.join(root, "a.txt"), "needle"); + const tool = buildCodingTools(root).find((tool) => tool.name === "read_file")!; + const started = deferred(); const release = deferred(); const metadataSettled = deferred(); + const stat = fsPromises.stat; const realpath = fsPromises.realpath; + let pendingStats = 0; let completedStats = 0; + t.mock.method(fsPromises, "realpath", async (...args: Parameters) => { + if (args[0] === root) throw new Error("first metadata request failed"); + return realpath(...args); + }); + t.mock.method(fsPromises, "stat", async (...args: Parameters) => { + if (args[0] === root) { + if (++pendingStats === 4) started.resolve(); + await release.promise; + } + const result = await stat(...args); + if (args[0] === root && ++completedStats === 4) metadataSettled.resolve(); + return result; + }); + syncBuiltinESMExports(); + const controllers = Array.from({ length: 4 }, () => new AbortController()); + const calls = controllers.map((controller) => tool.execute("pending", { path: "a.txt" }, controller.signal)); + const rejections = calls.map((call) => assert.rejects(call, /metadata cancelled/)); + try { + await started.promise; + controllers.forEach((controller) => controller.abort(new Error("metadata cancelled"))); + await Promise.all(rejections); + for (let i = 0; i < 8; i++) await assert.rejects(tool.execute("excess", { path: "a.txt" }), /operations are busy/); + assert.equal(pendingStats, 4, "excess calls never start another syscall"); + } finally { + release.resolve(); t.mock.restoreAll(); syncBuiltinESMExports(); + // Let all four original owners observe the completed metadata pair. + await metadataSettled.promise; + await new Promise((resolve) => setImmediate(resolve)); + } + try { + assert.equal(await foregroundText(root, "read_file", { path: "a.txt" }), "needle"); + } finally { await fs.rm(root, { recursive: true, force: true }); } +}); diff --git a/main/services/coding-tools.ts b/main/services/coding-tools.ts index 9fe9ca8b6..a94069cd7 100644 --- a/main/services/coding-tools.ts +++ b/main/services/coding-tools.ts @@ -6,6 +6,7 @@ // Tool inputs use typebox schemas (pi's AgentTool.parameters), matching tools.ts. import { spawn } from "node:child_process"; +import { ForegroundReadOperations, ForegroundReadCleanupError, closeForegroundResource } from "./foreground-read-scope.js"; import { StringDecoder } from "node:string_decoder"; import { CodingToolMatchTimeout, withCodingToolGlob, withCodingToolMatcher } from "./coding-tool-matcher.js"; import { agentCommandEnvironment } from "./agent-command-environment.js"; @@ -317,10 +318,13 @@ async function verifyWorkspaceRoot( let canonical: string; let identity: Stats; try { - [canonical, identity] = await Promise.all([ + const results = await Promise.allSettled([ fs.realpath(workspace.lexical), fs.stat(workspace.lexical), ]); + if (results[0].status === "rejected") throw results[0].reason; + if (results[1].status === "rejected") throw results[1].reason; + [canonical, identity] = [results[0].value, results[1].value]; } catch { throwIfAborted(signal, "Filesystem operation cancelled."); throw new Error("The authorized workspace root changed during this generation."); @@ -921,14 +925,19 @@ async function readParentFile( await workspace.testObserver?.beforeEntryAccess?.(full); await verifyWorkspaceRoot(workspace, signal); const canonical = assertRealPathInRoot(root, await fs.realpath(full), full); + throwIfAborted(signal, "File read cancelled."); rejectEnvironmentSecret(root, canonical); const handle = await fs.open(canonical, fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) | (fsConstants.O_NONBLOCK ?? 0)); try { + throwIfAborted(signal, "File read cancelled."); const stat = await handle.stat(); + throwIfAborted(signal, "File read cancelled."); if (!stat.isFile()) throw new Error("The requested path is not a regular file."); const verified = assertRealPathInRoot(root, await fs.realpath(full), full); + throwIfAborted(signal, "File read cancelled."); rejectEnvironmentSecret(root, verified); if (!sameFile(stat, await fs.stat(verified))) throw new Error("File changed while opening."); + throwIfAborted(signal, "File read cancelled."); await workspace.testObserver?.afterFileStat?.(full); const buffer = Buffer.allocUnsafe(maxBytes + 1); let bytesRead = 0; @@ -943,7 +952,7 @@ async function readParentFile( await verifyWorkspaceRoot(workspace, signal); return { buffer: buffer.subarray(0, Math.min(bytesRead, maxBytes)), truncated: bytesRead > maxBytes, bytesRead }; } finally { - await handle.close(); + await closeForegroundResource(handle); } } @@ -978,13 +987,18 @@ async function parentDirectoryEntries( if (parentScanStopped(budget, signal)) return []; const root = await verifyWorkspaceRoot(workspace, signal); await workspace.testObserver?.beforeDirectoryOpen?.(full); + throwIfAborted(signal, "Directory traversal cancelled."); const canonical = assertRealPathInRoot(root, await fs.realpath(full), full); + throwIfAborted(signal, "Directory traversal cancelled."); const before = await fs.stat(canonical); + throwIfAborted(signal, "Directory traversal cancelled."); const directory = await fs.opendir(canonical, { bufferSize: 1 }); try { + throwIfAborted(signal, "Directory traversal cancelled."); await workspace.testObserver?.afterDirectoryOpen?.(full); await verifyWorkspaceRoot(workspace, signal); const after = assertRealPathInRoot(root, await fs.realpath(full), full); + throwIfAborted(signal, "Directory traversal cancelled."); if (canonical !== after || !sameFile(before, await fs.stat(after))) throw new Error("Directory changed while opening."); const entries: Dirent[] = []; while (!parentScanStopped(budget, signal)) { @@ -1005,7 +1019,7 @@ async function parentDirectoryEntries( // order to select the returned subset. Earlier complete directories survive. return budget.stopped ? [] : entries.sort(compareEntryNames); } finally { - await directory.close(); + await closeForegroundResource(directory); } } @@ -1429,6 +1443,7 @@ function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { const resolved = await resolveExistingInRoot(workspace, task.path, signal); full = resolved.full; if (isEnvironmentSecretPath(task.path) || isEnvironmentSecretPath(path.relative(root, full))) continue; + throwIfAborted(signal, "File search cancelled."); directory = (await fs.stat(full)).isDirectory(); } catch { throwIfAborted(signal, "File search cancelled."); @@ -1449,6 +1464,7 @@ function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { try { const lexical = resolveInRoot(workspace.lexical, path.join(task.path, prepared.literal)); await workspace.testObserver?.beforeEntryAccess?.(lexical); + throwIfAborted(signal, "File search cancelled."); const stat = await fs.lstat(lexical); workspace.testObserver?.onDirectoryEntry?.(lexical); entries = [{ name: prepared.literal, directory: stat.isDirectory(), link: stat.isSymbolicLink() }]; @@ -1459,7 +1475,8 @@ function makeParentGlob(workspace: WorkspaceRootGuard): AgentTool { } else { try { entries = (await parentDirectoryEntries(workspace, full, budget, signal)).map((entry) => ({ name: entry.name, directory: entry.isDirectory(), link: entry.isSymbolicLink() })); - } catch { + } catch (error) { + if (error instanceof ForegroundReadCleanupError) throw error; throwIfAborted(signal, "File search cancelled."); await verifyWorkspaceRoot(workspace, signal); budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); @@ -1709,7 +1726,8 @@ function makeParentGrep(workspace: WorkspaceRootGuard): AgentTool { if (stat.isDirectory()) { let entries: Dirent[]; try { entries = await parentDirectoryEntries(workspace, full, budget, signal); } - catch { + catch (error) { + if (error instanceof ForegroundReadCleanupError) throw error; throwIfAborted(signal, "File search cancelled."); await verifyWorkspaceRoot(workspace, signal); budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); @@ -1733,7 +1751,8 @@ function makeParentGrep(workspace: WorkspaceRootGuard): AgentTool { let read: Awaited>; try { read = await readParentFile(workspace, root, full, Math.min(512_000, remaining - 1), signal, (bytes) => { budget.bytes += bytes; }); - } catch { + } catch (error) { + if (error instanceof ForegroundReadCleanupError) throw error; throwIfAborted(signal, "File search cancelled."); await verifyWorkspaceRoot(workspace, signal); budget.warnings.add("… [search incomplete: unreadable or changed paths skipped]"); @@ -1917,12 +1936,28 @@ function makeRunCommand(workspace: WorkspaceRootGuard, options: CodingToolOption }; } +const foregroundReads = new ForegroundReadOperations(); + +function withForegroundReadScope(tool: AgentTool): AgentTool { + return { + ...tool, + execute: (id, params, signal, onUpdate) => foregroundReads.run( + signal, + tool.name === "read_file" ? undefined : MAX_GREP_DURATION_MS, + (operationSignal) => tool.execute(id, params, operationSignal, onUpdate ? (update) => { + if (!operationSignal.aborted) onUpdate(update); + } : undefined), + () => textResult(`… [search stopped after ${MAX_GREP_DURATION_MS} ms]`), + ), + }; +} + function buildParentCodingToolSet(workspace: WorkspaceRootGuard, options: CodingToolOptions = {}): AgentTool[] { return [ - declarePiRuntimeReplay(makeParentReadFile(workspace), "safe"), - declarePiRuntimeReplay(makeParentListDir(workspace), "safe"), - declarePiRuntimeReplay(makeParentGlob(workspace), "safe"), - declarePiRuntimeReplay(makeParentGrep(workspace), "safe"), + declarePiRuntimeReplay(withForegroundReadScope(makeParentReadFile(workspace)), "safe"), + declarePiRuntimeReplay(withForegroundReadScope(makeParentListDir(workspace)), "safe"), + declarePiRuntimeReplay(withForegroundReadScope(makeParentGlob(workspace)), "safe"), + declarePiRuntimeReplay(withForegroundReadScope(makeParentGrep(workspace)), "safe"), declarePiRuntimeReplay(makeEditFile(workspace), "never"), declarePiRuntimeReplay(makeWriteFile(workspace), "never"), declarePiRuntimeReplay(makeRunCommand(workspace, options), "never"), diff --git a/main/services/foreground-read-scope.ts b/main/services/foreground-read-scope.ts new file mode 100644 index 000000000..e9c6dba48 --- /dev/null +++ b/main/services/foreground-read-scope.ts @@ -0,0 +1,70 @@ +/** A failed close cannot establish that the descriptor was released. */ +export class ForegroundReadCleanupError extends Error { + constructor(message: string, readonly cause: unknown) { super(message); } +} + +/** + * Cancellation settles the caller, but cannot cancel an issued kernel syscall. + * Keep a bounded owner until its original operation and all cleanup settle. + */ +export class ForegroundReadOperations { + private active = 0; + + constructor(private readonly limit = 4) {} + + run( + signal: AbortSignal | undefined, + durationMs: number | undefined, + operation: (signal: AbortSignal) => Promise, + timeoutResult: () => T, + ): Promise { + if (signal?.aborted) return Promise.reject(signal.reason); + if (this.active >= this.limit) { + return Promise.reject(new Error("Filesystem operations are busy; try again shortly.")); + } + this.active++; + const controller = new AbortController(); + return new Promise((resolve, reject) => { + let settled = false; + const finish = (outcome: { result: T } | { error: unknown }) => { + if (settled) return; + settled = true; + clearTimeout(timer); + signal?.removeEventListener("abort", abort); + if ("error" in outcome) reject(outcome.error); + else resolve(outcome.result); + }; + const abort = () => { + controller.abort(signal?.reason); + finish({ error: controller.signal.reason }); + }; + const timer = durationMs === undefined ? undefined : setTimeout(() => { + controller.abort(new Error("File search deadline reached.")); + try { finish({ result: timeoutResult() }); } + catch (error) { finish({ error }); } + }, durationMs); + signal?.addEventListener("abort", abort, { once: true }); + if (signal?.aborted) abort(); + // Both handlers remain installed after caller settlement. Late failures + // are observed and never produce a second result or release capacity early. + void Promise.resolve().then(() => { + controller.signal.throwIfAborted(); + return operation(controller.signal); + }).then( + (result) => { this.active--; finish({ result }); }, + (error: unknown) => { + if (!(error instanceof ForegroundReadCleanupError)) this.active--; + finish({ error }); + }, + ); + }); + } +} + +export async function closeForegroundResource(resource: { close(): Promise }): Promise { + try { + await resource.close(); + } catch (error) { + throw new ForegroundReadCleanupError("Filesystem cleanup failed; capacity remains reserved.", error); + } +} diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts index 51862ab14..c0870c4a4 100644 --- a/scripts/smoke-foreground-file-tools.ts +++ b/scripts/smoke-foreground-file-tools.ts @@ -34,6 +34,7 @@ void app.whenReady().then(async () => { "**/*/*.ts", "{src,link}/**/*", "src/**/..", + `{${path.join(root, "src/*.ts")},src/sub/*.ts}`, ]; for (const pattern of globs) { const expected: string[] = []; From b28f681e863ecdccabd7aba551b8cc79c71f6535 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 15:58:11 -0400 Subject: [PATCH 05/11] fix(tools): preserve normalized Windows glob root offsets --- .memory/foreground-file-tool-bounds.md | 6 ++++ .../README.md | 3 +- main/services/coding-tool-glob-worker.ts | 4 +-- main/services/coding-tools.test.ts | 30 +++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 7179dc465..73c69ebd8 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -51,3 +51,9 @@ without further reads; failed cleanup quarantines admission. Root verification awaits both started metadata requests even if one fails. Worker termination starts on lifetime abort/deadline while its traversal callback may still be pending. The stalled-I/O deadline result is an explicit notice without partial output. + +Completion audit also caught the expanded-root offset using host separators even +though minimatch globParts are slash-normalized. Count normalized slashes so +Windows drive and UNC roots skip exactly their parsed prefix. Production-worker +VM fixtures use path.win32 and minimatch platform win32, covering slash/backslash +drive spelling, UNC, and mixed absolute/relative arms; prior code fails the fixture. diff --git a/docs/performance/foreground-file-tools-2026-09-28/README.md b/docs/performance/foreground-file-tools-2026-09-28/README.md index 5ae595eab..9c66b2cfd 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/README.md +++ b/docs/performance/foreground-file-tools-2026-09-28/README.md @@ -53,7 +53,8 @@ segment positions and link traversal states without prematurely normalizing `**/..`. Filesystem access stays on the bounded host. The first independent review found gaps in a flattened-path matcher; the replacement has differential tests for absolute paths, mixed absolute/relative brace arms, braces/extglobs, globstars, linked prefixes, linked wildcard -paths, and glob-dependent parent segments. Node traversal attribution is in +paths, and glob-dependent parent segments. Worker-engine fixtures also exercise +Windows drive and UNC roots with Windows path/parser semantics. Node traversal attribution is in THIRD_PARTY_NOTICES.md. Pinned Electron 43.1.1 / Node 24.18.0 smoke uses a bundled entry and real app main diff --git a/main/services/coding-tool-glob-worker.ts b/main/services/coding-tool-glob-worker.ts index 132b8e57d..cad8bf03b 100644 --- a/main/services/coding-tool-glob-worker.ts +++ b/main/services/coding-tool-glob-worker.ts @@ -5,7 +5,7 @@ * rather than matching a flattened pathname. Filesystem access stays on the host. */ export const CODING_GLOB_WORKER_SOURCE = ` -const { join, isAbsolute, parse, sep } = require('node:path'); +const { join, isAbsolute, parse } = require('node:path'); const { Minimatch, GLOBSTAR } = require(workerData.minimatchPath); const matcher = new Minimatch(workerData.pattern, { nocase: process.platform === 'win32' || process.platform === 'darwin', @@ -19,7 +19,7 @@ const seeds = matcher.set.map((parts, pattern) => { const expanded = matcher.globParts[pattern].join('/'); const root = isAbsolute(expanded) ? parse(expanded).root : ''; let current = root || '.'; - let index = root ? root.split(sep).length - 1 : 0; + let index = root ? root.split('/').length - 1 : 0; // Resolve literal prefixes only. A globstar followed by .. is never collapsed. while (index < parts.length - 1 && typeof parts[index] === 'string') { current = join(current, parts[index++]); diff --git a/main/services/coding-tools.test.ts b/main/services/coding-tools.test.ts index 66b5d4751..a1b63e05f 100644 --- a/main/services/coding-tools.test.ts +++ b/main/services/coding-tools.test.ts @@ -1,6 +1,9 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import fsPromises, * as fs from "node:fs/promises"; +import { runInNewContext } from "node:vm"; +import { createRequire } from "node:module"; +import { CODING_GLOB_WORKER_SOURCE, type CodingGlobTask, type CodingGlobReply } from "./coding-tool-glob-worker.js"; import { Worker } from "node:worker_threads"; import { syncBuiltinESMExports } from "node:module"; import { ForegroundReadOperations, ForegroundReadCleanupError, closeForegroundResource } from "./foreground-read-scope.js"; @@ -2540,3 +2543,30 @@ test("foreground tools retain global admission until both root metadata requests assert.equal(await foregroundText(root, "read_file", { path: "a.txt" }), "needle"); } finally { await fs.rm(root, { recursive: true, force: true }); } }); + + +test("foreground glob worker preserves Windows drive and UNC roots in expanded arms", () => { + const require = createRequire(import.meta.url); + const fixtures = [ + { pattern: "C:/repo/*.ts", expected: "C:\\repo\\a.ts" }, + { pattern: "C:\\repo\\*.ts", expected: "C:\\repo\\a.ts" }, + { pattern: "//server/share/repo/*.ts", expected: "\\\\server\\share\\repo\\a.ts" }, + { pattern: "{C:/repo/*.ts,src/*.js}", expected: "C:\\repo\\a.ts" }, + { pattern: "{src/*.js,//server/share/repo/*.ts}", expected: "\\\\server\\share\\repo\\a.ts" }, + ]; + for (const { pattern, expected } of fixtures) { + // Execute the production worker engine with Windows path/parser semantics. + const engine = runInNewContext(`${CODING_GLOB_WORKER_SOURCE}; ({ seeds, globStep });`, { + process: { platform: "win32" }, + workerData: { pattern, minimatchPath: require.resolve("minimatch") }, + require: (name: string) => name === "node:path" ? path.win32 : require(name), + }) as { seeds: CodingGlobTask[]; globStep(request: unknown): CodingGlobReply }; + const matches: string[] = []; + for (const task of engine.seeds) { + const prepared = engine.globStep({ task, directory: true }); + matches.push(...prepared.matches); + if (!prepared.done) matches.push(...engine.globStep({ task, directory: true, entries: [{ name: "a.ts", directory: false, link: false }] }).matches); + } + assert.deepEqual(matches, [expected], pattern); + } +}); From 73f8664532eb571380d2a9a609077fea0108af81 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 16:30:46 -0400 Subject: [PATCH 06/11] test(tools): run pinned Electron foreground smoke in CI --- .github/workflows/ci.yml | 2 + .memory/foreground-file-tool-bounds.md | 8 + .papercuts/troubleshooting.md | 13 ++ .../README.md | 12 +- .../electron.json | 14 +- package.json | 1 + scripts/check-ci-policy.test.mjs | 11 ++ scripts/run-foreground-file-tools-smoke.mjs | 83 ++++++++ scripts/smoke-foreground-file-tools.ts | 178 +++++++++--------- 9 files changed, 224 insertions(+), 98 deletions(-) create mode 100644 scripts/run-foreground-file-tools-smoke.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9596368ab..c57ff2c06 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,6 +138,8 @@ jobs: run: npm run test:model-catalog - name: Build production bundles once run: npm run build + - name: Verify foreground file tools in pinned Electron + run: npm run test:foreground-file-tools:electron - name: Verify production-profile diagnostics support workflow run: npm run test:e2e:diagnostics:production:run diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 73c69ebd8..670429764 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -57,3 +57,11 @@ though minimatch globParts are slash-normalized. Count normalized slashes so Windows drive and UNC roots skip exactly their parsed prefix. Production-worker VM fixtures use path.win32 and minimatch platform win32, covering slash/backslash drive spelling, UNC, and mixed absolute/relative arms; prior code fails the fixture. + + +The assertion-based Electron smoke is registered as +`test:foreground-file-tools:electron` and required in the existing Desktop build +and diagnostics CI lane. Its Node runner bundles an isolated entry (Electron +explicitly external), owns workspace/profile fixtures, clears ELECTRON_RUN_AS_NODE, +and waits for child close after success/error/30s deadline/SIGINT/SIGTERM before +cleanup. CI policy coverage enforces this mandatory package-script invocation. diff --git a/.papercuts/troubleshooting.md b/.papercuts/troubleshooting.md index 2c7d42ee7..f4a194fcd 100644 --- a/.papercuts/troubleshooting.md +++ b/.papercuts/troubleshooting.md @@ -1421,3 +1421,16 @@ listener is removed, even for a live idle worker. The initial lifecycle test fai `0 !== 4`; corrected the oracle to await the actual Worker.terminate promises while traversal stays deferred, then independently assert held admission and recovery. This was a deterministic test-oracle failure, not a passing rerun or timeout change. + + +### Register standalone Electron assertions in CI (2026-09-28, PR #288) + +A final audit caught the foreground smoke's manual-only execution: assertion-based +smoke entries also need a package test command and a CI lane, even without a +`.test` suffix. Registered it in Desktop build and diagnostics. When moving the +esbuild CLI invocation into its API, retain explicit `external: ["electron"]`: +`packages: "external"` alone allowed repository TS path resolution to bundle the +npm launcher, producing “Dynamic require of child_process is not supported”. The +runner's hard deadline caught that pre-handler failure and removed fixtures; the +corrected registered smoke passes. SIGTERM validation also proved nonzero exit, +reaped Electron, and removal of the temporary bundle/workspace/profile. diff --git a/docs/performance/foreground-file-tools-2026-09-28/README.md b/docs/performance/foreground-file-tools-2026-09-28/README.md index 9c66b2cfd..a9fe2915d 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/README.md +++ b/docs/performance/foreground-file-tools-2026-09-28/README.md @@ -80,10 +80,16 @@ npx tsx --test main/services/coding-tools.test.ts main/services/generation-runti npm run type-check npx eslint main/services/coding-tools.ts main/services/coding-tool-matcher.ts main/services/coding-tool-glob-worker.ts main/services/foreground-read-scope.ts main/services/coding-tools.test.ts scripts/benchmark-foreground-file-tools.ts scripts/smoke-foreground-file-tools.ts node node_modules/electron/install.js -npx esbuild scripts/smoke-foreground-file-tools.ts --bundle --platform=node --format=esm --packages=external --external:electron --outfile=build/main/foreground-file-tools-smoke.mjs -node_modules/electron/dist/Electron.app/Contents/MacOS/Electron build/main/foreground-file-tools-smoke.mjs +npm run test:foreground-file-tools:electron +npm run test:ci-policy ``` The existing coding-tools test file is already in the root test chain and CI -registry. No shared server/native contract or transcript UI changed; no mobile +registry. The Electron smoke has its own package command and runs as a required +step in the existing Desktop build and diagnostics CI lane. Its runner bundles +into an isolated directory, uses the installed pinned Electron, owns the synthetic +workspace and user-data profile, and waits for process close before cleanup. A +30-second hard process deadline also catches module-load failures before the +smoke's own error handler can run; SIGINT/SIGTERM terminate its owned process group. +The CI policy suite verifies the package command remains a required CI step. No shared server/native contract or transcript UI changed; no mobile implementation or plan-status update is needed for these internal tools. diff --git a/docs/performance/foreground-file-tools-2026-09-28/electron.json b/docs/performance/foreground-file-tools-2026-09-28/electron.json index faf24bd52..2a2bdfd10 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/electron.json +++ b/docs/performance/foreground-file-tools-2026-09-28/electron.json @@ -14,15 +14,15 @@ "**/*/*.ts", "{src,link}/**/*", "src/**/..", - "{/var/folders/bb/wd_m6tl14y5c1wklxz3sj9840000gn/T/aiden-electron-file-tools-dzVPQW/src/*.ts,src/sub/*.ts}" + "{/Users/sambitbiswas/.codex/worktrees/4e14/aiden-agent/build/foreground-smoke-0uGmSU/workspace/src/*.ts,src/sub/*.ts}" ], "cancellationMs": [ - 152.08937500000002, - 152.27324999999996, - 152.46399999999994, - 152.52579100000003, - 151.98649999999998, - 151.38937499999997 + 151.54775000000006, + 151.97033299999998, + 152.3012500000001, + 152.90824999999995, + 152.47225000000003, + 152.56825000000003 ], "deadline": "settled with incomplete notice", "workerPortsAfter": 0, diff --git a/package.json b/package.json index 2fa00bf65..f95cc4d7c 100644 --- a/package.json +++ b/package.json @@ -176,6 +176,7 @@ "computer-use:linux-host-preflight": "node scripts/linux-computer-use-host-preflight.mjs", "test:devices": "tsx --test main/services/aiden-remote-simulators.test.ts main/services/devices/device-actions.test.ts main/services/devices/device-hub-proxy.test.ts main/services/devices/device-ipc.test.ts main/services/devices/device-service.test.ts main/services/devices/device-toolchain.test.ts main/services/devices/device-tools.test.ts main/services/devices/feature-flag.test.ts main/services/devices/local-device-host.test.ts main/services/devices/peer-devices.test.ts renderer/components/device-tools-panel.test.tsx renderer/components/devices-panel.test.tsx renderer/components/settings/simulator-settings.test.tsx renderer/lib/composer-attach.test.ts renderer/lib/device-3d/device-3d.test.ts renderer/lib/device-controls.test.ts renderer/lib/device-duo-control.test.ts renderer/lib/device-foreground.test.ts renderer/lib/device-stream.test.ts renderer/lib/environment-panel-state.test.ts renderer/shared/devices.test.ts", "test:custom-model-options": "tsx --test main/services/custom-model-options.test.ts", + "test:foreground-file-tools:electron": "node scripts/run-foreground-file-tools-smoke.mjs", "test:tool-outputs": "tsx --test main/services/tool-output-store.test.ts main/services/coding-tools.test.ts main/services/mcp-tool-result.test.ts main/services/generation-timeline.test.ts main/services/chat-application-service.test.ts main/services/aiden-remote-protocol.test.ts renderer/components/activity-feed.test.tsx renderer/components/onboarding-flow.test.tsx", "test:chat-pull-requests": "tsx --test renderer/shared/chat-pull-requests.test.ts main/services/chat-pull-request-store.test.ts main/services/chat-pull-request-service.test.ts main/services/pull-request-current-resolver.test.ts main/services/pull-request-create-reconciliation.test.ts main/services/github-pull-request.test.ts", "test:ci": "node scripts/run-ci-tests.mjs", diff --git a/scripts/check-ci-policy.test.mjs b/scripts/check-ci-policy.test.mjs index bb6289db4..f7d16fdef 100644 --- a/scripts/check-ci-policy.test.mjs +++ b/scripts/check-ci-policy.test.mjs @@ -360,3 +360,14 @@ test("iOS-only PRs retain shipping and TestFlight policy checks when desktop lan const registry = readRegistry(); assert.ok(registry.lanes.some((lane) => lane.preserved.includes("ios-release-policy"))); }); + +test("foreground Electron smoke is a required package command in desktop verification", async () => { + const { scripts } = JSON.parse(await readFile(new URL("../package.json", import.meta.url), "utf8")); + const { jobs } = parse(await readFile(workflowUrl, "utf8")); + const command = "test:foreground-file-tools:electron"; + assert.ok(scripts[command], "Electron smoke must be runnable from the package manifest"); + const smoke = jobs.verify.steps.find((step) => step.run === `npm run ${command}`); + assert.ok(smoke, "Desktop CI must execute the foreground runtime assertions"); + assert.notEqual(smoke["continue-on-error"], true, "Smoke failures must fail CI"); + assert.equal(smoke.if, undefined, "Every desktop verification run must execute the smoke"); +}); diff --git a/scripts/run-foreground-file-tools-smoke.mjs b/scripts/run-foreground-file-tools-smoke.mjs new file mode 100644 index 000000000..b951b59a8 --- /dev/null +++ b/scripts/run-foreground-file-tools-smoke.mjs @@ -0,0 +1,83 @@ +/* global clearTimeout, console, process, setTimeout */ + +import { spawn } from "node:child_process"; +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath, URL } from "node:url"; +import { build, stop } from "esbuild"; +import electron from "electron"; + +const repository = fileURLToPath(new URL("../", import.meta.url)); + +async function run() { + await mkdir(path.join(repository, "build"), { recursive: true }); + const temporary = await mkdtemp(path.join(repository, "build", "foreground-smoke-")); + try { + const entry = path.join(temporary, "smoke.mjs"); + const workspace = path.join(temporary, "workspace"); + const profile = path.join(temporary, "profile"); + await Promise.all([mkdir(workspace), mkdir(profile)]); + try { + await build({ + entryPoints: [path.join(repository, "scripts/smoke-foreground-file-tools.ts")], + outfile: entry, + bundle: true, + platform: "node", + format: "esm", + packages: "external", + external: ["electron"], + }); + } finally { + stop(); + } + const env = { ...process.env }; + delete env.ELECTRON_RUN_AS_NODE; + await new Promise((resolve, reject) => { + const child = spawn(electron, [entry, workspace, profile], { + cwd: repository, + env, + stdio: "inherit", + detached: process.platform !== "win32", + }); + let failure; + const terminate = (reason) => { + failure ??= new Error(reason); + if (child.pid && child.exitCode === null && child.signalCode === null) { + try { + if (process.platform === "win32") child.kill("SIGKILL"); + else process.kill(-child.pid, "SIGKILL"); + } catch (error) { + if (error.code !== "ESRCH") failure = error; + } + } + }; + const interrupt = () => terminate("Electron foreground smoke interrupted."); + const timer = setTimeout( + () => terminate("Electron foreground smoke exceeded 30 seconds."), + 30_000, + ); + process.once("SIGINT", interrupt); + process.once("SIGTERM", interrupt); + child.once("error", (error) => { + failure ??= error; + }); + // Wait for close even after timeout/signal before removing owned fixtures. + child.once("close", (code, signal) => { + clearTimeout(timer); + process.off("SIGINT", interrupt); + process.off("SIGTERM", interrupt); + if (failure) reject(failure); + else if (code !== 0) + reject(new Error(`Electron foreground smoke exited with ${signal ?? code}.`)); + else resolve(); + }); + }); + } finally { + await rm(temporary, { recursive: true, force: true }); + } +} + +run().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts index c0870c4a4..369d6db3d 100644 --- a/scripts/smoke-foreground-file-tools.ts +++ b/scripts/smoke-foreground-file-tools.ts @@ -1,98 +1,100 @@ -/** Bundle with esbuild and run using the pinned Electron executable (see evidence doc). */ +/** Run with npm run test:foreground-file-tools:electron (owns build and fixture cleanup). */ import { app } from "electron"; import assert from "node:assert/strict"; import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; import { buildCodingTools } from "../main/services/coding-tools.js"; -void app.whenReady().then(async () => { - const root = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-electron-file-tools-")); - try { - const tools = buildCodingTools(root); - const invoke = async (name: string, params: object, signal?: AbortSignal) => { - const result = await tools - .find((tool) => tool.name === name)! - .execute("electron-smoke", params, signal); - const block = result.content[0]; - return block?.type === "text" ? block.text : ""; - }; - await fs.mkdir(path.join(root, "src/sub"), { recursive: true }); - await fs.writeFile(path.join(root, "src/sub/b.ts"), ""); - await fs.writeFile(path.join(root, "src/a.ts"), "foobar foofoo\n"); - await fs.symlink("src", path.join(root, "link")); - const globs = [ - "*", - "**", - "**/*", - "**/*.ts", - "src/**", - "src", - "src/*.{ts,js}", - "link/*.ts", - "*/a.ts", - "**/*/*.ts", - "{src,link}/**/*", - "src/**/..", - `{${path.join(root, "src/*.ts")},src/sub/*.ts}`, - ]; - for (const pattern of globs) { - const expected: string[] = []; - for await (const entry of fs.glob(pattern, { cwd: root })) expected.push(entry); - assert.equal( - await invoke("glob", { pattern }), - expected.sort().join("\n") || "[no matches]", - pattern, - ); - } +async function smoke() { + const [root, profile] = process.argv.slice(2); + assert.ok(root && profile, "Run through the foreground smoke package script."); + app.setPath("userData", profile); + await app.whenReady(); + const tools = buildCodingTools(root); + const invoke = async (name: string, params: object, signal?: AbortSignal) => { + const result = await tools + .find((tool) => tool.name === name)! + .execute("electron-smoke", params, signal); + const block = result.content[0]; + return block?.type === "text" ? block.text : ""; + }; + await fs.mkdir(path.join(root, "src/sub"), { recursive: true }); + await fs.writeFile(path.join(root, "src/sub/b.ts"), ""); + await fs.writeFile(path.join(root, "src/a.ts"), "foobar foofoo\n"); + await fs.symlink("src", path.join(root, "link")); + const globs = [ + "*", + "**", + "**/*", + "**/*.ts", + "src/**", + "src", + "src/*.{ts,js}", + "link/*.ts", + "*/a.ts", + "**/*/*.ts", + "{src,link}/**/*", + "src/**/..", + `{${path.join(root, "src/*.ts")},src/sub/*.ts}`, + ]; + for (const pattern of globs) { + const expected: string[] = []; + for await (const entry of fs.glob(pattern, { cwd: root })) expected.push(entry); assert.equal( - await invoke("grep", { pattern: "(?<=foo)bar|\\b(foo)\\1\\b" }), - "src/a.ts:1: foobar foofoo", + await invoke("glob", { pattern }), + expected.sort().join("\n") || "[no matches]", + pattern, ); - for (let i = 0; i < 6; i++) - await assert.rejects(invoke("grep", { pattern: "[" }), /Invalid regular expression/); - await fs.writeFile(path.join(root, "src/a.ts"), "a".repeat(100_000) + "!"); - const cancellationMs: number[] = []; - for (let i = 0; i < 6; i++) { - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(new Error("electron cancelled")), 150); - const start = performance.now(); - try { - await assert.rejects( - invoke("grep", { pattern: "^(a+)+$" }, controller.signal), - /electron cancelled/, - ); - } finally { - clearTimeout(timer); - } - cancellationMs.push(performance.now() - start); + } + assert.equal( + await invoke("grep", { pattern: "(?<=foo)bar|\\b(foo)\\1\\b" }), + "src/a.ts:1: foobar foofoo", + ); + for (let i = 0; i < 6; i++) + await assert.rejects(invoke("grep", { pattern: "[" }), /Invalid regular expression/); + await fs.writeFile(path.join(root, "src/a.ts"), "a".repeat(100_000) + "!"); + const cancellationMs: number[] = []; + for (let i = 0; i < 6; i++) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(new Error("electron cancelled")), 150); + const start = performance.now(); + try { + await assert.rejects( + invoke("grep", { pattern: "^(a+)+$" }, controller.signal), + /electron cancelled/, + ); + } finally { + clearTimeout(timer); } - assert.match(await invoke("grep", { pattern: "^(a+)+$" }), /search stopped after 5000 ms/); - assert.equal(await invoke("grep", { pattern: "missing" }), "[no matches]"); - const resources = process - .getActiveResourcesInfo() - .filter((resource) => resource === "MessagePort"); - assert.deepEqual(resources, []); - console.log( - JSON.stringify( - { - electron: process.versions.electron, - node: process.version, - globs, - cancellationMs, - deadline: "settled with incomplete notice", - workerPortsAfter: resources.length, - result: "passed", - }, - null, - 2, - ), - ); - } catch (error) { - console.error(error); - process.exitCode = 1; - } finally { - await fs.rm(root, { recursive: true, force: true }); - app.exit(process.exitCode ?? 0); + cancellationMs.push(performance.now() - start); } -}); + assert.match(await invoke("grep", { pattern: "^(a+)+$" }), /search stopped after 5000 ms/); + assert.equal(await invoke("grep", { pattern: "missing" }), "[no matches]"); + const resources = process + .getActiveResourcesInfo() + .filter((resource) => resource === "MessagePort"); + assert.deepEqual(resources, []); + console.log( + JSON.stringify( + { + electron: process.versions.electron, + node: process.version, + globs, + cancellationMs, + deadline: "settled with incomplete notice", + workerPortsAfter: resources.length, + result: "passed", + }, + null, + 2, + ), + ); +} + +void smoke().then( + () => app.exit(0), + (error) => { + console.error(error); + app.exit(1); + }, +); From 4d5ce2d6cfe2cd26b3d80a4b574a225928ac02e6 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 16:33:51 -0400 Subject: [PATCH 07/11] fix(tools): consume complete bare UNC glob roots --- .memory/foreground-file-tool-bounds.md | 5 +++++ main/services/coding-tool-glob-worker.ts | 4 +++- main/services/coding-tools.test.ts | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 670429764..6f1ed006d 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -65,3 +65,8 @@ and diagnostics CI lane. Its Node runner bundles an isolated entry (Electron explicitly external), owns workspace/profile fixtures, clears ELECTRON_RUN_AS_NODE, and waits for child close after success/error/30s deadline/SIGINT/SIGTERM before cleanup. CI policy coverage enforces this mandatory package-script invocation. + +A later Pullfrog run found bare UNC share roots without a terminal slash. Consume +the complete platform root, and use an empty terminal segment for directory-self +matching when that consumes the whole pattern. Existing Windows worker fixtures +now cover drive roots, bare UNC shares with/without slash, and mixed relative arms. diff --git a/main/services/coding-tool-glob-worker.ts b/main/services/coding-tool-glob-worker.ts index cad8bf03b..a08df6e61 100644 --- a/main/services/coding-tool-glob-worker.ts +++ b/main/services/coding-tool-glob-worker.ts @@ -19,7 +19,9 @@ const seeds = matcher.set.map((parts, pattern) => { const expanded = matcher.globParts[pattern].join('/'); const root = isAbsolute(expanded) ? parse(expanded).root : ''; let current = root || '.'; - let index = root ? root.split('/').length - 1 : 0; + let index = root ? root.split('/').length - Number(root.endsWith('/')) : 0; + // A bare UNC share consumes every segment; match the root directory itself. + if (index === parts.length) parts.push(''); // Resolve literal prefixes only. A globstar followed by .. is never collapsed. while (index < parts.length - 1 && typeof parts[index] === 'string') { current = join(current, parts[index++]); diff --git a/main/services/coding-tools.test.ts b/main/services/coding-tools.test.ts index a1b63e05f..f45729754 100644 --- a/main/services/coding-tools.test.ts +++ b/main/services/coding-tools.test.ts @@ -2549,6 +2549,10 @@ test("foreground glob worker preserves Windows drive and UNC roots in expanded a const require = createRequire(import.meta.url); const fixtures = [ { pattern: "C:/repo/*.ts", expected: "C:\\repo\\a.ts" }, + { pattern: "C:/", expected: "C:\\" }, + { pattern: "//server/share", expected: "\\\\server\\share\\" }, + { pattern: "//server/share/", expected: "\\\\server\\share\\" }, + { pattern: "{src/*.js,//server/share}", expected: "\\\\server\\share\\" }, { pattern: "C:\\repo\\*.ts", expected: "C:\\repo\\a.ts" }, { pattern: "//server/share/repo/*.ts", expected: "\\\\server\\share\\repo\\a.ts" }, { pattern: "{C:/repo/*.ts,src/*.js}", expected: "C:\\repo\\a.ts" }, From 5059c78dbecd7c516e39034311a76fade671bc27 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 16:39:31 -0400 Subject: [PATCH 08/11] test(tools): normalize workspace paths in smoke receipts --- .memory/foreground-file-tool-bounds.md | 4 ++++ .../foreground-file-tools-2026-09-28/electron.json | 14 +++++++------- scripts/smoke-foreground-file-tools.ts | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 6f1ed006d..5c2f286aa 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -70,3 +70,7 @@ A later Pullfrog run found bare UNC share roots without a terminal slash. Consum the complete platform root, and use an empty terminal segment for directory-self matching when that consumes the whole pattern. Existing Windows worker fixtures now cover drive roots, bare UNC shares with/without slash, and mixed relative arms. + +Published Electron smoke receipts normalize the synthetic workspace prefix to +`` while assertions retain real absolute paths. The counter receipts +already use repository-relative module paths and contain no author-local root. diff --git a/docs/performance/foreground-file-tools-2026-09-28/electron.json b/docs/performance/foreground-file-tools-2026-09-28/electron.json index 2a2bdfd10..5f0925319 100644 --- a/docs/performance/foreground-file-tools-2026-09-28/electron.json +++ b/docs/performance/foreground-file-tools-2026-09-28/electron.json @@ -14,15 +14,15 @@ "**/*/*.ts", "{src,link}/**/*", "src/**/..", - "{/Users/sambitbiswas/.codex/worktrees/4e14/aiden-agent/build/foreground-smoke-0uGmSU/workspace/src/*.ts,src/sub/*.ts}" + "{/src/*.ts,src/sub/*.ts}" ], "cancellationMs": [ - 151.54775000000006, - 151.97033299999998, - 152.3012500000001, - 152.90824999999995, - 152.47225000000003, - 152.56825000000003 + 150.49379199999998, + 151.28300000000002, + 151.60524999999996, + 152.44187499999998, + 153.01933400000007, + 152.536333 ], "deadline": "settled with incomplete notice", "workerPortsAfter": 0, diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts index 369d6db3d..12beda348 100644 --- a/scripts/smoke-foreground-file-tools.ts +++ b/scripts/smoke-foreground-file-tools.ts @@ -79,7 +79,7 @@ async function smoke() { { electron: process.versions.electron, node: process.version, - globs, + globs: globs.map((pattern) => pattern.replaceAll(root, "")), cancellationMs, deadline: "settled with incomplete notice", workerPortsAfter: resources.length, From 30b042b44704771335cdfbb7ad1116d8374e55a4 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 17:53:22 -0400 Subject: [PATCH 09/11] test(ios): isolate stream gate and preserve failure diagnostics --- .github/workflows/ci.yml | 10 ++++++ .memory/foreground-file-tool-bounds.md | 9 +++++ ios/AidenOnTheGoTests/AidenChatTests.swift | 41 +++++++++++++--------- scripts/check-ci-policy.test.mjs | 9 +++++ 4 files changed, 52 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c57ff2c06..d5b0ed61c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -271,8 +271,18 @@ jobs: -configuration Debug -destination 'platform=iOS Simulator,id=${{ steps.simulator.outputs.udid }}' -derivedDataPath '${{ runner.temp }}/AidenOnTheGoSimulatorDerivedData' + -resultBundlePath '${{ runner.temp }}/AidenOnTheGoSimulator.xcresult' CODE_SIGNING_ALLOWED=NO + - name: Preserve failed iOS simulator test results + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + with: + name: ios-simulator-results-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/AidenOnTheGoSimulator.xcresult + retention-days: 7 + if-no-files-found: warn + android: name: Android build and APK needs: changes diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 5c2f286aa..35808e4f5 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -74,3 +74,12 @@ now cover drive roots, bare UNC shares with/without slash, and mixed relative ar Published Electron smoke receipts normalize the synthetic workspace prefix to `` while assertions retain real absolute paths. The counter receipts already use repository-relative module paths and contain no author-local root. + +Hosted CI follow-up reused the narrowly scoped consumer-gate correction already +present in PRs #278/#280: hold the exact recovery stream endpoint, then deliberately +run unrelated progress before sending. Reverting only the exact endpoint to +`/events` reproduces a progress-observer timeout in the simulator. The separate +completed-upload failure remains unreproduced (unchanged full chat suite passed +208/208 locally); per-mode assertion labels and failed-CI xcresult preservation +provide diagnostic evidence without relaxing assertions or timeouts. Parent audit +papercuts retain both original failing run attempts and the unresolved upload flake. diff --git a/ios/AidenOnTheGoTests/AidenChatTests.swift b/ios/AidenOnTheGoTests/AidenChatTests.swift index 15f61e472..7d6f087d4 100644 --- a/ios/AidenOnTheGoTests/AidenChatTests.swift +++ b/ios/AidenOnTheGoTests/AidenChatTests.swift @@ -950,12 +950,19 @@ final class AidenChatTests: XCTestCase { } return fixture.response(request) } - await model.load() + await model.load(observeProgress: false) model.draft = "Hello" XCTAssertTrue(model.canSend) let arrived = expectation(description: "consumer events held") - AidenChatProgressLifecycleURLProtocol.holdNextRequest(endingIn: "/events") { arrived.fulfill() } + AidenChatProgressLifecycleURLProtocol.holdNextRequest(endingIn: "/streams/stream-recovery/events") { arrived.fulfill() } defer { AidenChatProgressLifecycleURLProtocol.releaseHeldRequest() } + // The chat's unrelated progress stream also ends in `/events`. Exercise + // it while the stream-consumer hold is armed to prove it cannot steal + // the intended gate. + model.startProgressObservation() + try await waitForProgressRequestCount(1) + try await waitForProgressObservationToStop(model) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.progressRequestCount, 1) await model.send() await fulfillment(of: [arrived], timeout: 2) let admitted = await cache.loadChat(instanceId: "instance-progress-lifecycle", chatId: model.chat.id) @@ -2608,18 +2615,18 @@ final class AidenChatTests: XCTestCase { } let failed = await model.upload(.text(name: "fixture.txt", mimeType: "text/plain", text: "fixture")) if mode == "invalid" { - XCTAssertEqual(failed, 1) + XCTAssertEqual(failed, 1, mode) await cache.removeChat(instanceId: "instance-progress-lifecycle", chatId: model.chat.id) - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 0) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 0, mode) continue } - XCTAssertEqual(failed, 0) - XCTAssertFalse(model.isUploadingAttachment) - let reference = try XCTUnwrap(model.pendingAttachments.first) + XCTAssertEqual(failed, 0, mode) + XCTAssertFalse(model.isUploadingAttachment, mode) + let reference = try XCTUnwrap(model.pendingAttachments.first, mode) if mode == "removal_wins" { await model.load(observeProgress: false) model.draft = "Hello" - let turnArrived = expectation(description: "turn receipt held") + let turnArrived = expectation(description: "\(mode): turn receipt held") AidenChatProgressLifecycleURLProtocol.holdNextRequest(endingIn: "/turns") { turnArrived.fulfill() } let sending = Task { await model.send() } await fulfillment(of: [turnArrived], timeout: 2) @@ -2632,22 +2639,22 @@ final class AidenChatTests: XCTestCase { await sending.value await gate.release() await removing.value - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 1) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 1, mode) continue } if mode == "consumed" || mode == "failed" { await model.load(observeProgress: false) model.draft = "Hello" - XCTAssertTrue(model.canSend) + XCTAssertTrue(model.canSend, mode) await model.send() - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.turnRequestCount, 1) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.turnRequestCount, 1, mode) } if mode == "consumed" { await cache.removeChat(instanceId: "instance-progress-lifecycle", chatId: model.chat.id) - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 0) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 0, mode) continue } - let arrived = expectation(description: "completed reference DELETE held") + let arrived = expectation(description: "\(mode): completed reference DELETE held") AidenChatProgressLifecycleURLProtocol.holdNextRequest(endingIn: "/attachments/" + reference.id) { arrived.fulfill() } defer { AidenChatProgressLifecycleURLProtocol.releaseHeldRequest() } var explicit: Task? @@ -2655,7 +2662,7 @@ final class AidenChatTests: XCTestCase { explicit = Task { await model.removeAttachment(reference) } await fulfillment(of: [arrived], timeout: 2) } - let early = expectation(description: "removal waits for completed upload cleanup") + let early = expectation(description: "\(mode): removal waits for completed upload cleanup") early.isInverted = true var held = true let removal = Task { @@ -2670,9 +2677,9 @@ final class AidenChatTests: XCTestCase { AidenChatProgressLifecycleURLProtocol.releaseHeldRequest() await removal.value await explicit?.value - XCTAssertTrue(model.pendingAttachments.isEmpty) - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.uploadRequestCount, 1) - XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 1) + XCTAssertTrue(model.pendingAttachments.isEmpty, mode) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.uploadRequestCount, 1, mode) + XCTAssertEqual(AidenChatProgressLifecycleURLProtocol.attachmentDeleteCount, 1, mode) } } diff --git a/scripts/check-ci-policy.test.mjs b/scripts/check-ci-policy.test.mjs index f7d16fdef..a4d5028da 100644 --- a/scripts/check-ci-policy.test.mjs +++ b/scripts/check-ci-policy.test.mjs @@ -202,6 +202,15 @@ test("desktop E2E and unit work are sharded with independent Apple and iOS check assert.ok(receipt.with.name.includes("${{ matrix.shard }}")); assert.ok(jobs.apple.steps.some((step) => step.run === "npm run test:native")); assert.ok(jobs.ios.steps.some((step) => step.run?.includes("xcodebuild build-for-testing"))); + const simulator = jobs["ios-simulator"]; + const simulatorTest = simulator.steps.find((step) => step.run?.includes("xcodebuild test")); + const simulatorResults = simulator.steps.find((step) => step.uses?.startsWith("actions/upload-artifact@")); + assert.ok(simulatorTest.run.includes("-resultBundlePath '${{ runner.temp }}/AidenOnTheGoSimulator.xcresult'")); + assert.equal(simulatorTest["continue-on-error"], undefined); + assert.equal(simulatorResults.if, "failure()"); + assert.equal(simulatorResults.uses, "actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02"); + assert.equal(simulatorResults.with.path, "${{ runner.temp }}/AidenOnTheGoSimulator.xcresult"); + assert.equal(simulatorResults.with["retention-days"], 7); assert.equal(jobs.verify.steps.filter((step) => step.run === "npm run build").length, 1); assert.ok(jobs.verify.steps.some((step) => step.run === "npm run test:e2e:diagnostics:production:run")); const manifest = JSON.parse(await readFile(new URL("../package.json", import.meta.url), "utf8")); From 0beec5804ad2612171fedf9d6775c4b131d6bea9 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Mon, 28 Sep 2026 18:28:26 -0400 Subject: [PATCH 10/11] test(tools): join owned cleanup between Electron smoke samples --- .memory/foreground-file-tool-bounds.md | 9 +++++++ .papercuts/troubleshooting.md | 17 +++++++++++++ scripts/smoke-foreground-file-tools.ts | 35 ++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/.memory/foreground-file-tool-bounds.md b/.memory/foreground-file-tool-bounds.md index 6b6cbefcd..467210cc1 100644 --- a/.memory/foreground-file-tool-bounds.md +++ b/.memory/foreground-file-tool-bounds.md @@ -91,3 +91,12 @@ reused exact iOS stream gate remains alongside main's catalog/publication tests. The Remote revision is unchanged by this branch. Advisor drift coverage passes; a broader direct CLI extension invocation lacked its required built dist/app, so its process-launch failure is not runtime validation. + +Integration smoke revealed that sequential caller cancellations can fill retained +cleanup slots under host load. The smoke now observes and joins original operation +promises before each next sample and after the deadline sample; the production +owner contract, caller timing, and process hard deadline are unchanged. Merged iOS +chat XCTest passed 214/214 on the selected iOS 27 simulator. +The observer retains late cleanup errors and asserts none; fault injection after +real worker termination confirms cleanup quarantine fails the smoke even after +caller cancellation has already settled. diff --git a/.papercuts/troubleshooting.md b/.papercuts/troubleshooting.md index aeb2382eb..14e75d6f9 100644 --- a/.papercuts/troubleshooting.md +++ b/.papercuts/troubleshooting.md @@ -1445,3 +1445,20 @@ reaped Electron, and removal of the temporary bundle/workspace/profile. - PR #287 local full suite (`/tmp/aiden-git-publish-full.log`, head `96f3e527`): `GitService retries a read across root replacement and discovers newly nested repositories` intermittently failed with "not a git repository" while another Git subprocess overlapped the rename. Earlier local pointer-race validation also exposed a missing `commondir` failure. Fixed the underlying changed-identity error path and made both tests deterministic; no timeout/retry increase. The cache-publication fixture now waits for the initial command batch before swapping, preventing an unrelated command failure from stranding its cache gate. These were local failures, so there is no hosted run URL. - PR #287 hosted run [36476435704, core-git job](https://github.com/sambitcreate/aiden-agent/actions/runs/36476435704/job/109111328876), head `2ce5e3e4`: `GitService retries config and linked common-directory changes during discovery` returned correct branches but observed three discoveries instead of the expected two. The deliberate missing-pointer fixture could also fail the parallel initial `--show-toplevel`, admitting a retry before pointer restoration. Gate pointer removal on the sibling command's completion; keep the exact two-discovery assertion and all behavioral checks. No job rerun or timeout/retry increase. Completed job logs are accessible while the overall run remains active using `gh api --allow-escape-sequences .../actions/jobs//logs`; `gh run view --log-failed` waits for the whole run. + +### Foreground Electron smoke cleanup admission (2026-09-28, PR #288) + +During main integration, the local pinned-Electron smoke overlapped iOS compilation +and its repeated cancellation loop rejected with “Filesystem operations are busy” +instead of the expected cancellation reason (`/tmp/foreground-integration-electron.log`). +The harness treated caller rejection as disposal even though bounded operation +owners intentionally retain pending I/O and worker cleanup. Observe and join the +original operation promises between samples, then yield to their admission-release +handler; retain the existing hard process deadline, production four-owner limit, +cancellation timing samples, and final no-MessagePort assertion. No blind retry or +fixed sleep is used to hide admission failure. +Astra review required retaining late ForegroundReadCleanupError values even after +removing settled promises from the observed set. The smoke now fails on any such +cleanup error. A temporary Worker.terminate wrapper that waited for real termination +then rejected reproduced the new assertion failure; removing the injection restores +the normal smoke. This proves a late quarantine error cannot be hidden by joining. diff --git a/scripts/smoke-foreground-file-tools.ts b/scripts/smoke-foreground-file-tools.ts index 12beda348..28b5a596d 100644 --- a/scripts/smoke-foreground-file-tools.ts +++ b/scripts/smoke-foreground-file-tools.ts @@ -4,6 +4,39 @@ import assert from "node:assert/strict"; import fs from "node:fs/promises"; import path from "node:path"; import { buildCodingTools } from "../main/services/coding-tools.js"; +import { ForegroundReadCleanupError, ForegroundReadOperations } from "../main/services/foreground-read-scope.js"; + +// Caller cancellation deliberately precedes cleanup. Observe the original +// operations so sequential smoke samples join cleanup rather than racing the +// four-owner admission limit. The runner's hard deadline still bounds this wait. +const pendingOperations = new Set>(); +const cleanupFailures: ForegroundReadCleanupError[] = []; +const originalRun = ForegroundReadOperations.prototype.run; +ForegroundReadOperations.prototype.run = function ( + signal: AbortSignal | undefined, + durationMs: number | undefined, + operation: (signal: AbortSignal) => Promise, + timeoutResult: () => T, +): Promise { + return originalRun.call(this, signal, durationMs, async (ownedSignal) => { + const pending = operation(ownedSignal); + pendingOperations.add(pending); + try { return await pending; } + catch (error) { + if (error instanceof ForegroundReadCleanupError) cleanupFailures.push(error); + throw error; + } + finally { pendingOperations.delete(pending); } + }, timeoutResult) as Promise; +}; + +async function settleForegroundOperations() { + await Promise.allSettled([...pendingOperations]); + // Let the scope's completion handler release its admission after cleanup. + await new Promise((resolve) => setImmediate(resolve)); + assert.equal(pendingOperations.size, 0); + assert.deepEqual(cleanupFailures, [], "Cancelled operations must release every owned resource."); +} async function smoke() { const [root, profile] = process.argv.slice(2); @@ -67,8 +100,10 @@ async function smoke() { clearTimeout(timer); } cancellationMs.push(performance.now() - start); + await settleForegroundOperations(); } assert.match(await invoke("grep", { pattern: "^(a+)+$" }), /search stopped after 5000 ms/); + await settleForegroundOperations(); assert.equal(await invoke("grep", { pattern: "missing" }), "[no matches]"); const resources = process .getActiveResourcesInfo() From e980e5993a4674739ccccf7e43ff9c65eb8523b2 Mon Sep 17 00:00:00 2001 From: Sambit Biswas Date: Tue, 29 Sep 2026 21:37:38 -0400 Subject: [PATCH 11/11] chore: drop .papercuts changes from PR Co-Authored-By: Claude Opus 5.5 --- .papercuts/troubleshooting.md | 47 ----------------------------------- 1 file changed, 47 deletions(-) diff --git a/.papercuts/troubleshooting.md b/.papercuts/troubleshooting.md index 16696871f..34320dfd2 100644 --- a/.papercuts/troubleshooting.md +++ b/.papercuts/troubleshooting.md @@ -1398,53 +1398,6 @@ because their native file-mutator test binary had not been built. Run - `node scripts/build-native-helpers.mjs --docker ...` bind-mounts the repo and leaves Linux ELF helpers in `build/native`. Rebuild the macOS helpers before you run local native tests, or they fail with `spawn ENOEXEC`. - After merging main, run `npm ci`. #71 added `bonjour-service`, and without it `tsc` fails. -## 2026-09-28 — Foreground file-tool performance fixtures - -- Node built-in ESM namespaces retain their bindings when a benchmark wraps the default fs/promises export. Call `syncBuiltinESMExports()` after instrumentation and restoration; otherwise byte counters misleadingly report zero. The clean 0.87.1 runner now reproduces the original 16 MiB read. -- A standalone Electron ESM smoke must externalize `electron` explicitly (tsconfig path resolution otherwise bundles its npm launcher), and register `app.whenReady().then(...)` without top-level-awaiting readiness. Electron waits for module evaluation before ready; top-level await deadlocks the fixture. The resulting fixed-source worker is validated in pinned Electron 43.1.1 / Node 24.18.0. - - -### Foreground pending-I/O review follow-up (2026-09-28, PR #288) - -Pullfrog found mixed-root brace glob arms and cancellation waiting on pending -filesystem reads. Cover syscall ownership as well as matcher termination: retain -admission through original I/O/cleanup, and fence late handle acquisitions. Worker -message ports are absent from Node `_getActiveHandles()` after its one-shot ready -listener is removed, even for a live idle worker. The initial lifecycle test failed -`0 !== 4`; corrected the oracle to await the actual Worker.terminate promises while -traversal stays deferred, then independently assert held admission and recovery. -This was a deterministic test-oracle failure, not a passing rerun or timeout change. - - -### Register standalone Electron assertions in CI (2026-09-28, PR #288) - -A final audit caught the foreground smoke's manual-only execution: assertion-based -smoke entries also need a package test command and a CI lane, even without a -`.test` suffix. Registered it in Desktop build and diagnostics. When moving the -esbuild CLI invocation into its API, retain explicit `external: ["electron"]`: -`packages: "external"` alone allowed repository TS path resolution to bundle the -npm launcher, producing “Dynamic require of child_process is not supported”. The -runner's hard deadline caught that pre-handler failure and removed fixtures; the -corrected registered smoke passes. SIGTERM validation also proved nonzero exit, -reaped Electron, and removal of the temporary bundle/workspace/profile. - -### Foreground Electron smoke cleanup admission (2026-09-28, PR #288) - -During main integration, the local pinned-Electron smoke overlapped iOS compilation -and its repeated cancellation loop rejected with “Filesystem operations are busy” -instead of the expected cancellation reason (`/tmp/foreground-integration-electron.log`). -The harness treated caller rejection as disposal even though bounded operation -owners intentionally retain pending I/O and worker cleanup. Observe and join the -original operation promises between samples, then yield to their admission-release -handler; retain the existing hard process deadline, production four-owner limit, -cancellation timing samples, and final no-MessagePort assertion. No blind retry or -fixed sleep is used to hide admission failure. -Astra review required retaining late ForegroundReadCleanupError values even after -removing settled promises from the observed set. The smoke now fails on any such -cleanup error. A temporary Worker.terminate wrapper that waited for real termination -then rejected reproduced the new assertion failure; removing the injection restores -the normal smoke. This proves a late quarantine error cannot be hidden by joining. - ## 2026-09-26 PR #121 merge of #246 (pi 0.87.1) - The CLI bundles main/services, so it has to use the same pi version as root. Bump `packages/cli` `@earendil-works/pi-coding-agent` to match, otherwise mixing 0.84 and 0.87 types breaks `tsc`. - In 0.87 chord is nested, not hoisted, so declare it directly in `packages/cli`. Its exports are import-only, so `require.resolve` in the build's external check fails even when chord is installed. The check needs an ESM resolve fallback.