feat(approvals): record the pre-decision figure on budget assertions (BLO-34008)

packages/shared/src/validators/approval.ts

@@ -86,11 +86,12 @@ const approvalPayloadSchema = z.object({
     "`budget_override_required`: such a card carrying no parseable assertion is refused with " +
     "`budget_approval_missing_enforcement_assertion` (BLO-34008), whose `details.remediation` " +
     "gives the full shape — approving one writes nothing to `budget_policies`, so an undeclared " +
-    "figure is unverifiable forever. Optionally add the figure the change starts from as " +
-    "`from_usd` / `from_amount_cents` when you already know it: the approval-enforcement " +
-    "reconciler reads it as the recorded prior, which is what lets it tell a decision that was " +
-    "never applied from one that was later superseded; without it a mismatch can only be " +
-    "reported as `unverifiable_mismatch`. Never invent one. For every other type it stays " +
+    "figure is unverifiable forever. You do not need to supply the figure the change starts " +
+    "from: the server reads it off the policy the assertion names and records it as " +
+    "`from_amount_cents`, which is what lets the approval-enforcement reconciler tell a decision " +
+    "that was never applied from one that was later superseded. Send `from_usd` / " +
+    "`from_amount_cents` yourself only if you know a prior the policy row no longer shows — " +
+    "never invent one. For every other type it stays " +
     "optional; today only `budget_policy_amount` is " +
     "checked, and unknown kinds are ignored.",
 );

server/src/__tests__/approval-budget-assertion-required.test.ts

@@ -53,20 +53,37 @@ function registerModuleMocks() {
   }));
 }
 
