fix(acpx-engine): give the unsettled permission event its own budget
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 <noreply@paperclip.ing>
This commit is contained in:
parent
5536282bbe
commit
df9b4d69d4
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue