Skip to content

Commit 5d67a01

Browse files
committed
fix(permission): forget cached denies on a new user turn
Same-turn reactor retries still short-circuit. A later message must re-ask the same URL instead of canned-replying Tool call rejected.
1 parent 78ee45f commit 5d67a01

11 files changed

Lines changed: 193 additions & 7 deletions

File tree

‎src/agent/director.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,9 @@ class ChatDirectorImpl extends DefaultDirector {
559559
private currentSourceId: string | undefined;
560560
/** CL-7918 live replacement for the former getLiveFleetCount closure. */
561561
private allowIdleWithFleet: boolean;
562+
// Forget cached permission denies on the next inbound user message. Session
563+
// wiring points this at PermissionGate.clearDenials; unset in unit tests.
564+
private clearDenials: (() => void) | undefined;
562565
// Consecutive assistant turns that contain tool calls and no text. Reset on
563566
// any turn with text and on every fresh user message — a weak model that
564567
// spins in place on one thread of tool calls still converges to the
@@ -749,6 +752,10 @@ class ChatDirectorImpl extends DefaultDirector {
749752
this.allowIdleWithFleet = value;
750753
}
751754

755+
setClearDenials(clear: (() => void) | undefined): void {
756+
this.clearDenials = clear;
757+
}
758+
752759
updateToolDefinitions(toolDefinitions: ToolDefinition[]): void {
753760
const before = toolSetDigest(this._toolDefinitions);
754761
const after = toolSetDigest(toolDefinitions);
@@ -1012,6 +1019,7 @@ class ChatDirectorImpl extends DefaultDirector {
10121019
this.toolOnlyStreak = 0;
10131020
this.toolOnlyNudgeFired = false;
10141021
this.pendingToolOnlyNudge = false;
1022+
this.clearDenials?.();
10151023
}
10161024
if (onTurnBoundary(event)) this.inferenceRecoveries = 0;
10171025

@@ -1357,6 +1365,7 @@ export interface ChatDirector extends ReactorDirector {
13571365
updateToolDefinitions(toolDefinitions: ToolDefinition[]): void;
13581366
setWorkflowCoordinator(coordinator: WorkflowCoordinator | undefined): void;
13591367
setAllowIdleWithFleet(value: boolean): void;
1368+
setClearDenials(clear: (() => void) | undefined): void;
13601369
getTasks(): Task[];
13611370
restoreTasks(tasks: Task[]): void;
13621371
getContextEstimate(): { tokens: number; isEstimate: boolean };

‎src/director.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,51 @@ describe("open-task termination guard", () => {
712712
).toBe(true);
713713
expect(actions.some((a) => a.type === "infer")).toBe(false);
714714
});
715+
716+
test("a new user turn after a canned decline infers instead of canned-replying", async () => {
717+
const director = createChatDirector("base", [], {});
718+
let cleared = 0;
719+
director.setClearDenials(() => {
720+
cleared++;
721+
});
722+
const declinedTurn = actionsArray(
723+
await director.decide(
724+
makeToolErrorEvent("c", declined),
725+
mockState,
726+
mockCapabilities,
727+
),
728+
);
729+
expect(
730+
declinedTurn.some(
731+
(a) =>
732+
a.type === "reply" &&
733+
"content" in a &&
734+
a.content === "Tool call rejected by operator.",
735+
),
736+
).toBe(true);
737+
expect(cleared).toBe(0);
738+
739+
const next = actionsArray(
740+
await director.decide(
741+
{
742+
type: "message.received",
743+
message: { role: "user", content: "just talk" },
744+
} as unknown as ReactorInboundEvent,
745+
mockState,
746+
mockCapabilities,
747+
),
748+
);
749+
expect(
750+
next.some(
751+
(a) =>
752+
a.type === "reply" &&
753+
"content" in a &&
754+
a.content === "Tool call rejected by operator.",
755+
),
756+
).toBe(false);
757+
expect(next.some((a) => a.type === "infer")).toBe(true);
758+
expect(cleared).toBe(1);
759+
});
715760
});
716761

717762
describe("chatDirector compaction", () => {

‎src/exec/runner.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
856856
computeAdvertised,
857857
inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000,
858858
totalTimeoutMs: config.totalTimeoutMs,
859+
clearDenials: () => permissionGate.clearDenials(),
859860
getProvider: () => config,
860861
getWorkdir: () => workdir,
861862
getSessionId: () => sessionId,

‎src/permission/gate.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,9 @@ export interface PermissionGate {
436436
getApprovals: () => readonly Approval[];
437437
// Forget every remembered approval so a fresh session re-prompts from scratch.
438438
reset: () => void;
439+
// Drop cached operator/headless denies so the next user turn re-asks. Same-turn
440+
// retries still short-circuit until this runs.
441+
clearDenials: () => void;
439442
// The approvals granted only for this session (not persisted to any store).
440443
getSessionApprovals: () => readonly Approval[];
441444
// Drop one approval from the gate's live list and from the session set so the
@@ -633,9 +636,10 @@ export function createPermissionGate(
633636

634637
// Same-turn denial memory: stable fingerprints of headless and
635638
// operator-declined denies so a retry with a fresh tool_call.id returns the
636-
// identical cached reason instead of re-evaluating. Cleared by reset() and
637-
// by every state change that can flip a deny to an allow (new grants,
638-
// re-seeded approvals, auto/skip toggles, provider-identity switches).
639+
// identical cached reason instead of re-evaluating. Cleared on inbound user
640+
// turns (clearDenials), by reset(), and by every state change that can flip a
641+
// deny to an allow (new grants, re-seeded approvals, auto/skip toggles,
642+
// provider-identity switches). Timeouts and aborts are never recorded.
639643
const denialMemory = new DenialMemory();
640644

641645
// Non-blocking policy decision for one tool call: everything the gate owns —
@@ -1158,6 +1162,7 @@ export function createPermissionGate(
11581162
isReactorGated: () => reactorGated,
11591163
getApprovals: () => approvals,
11601164
reset,
1165+
clearDenials: () => denialMemory.clear(),
11611166
getSessionApprovals,
11621167
removeSessionApproval,
11631168
setSeededApprovals,

‎src/permission/permission.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,6 +2375,45 @@ describe("createPermissionGate", () => {
23752375
expect(asked).toBe(1);
23762376
});
23772377

2378+
// A denied URL is remembered only for same-turn retries. An inbound user
2379+
// turn (clearDenials) must forget it so the same URL can be re-asked.
2380+
test("clearDenials drops cached denies so a later turn re-asks the same URL", async () => {
2381+
let asked = 0;
2382+
const gate = createPermissionGate({
2383+
approvals: [],
2384+
interactive: true,
2385+
skipPermissions: false,
2386+
reactorGated: false,
2387+
requestApproval: async () => {
2388+
asked++;
2389+
return { allow: false };
2390+
},
2391+
});
2392+
const args = { url: "https://example.com/docs", format: "markdown" };
2393+
const first = await gate.evaluate({
2394+
id: "call_0",
2395+
name: "web_fetch",
2396+
arguments: args,
2397+
});
2398+
if (first.allowed) throw new Error("expected the first call declined");
2399+
expect(asked).toBe(1);
2400+
const sameTurn = await gate.evaluate({
2401+
id: "call_1",
2402+
name: "web_fetch",
2403+
arguments: args,
2404+
});
2405+
if (sameTurn.allowed) throw new Error("expected the same-turn retry denied");
2406+
expect(asked).toBe(1);
2407+
gate.clearDenials();
2408+
const later = await gate.evaluate({
2409+
id: "call_2",
2410+
name: "web_fetch",
2411+
arguments: args,
2412+
});
2413+
if (later.allowed) throw new Error("expected the later-turn call declined");
2414+
expect(asked).toBe(2);
2415+
});
2416+
23782417
// CL-8002: a reactor-path timeout is not an operator decision, so
23792418
// resolveSuspended must not cache it — the retry re-asks the operator.
23802419
test("reactor-path timeout is not cached: retry re-asks", async () => {

‎src/permission/reactor-authorize.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export function workerPermissionGate(gate: PermissionGate): PermissionGate {
107107
isReactorGated: () => true,
108108
getApprovals: () => gate.getApprovals(),
109109
reset: () => gate.reset(),
110+
clearDenials: () => gate.clearDenials(),
110111
getSessionApprovals: () => gate.getSessionApprovals(),
111112
removeSessionApproval: (target) => gate.removeSessionApproval(target),
112113
setSeededApprovals: (seeded) => gate.setSeededApprovals(seeded),

‎src/session/assemble-runtime.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,11 @@ export interface ChatAgentWiring {
477477
* the TUI seeds it (fleet lanes may appear mid-session).
478478
*/
479479
allowIdleWithFleet?: boolean;
480+
/**
481+
* Bound to PermissionGate.clearDenials so a later user turn re-asks a URL
482+
* that was declined this turn. Same-turn reactor retries still short-circuit.
483+
*/
484+
clearDenials?: () => void;
480485
getProvider: () => { providerName: string; model: string };
481486
/** Pre-created holder so the workflow controller can close over it first. */
482487
directorHolder?: { instance?: ChatDirector };
@@ -535,6 +540,8 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent {
535540
allowIdleWithFleet: wiring.allowIdleWithFleet,
536541
},
537542
);
543+
if (wiring.clearDenials !== undefined)
544+
d.setClearDenials(wiring.clearDenials);
538545
directorHolder.instance = d;
539546
return d;
540547
},

‎src/tui/runner/session.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ export async function assembleTUISession(
631631
// instead of a host closure; no getLiveFleetCount: the publisher drives
632632
// the allowance through setAllowIdleWithFleet.)
633633
allowIdleWithFleet: true,
634+
clearDenials: () => permissionGate.clearDenials(),
634635
getProvider: () => state.config,
635636
directorHolder,
636637
getWorkdir: () => state.workdir,

‎tests/integration/harness.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,17 @@ export async function openIntegrationSession(
129129
const chatDirectorDef = defineDirector({
130130
id: `${ID_PREFIX}/chat`,
131131
configSchema: type({}),
132-
factory: (_config, _env, agentCtx) =>
133-
createChatDirector(agentCtx.systemPrompt, [...agentCtx.toolDefinitions], {
134-
inactivityTimeoutMs: 750_000,
135-
}),
132+
factory: (_config, _env, agentCtx) => {
133+
const d = createChatDirector(
134+
agentCtx.systemPrompt,
135+
[...agentCtx.toolDefinitions],
136+
{
137+
inactivityTimeoutMs: 750_000,
138+
},
139+
);
140+
d.setClearDenials(() => opts.permissionGate.clearDenials());
141+
return d;
142+
},
136143
});
137144

138145
const toolsFactory = defineTool({

‎tests/integration/reactor-permission-multi-turn.test.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,4 +173,74 @@ describe("integration — reactor permission + multi-turn", () => {
173173
}
174174
},
175175
);
176+
177+
test.serial(
178+
"denied run_shell does not canned-reply a later prose turn and re-asks the same command",
179+
async () => {
180+
let asked = 0;
181+
const session = await openIntegrationSession({
182+
permissionGate: createPermissionGate({
183+
approvals: [],
184+
interactive: true,
185+
skipPermissions: false,
186+
reactorGated: false,
187+
requestApproval: async () => {
188+
asked++;
189+
return { allow: false };
190+
},
191+
}),
192+
});
193+
194+
try {
195+
session.harness.scenario.replyOnce("anthropic", {
196+
toolCalls: [
197+
{
198+
name: "run_shell",
199+
args: { command: "curl https://example.com/docs" },
200+
},
201+
],
202+
});
203+
204+
const denied = await runUntilDone(
205+
session,
206+
"Fetch https://example.com/docs.",
207+
);
208+
expect(asked).toBeGreaterThan(0);
209+
const askedAfterDeny = asked;
210+
expect(
211+
denied.events.some((e) => e.type === "reactor.error"),
212+
).toBe(false);
213+
214+
session.harness.scenario.replyOnce("anthropic", {
215+
text: "I'll continue without fetching.",
216+
});
217+
const prose = await runUntilDone(
218+
session,
219+
"Do not fetch anything. Confirm in prose.",
220+
);
221+
expect(prose.reply).toContain("I'll continue without fetching.");
222+
expect(prose.reply).not.toContain("Tool call rejected by operator.");
223+
expect(asked).toBe(askedAfterDeny);
224+
225+
session.harness.scenario.replyOnce("anthropic", {
226+
toolCalls: [
227+
{
228+
name: "run_shell",
229+
args: { command: "curl https://example.com/docs" },
230+
},
231+
],
232+
});
233+
const retried = await runUntilDone(
234+
session,
235+
"Try fetching https://example.com/docs again.",
236+
);
237+
expect(asked).toBe(askedAfterDeny + 1);
238+
expect(
239+
retried.events.some((e) => e.type === "reactor.error"),
240+
).toBe(false);
241+
} finally {
242+
await closeIntegrationSession(session);
243+
}
244+
},
245+
);
176246
});

0 commit comments

Comments
 (0)