Score each action once in a policy dry-run, not once per audit row it wrote (#294)

Testing a candidate boundary against recent history replays the audit trail's
computer-action rows. It replayed three event types, and one is a duplicate: a
permitted action that fails is recorded twice — the decision row written before
it was attempted (action_allowed / action_refused) and a failure row written
when it did not succeed (action_failed) — and both carry the same action and are
returned by the query.

So every failed action was scanned and scored twice. And because a dry-run
policy carries a refused action out, a refused action can fail too, leaving two
rows that disagree on the baseline: the decision row records "refused", while
the failure row has no decision of its own and fell to the "allowed" branch. A
candidate policy that refused the same action identically was then reported as a
new refusal it never introduced.

The failure row is an outcome, not a decision. The replay now skips it and
scores each action once from its decision row, and the query no longer fetches
it — so it also stops spending the scan budget on rows that would be dropped.
The baseline for a permitted-but-failed action is still "allowed", because that
is what its decision row says.

Co-authored-by: kevin9327 <kevin9327@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: David McKay <davidmckayv@users.noreply.github.com>

Author: 3 people

CHANGELOG.md

@@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A policy dry-run no longer counts a failed action twice, or invents a change it did not make
+
+Testing a boundary against recent history replayed three kinds of audit row, and one of them is a
+duplicate: a permitted action that fails is recorded both as the decision that allowed it and as a
+separate failure row, so every failed action was scanned and scored twice. Worse, a dry-run policy
+carries a refused action out, so a refused action can fail too — and its two rows disagree, the
+decision row saying "refused" and the failure row reading as "allowed", so a candidate policy that
+refused it identically was reported as a new refusal it never introduced. The replay now scores each
+action once, from the row that recorded its decision.
 ### A message no longer routes to a specialist because a longer word contained a connector's name
 
 When the intent router falls back — it is unreachable, or it declines — and exactly one coworker can

server/src/computer/policy-dry-run.ts

@@ -20,11 +20,17 @@ import {
   type PolicyContext,
 } from "./policy";
 
-/** The event types the gateway writes for a judged computer action. In one place, for the query. */
+/**
+ * The event types that record a policy DECISION about a computer action. In one place, for the query.
+ *
+ * Not `computer.action_failed`: the gateway writes that beside the decision row when a permitted
+ * action is attempted and does not succeed, so it is a second row for an action already recorded
+ * here, not a decision of its own. Replaying it too would score the action twice — see
+ * {@link dryRunAgainstHistory}.
+ */
 export const REPLAYABLE_EVENT_TYPES = [
   "computer.action_allowed",
   "computer.action_refused",
-  "computer.action_failed",
 ] as const;
 
 /** One action the candidate policy would have decided differently. */
@@ -38,7 +44,7 @@ export type DryRunChange = {
   element: { role: string; name: string } | null;
   command: string | null;
   file: string | null;
-  /** What actually happened, from the trail. A failed action was permitted first, so it was allowed. */
+  /** What the policy in force decided, from the action's decision row. */
   was: "allowed" | "refused";
   would: "allowed" | "refused";
   /** The candidate rule that decided it, or null for the default refusal. */
@@ -120,6 +126,14 @@ export function contextFromAuditPayload(
  * a dry-run policy's refusals, which were recorded and then carried out. That is the honest
  * baseline: the question this answers is "what would decide differently than was decided", not
  * "what would run differently than ran".
+ *
+ * A `computer.action_failed` row is skipped rather than scored. It is the outcome of an action whose
+ * decision row is already in this history, so counting it would score that action twice — and worse,
+ * a dry-run policy carries a refused action out, so a refused-then-failed action would arrive as a
+ * decision row that says "refused" and a failure row that reads as "allowed", inventing a change no
+ * policy made. The decision is on the decision row; the failure row only says it did not finish.
+ * Callers should exclude it from the query too ({@link REPLAYABLE_EVENT_TYPES}); this guards the
+ * function against being handed one regardless.
  */
 export function dryRunAgainstHistory(
   policy: ActionPolicy,
@@ -134,6 +148,7 @@ export function dryRunAgainstHistory(
   };
 
   for (const event of events) {
+    if (event.eventType === "computer.action_failed") continue;
     const context = contextFromAuditPayload(event.payload);
     if (!context) continue;
     report.scanned += 1;

server/tests/policy-dry-run.test.ts

@@ -122,23 +122,62 @@ describe("dryRunAgainstHistory", () => {
     expect(report.changes[0]?.would).toBe("allowed");
   });
 
-  test("a failed action was permitted first, so it counts as allowed", () => {
+  test("a permitted action that failed is counted once, from its decision row", () => {
+    // The gateway records a permitted-but-failed action twice: the decision row written before the
+    // attempt, and a failure row written when it did not succeed. Both carry the same action, and
+    // both are returned by the trail query, so scoring the failure row too would count the action a
+    // second time. Its baseline is still "allowed" — from the decision row that permitted it.
     const deny: ActionPolicy = {
       mode: "enforce",
       deny: ['tool.name == "computer_click"'],
       allow: ["true"],
     };
     const report = dryRunAgainstHistory(deny, [
       event({
-        id: "f",
+        id: "dec",
+        eventType: "computer.action_allowed",
+        payload: CLICK_SUBMIT,
+      }),
+      event({
+        id: "fail",
         eventType: "computer.action_failed",
         payload: CLICK_SUBMIT,
       }),
     ]);
+    expect(report.scanned).toBe(1);
     expect(report.wouldRefuse).toBe(1);
+    expect(report.changes).toHaveLength(1);
+    expect(report.changes[0]?.id).toBe("dec");
     expect(report.changes[0]?.was).toBe("allowed");
   });
 
+  test("a dry-run refusal that was carried out and then failed invents no change", () => {
+    // In dry-run mode a refused action is still carried out, so a refused action can also fail. The
+    // decision row says "refused"; the failure row, read on its own, would read as "allowed" and a
+    // candidate that refuses the same action would then look like a new refusal. Skipping the failure
+    // row leaves only the honest baseline: it was refused, a policy that refuses it changes nothing.
+    const denyClicks: ActionPolicy = {
+      mode: "enforce",
+      deny: ['tool.name == "computer_click"'],
+      allow: ["true"],
+    };
+    const report = dryRunAgainstHistory(denyClicks, [
+      event({
+        id: "dec",
+        eventType: "computer.action_refused",
+        payload: CLICK_SUBMIT,
+      }),
+      event({
+        id: "fail",
+        eventType: "computer.action_failed",
+        payload: CLICK_SUBMIT,
+      }),
+    ]);
+    expect(report.scanned).toBe(1);
+    expect(report.wouldRefuse).toBe(0);
+    expect(report.unchanged).toBe(1);
+  });
+
   test("a rule naming a command does not refuse a click, because absent facts are neutral", () => {
     const candidate: ActionPolicy = {
       mode: "enforce",