From df9b4d69d46d46ad08881e3bc93e4f99ba4eb7cf Mon Sep 17 00:00:00 2001 From: nickyleach <331803+nickyleach@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:09:47 +0000 Subject: [PATCH] fix(acpx-engine): give the unsettled permission event its own budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settled and unsettled permission events shared one counter and one limit of 256. A normal long run settles 256 permission requests during the turn, so the shared budget was empty by the time the run ended. A later stall then emitted no acpx.permission_unsettled event — the one signal that tells an operator a handoff stalled. Split the shared counter into four independent per-run budgets: the ledger, the observed count, the settled count, and the unsettled count. Each stays at 256 and each is bounded by the same 256-entry ledger cap, so the worst case per run stays a constant. Make finalizeRun idempotent so a second settle-step call cannot emit a second summary event. Co-authored-by: Paperclip --- .../acpx-engine/permission-observer.test.ts | 78 +++++++++++++++++-- .../src/acpx-engine/permission-observer.ts | 63 +++++++++------ 2 files changed, 114 insertions(+), 27 deletions(-) diff --git a/packages/adapter-utils/src/acpx-engine/permission-observer.test.ts b/packages/adapter-utils/src/acpx-engine/permission-observer.test.ts index 51a9233a1f..c281659a46 100644 --- a/packages/adapter-utils/src/acpx-engine/permission-observer.test.ts +++ b/packages/adapter-utils/src/acpx-engine/permission-observer.test.ts @@ -328,7 +328,7 @@ describe("createAcpPermissionObserver — per-run budgets", () => { expect(summaries).toHaveLength(1); }); - it("bounds cumulative emissions at 512 detailed events across more than 1000 open-and-settle cycles", async () => { + it("bounds cumulative settled emissions at 256 across more than 1000 open-and-settle cycles", async () => { const events: PermissionObserverLogEvent[] = []; const observer = createAcpPermissionObserver({ emitLog: (event) => events.push(event), @@ -343,14 +343,75 @@ describe("createAcpPermissionObserver — per-run budgets", () => { observer.noteToolCallEvent(sessionId, { toolCallId, status: "completed" }); } - const detailed = events.filter((event) => event.type !== "acpx.permission_observer_truncated"); - expect(detailed.length).toBeLessThanOrEqual(512); + const settled = events.filter((event) => event.type === "acpx.permission_settled"); + expect(settled.length).toBeLessThanOrEqual(256); events.length = 0; await observer.finalizeRun(); expect(events.filter((event) => event.type === "acpx.permission_observer_truncated")).toHaveLength(1); }); + it("keeps the unsettled-event budget reachable after a normal run spends the settled budget", async () => { + const events: PermissionObserverLogEvent[] = []; + const observer = createAcpPermissionObserver({ + emitLog: (event) => events.push(event), + permissionMode: "approve-all", + transport: "sandbox", + }); + const signal = new AbortController().signal; + // A normal run: 256 requests open and settle during the turn. + for (let i = 0; i < 256; i += 1) { + const sessionId = `session-${i}`; + const toolCallId = `tool-${i}`; + await observer.handlePermissionRequest(buildRequestFor(sessionId, toolCallId), { signal }); + observer.noteToolCallEvent(sessionId, { toolCallId, status: "completed" }); + } + // One more request opens and never settles. This is the stall. + await observer.handlePermissionRequest(buildRequestFor("session-stall", "tool-stall"), { signal }); + + events.length = 0; + await observer.finalizeRun(); + const unsettled = events.filter((event) => event.type === "acpx.permission_unsettled"); + expect(unsettled).toHaveLength(1); + expect(unsettled[0]).toMatchObject({ toolCallId: "tool-stall" }); + }); + + it("bounds cumulative unsettled emissions at 256 when more than 256 requests never settle", async () => { + const events: PermissionObserverLogEvent[] = []; + const observer = createAcpPermissionObserver({ + emitLog: (event) => events.push(event), + permissionMode: "approve-all", + transport: "sandbox", + }); + const signal = new AbortController().signal; + for (let i = 0; i < 300; i += 1) { + await observer.handlePermissionRequest(buildRequestFor(`session-${i}`, `tool-${i}`), { signal }); + } + + events.length = 0; + await observer.finalizeRun(); + const unsettled = events.filter((event) => event.type === "acpx.permission_unsettled"); + expect(unsettled.length).toBeLessThanOrEqual(256); + }); + + it("emits exactly one truncated summary event across two finalizeRun calls", async () => { + const events: PermissionObserverLogEvent[] = []; + const observer = createAcpPermissionObserver({ + emitLog: (event) => events.push(event), + permissionMode: "approve-all", + transport: "sandbox", + }); + const signal = new AbortController().signal; + for (let i = 0; i < 300; i += 1) { + await observer.handlePermissionRequest(buildRequestFor(`session-${i}`, `tool-${i}`), { signal }); + } + + events.length = 0; + await observer.finalizeRun(); + await observer.finalizeRun(); + expect(events.filter((event) => event.type === "acpx.permission_observer_truncated")).toHaveLength(1); + }); + it("keeps the settled-event budget reachable after the observed budget is spent", async () => { const events: PermissionObserverLogEvent[] = []; const observer = createAcpPermissionObserver({ @@ -397,11 +458,18 @@ describe("createAcpPermissionObserver — per-run budgets", () => { const summary = events.find((event) => event.type === "acpx.permission_observer_truncated"); expect(summary).toBeDefined(); expect(Object.keys(summary as object).sort()).toEqual( - ["suppressedLedgerEntries", "suppressedObservedEvents", "suppressedTerminalEvents", "type"].sort(), + [ + "suppressedLedgerEntries", + "suppressedObservedEvents", + "suppressedSettledEvents", + "suppressedUnsettledEvents", + "type", + ].sort(), ); expect(typeof summary?.suppressedLedgerEntries).toBe("number"); expect(typeof summary?.suppressedObservedEvents).toBe("number"); - expect(typeof summary?.suppressedTerminalEvents).toBe("number"); + expect(typeof summary?.suppressedSettledEvents).toBe("number"); + expect(typeof summary?.suppressedUnsettledEvents).toBe("number"); }); it("still resolves handlePermissionRequest to undefined, and throws nothing, once every cap is reached", async () => { diff --git a/packages/adapter-utils/src/acpx-engine/permission-observer.ts b/packages/adapter-utils/src/acpx-engine/permission-observer.ts index 7da8f15573..822f2b9cf2 100644 --- a/packages/adapter-utils/src/acpx-engine/permission-observer.ts +++ b/packages/adapter-utils/src/acpx-engine/permission-observer.ts @@ -18,16 +18,20 @@ const MAX_AGE_MS = 24 * 60 * 60 * 1000; // input. Each run gets its own budget: every counter below lives inside the // observer closure, not at module scope. // -// The ledger, the "observed" count, and the "settled or unsettled" count each -// get a separate budget of 256, instead of one shared budget of 512. A shared -// budget lets an agent that sends 512 requests spend the whole budget on -// "observed" events, so the observer would then emit no "unsettled" event at -// finalization — the one signal this observer exists to produce. Two budgets -// keep that signal reachable no matter how the agent spends the "observed" -// side. +// The ledger, the "observed" count, the "settled" count, and the "unsettled" +// count each get a separate budget of 256, instead of one shared budget. A +// diagnostic event class must never share a budget with an event class that +// the agent drives. "acpx.permission_settled" fires once for each tool call +// the agent completes, so a normal long run can spend a shared budget before +// the run ends. "acpx.permission_unsettled" is the one signal this observer +// exists to produce: it tells an operator that a handoff stalled. A shared +// budget would let a normal run silence that signal at the exact time it +// matters most. Four separate budgets keep each event class reachable no +// matter how the agent spends the others. const MAX_LEDGER_ENTRIES = 256; const MAX_OBSERVED_EVENTS = 256; -const MAX_TERMINAL_EVENTS = 256; +const MAX_SETTLED_EVENTS = 256; +const MAX_UNSETTLED_EVENTS = 256; export const PERMISSION_OBSERVER_METHODS = ["session/request_permission"] as const; export type PermissionObserverMethod = (typeof PERMISSION_OBSERVER_METHODS)[number] | "unknown"; @@ -179,10 +183,13 @@ export function createAcpPermissionObserver(options: AcpPermissionObserverOption // events, not the live ledger size, so an agent cannot reset a counter by // opening and settling requests in a loop (the churn case). let observedEventCount = 0; - let terminalEventCount = 0; + let settledEventCount = 0; + let unsettledEventCount = 0; let suppressedLedgerEntries = 0; let suppressedObservedEvents = 0; - let suppressedTerminalEvents = 0; + let suppressedSettledEvents = 0; + let suppressedUnsettledEvents = 0; + let hasFinalized = false; const ledgerKey = (sessionId: string, toolCallId: string) => sessionId + "\u0000" + toolCallId; @@ -255,12 +262,12 @@ export function createAcpPermissionObserver(options: AcpPermissionObserverOption entry.lastStage = stage; const outcome = mapPermissionObserverOutcome(event?.status); if (outcome === "unknown") return; - // Delete the entry (freeing the ledger memory) even when the terminal + // Delete the entry (freeing the ledger memory) even when the settled // budget below is spent: the ledger must not hold a settled entry just // because the observer could not log its settlement. ledger.delete(key); - if (terminalEventCount < MAX_TERMINAL_EVENTS) { - terminalEventCount += 1; + if (settledEventCount < MAX_SETTLED_EVENTS) { + settledEventCount += 1; options.emitLog({ type: "acpx.permission_settled", sessionId: entry.sessionId, @@ -269,7 +276,7 @@ export function createAcpPermissionObserver(options: AcpPermissionObserverOption ageMs: boundedAgeMs(now() - entry.openedAtMs), }); } else { - suppressedTerminalEvents += 1; + suppressedSettledEvents += 1; } } catch { // Diagnostic only; never let a logging failure surface into the event @@ -278,11 +285,17 @@ export function createAcpPermissionObserver(options: AcpPermissionObserverOption }; const finalizeRun: AcpPermissionObserver["finalizeRun"] = async () => { + // The engine's settle step can call finalizeRun more than one time for + // the same run. Only the first call may drain the ledger and emit the + // summary event; every later call is a no-op. + if (hasFinalized) return; + hasFinalized = true; + const openEntries = [...ledger.values()]; ledger.clear(); for (const entry of openEntries) { - if (terminalEventCount < MAX_TERMINAL_EVENTS) { - terminalEventCount += 1; + if (unsettledEventCount < MAX_UNSETTLED_EVENTS) { + unsettledEventCount += 1; options.emitLog({ type: "acpx.permission_unsettled", sessionId: entry.sessionId, @@ -291,19 +304,25 @@ export function createAcpPermissionObserver(options: AcpPermissionObserverOption ageMs: boundedAgeMs(now() - entry.openedAtMs), }); } else { - suppressedTerminalEvents += 1; + suppressedUnsettledEvents += 1; } } // Emit one summary event for the whole run, and only when the observer - // suppressed something. It carries only the three counters: no session - // identifier, no tool-call identifier, and no other agent-controlled - // value. - if (suppressedLedgerEntries > 0 || suppressedObservedEvents > 0 || suppressedTerminalEvents > 0) { + // suppressed something. It carries only the four counters and the type + // field: no session identifier, no tool-call identifier, and no other + // agent-controlled value. + if ( + suppressedLedgerEntries > 0 || + suppressedObservedEvents > 0 || + suppressedSettledEvents > 0 || + suppressedUnsettledEvents > 0 + ) { options.emitLog({ type: "acpx.permission_observer_truncated", suppressedLedgerEntries, suppressedObservedEvents, - suppressedTerminalEvents, + suppressedSettledEvents, + suppressedUnsettledEvents, }); } };