From ab70e07cec9282eb5f0da711d4be557bfcc0ba8b Mon Sep 17 00:00:00 2001 From: dbarr5 Date: Sun, 27 Sep 2026 16:53:01 -0400 Subject: [PATCH] fix: bound nested instruction scan before chat turns --- src/commands/chat.ts | 1 + .../instructions/instruction_discovery.ts | 58 +++++++++++++++---- src/core/instructions/instruction_resolver.ts | 3 +- src/core/instructions/instruction_types.ts | 2 + src/core/skills/run_session.ts | 8 +++ src/core/skills/skill_bounds.ts | 4 ++ src/core/skills/skill_errors.ts | 1 + test/instruction_resolver.test.ts | 32 ++++++++++ 8 files changed, 98 insertions(+), 11 deletions(-) diff --git a/src/commands/chat.ts b/src/commands/chat.ts index a8c30775..7a19c86c 100644 --- a/src/commands/chat.ts +++ b/src/commands/chat.ts @@ -434,6 +434,7 @@ export async function runTurn( const opened = openRunSession({ projectRoot: ctx.flags.cwd, prompt, + allowIncompleteInstructionDiscovery: backend === "cloud", ...(skillOpts.explicitSkill ? { explicitSkill: skillOpts.explicitSkill } : {}), ...(skillOpts.noSkills ? { noSkills: true } : {}), }); diff --git a/src/core/instructions/instruction_discovery.ts b/src/core/instructions/instruction_discovery.ts index ef36a74d..baf60323 100644 --- a/src/core/instructions/instruction_discovery.ts +++ b/src/core/instructions/instruction_discovery.ts @@ -5,8 +5,10 @@ // fetches includes, and caps file size and source count honestly. import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync } from "node:fs"; +import type { Dirent } from "node:fs"; import { join, relative, resolve, sep } from "node:path"; import { createHash } from "node:crypto"; +import { performance } from "node:perf_hooks"; import { configDir } from "../config.js"; import { SKILL_BOUNDS } from "../skills/skill_bounds.js"; import type { InstructionSource, InstructionSourceKind } from "./instruction_types.js"; @@ -87,21 +89,49 @@ export function parseCursorGlobs(content: string): { globs: readonly string[] | return { globs: globs.length ? globs : null, warnings, body }; } -/** Locate nested AGENTS.md files, bounded by depth; skips dot/vendor dirs. */ -function findNestedAgents(projectRoot: string): string[] { +/** Locate nested AGENTS.md files with explicit work and wall-clock limits. */ +function findNestedAgents(projectRoot: string): { paths: string[]; complete: boolean; reason: string } { const found: string[] = []; const skip = new Set(["node_modules", "dist", "build", "vendor", "target", ".git"]); + const started = performance.now(); + let directories = 0; + let entriesSeen = 0; + let reason = ""; + const overBudget = (): boolean => { + if (reason) return true; + if (performance.now() - started >= SKILL_BOUNDS.maxNestedInstructionScanMs) { + reason = `nested AGENTS.md scan exceeded ${SKILL_BOUNDS.maxNestedInstructionScanMs} ms`; + return true; + } + if (directories >= SKILL_BOUNDS.maxNestedInstructionDirectories) { + reason = `nested AGENTS.md scan reached ${SKILL_BOUNDS.maxNestedInstructionDirectories} directories`; + return true; + } + return false; + }; const walk = (dir: string, depth: number): void => { if (depth > SKILL_BOUNDS.maxNestedInstructionDepth) return; - let entries: string[]; + if (overBudget()) return; + directories++; + let entries: Dirent[]; try { - entries = readdirSync(dir); + entries = readdirSync(dir, { withFileTypes: true }); } catch { return; } - for (const entry of entries.sort()) { - if (entry.startsWith(".") || skip.has(entry)) continue; - const full = join(dir, entry); + // A single directory can contain tens of thousands of names. Do not sort + // or stat any of them when the remaining entry budget cannot cover it. + // The readdirSync call itself is one synchronous filesystem operation; the + // incomplete marker below makes this limit explicit to callers. + if (entriesSeen + entries.length > SKILL_BOUNDS.maxNestedInstructionEntries) { + reason = `nested AGENTS.md scan reached ${SKILL_BOUNDS.maxNestedInstructionEntries} entries`; + return; + } + for (const entry of entries.sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0)) { + if (overBudget()) return; + entriesSeen++; + if (entry.name.startsWith(".") || skip.has(entry.name) || !entry.isDirectory()) continue; + const full = join(dir, entry.name); let isDirectory = false; try { isDirectory = lstatSync(full).isDirectory(); @@ -116,12 +146,13 @@ function findNestedAgents(projectRoot: string): string[] { } }; walk(projectRoot, 1); - return found; + return { paths: found, complete: !reason, reason }; } export function discoverInstructionSources(projectRoot: string): { sources: InstructionSource[]; skipped: { path: string; reason: string }[]; + nestedScanComplete: boolean; } { const root = resolve(projectRoot); const candidates: DiscoveredFile[] = []; @@ -132,7 +163,8 @@ export function discoverInstructionSources(projectRoot: string): { addIfPresent("aether-project", join(root, ".aether", "instructions.md")); addIfPresent("agents-root", join(root, "AGENTS.md")); - for (const nested of findNestedAgents(root)) { + const nestedScan = findNestedAgents(root); + for (const nested of nestedScan.paths) { const scopeDir = relative(root, join(nested, "..")).split(sep).join("/"); candidates.push({ kind: "agents-nested", path: nested, scopeDir, globs: null, warnings: [] }); } @@ -156,6 +188,12 @@ export function discoverInstructionSources(projectRoot: string): { const sources: InstructionSource[] = []; const skipped: { path: string; reason: string }[] = []; + if (!nestedScan.complete) { + skipped.push({ + path: "**/AGENTS.md", + reason: nestedScan.reason + "; nested project rules may be missing — start in a smaller project directory", + }); + } for (const candidate of candidates) { if (sources.length >= SKILL_BOUNDS.maxInstructionSources) { @@ -201,5 +239,5 @@ export function discoverInstructionSources(projectRoot: string): { }); } - return { sources, skipped }; + return { sources, skipped, nestedScanComplete: nestedScan.complete }; } diff --git a/src/core/instructions/instruction_resolver.ts b/src/core/instructions/instruction_resolver.ts index 14a5beed..d0c70ac2 100644 --- a/src/core/instructions/instruction_resolver.ts +++ b/src/core/instructions/instruction_resolver.ts @@ -150,7 +150,7 @@ export function runScopedSources(sources: readonly InstructionSource[]): Instruc /** Build the full graph for a project: discovery + run-scope conflict pass. */ export function resolveInstructionGraph(projectRoot: string): InstructionGraph { - const { sources, skipped } = discoverInstructionSources(projectRoot); + const { sources, skipped, nestedScanComplete } = discoverInstructionSources(projectRoot); // runScopedSources drops a source whose glob frontmatter would not parse: its // scope is unknown, so applying it to the whole run would be a guess about // which files it governs. Dropping it is right; dropping it SILENTLY is not — @@ -165,6 +165,7 @@ export function resolveInstructionGraph(projectRoot: string): InstructionGraph { sources, conflicts: detectConflicts(runScopedSources(sources)), skipped: [...skipped, ...unparsable], + nestedScanComplete, }; } diff --git a/src/core/instructions/instruction_types.ts b/src/core/instructions/instruction_types.ts index e5d182b1..6340d154 100644 --- a/src/core/instructions/instruction_types.ts +++ b/src/core/instructions/instruction_types.ts @@ -66,4 +66,6 @@ export interface InstructionGraph { conflicts: readonly InstructionConflict[]; /** Sources discovered but skipped, with a visible reason (over cap, bad encoding). */ skipped: readonly { path: string; reason: string }[]; + /** False when nested AGENTS.md discovery stopped at a scan limit. */ + nestedScanComplete: boolean; } diff --git a/src/core/skills/run_session.ts b/src/core/skills/run_session.ts index abd32654..7cd2b799 100644 --- a/src/core/skills/run_session.ts +++ b/src/core/skills/run_session.ts @@ -51,6 +51,8 @@ export interface RunSessionOptions { noSkills?: boolean; /** Injected for tests. */ builtinRoot?: string; + /** Hosted cloud chat has no local tool authority; it may proceed with a visible scan warning. */ + allowIncompleteInstructionDiscovery?: boolean; /** * The operator's live permissions. Skills never contribute to this set — it * is passed in so a caller can narrow it, and so no cached policy can widen @@ -392,6 +394,12 @@ export function openRunSession(options: RunSessionOptions): OpenRunSession { ...(options.noSkills ? { noSkills: true } : {}), ...(options.builtinRoot ? { builtinRoot: options.builtinRoot } : {}), }); + if (!session.instructionGraph.nestedScanComplete && !options.allowIncompleteInstructionDiscovery) { + return refused({ + code: "skill.instruction_scan_incomplete", + detail: "Nested AGENTS.md discovery stopped at a scan limit. Start from a smaller project directory so all local rules can be checked before local tools run", + }); + } // A skill the user NAMED that requires authority this session does not hold // does not run at all: the user asked for it, so a silent downgrade would // run something other than what was asked for. diff --git a/src/core/skills/skill_bounds.ts b/src/core/skills/skill_bounds.ts index cc5ebe0f..fbf79679 100644 --- a/src/core/skills/skill_bounds.ts +++ b/src/core/skills/skill_bounds.ts @@ -32,6 +32,10 @@ export const SKILL_BOUNDS = { maxContextPacketBytes: 512 * 1024, /** Nested AGENTS.md depth below the project root. */ maxNestedInstructionDepth: 6, + /** Bound the synchronous nested-rule walk before any chat pulse or request. */ + maxNestedInstructionDirectories: 256, + maxNestedInstructionEntries: 2048, + maxNestedInstructionScanMs: 500, /** One instruction file. */ maxInstructionFileBytes: 64 * 1024, /** Description / name / trigger phrase field lengths. */ diff --git a/src/core/skills/skill_errors.ts b/src/core/skills/skill_errors.ts index 0c2b6fb0..e87cbf35 100644 --- a/src/core/skills/skill_errors.ts +++ b/src/core/skills/skill_errors.ts @@ -13,6 +13,7 @@ export const SKILL_ERROR_CODES = [ "skill.dependency_missing", "skill.dependency_cycle", "skill.context_budget_exceeded", + "skill.instruction_scan_incomplete", "skill.tool_not_declared", "skill.permission_unavailable", "skill.permission_denied", diff --git a/test/instruction_resolver.test.ts b/test/instruction_resolver.test.ts index 1d1b2075..beee965e 100644 --- a/test/instruction_resolver.test.ts +++ b/test/instruction_resolver.test.ts @@ -13,6 +13,7 @@ import { sourceAppliesTo, } from "../src/core/instructions/instruction_resolver.js"; import { SKILL_BOUNDS } from "../src/core/skills/skill_bounds.js"; +import { openRunSession } from "../src/core/skills/run_session.js"; import type { InstructionSource } from "../src/core/instructions/instruction_types.js"; function withEnv(key: string, value: string, fn: () => T): T { @@ -66,6 +67,37 @@ test("nested AGENTS.md scopes to its subtree only", () => { }); }); +test("a wide project stops nested discovery visibly and refuses local tool authority", () => { + const root = makeProject(); + writeFileSync(join(root, "AGENTS.md"), "Keep the root rule.\n"); + const userDir = mkdtempSync(join(tmpdir(), "aether-cfg-")); + writeFileSync(join(userDir, "instructions.md"), "Keep the user rule.\n"); + // The old depth-only walk could spend minutes traversing a home directory + // before even starting the chat pulse. This exceeds the directory budget + // without depending on a machine-specific stopwatch threshold. + for (let i = 0; i <= SKILL_BOUNDS.maxNestedInstructionDirectories; i++) { + mkdirSync(join(root, `child-${String(i).padStart(4, "0")}`)); + } + withEnv("AETHER_CONFIG_DIR", userDir, () => { + const graph = resolveInstructionGraph(root); + assert.equal(graph.nestedScanComplete, false); + assert.ok(graph.sources.some((source) => source.kind === "agents-root" && source.content.includes("Keep the root rule"))); + assert.ok(graph.sources.some((source) => source.kind === "aether-user" && source.content.includes("Keep the user rule"))); + assert.ok(graph.skipped.some((item) => item.path === "**/AGENTS.md" && item.reason.includes("may be missing"))); + + const local = openRunSession({ projectRoot: root, prompt: "edit files" }); + assert.equal(local.ok, false); + if (!local.ok) assert.equal(local.refusal.code, "skill.instruction_scan_incomplete"); + + const cloudChat = openRunSession({ projectRoot: root, prompt: "reply 1", allowIncompleteInstructionDiscovery: true }); + assert.equal(cloudChat.ok, true, cloudChat.ok ? "" : cloudChat.lines.join("\n")); + if (cloudChat.ok) { + assert.equal(cloudChat.run.hasWarnings, true); + assert.match(cloudChat.run.headerLines.join("\n"), /nested project rules may be missing/); + } + }); +}); + test("precedence: canonical Aether project instruction beats root AGENTS.md", () => { const root = makeProject(); writeFileSync(join(root, "AGENTS.md"), "Always run `npm test`.\n");