Skip to content

Commit a4fcce5

Browse files
committed
fix(subagent): thread grant_request_id through ask_director leaf
1 parent 72c5654 commit a4fcce5

3 files changed

Lines changed: 249 additions & 0 deletions

File tree

‎src/permission/worker-grant.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,71 @@ describe("WorkerGrantStore lifecycle", () => {
328328
]);
329329
});
330330

331+
describe("two-denies correlation", () => {
332+
function twoDenies() {
333+
const store = new WorkerGrantStore();
334+
const envelopeA = store.register(
335+
createDeniedCallEnvelope(
336+
descriptor({ callId: "call-A", args: { command: "npm test" } }),
337+
),
338+
);
339+
const envelopeB = store.register(
340+
createDeniedCallEnvelope(
341+
descriptor({ callId: "call-B", args: { command: "npm run lint" } }),
342+
),
343+
);
344+
return { store, envelopeA, envelopeB };
345+
}
346+
347+
test("named ask binds its exact denial, not the first pending", () => {
348+
const { store, envelopeA, envelopeB } = twoDenies();
349+
expect(
350+
store.attachToAsk("worker-1", "ask-B", envelopeB.requestId, 1_000_001),
351+
).toBe(envelopeB);
352+
expect(envelopeB.questionId).toBe("ask-B");
353+
expect(envelopeA.questionId).toBeUndefined();
354+
expect(store.byQuestion("ask-B")).toBe(envelopeB);
355+
});
356+
357+
test("unnamed ask keeps the legacy first-pending bind", () => {
358+
const { store, envelopeA, envelopeB } = twoDenies();
359+
expect(store.attachToAsk("worker-1", "ask-x", undefined, 1_000_001)).toBe(
360+
envelopeA,
361+
);
362+
expect(envelopeA.questionId).toBe("ask-x");
363+
expect(envelopeB.questionId).toBeUndefined();
364+
});
365+
366+
test("bogus id fails closed with no fallback to another denial", () => {
367+
const { store, envelopeA, envelopeB } = twoDenies();
368+
expect(
369+
store.attachToAsk("worker-1", "ask-x", "not-a-real-id", 1_000_001),
370+
).toBeUndefined();
371+
expect(envelopeA.questionId).toBeUndefined();
372+
expect(envelopeB.questionId).toBeUndefined();
373+
expect(store.byQuestion("ask-x")).toBeUndefined();
374+
});
375+
376+
test("cross-session id fails closed with no fallback", () => {
377+
const { store, envelopeA, envelopeB } = twoDenies();
378+
const other = store.register(
379+
createDeniedCallEnvelope(
380+
descriptor({
381+
callId: "call-other",
382+
args: { command: "npm run build" },
383+
workerSessionId: "worker-9",
384+
}),
385+
),
386+
);
387+
expect(
388+
store.attachToAsk("worker-1", "ask-x", other.requestId, 1_000_001),
389+
).toBeUndefined();
390+
expect(envelopeA.questionId).toBeUndefined();
391+
expect(envelopeB.questionId).toBeUndefined();
392+
expect(other.questionId).toBeUndefined();
393+
});
394+
});
395+
331396
test("audit trail orders deny → ask → consume", () => {
332397
const store = new WorkerGrantStore();
333398
const envelope = store.register(createDeniedCallEnvelope(descriptor()));
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/**
2+
* run.ts must thread grant_request_id from the ask_director tool call through
3+
* the leaf handler into handleAskDirector and out the askDirectorPort.
4+
* Dropping it at the schema or the handler silently forces every production
5+
* ask down the legacy first-pending path, so this test drives the real
6+
* handler path end to end — unit tests on handleAskDirector alone cannot
7+
* see the two call sites this covers.
8+
*/
9+
import { describe, expect, test } from "bun:test";
10+
import { mkdtemp } from "node:fs/promises";
11+
import { tmpdir } from "node:os";
12+
import { join } from "node:path";
13+
14+
import type { DirectorFactory } from "@intx/agent";
15+
16+
import { withMockedModuleDuring } from "../../tests/helpers/mock-module.js";
17+
import { createPermissionGate } from "../permission/gate.js";
18+
import type { RunSubAgentParams } from "./types.js";
19+
20+
const testPermissionGate = createPermissionGate({
21+
approvals: [],
22+
interactive: false,
23+
skipPermissions: true,
24+
reactorGated: false,
25+
});
26+
27+
function createHangingStubAgent() {
28+
return {
29+
async send(_content: string, optsSend?: { signal?: AbortSignal }) {
30+
return await new Promise((_, reject) => {
31+
if (optsSend?.signal?.aborted === true) {
32+
reject(
33+
optsSend.signal.reason instanceof Error
34+
? optsSend.signal.reason
35+
: new Error("aborted"),
36+
);
37+
return;
38+
}
39+
optsSend?.signal?.addEventListener(
40+
"abort",
41+
() => {
42+
const reason = optsSend.signal?.reason;
43+
reject(reason instanceof Error ? reason : new Error("aborted"));
44+
},
45+
{ once: true },
46+
);
47+
});
48+
},
49+
stream: () =>
50+
(async function* () {
51+
yield* [];
52+
})(),
53+
deliver: () => undefined,
54+
close: async () => undefined,
55+
setSource: () => undefined,
56+
setSources: () => undefined,
57+
history: async () => [],
58+
checkpoints: async () => [],
59+
readAt: async () => [],
60+
blobReader: {},
61+
};
62+
}
63+
64+
describe("runSubAgent ask_director grant_request_id threading", () => {
65+
test("leaf forwards grant_request_id to the port; absent stays legacy", async () => {
66+
const cwd = await mkdtemp(join(tmpdir(), "cl9475-ask-grant-"));
67+
const registered: {
68+
question: string;
69+
questionId: string;
70+
grantRequestId?: string;
71+
}[] = [];
72+
let capturedAskHandler:
73+
| ((
74+
rawArgs: Record<string, unknown>,
75+
signal: AbortSignal,
76+
) => Promise<string> | string)
77+
| undefined;
78+
let capturedFactory: DirectorFactory | undefined;
79+
let handles: { close: (ms?: number) => Promise<void> } | undefined;
80+
81+
const outcome = await withMockedModuleDuring(
82+
import.meta.resolve("@intx/agent"),
83+
(real: typeof import("@intx/agent")) => ({
84+
...real,
85+
defineDirector: (opts: Parameters<typeof real.defineDirector>[0]) => {
86+
capturedFactory = opts.factory;
87+
return real.defineDirector(opts);
88+
},
89+
stringTool: (args: Parameters<typeof real.stringTool>[0]) => {
90+
const tool = real.stringTool(args);
91+
if (args.definition.name === "ask_director") {
92+
capturedAskHandler = args.handler;
93+
}
94+
return tool;
95+
},
96+
}),
97+
async () =>
98+
await withMockedModuleDuring(
99+
import.meta.resolve("../agent/live-tool-dispatch.js"),
100+
(real: typeof import("../agent/live-tool-dispatch.js")) => ({
101+
...real,
102+
createAgentWithLiveToolDispatch: async () =>
103+
createHangingStubAgent() as unknown as Awaited<
104+
ReturnType<typeof real.createAgentWithLiveToolDispatch>
105+
>,
106+
}),
107+
async () => {
108+
const { runSubAgent } = await import("./run.js");
109+
110+
const params: RunSubAgentParams = {
111+
cwd,
112+
workdirBase: join(cwd, ".ctx"),
113+
permissionGate: testPermissionGate,
114+
provider: {
115+
providerName: "test",
116+
baseURL: "http://localhost",
117+
model: "test-model",
118+
},
119+
description: "ask_director grant threading",
120+
prompt: "hold for ask_director",
121+
persist: true,
122+
tier: "leaf",
123+
askDirectorPort: {
124+
register: (input) => {
125+
registered.push({ ...input });
126+
return Promise.resolve(`answer:${registered.length}`);
127+
},
128+
cancel: () => undefined,
129+
},
130+
onAgentReady: (h) => {
131+
handles = h;
132+
},
133+
};
134+
135+
const runPromise = runSubAgent(params);
136+
for (let i = 0; i < 500 && handles === undefined; i++) {
137+
await new Promise((resolve) => setTimeout(resolve, 1));
138+
}
139+
if (handles === undefined)
140+
throw new Error("onAgentReady never fired");
141+
if (capturedFactory === undefined)
142+
throw new Error("defineDirector factory not seen");
143+
if (capturedAskHandler === undefined)
144+
throw new Error("ask_director tool was not mounted");
145+
146+
// Exercise the factory so the leaf tool wiring is live.
147+
capturedFactory({}, {} as never, {
148+
systemPrompt: "system",
149+
toolDefinitions: [],
150+
compactorNames: [],
151+
});
152+
153+
const named = await capturedAskHandler(
154+
{ question: "may I retry?", grant_request_id: "grant-123" },
155+
new AbortController().signal,
156+
);
157+
expect(named).toBe("answer:1");
158+
159+
const legacy = await capturedAskHandler(
160+
{ question: "plain question" },
161+
new AbortController().signal,
162+
);
163+
expect(legacy).toBe("answer:2");
164+
165+
await handles.close().catch(() => undefined);
166+
await runPromise.catch(() => undefined);
167+
return { registered };
168+
},
169+
),
170+
);
171+
172+
expect(outcome.registered.length).toBe(2);
173+
expect(outcome.registered[0]?.question).toBe("may I retry?");
174+
expect(outcome.registered[0]?.grantRequestId).toBe("grant-123");
175+
expect(outcome.registered[1]?.question).toBe("plain question");
176+
expect(outcome.registered[1]?.grantRequestId).toBeUndefined();
177+
});
178+
});

‎src/subagent/run.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,11 @@ const askDirectorDefinition: ToolDefinition = {
535535
type: "string",
536536
description: "The question for the spawning director (non-empty).",
537537
},
538+
grant_request_id: {
539+
type: "string",
540+
description:
541+
"Grant request id quoted from the deny reason; binds this ask to its own denial.",
542+
},
538543
},
539544
required: ["question"],
540545
},
@@ -859,6 +864,7 @@ async function runSubAgentInner(
859864
try {
860865
return await handleAskDirector({
861866
question: rawArgs.question,
867+
grantRequestId: rawArgs.grant_request_id,
862868
state: askDirectorState,
863869
port,
864870
signal,

0 commit comments

Comments
 (0)