-function createRouteDb() {
+/**
+ * Two readers reach `select()` on these routes: the run-context lookup, and
+ * `loadEnforcedBudgetPolicies` (BLO-34008). They are told apart by the projection,
+ * because that is the only thing this stub sees — distinguishing on the table
+ * would mean reimplementing enough of drizzle to read `.from()`. `policyId` is
+ * the discriminator rather than `amount`: `amount` is a plausible column for any
+ * future select on these routes to project, and a misroute would surface as a
+ * confusing run-context failure instead of a clear one.
+ *
+ * The `where` clause is ignored, so the company/policy filter inside
+ * `loadEnforcedBudgetPolicies` is not exercised here — these cases are about the
+ * routes' stamping behaviour, and that filter is covered where the function is
+ * tested directly.
+ */
+function createRouteDb(policyRows: Array<Record<string, unknown>> = []) {
+  const runContextRow = { id: "run-1", companyId: "company-1", agentId: "agent-1", contextSnapshot: {} };
   return {
-    select: vi.fn(() => ({
-      from: vi.fn(() => ({
-        where: vi.fn(() => ({
-          then: async (resolve: (rows: unknown[]) => unknown) =>
-            resolve([{ id: "run-1", companyId: "company-1", agentId: "agent-1", contextSnapshot: {} }]),
+    select: vi.fn((projection?: Record<string, unknown>) => {
+      const rows = projection && "policyId" in projection ? policyRows : [runContextRow];
+      return {
+        from: vi.fn(() => ({
+          where: vi.fn(() => ({
+            then: async (resolve: (rows: unknown[]) => unknown) => resolve(rows),
+          })),
         })),
-      })),
-    })),
+      };
+    }),
   } as any;
 }
 
-async function createAgentApp() {
+async function createAgentApp(policyRows: Array<Record<string, unknown>> = []) {
   const [{ errorHandler }, { approvalRoutes }] = await Promise.all([
     import("../middleware/index.js"),
     import("../routes/approvals.js"),
@@ -84,7 +101,7 @@ async function createAgentApp() {
     };
     next();
   });
-  app.use("/api", approvalRoutes(createRouteDb()));
+  app.use("/api", approvalRoutes(createRouteDb(policyRows)));
   app.use(errorHandler);
   return app;
 }
@@ -390,8 +407,60 @@ describe("budget_override_required requires an enforcement assertion (BLO-34008)
     expect(mockApprovalService.resubmit).toHaveBeenCalledTimes(1);
   });
 
-  it("leaves resubmit of every other approval type alone", async () => {
-    mockApprovalService.getById.mockResolvedValue({
+  // The cohort the creation stamp cannot reach: a card filed before it existed
+  // carries an assertion with no prior. It passes the refusal above — that asks
+  // for *an* assertion, not a prior — so stamping only the supplied payload let
+  // it return to `pending` classifying as `unverifiable_mismatch` forever, which
+  // is the state BLO-34008 exists to eliminate. Ally's review of `b8d4f5e`.
+  const RESUBMIT_POLICY_AT_19K = {
+    policyId: POLICY_ID,
+    amount: 1900000,
+    isActive: true,
+    amountUpdatedAt: new Date("2026-08-01T00:00:00.000Z"),
+  };
+
+  it("stamps the stored payload on an empty resubmit when the card predates the creation stamp", async () => {
+    const priorless = { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000, label: "CTO" };
+    mockApprovalService.getById.mockResolvedValue(
+      revisionRequestedBudgetCard({ title: "Raise the CTO cap", enforcement_assertions: [priorless] }),
+    );
+    mockApprovalService.resubmit.mockImplementation(async (id: string, payload?: Record<string, unknown>) => ({
+      ...revisionRequestedBudgetCard(payload ?? {}),
+      id,
+      status: "pending",
+    }));
+
+    const res = await resubmit(await createAgentApp([RESUBMIT_POLICY_AT_19K]), {});
+
+    expect(res.status, JSON.stringify(res.body)).toBe(200);
+    const [, written] = mockApprovalService.resubmit.mock.calls.at(-1) ?? [];
+    expect((written as any)?.enforcement_assertions?.[0]).toMatchObject({
+      from_amount_cents: 1900000,
+      from_source: "server_policy_read",
+    });
+  });
+
+  it("still keeps the stored payload on an empty resubmit when there is no prior to add", async () => {
+    // The other half of the same branch: stamping must not turn every empty
+    // resubmit into an explicit payload write. `stampAssertionPriors` returns its
+    // argument by reference when it changes nothing, and that is what the route
+    // tests to decide whether to send one at all.
+    mockApprovalService.getById.mockResolvedValue(
+      revisionRequestedBudgetCard({ title: "Raise the CTO cap", enforcement_assertions: [VALID_ASSERTION] }),
+    );
+    mockApprovalService.resubmit.mockImplementation(async (id: string, payload?: Record<string, unknown>) => ({
+      ...revisionRequestedBudgetCard(payload ?? {}),
+      id,
+      status: "pending",
+    }));
+
+    const res = await resubmit(await createAgentApp([RESUBMIT_POLICY_AT_19K]), {});
+
+    expect(res.status, JSON.stringify(res.body)).toBe(200);
+    expect(mockApprovalService.resubmit).toHaveBeenCalledWith("approval-budget-1", undefined);
+  });
+
+  it("leaves resubmit of every other approval type alone", async () => {    mockApprovalService.getById.mockResolvedValue({
       ...revisionRequestedBudgetCard({ title: "Approve hosting spend" }),
       type: "request_board_approval",
     });
@@ -518,3 +587,147 @@ describe("the budget watcher's own threshold cards are exempt by construction (B
     expect(extractEnforcementAssertions(watcherPayload)).toEqual([]);
   });
 });
+
+/**
+ * BLO-34008 — the server records the figure the change starts from.
+ *
+ * `classifyEnforcementAssertion` needs three numbers (prior, decided, enforced)
+ * to tell a decision that never landed from one a later decision superseded.
+ * With only two it answers `unverifiable_mismatch` for every disagreement:
+ * still reported as drift, but not auto-appliable, and it cannot distinguish the
+ * real gap from the false-positive class that filed BLO-33160, BLO-33397,
+ * BLO-33416 and BLO-33772.
+ *
+ * Requiring the caller to supply it is not available: the refusal's own
+ * remediation says never to invent a starting figure, and its example payload
+ * deliberately omits one. So the server reads it from the policy the assertion
+ * already names. That is a database read, not a guess — the distinction the
+ * whole refusal exists to protect.
+ */
+describe("the starting figure is stamped from the enforcing policy (BLO-34008)", () => {
+  const POLICY_AT_19K = { policyId: POLICY_ID, amount: 1900000, isActive: true, amountUpdatedAt: new Date("2026-08-01T00:00:00.000Z") };
+
+  beforeEach(() => {
+    vi.resetModules();
+    vi.doUnmock("../services/index.js");
+    registerModuleMocks();
+    vi.clearAllMocks();
+
+    mockApprovalService.createWithIdempotency.mockImplementation(
+      async (companyId: string, data: Record<string, unknown>) => ({
+        approval: { id: "approval-1", companyId, status: "pending", ...data },
+        deduplicated: false,
+      }),
+    );
+    mockLogActivity.mockResolvedValue(mockDeferredActivityPublish);
+    mockAccessService.decide.mockResolvedValue({
+      allowed: true,
+      action: "company_scope:read",
+      reason: "allow_test",
+      explanation: "Allowed by test mock.",
+    });
+    mockIssueApprovalService.listIssuesForApproval.mockResolvedValue([]);
+  });
+
+  /** The `enforcement_assertions` array as it was actually persisted. */
+  function persistedAssertions() {
+    const [, data] = mockApprovalService.createWithIdempotency.mock.calls.at(-1) ?? [];
+    return (data as any)?.payload?.enforcement_assertions ?? [];
+  }
+
+  function fileCard(app: express.Express, assertion: Record<string, unknown>) {
+    return postApproval(app, {
+      type: "budget_override_required",
+      payload: { title: "Raise the CTO cap", enforcement_assertions: [assertion] },
+    });
+  }
+
+  it("fills the prior from the policy when the caller states none", async () => {
+    const app = await createAgentApp([POLICY_AT_19K]);
+
+    const res = await fileCard(app, { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000, label: "CTO" });
+
+    expect([200, 201], JSON.stringify(res.body)).toContain(res.status);
+    expect(persistedAssertions()[0]).toMatchObject({
+      from_amount_cents: 1900000,
+      from_source: "server_policy_read",
+    });
+  });
+
+  it("does not overwrite a prior the caller stated", async () => {
+    // The caller may know a pre-decision figure the current row no longer shows
+    // (a card filed after a manual edit). Silently replacing it would also hide
+    // a wrong one, which `policyAmountChangedAfterDecision` is there to catch.
+    const app = await createAgentApp([{ ...POLICY_AT_19K, amount: 2500000 }]);
+
+    await fileCard(app, { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000, from_usd: 19000 });
+
+    expect(persistedAssertions()[0]).toMatchObject({ from_usd: 19000 });
+    expect(persistedAssertions()[0]).not.toHaveProperty("from_source");
+  });
+
+  it("leaves an unresolvable policy unstamped so it still reports as missing_policy", async () => {
+    // Stamping nothing is the point: a card naming a policy that does not exist
+    // in this company must keep reading as broken, not acquire a figure that
+    // makes it look serviceable.
+    const app = await createAgentApp([]);
+
+    await fileCard(app, { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000 });
+
+    expect(persistedAssertions()[0]).not.toHaveProperty("from_amount_cents");
+    expect(persistedAssertions()[0]).not.toHaveProperty("from_source");
+  });
+
+  it("turns the 0-of-8 shape from unverifiable_mismatch into never_applied", async () => {
+    // The whole reason the stamp exists, asserted end to end rather than on the
+    // field: file the card, take the payload that was actually persisted, and
+    // classify it against a policy still sitting at the pre-approval amount.
+    // That is card 304ea443's shape — approved, never applied.
+    const app = await createAgentApp([POLICY_AT_19K]);
+    await fileCard(app, { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000, label: "CTO" });
+
+    const { extractEnforcementAssertions, classifyEnforcementAssertion } = await import(
+      "../services/approval-enforcement-reconciler.js"
+    );
+    const [, data] = mockApprovalService.createWithIdempotency.mock.calls.at(-1) ?? [];
+    const [assertion] = extractEnforcementAssertions((data as any).payload);
+    const decidedAt = new Date("2026-08-04T10:04:45.877Z");
+
+    expect(assertion?.priorAmountCents).toBe(1900000);
+    expect(classifyEnforcementAssertion(assertion!, POLICY_AT_19K, decidedAt)).toBe("never_applied");
+  });
+
+  it("still classifies a genuine supersession as superseded, not drift", async () => {
+    // The other half: the stamp must not convert the legitimate case into a
+    // false positive. Enforced figure is neither prior nor decided, and the
+    // amount moved after the decision.
+    const app = await createAgentApp([POLICY_AT_19K]);
+    await fileCard(app, { kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000 });
+
+    const { extractEnforcementAssertions, classifyEnforcementAssertion } = await import(
+      "../services/approval-enforcement-reconciler.js"
+    );
+    const [, data] = mockApprovalService.createWithIdempotency.mock.calls.at(-1) ?? [];
+    const [assertion] = extractEnforcementAssertions((data as any).payload);
+
+    const movedLater = { ...POLICY_AT_19K, amount: 2500000, amountUpdatedAt: new Date("2026-08-20T00:00:00.000Z") };
+    expect(classifyEnforcementAssertion(assertion!, movedLater, new Date("2026-08-04T10:04:45.877Z"))).toBe("superseded");
+  });
+
+  it("writes the stamp back to the key it read, not the key that merely exists", async () => {
+    // `??` falls through a present-but-null `enforcement_assertions` to the
+    // camelCase array. Choosing the write key with `in` would then stamp the
+    // snake_case key and leave the camelCase array this actually read in place
+    // unstamped — two divergent assertion arrays on one money payload.
+    const { stampAssertionPriors } = await import("../services/approval-enforcement-reconciler.js");
+    const payload = {
+      enforcement_assertions: null,
+      enforcementAssertions: [{ kind: "budget_policy_amount", policyId: POLICY_ID, expected_usd: 32000 }],
+    };
+
+    const out = stampAssertionPriors(payload, new Map([[POLICY_ID, POLICY_AT_19K]])) as any;
+
+    expect(out.enforcementAssertions[0]).toMatchObject({ from_amount_cents: 1900000 });
+    expect(out.enforcement_assertions).toBeNull();
+  });
+});

server/src/routes/approvals.ts

@@ -24,6 +24,8 @@ import { redactApprovalPayloadForDisplay, withholdAgentConfigFromApprovalPayload
 import {
   BUDGET_POLICY_AMOUNT_ASSERTION,
   extractEnforcementAssertions,
+  loadEnforcedBudgetPolicies,
+  stampAssertionPriors,
 } from "../services/approval-enforcement-reconciler.js";
 import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
 import { resolveApprovalWithSideEffects } from "../services/approval-resolution.js";
@@ -154,10 +156,11 @@ function budgetAssertionRefusal(type: string, payload: unknown) {
         "policy that enforces the cap. Give the target as `expected_usd` (dollars) or " +
         "`expected_amount_cents` (integer cents). `label` is printed into the drift report this " +
         "assertion raises, so set it to the agent or scope this policy actually caps — a label " +
-        "left over from the example misattributes the drift. If you have the figure the change " +
-        "starts from, record it as `from_usd` / `from_amount_cents`: it is retained on the card " +
-        "so a later reader can tell 'never applied' from 'applied and then superseded'. Nothing " +
-        "reads it yet, so never invent one — only the target is required. On resubmit, send the " +
+        "left over from the example misattributes the drift. You do not need the figure the change " +
+        "starts from: the server reads it off the policy you named and records it as " +
+        "`from_amount_cents`, which is what lets a later reader tell 'never applied' from " +
+        "'applied and then superseded'. State `from_usd` / `from_amount_cents` yourself only if " +
+        "you know a prior the policy row no longer shows — never invent one. On resubmit, send the " +
         "corrected assertions in the resubmit body: the check runs against the payload that will " +
         "end up pending, and a card filed before this guard existed has none stored.",
     },
@@ -212,6 +215,20 @@ export function approvalRoutes(
   });
   const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true";
 
+  // Stamp the pre-decision figure onto any canonical assertion that omits one,
+  // reading it from the policy the assertion names (BLO-34008). Runs on both
+  // routes that can leave a card `pending`, for the same reason the refusal
+  // does: what matters is the payload that ends up decided.
+  //
+  // Called only after `budgetAssertionRefusal` has passed, so a payload with no
+  // usable assertion never reaches the lookup.
+  async function stampPriors(companyId: string, type: string, payload: unknown) {
+    if (type !== "budget_override_required") return payload;
+    const policyIds = extractEnforcementAssertions(payload).map((a) => a.policyId);
+    if (policyIds.length === 0) return payload;
+    return stampAssertionPriors(payload, await loadEnforcedBudgetPolicies(db, companyId, policyIds));
+  }
+
   async function requireApprovalAccess(req: Request, id: string) {
     const approval = await svc.getById(id);
     if (!approval || !hasCompanyAccess(req, approval.companyId)) {
@@ -529,13 +546,14 @@ export function approvalRoutes(
       res.status(422).json(budgetRefusal);
       return;
     }
+    const persistedPayload = await stampPriors(companyId, approvalInput.type, normalizedPayload);
 
     const actor = getActorInfo(req);
     const requestedByAgentId = actor.actorType === "agent" ? actor.actorId : null;
     const requestedByUserId = actor.actorType === "user" ? actor.actorId : null;
     const payloadObj =
-      typeof normalizedPayload === "object" && normalizedPayload !== null
-        ? (normalizedPayload as Record<string, unknown>)
+      typeof persistedPayload === "object" && persistedPayload !== null
+        ? (persistedPayload as Record<string, unknown>)
         : {};
     const approvalTitle =
       typeof payloadObj.title === "string" ? payloadObj.title : undefined;
@@ -554,7 +572,7 @@ export function approvalRoutes(
     const coalesceKey = runContextDecision.boardEscalationCoalesceKey;
     const { approval, deduplicated } = await svc.createWithIdempotency(companyId, {
       ...approvalInput,
-      payload: normalizedPayload,
+      payload: persistedPayload,
       // Requester identity is derived only from the authenticated actor, and exactly one
       // requester column is populated. Letting a user also nominate `requestedByAgentId`
       // makes the idempotency key ambiguous because both requester-scoped unique indexes
@@ -788,7 +806,29 @@ export function approvalRoutes(
       return;
     }
 
-    const approval = await svc.resubmit(id, normalizedPayload);
+    // Stamp whichever payload will actually end up `pending`, for the same reason
+    // the refusal checks that one. Stamping only a *supplied* payload left the
+    // motivating cohort uncovered: a card filed before the creation stamp existed
+    // carries an assertion with no prior, which passes the refusal above (that
+    // only requires *an* assertion, not a prior) and walks back to `pending`
+    // classifying as `unverifiable_mismatch` — the exact state this exists to
+    // eliminate. Found by Ally reviewing `b8d4f5e`.
+    //
+    // `stampAssertionPriors` returns its argument by reference when it changes
+    // nothing, so keep-vs-overwrite semantics are untouched: an empty resubmit
+    // still sends `undefined` and lets `svc.resubmit()` keep the stored payload
+    // unless there was genuinely a prior to add.
+    const stamped = await stampPriors(
+      existing.companyId,
+      existing.type,
+      normalizedPayload ?? existing.payload,
+    );
+    const resubmitPayload =
+      normalizedPayload === undefined && stamped === existing.payload
+        ? undefined
+        : (stamped as Record<string, unknown> | undefined);
+
+    const approval = await svc.resubmit(id, resubmitPayload);
     const actor = getActorInfo(req);
     await logActivity(db, {
       companyId: approval.companyId,