fix(acp): plan output charged to per-turn budget; HITL requires readable decision

Two valid P1s from PR review threads:
- event-bridge: plan output bypassed the per-turn output cap — entry size was
  bounded but entry COUNT wasn't (1000 entries ~ 64MB through onThinking).
  Plans are now suppressed once the cap flags, capped at MAX_PLAN_ENTRIES=100
  with a truncation marker, bounded, and charged to the budget. +2 tests.
- control-handler: with pauseForApproval but no findApprovalByDedupeKey, a
  human approval was silently discarded (unreadable status -> deny). HITL now
  requires BOTH closures upfront and default-denies before creating a request,
  so no approval is wasted and no pending record orphaned.

184 tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-03 12:56:58 -07:00
parent b9d824b41f
commit d3e1a355dc
3 changed files with 74 additions and 4 deletions

View File

@@ -158,3 +158,47 @@ describe("event bridge bounds: toolCall correlation map (Risk S5)", () => {
expect(onToolEnd).toHaveBeenCalledWith("Sneaky", false, undefined);
});
});
describe("plan output bounds (S5)", () => {
it("caps plan entry count and charges the per-turn budget", async () => {
const { createEventBridge, MAX_PLAN_ENTRIES, PER_TURN_OUTPUT_CAP_CHARS } = await import(
"../event-bridge.js"
);
const thinking: string[] = [];
const bridge = createEventBridge({ onThinking: (t) => thinking.push(t) });
const entries = Array.from({ length: MAX_PLAN_ENTRIES + 50 }, (_, i) => ({
content: `step ${i}`,
priority: "low",
status: "pending",
}));
bridge.handleSessionUpdate({ sessionUpdate: "plan", entries } as never);
expect(thinking).toHaveLength(1);
// Truncation marker present; not all entries formatted.
expect(thinking[0]).toContain("50 more entries truncated");
expect(thinking[0].length).toBeLessThan(PER_TURN_OUTPUT_CAP_CHARS);
});
it("suppresses plan output once the per-turn cap has flagged", async () => {
const { createEventBridge, PER_CHUNK_CAP_CHARS, PER_TURN_OUTPUT_CAP_CHARS } = await import(
"../event-bridge.js"
);
const thinking: string[] = [];
const bridge = createEventBridge({ onThinking: (t) => thinking.push(t) });
// Flood text until the per-turn cap flags.
const chunk = "x".repeat(PER_CHUNK_CAP_CHARS);
const chunksNeeded = Math.ceil(PER_TURN_OUTPUT_CAP_CHARS / PER_CHUNK_CAP_CHARS) + 2;
for (let i = 0; i < chunksNeeded; i += 1) {
bridge.handleSessionUpdate({
sessionUpdate: "agent_thought_chunk",
content: { type: "text", text: chunk },
} as never);
}
const before = thinking.length;
bridge.handleSessionUpdate({
sessionUpdate: "plan",
entries: [{ content: "late plan", priority: "low", status: "pending" }],
} as never);
// No plan line after the cap flagged.
expect(thinking.length).toBe(before);
});
});

View File

@@ -191,9 +191,16 @@ export async function runApprovalForCategory(
}
}
// No way to block for a human decision → default-deny BEFORE creating a
// request, so we never orphan a perpetually-`pending` record in the store.
if (typeof gate.pauseForApproval !== "function") {
// Default-deny BEFORE creating a request when the HITL round-trip cannot
// complete: without `pauseForApproval` we cannot block for a decision, and
// without `findApprovalByDedupeKey` we cannot READ the decision after the
// pause — a human approval would be silently discarded (mapStatus(undefined)
// → deny). Denying upfront never orphans a pending record and never wastes
// a human's approval on an outcome that would be denied anyway.
if (
typeof gate.pauseForApproval !== "function" ||
typeof gate.findApprovalByDedupeKey !== "function"
) {
return "deny";
}

View File

@@ -48,6 +48,13 @@ export const PER_CHUNK_CAP_CHARS = 64_000;
*/
export const TOOL_CALL_MAP_CAP = 1000;
/**
* Max plan entries formatted into the plan log line. Entry size is bounded in
* formatPlan; this bounds the COUNT so one plan event cannot bypass the
* per-turn output budget with thousands of 64KB entries (Risk S5).
*/
export const MAX_PLAN_ENTRIES = 100;
/** Tracked metadata for an in-flight tool call, keyed by `toolCallId`. */
interface TrackedToolCall {
title?: string | null;
@@ -223,8 +230,20 @@ export function createEventBridge(callbacks: AcpCallbacks): EventBridge {
function handlePlan(entries: PlanEntry[] | undefined): void {
// FULL REPLACEMENT: drop any prior snapshot, surface the new one once.
// Plan output is charged against the same per-turn budget as text/thinking
// (Risk S5): entry SIZE is bounded in formatPlan, but entry COUNT is
// agent-controlled — without the cap below, one plan event with thousands
// of entries bypasses the per-turn ceiling entirely.
if (outputCapFlagged) return;
const list = Array.isArray(entries) ? entries : [];
callbacks.onThinking?.(formatPlan(list));
const capped = list.slice(0, MAX_PLAN_ENTRIES);
let line = formatPlan(capped);
if (list.length > capped.length) {
line += `\n- … ${list.length - capped.length} more entries truncated`;
}
line = boundString(line, PER_CHUNK_CAP_CHARS);
cumulativeOutputChars += line.length;
callbacks.onThinking?.(line);
}
function handleSessionUpdate(update: SessionUpdate): void {