Skip to content

Commit 069a4e0

Browse files
committed
Stop caching middleware timeouts and aborts as operator declines
1 parent b1ba74e commit 069a4e0

2 files changed

Lines changed: 74 additions & 9 deletions

File tree

‎src/permission/gate.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -988,14 +988,25 @@ export function createPermissionGate(
988988
const outcome = await resolveInteractiveAsk(decision);
989989
if (outcome === undefined || !outcome.allow) {
990990
const reason = declineReason(decision.request, outcome);
991-
denialMemory.record(
992-
stableRequestId(
993-
call,
994-
getSubAgentIdentity()?.cwd ?? resolvedCwd,
995-
rootsProvider,
996-
),
997-
reason,
998-
);
991+
// Cache operator declines so a same-turn retry (fresh tool_call.id,
992+
// same stable fingerprint) denies with the identical reason instead of
993+
// re-asking. Timeouts, aborts, and missing outcomes are never cached —
994+
// the operator made no decision, so the retry must ask again (mirrors
995+
// resolveSuspended).
996+
if (
997+
outcome !== undefined &&
998+
!outcome.allow &&
999+
classifyOutcome(outcome) === "deny"
1000+
) {
1001+
denialMemory.record(
1002+
stableRequestId(
1003+
call,
1004+
getSubAgentIdentity()?.cwd ?? resolvedCwd,
1005+
rootsProvider,
1006+
),
1007+
reason,
1008+
);
1009+
}
9991010
return { allowed: false, reason };
10001011
}
10011012
return { allowed: true };

‎src/permission/permission.test.ts‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ import {
4040
createPathRestriction,
4141
resolveWorkspacePath,
4242
} from "./path-restriction.js";
43-
import type { Approval, PermissionRequest } from "./types.js";
43+
import type {
44+
Approval,
45+
ApprovalOutcome,
46+
PermissionRequest,
47+
} from "./types.js";
4448
import { initTemporaryGitRepo } from "../../tests/helpers/temporary-git-repo.js";
4549
import { secretGuardPlugin } from "../plugins/secret-guard-plugin.js";
4650
import { pathEscapePlugin } from "../plugins/path-escape-plugin.js";
@@ -2410,6 +2414,56 @@ describe("createPermissionGate", () => {
24102414
expect(asked).toBe(1);
24112415
});
24122416

2417+
// The middleware path must mirror the reactor-path guard above: only a real
2418+
// operator decline populates denial memory. Timeouts, aborts, and missing
2419+
// outcomes are never cached — the operator made no decision, so a same-turn
2420+
// retry with a fresh tool_call.id must re-ask instead of denying from cache.
2421+
test("middleware-path timeout/abort/missing outcomes are not cached: retry re-asks", async () => {
2422+
const args = { url: "https://example.com/docs", format: "markdown" };
2423+
const outcomes: { name: string; outcome: ApprovalOutcome | undefined }[] = [
2424+
{
2425+
name: "timeout",
2426+
outcome: { allow: false, message: APPROVAL_TIMEOUT_RESULT_TEXT },
2427+
},
2428+
{
2429+
name: "abort",
2430+
outcome: {
2431+
allow: false,
2432+
message: "tool no longer running; permission request denied",
2433+
},
2434+
},
2435+
{ name: "missing", outcome: undefined },
2436+
];
2437+
for (const { name, outcome } of outcomes) {
2438+
let asked = 0;
2439+
const gate = createPermissionGate({
2440+
approvals: [],
2441+
interactive: true,
2442+
skipPermissions: false,
2443+
reactorGated: false,
2444+
requestApproval: async () => {
2445+
asked++;
2446+
return outcome as ApprovalOutcome;
2447+
},
2448+
});
2449+
const first = await gate.evaluate({
2450+
id: "call_0",
2451+
name: "web_fetch",
2452+
arguments: args,
2453+
});
2454+
if (first.allowed)
2455+
throw new Error(`expected the first ${name} call denied`);
2456+
const retry = await gate.evaluate({
2457+
id: "call_1",
2458+
name: "web_fetch",
2459+
arguments: args,
2460+
});
2461+
if (retry.allowed)
2462+
throw new Error(`expected the ${name} retry denied after re-asking`);
2463+
expect(asked).toBe(2);
2464+
}
2465+
});
2466+
24132467
// CL-8002: distinct URLs deny independently, and reset() clears the denial
24142468
// memory so the next turn re-denies cleanly with no stale state.
24152469
test("headless denies distinct web_fetch URLs independently; reset clears denials", async () => {

0 commit comments

Comments
 (0)