Skip to content

Commit e08ae90

Browse files
TheGreatAxiosTheGreatAxios
committed
feat(subagent): verify requires_tools pre-spawn with fail-closed preflight
1 parent b3a95ad commit e08ae90

10 files changed

Lines changed: 1156 additions & 5 deletions

‎src/agent/profiles-snapshot.test.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { mkdtemp, writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
6+
import {
7+
currentProfileSnapshotRevision,
8+
loadAgentProfiles,
9+
loadAgentProfilesWithDiagnostics,
10+
} from "./profiles.js";
11+
12+
async function makeAgentsDir(files: Record<string, string>): Promise<string> {
13+
const dir = await mkdtemp(join(tmpdir(), "cl9476-profiles-"));
14+
for (const [name, content] of Object.entries(files)) {
15+
await writeFile(join(dir, name), content);
16+
}
17+
return dir;
18+
}
19+
20+
const VALID_PROFILE = JSON.stringify({
21+
id: "local-reader",
22+
systemPromptRole: "You read files.",
23+
capabilities: { mode: "allow", tools: ["read_file"] },
24+
});
25+
26+
describe("loadAgentProfilesWithDiagnostics", () => {
27+
test("malformed file is reported and never poisons the load", async () => {
28+
const dir = await makeAgentsDir({
29+
"good.json": VALID_PROFILE,
30+
"broken.json": "{ not valid json",
31+
"notes.txt": "ignored, not a profile extension",
32+
});
33+
34+
const { profiles, diagnostics } =
35+
await loadAgentProfilesWithDiagnostics(dir);
36+
37+
expect(profiles.some((p) => p.id === "local-reader")).toBe(true);
38+
expect(diagnostics.malformed).toHaveLength(1);
39+
expect(diagnostics.malformed[0]?.path).toBe(join(dir, "broken.json"));
40+
expect(diagnostics.malformed[0]?.reason).toBe("invalid JSON");
41+
expect(typeof diagnostics.revision).toBe("number");
42+
});
43+
44+
test("schema-invalid file is reported with its reason", async () => {
45+
const dir = await makeAgentsDir({
46+
"bad-shape.json": JSON.stringify({ id: 42 }),
47+
});
48+
49+
const { profiles, diagnostics } =
50+
await loadAgentProfilesWithDiagnostics(dir);
51+
52+
expect(profiles.some((p) => p.id === "local-reader")).toBe(false);
53+
expect(diagnostics.malformed).toHaveLength(1);
54+
expect(diagnostics.malformed[0]?.reason).toBe("schema validation failed");
55+
});
56+
57+
test("loadAgentProfiles delegates and resolves identically", async () => {
58+
const dir = await makeAgentsDir({
59+
"good.json": VALID_PROFILE,
60+
"broken.json": "{ nope",
61+
});
62+
63+
const viaDiagnostics = await loadAgentProfilesWithDiagnostics(dir);
64+
const direct = await loadAgentProfiles(dir);
65+
66+
expect(direct.map((p) => p.id).sort()).toEqual(
67+
viaDiagnostics.profiles.map((p) => p.id).sort(),
68+
);
69+
});
70+
71+
test("every load bumps the snapshot revision", async () => {
72+
const dir = await makeAgentsDir({ "good.json": VALID_PROFILE });
73+
const before = currentProfileSnapshotRevision();
74+
75+
const first = await loadAgentProfilesWithDiagnostics(dir);
76+
const second = await loadAgentProfilesWithDiagnostics(dir);
77+
78+
expect(second.diagnostics.revision).toBeGreaterThan(
79+
first.diagnostics.revision,
80+
);
81+
expect(first.diagnostics.revision).toBeGreaterThan(before);
82+
expect(currentProfileSnapshotRevision()).toBe(second.diagnostics.revision);
83+
});
84+
85+
test("missing directory resolves defaults with no malformed entries", async () => {
86+
const { profiles, diagnostics } = await loadAgentProfilesWithDiagnostics(
87+
join(tmpdir(), "cl9476-does-not-exist"),
88+
);
89+
90+
expect(profiles.length).toBeGreaterThan(0);
91+
expect(diagnostics.malformed).toEqual([]);
92+
});
93+
});

‎src/agent/profiles.ts‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,28 @@ function isENOENT(err: unknown): boolean {
7171
// DIRECTOR_IDS, which are reserved and skipped at load (CL-7015).
7272
const registry: AgentProfile[] = [...defaultPlugin.agents];
7373

74+
// Load diagnostics: additive over loadAgentProfiles. `revision` stamps the
75+
// profile snapshot a dispatch was verified against (agent-fleet records it on
76+
// the session); `malformed` names local files that failed to load and why.
77+
// A malformed file never blocks the load — it fails closed only when a
78+
// dispatch's requires_tools preflight must verify against it (CL-9476).
79+
export interface MalformedAgentProfile {
80+
path: string;
81+
reason: string;
82+
}
83+
84+
export interface AgentProfileDiagnostics {
85+
revision: number;
86+
malformed: MalformedAgentProfile[];
87+
}
88+
89+
let profileSnapshotRevision = 0;
90+
91+
/** Revision of the most recent profile snapshot load. */
92+
export function currentProfileSnapshotRevision(): number {
93+
return profileSnapshotRevision;
94+
}
95+
7496
// Merge a profile into a list: replace a same-id entry or append. Used to layer
7597
// profiles by precedence (defaults < plugin < local).
7698
function mergeProfileInto(list: AgentProfile[], profile: AgentProfile): void {
@@ -95,6 +117,28 @@ export async function loadAgentProfiles(
95117
dir: string,
96118
extraProfiles: AgentProfile[] = [],
97119
): Promise<AgentProfile[]> {
120+
return (await loadAgentProfilesWithDiagnostics(dir, extraProfiles)).profiles;
121+
}
122+
123+
/**
124+
* loadAgentProfiles plus load diagnostics. Additive: profiles resolve
125+
* exactly as before; unreadable/unparseable/invalid local files are named in
126+
* `diagnostics.malformed` instead of skipped silently. Every call bumps the
127+
* snapshot revision a dispatch stamps on its session record.
128+
*/
129+
export async function loadAgentProfilesWithDiagnostics(
130+
dir: string,
131+
extraProfiles: AgentProfile[] = [],
132+
): Promise<{ profiles: AgentProfile[]; diagnostics: AgentProfileDiagnostics }> {
133+
profileSnapshotRevision += 1;
134+
const revision = profileSnapshotRevision;
135+
const malformed: MalformedAgentProfile[] = [];
136+
const done = (
137+
profiles: AgentProfile[],
138+
): { profiles: AgentProfile[]; diagnostics: AgentProfileDiagnostics } => ({
139+
profiles,
140+
diagnostics: { revision, malformed },
141+
});
98142
let entries: string[];
99143
try {
100144
entries = await readdir(dir);
@@ -105,7 +149,7 @@ export async function loadAgentProfiles(
105149
if (isReservedDirectorProfile(p)) continue;
106150
mergeProfileInto(merged, p);
107151
}
108-
return merged;
152+
return done(merged);
109153
}
110154
throw err;
111155
}
@@ -121,16 +165,24 @@ export async function loadAgentProfiles(
121165
try {
122166
raw = await readFile(filePath, "utf8");
123167
} catch {
168+
malformed.push({ path: filePath, reason: "unreadable file" });
124169
continue;
125170
}
126171
let parsed: unknown;
127172
try {
128173
parsed = isJSON ? JSON.parse(raw) : Bun.YAML.parse(raw);
129174
} catch {
175+
malformed.push({
176+
path: filePath,
177+
reason: isJSON ? "invalid JSON" : "invalid YAML",
178+
});
130179
continue;
131180
}
132181
const result = AgentProfileSchema(parsed);
133-
if (result instanceof type.errors) continue;
182+
if (result instanceof type.errors) {
183+
malformed.push({ path: filePath, reason: "schema validation failed" });
184+
continue;
185+
}
134186
const profile = result as AgentProfile;
135187
if (isReservedDirectorProfile(profile)) continue;
136188
// Resolve systemPromptPath relative to this directory. The file content
@@ -158,5 +210,5 @@ export async function loadAgentProfiles(
158210
mergeProfileInto(merged, profile);
159211
}
160212
for (const profile of local) mergeProfileInto(merged, profile);
161-
return merged;
213+
return done(merged);
162214
}

‎src/subagent/agent-fleet-requires-tools.test.ts‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import {
4+
createFleetMailbox,
5+
createSpawnAgentTool,
6+
type AgentFleetDeps,
7+
} from "./agent-fleet.js";
8+
import { unlimitedAdmissionQueue } from "./admission.js";
9+
import { createPermissionGate } from "../permission/gate.js";
10+
import { NOOP_TELEMETRY, type TelemetryEvent } from "../telemetry/index.js";
11+
import type { AgentProfile } from "../agent/profiles.js";
12+
import { createSubAgentSessionStore } from "./session-store.js";
13+
import type { RunSubAgentParams, RunSubAgentResult } from "./types.js";
14+
15+
const testPermissionGate = createPermissionGate({
16+
approvals: [],
17+
interactive: false,
18+
skipPermissions: true,
19+
reactorGated: false,
20+
});
21+
22+
const provider = {
23+
providerName: "test-provider",
24+
baseURL: "http://localhost",
25+
model: "test-model",
26+
};
27+
28+
const READ_ONLY_PROFILE: AgentProfile = {
29+
id: "read-only-worker",
30+
systemPromptRole: "You read files.",
31+
capabilities: { mode: "allow", tools: ["read_file"] },
32+
};
33+
34+
function makeDeps(
35+
run: (params: RunSubAgentParams) => Promise<RunSubAgentResult>,
36+
capturedTelemetry: TelemetryEvent[],
37+
): AgentFleetDeps {
38+
const sessions = createSubAgentSessionStore();
39+
return {
40+
permissionGate: testPermissionGate,
41+
cwd: "/tmp",
42+
getWorkdirBase: () => "/tmp/workdir",
43+
provider,
44+
run,
45+
sessions,
46+
fleetRecords: createFleetMailbox(sessions),
47+
admission: unlimitedAdmissionQueue(),
48+
profiles: [READ_ONLY_PROFILE],
49+
telemetry: {
50+
...NOOP_TELEMETRY,
51+
capture: (event: TelemetryEvent) => {
52+
capturedTelemetry.push(event);
53+
},
54+
},
55+
};
56+
}
57+
58+
async function callSpawn(
59+
tool: ReturnType<typeof createSpawnAgentTool>,
60+
args: Record<string, unknown>,
61+
): Promise<{ content: string; isError?: boolean }> {
62+
if (tool.kind !== "full") throw new Error("expected full tool");
63+
const result = await tool.handler(
64+
{
65+
id: `spawn-${Math.random()}`,
66+
name: "spawn_agent",
67+
arguments: args,
68+
},
69+
new AbortController().signal,
70+
);
71+
const content =
72+
typeof result.content === "string"
73+
? result.content
74+
: JSON.stringify(result.content);
75+
return {
76+
content,
77+
...(result.isError !== undefined ? { isError: result.isError } : {}),
78+
};
79+
}
80+
81+
describe("spawn_agent requires_tools preflight", () => {
82+
test("rejects a tool outside the allowlist with no session, telemetry, or run", async () => {
83+
const telemetry: TelemetryEvent[] = [];
84+
let runCalled = false;
85+
const deps = makeDeps(async () => {
86+
runCalled = true;
87+
return { report: "done" };
88+
}, telemetry);
89+
const spawn = createSpawnAgentTool(deps);
90+
91+
const result = await callSpawn(spawn, {
92+
description: "shell job",
93+
prompt: "run a command",
94+
agent: "read-only-worker",
95+
requires_tools: ["run_shell"],
96+
});
97+
98+
expect(result.isError).toBe(true);
99+
expect(result.content.startsWith("Error:")).toBe(true);
100+
expect(result.content).toContain("run_shell");
101+
expect(result.content).toContain("Re-dispatch");
102+
expect(result.content).not.toContain("continuable");
103+
expect(runCalled).toBe(false);
104+
expect(deps.sessions.list()).toEqual([]);
105+
expect(telemetry).toEqual([]);
106+
});
107+
108+
test("rejects an unknown tool with a did-you-mean hint and no session", async () => {
109+
const telemetry: TelemetryEvent[] = [];
110+
let runCalled = false;
111+
const deps = makeDeps(async () => {
112+
runCalled = true;
113+
return { report: "done" };
114+
}, telemetry);
115+
const spawn = createSpawnAgentTool(deps);
116+
117+
const result = await callSpawn(spawn, {
118+
description: "typo job",
119+
prompt: "do it",
120+
agent: "read-only-worker",
121+
requires_tools: ["run_shel"],
122+
});
123+
124+
expect(result.isError).toBe(true);
125+
expect(result.content).toContain('Did you mean "run_shell"?');
126+
expect(runCalled).toBe(false);
127+
expect(deps.sessions.list()).toEqual([]);
128+
expect(telemetry).toEqual([]);
129+
});
130+
131+
test("accepts a mounted tool and stamps the session record", async () => {
132+
const telemetry: TelemetryEvent[] = [];
133+
let seenRequires: readonly string[] | undefined;
134+
const deps = makeDeps(async (params) => {
135+
seenRequires = params.requiresTools;
136+
return { report: "done" };
137+
}, telemetry);
138+
const spawn = createSpawnAgentTool(deps);
139+
140+
const result = await callSpawn(spawn, {
141+
description: "read job",
142+
prompt: "read a file",
143+
agent: "read-only-worker",
144+
requires_tools: ["read_file"],
145+
});
146+
147+
expect(result.isError).not.toBe(true);
148+
const body = JSON.parse(result.content) as {
149+
agent_id: string;
150+
status: string;
151+
};
152+
const session = deps.sessions.get(body.agent_id);
153+
expect(session?.requiresTools).toEqual(["read_file"]);
154+
expect(typeof session?.snapshotRevision).toBe("number");
155+
expect(seenRequires).toEqual(["read_file"]);
156+
expect(telemetry).toContain("subagent_start");
157+
});
158+
159+
test("aliases collapse at dispatch (shell stamps run_shell)", async () => {
160+
const telemetry: TelemetryEvent[] = [];
161+
const deps = makeDeps(async () => ({ report: "done" }), telemetry);
162+
const spawn = createSpawnAgentTool(deps);
163+
164+
const result = await callSpawn(spawn, {
165+
description: "alias read job",
166+
prompt: "read a file",
167+
agent: "read-only-worker",
168+
requires_tools: ["read"],
169+
});
170+
171+
expect(result.isError).not.toBe(true);
172+
const body = JSON.parse(result.content) as { agent_id: string };
173+
expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([
174+
"read_file",
175+
]);
176+
});
177+
178+
test("omitted requires_tools leaves the session unstamped", async () => {
179+
const telemetry: TelemetryEvent[] = [];
180+
const deps = makeDeps(async () => ({ report: "done" }), telemetry);
181+
const spawn = createSpawnAgentTool(deps);
182+
183+
const result = await callSpawn(spawn, {
184+
description: "plain job",
185+
prompt: "read a file",
186+
agent: "read-only-worker",
187+
});
188+
189+
expect(result.isError).not.toBe(true);
190+
const body = JSON.parse(result.content) as { agent_id: string };
191+
const session = deps.sessions.get(body.agent_id);
192+
expect(session?.requiresTools).toBeUndefined();
193+
expect(session?.snapshotRevision).toBeUndefined();
194+
});
195+
});

0 commit comments

Comments
 (0)