fix(server): recover approved tool actions from committed changes (#17454)

## Thinking Path > - Paperclip manages agent work and its governed tool calls. > - A human approval can persist before the approving process dispatches the tool. > - Recovery currently scans approved requests on every maintenance tick. > - Completed and deliberately inert approvals do not owe another execution. > - Test-tab approval also needs a shared execution claim before recovery can safely race the foreground path. > - This PR wakes recovery from committed changes and retries only unresolved approvals. > - Idle history becomes quiet while existing approval and execution checks remain with the gateway. ## Linked Issues or Issue Description Refs #17453. This is the fifth slice of the recurring-server sleep migration, stacked on `codex/sleep-04-tool-review-expiry`. **Problem** Approved tool requests are scanned periodically even when no executable work remains. The Test-tab foreground path lacks the request claim needed to admit concurrent recovery safely. **Proposed Solution** Use the existing transaction-aware review notification boundary and shared deadline worker. Keep signed-payload interpretation, current policy and target checks, and provider dispatch in the gateway. Share a claim between Test-tab foreground and recovery, then persist both authoritative outcome rows atomically. **Alternatives Considered** A longer interval still scans idle history. A new durable queue would duplicate existing request/invocation state. Dispatching Test-tab approvals without a claim risks duplicate external calls. ## What Changed - Recheck both principals for delegated Test-tab approvals. Save their rate receipts in the same claim transaction, share only the same bucket, and roll back new charges on denial. - Preserve delegated gateway authorization and both issuer/target rate receipts inside the existing claimed-invocation transaction; retain the durable actor during approved replay. - Expire stale managed connector payloads before consuming approved-tool admission. - Persist current rate-policy charges and their invocation receipts atomically before dispatch, preserving exact approval claim fencing and read-only preflight. - Recover approved requests at startup and after committed review changes; retry only unresolved approvals. - Leave legacy agent approvals without execute-on-approve intent inert. Repair completed Test-tab history without another provider call. - Fence Test-tab execution and outcome settlement with an atomic claim shared by both callers. Revalidate the approved target and current policy before dispatch. - Keep provider calls outside database transactions. Retain quarantine for interrupted execution and unknown outcomes. - Add a concurrent, idempotent partial index for approved-request pagination. Reuse the parent's company-scoped review signals and shared scoped deadline worker. - Remove approved scanning from the aggregate tick. Preserve the public combined sweep and scheduler-disabled startup behavior. - Document producer ownership, quiet states, lifecycle behavior, and remaining recovery duties. ## Verification Latest published head: `23868c1751c100a27f58087f7ce16b93c52d4eba`, targeting master. The review-found delegated gateway pool deadlock is repaired by passing the admission transaction through token and issuer-authority reads. Foreground and recovery each reproduced the deadlock with a one-connection pool; all three constrained-pool cases, including revoked authority, now pass. All 81 HTTP integration cases pass. The historical foreground rate-slot regression also reproduced before its repair; verified preview hashes are accepted only by read-only preflight, while consuming admission still requires the complete identity under the exact claim. Five full suites passed 329 cases; the gateway service then passed all 141 cases after two additional mixed historical hash controls. Production types/build passed before the final normal integration of master `35820cf398` (including independent Grok/Runner changes); integration validation is running. Initial fixture/assertion errors and the prior E2E restoration/cancellation deadlock remain recorded in [the owning audit](doc/idle-approved-tool-recovery.md#gateway-admission-uses-its-transaction-connection). Fresh-head CI, Greptile 5/5, and passing Ponytail are still required before merge. - Master landing reconciliation at `8dfe3958db6e4e8a0b7499f3f5be0a396ca77361` after #17453 merged: production and server test files are byte-identical to the accepted 320-case repair. The landed migration journal prefix is unchanged, and #17928's independent eval reporting changes are preserved. Fresh CI and reviews on this exact head are required before merge. - Historical-identity repair published at `096b7c7198c3ca864aca9377e261cbaa0a8aad6f` (production/test source identical to integrated acceptance `81231824d773a79fc50c835eccf7d9b3977e79fc`): all 320 accepted gateway, policy, startup, preparation-timing and authenticated HTTP management cases pass. Server types and the production server build pass. Four real historical sensitive-argument regressions failed before repair; invalid-signature, foreign-hash and stale-claim controls remain denied without dispatch or identity changes. Both agent and Test-tab calls deliver the original signed arguments once, and new rate policies still apply. - The first integrated combined run passed 319 cases and hit a Slack bot ownership collision left in a reused HTTP test database. The guard correctly returned 409. The full 78-case HTTP suite passed on a fresh dedicated fixture without product or assertion changes. Earlier fixture-construction errors are distinguished from the four real regressions in the audit document. Fresh remote CI and reviews remain required. - Parent reconciliation at `736e7f56097055e21ce13b4e45d9033391483af8`: 299 cases pass (221 full gateway/policy/startup cases and 78 assembled-server connection management cases), with no skipped cases. Server types pass. This includes real delegated Test-tab approval admission for newly added shared and caller rate rules. Fresh remote CI and Greptile review remain required. Approved delegated Test-tab calls now evaluate caller and target policies in the existing claim transaction. Real HTTP regressions first showed a missing caller charge and shared-limit self-rejection. All 78 connection-management cases and 92 gateway cases now pass on the owning branch, along with server production typechecking. All 78 connection-management cases also pass at combined tip `d25f830db89c1623bff3547bfc68015c1c66e0cf`. The three new cases cover shared charges, separate charges, rollback on denial, duplicate approval, and rejection of a later authorized call. Intermediate counter/identity fixture failures and a reused-database Slack collision are recorded in [the owning audit](https://github.com/paperclipai/paperclip/blob/bcca5b006976a471f4afd1f70c4031aa40a2b3f7/doc/idle-approved-tool-recovery.md#delegated-test-tab-admission-after-policy-changes). The earlier combined checks below remain evidence for their stated revisions. Current-head CI and review remain required. Current head: `bcca5b006976a471f4afd1f70c4031aa40a2b3f7`. This remains a single-duty PR stacked on #17453. - Integrated pinned master `8548173fc9efdc0fa07644fb92dcc0b6851a95e5` with normal merges. Every branch passed generated-schema checks and its target-branch migration-order gate. Existing SQL is unchanged; the 25 stack migrations follow master’s new migration. - Duty-specific producer coverage, lifecycle tests, and earlier failure records: [2026-10-24-recurring-server-sleep-stack.md](https://github.com/paperclipai/paperclip/blob/a021467f04d22be42ee5b0c87b6583b2300a62a9/doc/plans/2026-10-24-recurring-server-sleep-stack.md). - Combined tip `51e6cbe67979b55b340ea9f0f434c5b5c682ccd6`: **355 startup/cancellation/recovery/dispatch/preservation tests**, **75 connection-management tests**, and production server typechecking pass. These combined checks do not mean every intermediate PR independently ran the full repository suite. - Real master-to-stack database upgrade passes: **379 unchanged prior migration receipts plus 25 new migrations**, with the gateway responsibility fixture preserved. - Owning-branch checks: 84 runner installation/settlement cases plus runner typechecking and generated TypeScript/Rust profile agreement; 253 GitHub review workflow cases. Stop-attribution and held-worker readiness regressions failed before their fixes. - Failure accounting: the initial combined run passed 349 cases and failed six idle assertions in a reused preservation database containing old cancelled runs. Repeating the combined command with a fresh preservation database passed. Earlier invalid project selection, merge import/type errors, and the interrupted full local suite are not counted as passing checks. - **Latest-head CI and review remain required.** Prior CI findings include GitHub idle observation, route teardown, attention cleanup, touch-picker E2E, workspace health, and voice-cleanup timing; local passes do not erase those failures. No full local repository-suite, deployed sleep, fleet cost, or live Copilot qualification claim is made. ## Risks - Notifications cover the existing database-owner, single-active-writer contract. Independent SQL writers and other replicas need an explicit wake path. - Unknown provider outcomes cannot be replayed safely. Failed result persistence retains an executing claim and the existing stale-execution quarantine. - Interrupted execution and terminal interaction projection repair remain periodic until their separate slices. This PR changes only approved recovery. - A pending-only review write can trigger one empty approved-recovery pass. Settled idle work schedules no further scan. - The partial index uses the existing top-level concurrent migration executor. Snapshot pruning follows repository policy. - Cloud sleep eligibility and external alarm coverage remain unchanged. Worker quiet is not proof of deployed instance suspension. ## Model Used OpenAI Codex, GPT-6 family, with reasoning and code execution. The exact serving model identifier and context window were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-29 04:25:52 +01:00 · 2026-10-27 16:10:44 -05:00
1 parent 35820cf398
commit 77295faebc
@@ -1,3 +1,4 @@
+import { writeToolReviewChange } from "./tool-review-work-notifications.js";
 import { approvalResolutionRecords, resolvableApprovalStatuses as resolvableStatuses } from "../lib/approval-records.js";
 import { notifyHireApproved } from "./hire-hook.js";

@@ -12,7 +13,15 @@ import { instanceSettingsService } from "./instance-settings.js";

 export function approvalRecords(db: Db) {
  const instanceSettings = instanceSettingsService(db);
-  const { getExistingApproval, resolveApproval } = approvalResolutionRecords(db);
+  const { getExistingApproval, resolveApproval: resolveApprovalRecord } = approvalResolutionRecords(db);
+
+  // Tool reviews waiting for a formal decision stay parked until this producer
+  // commits. The shared record owner retains approval validation and CAS rules.
+  async function resolveApproval(id: string, status: "approved" | "rejected", userId: string,
+    note: string | null | undefined, database: Db = db) {
+    const existing = await getExistingApproval(id, database);
+    return writeToolReviewChange(database, existing.companyId, tx => resolveApprovalRecord(id, status, userId, note, tx));
+  }

  function redactApprovalComment<T extends { body: string }>(comment: T, censorUsernameInLogs: boolean): T {
    return {
@@ -64,7 +73,9 @@ export function approvalRecords(db: Db) {
    // cleanup. Idempotent: a no-op on already-resolved approvals.
    cancel: async (id: string, reason?: string | null) => {
      const now = new Date();
-      const updated = await db
+      const [existing] = await db.select({ companyId: approvals.companyId }).from(approvals).where(eq(approvals.id, id));
+      if (!existing) return null;
+      const updated = await writeToolReviewChange(db, existing.companyId, tx => tx
        .update(approvals)
        .set({
          status: "cancelled",
@@ -74,7 +85,7 @@ export function approvalRecords(db: Db) {
        })
        .where(and(eq(approvals.id, id), inArray(approvals.status, resolvableStatuses)))
        .returning()
-        .then((rows) => rows[0] ?? null);
+        .then((rows) => rows[0] ?? null));
      return updated;
    },

@@ -9,6 +9,7 @@ export const DELIVERY_QUEUES = {
  executionStatus: "execution-status",
  executionDisposition: "execution-disposition",
  environmentCleanup: "environment-cleanup",
+  toolReviewRecovery: "tool-review-recovery",
  toolReviewExpiry: "tool-review-expiry",
  gatewayCredentialExpiry: "gateway-credential-expiry",
  secretProposalExpiry: "secret-proposal-expiry",
@@ -1,7 +1,8 @@
 import type { ConnectionManagementContext } from "./connection-management.js";
 import { createHash } from "node:crypto";
 import { boundRunIssueIdSql } from "./run-task-binding.js";
-import { and, asc, desc, eq, gt, inArray, isNull, ne, sql } from "drizzle-orm";
+import type { ToolApprovalClaim } from "./tool-approval-execution.js";
+import { and, asc, desc, eq, inArray, isNull, ne, sql } from "drizzle-orm";
 import type { Db } from "@paperclipai/db";
 import {
  agents,
@@ -53,6 +54,12 @@ import {
 import { recordToolRuntimeAuditWriteFailure } from "./tool-runtime-metrics.js";
 import { canonicalToolArguments, readSignedToolArgumentsPayload, toolRequestIdentityHash } from "./tool-content-guards.js";

+class RejectedInvocationAdmission extends Error {
+  constructor(readonly decision: ToolAccessDecision) {
+    super(decision.explanation);
+  }
+}
+
 type ToolAccessContext = {
  companyId: string;
  actorType: "agent" | "user" | "system" | "plugin";
@@ -717,7 +724,7 @@ function assertSupportedGenericPolicyShape(
  }
 }

-async function getGenericPolicyRow(db: Db, companyId: string, policyId: string) {
+async function getGenericPolicyRow(db: Pick<Db, "select">, companyId: string, policyId: string) {
  const [policy] = await db
    .select()
    .from(toolPolicies)
@@ -785,7 +792,7 @@ function scopeAllowsTool(scope: Record<string, unknown> | null, ctx: ToolAccessC
  return selectorMatches(selectors, ctx);
 }

-export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecret?: string } = {}) {
+export function toolAccessPolicyService(db: Pick<Db, "select" | "insert" | "update" | "delete" | "transaction">, options: { toolActionSigningSecret?: string } = {}) {
  // Policy administration includes authorized self-targeted changes. It cannot
  // resolve governed action requests or modify principal permission grants.
  async function listPolicies(companyId: string) {
@@ -1301,11 +1308,12 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
        ],
        set: {
          limit: rule.limit,
-          remaining: sql`greatest(0, least(${toolRateLimitCounters.remaining}, ${rule.limit}) - 1)`,
+          // Preserve the number already spent when an operator changes the limit.
+          remaining: sql`greatest(0, ${rule.limit} - (${toolRateLimitCounters.limit} - ${toolRateLimitCounters.remaining}) - 1)`,
          resetAt,
          updatedAt: now,
        },
-        setWhere: gt(toolRateLimitCounters.remaining, 0),
+        setWhere: sql`${toolRateLimitCounters.limit} - ${toolRateLimitCounters.remaining} < ${rule.limit}`,
      })
      .returning({ remaining: toolRateLimitCounters.remaining });
    if (!counter) {
@@ -1376,7 +1384,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
    });
  }

-  async function admittedRateLimitAdmissions(ctx: ToolAccessContext, invocationId: string): Promise<ConsumedRateAdmission[]> {
+  async function admittedRateLimitAdmissions(ctx: ToolAccessContext, invocationId: string, previewHash?: string): Promise<ConsumedRateAdmission[] | null> {
    const [invocation] = await db.select({
      actorType: toolInvocations.actorType, actorId: toolInvocations.actorId, agentId: toolInvocations.agentId,
      runId: toolInvocations.runId, issueId: toolInvocations.issueId, gatewayId: toolInvocations.gatewayId,
@@ -1387,6 +1395,9 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
      status: toolInvocations.status, policyDecision: toolInvocations.policyDecision, approvalState: toolInvocations.approvalState,
      policyExplanation: toolInvocations.policyExplanation,
    }).from(toolInvocations).where(and(eq(toolInvocations.companyId, ctx.companyId), eq(toolInvocations.id, invocationId)));
+    // A parked request already spent its slot before asking for review. Reusing
+    // that admission only skips the same rate rule; current policy and the
+    // gateway's approval/claim owners still decide whether it may execute.
    const delegated = invocation?.policyExplanation?.delegatedConnectionTest as { agentId?: string } | undefined;
    const gatewayIssuer = invocation?.gatewayId ? invocation.policyExplanation?.delegatedGatewayIssuer : undefined;
    const isDelegatedCaller = invocation?.actorType === "agent" && invocation.actorId === ctx.agentId
@@ -1401,13 +1412,81 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
      && invocation.catalogEntryId === ctx.catalogEntryId && invocation.toolName === ctx.toolName
      && invocation.providerType === ctx.providerType && invocation.applicationKey === ctx.applicationKey
      && invocation.upstreamToolName === ctx.upstreamToolName
-      && invocation.argumentsHash === ctx.argumentsHash;
-    const stored = authorized ? invocation.policyExplanation?.[isDelegatedCaller ? "delegatedCallerConsumedRateAdmissions" : "consumedRateAdmissions"] : null;
+      && (invocation.argumentsHash === ctx.argumentsHash || invocation.argumentsHash === previewHash);
+    if (!authorized) return null;
+    const stored = invocation.policyExplanation?.[isDelegatedCaller ? "delegatedCallerConsumedRateAdmissions" : "consumedRateAdmissions"];
    return Array.isArray(stored) ? stored.filter((item): item is ConsumedRateAdmission => isRecord(item)
      && typeof item.policyId === "string" && CANONICAL_UUID_RE.test(item.policyId)
      && typeof item.versionHash === "string" && /^[a-f0-9]{64}$/.test(item.versionHash)) : [];
  }

+  /** Revalidate claimed work and persist new rate charges before provider dispatch. */
+  async function admitInvocation(
+    input: ToolAccessDecisionInput,
+    invocationId: string,
+    claim?: ToolApprovalClaim,
+    evaluate?: (
+      policy: Pick<ReturnType<typeof toolAccessPolicyService>, "decide" | "writeAudit">,
+      input: ToolAccessDecisionInput,
+      transaction: Db,
+    ) => Promise<ToolAccessDecision>,
+  ): Promise<ToolAccessDecision> {
+    try {
+      return await db.transaction(async tx => {
+        const [invocation] = await tx.select().from(toolInvocations)
+          .where(and(eq(toolInvocations.companyId, input.companyId), eq(toolInvocations.id, invocationId)))
+          .for("update");
+        // Historical reviewed requests can still have a pending invocation.
+        // Accept that state only with the exact executing claim owned by the
+        // gateway, locked in the same invocation-then-request order as dispatch.
+        let ownsClaim = false;
+        if (claim && claim.invocationId === invocationId) {
+          const [request] = await tx.select().from(toolActionRequests).where(and(
+            eq(toolActionRequests.id, claim.actionRequestId),
+            eq(toolActionRequests.invocationId, invocationId),
+            eq(toolActionRequests.companyId, input.companyId),
+          )).for("update");
+          ownsClaim = request?.status === "executing" && request.updatedAt.getTime() === claim.claimedAt.getTime();
+        }
+        if (!invocation || (claim && !ownsClaim)
+          || !(invocation.status === "executing" && invocation.policyDecision === "allow"
+            || ["awaiting_approval", "executing"].includes(invocation.status)
+              && invocation.policyDecision === "require_approval" && invocation.approvalState === "approved"
+            || invocation.status === "awaiting_approval" && invocation.policyDecision === "require_approval"
+              && invocation.approvalState === "pending" && ownsClaim)) {
+          return decision("deny", "deny_policy_block", "Tool invocation admission is unavailable.", [], []);
+        }
+        const admissionPolicy = toolAccessPolicyService(tx, options);
+        const admissionInput = { ...input, consumeRateLimit: true };
+        // Composed caller/target policy must share this claim transaction so
+        // either all new charges and receipts commit or all roll back.
+        const result = evaluate
+          ? await evaluate(admissionPolicy, admissionInput, tx as unknown as Db)
+          : await admissionPolicy.decide(admissionInput, { admittedInvocationId: invocationId });
+        if (!result.allowed && result.decision !== "require_approval") {
+          // A later policy can reject after an earlier rate rule spent a slot.
+          // Roll back both the new charges and their receipt on any rejection.
+          throw new RejectedInvocationAdmission(result);
+        }
+        const admissions = result.policyExplanation?.consumedRateAdmissions;
+        const delegatedAdmissions = result.policyExplanation?.delegatedCallerConsumedRateAdmissions;
+        if (Array.isArray(admissions) || Array.isArray(delegatedAdmissions)) {
+          await tx.update(toolInvocations).set({
+            policyExplanation: {
+              ...invocation.policyExplanation,
+              ...(Array.isArray(admissions) ? { consumedRateAdmissions: admissions } : {}),
+              ...(Array.isArray(delegatedAdmissions) ? { delegatedCallerConsumedRateAdmissions: delegatedAdmissions } : {}),
+            },
+          }).where(and(eq(toolInvocations.companyId, input.companyId), eq(toolInvocations.id, invocationId)));
+        }
+        return result;
+      });
+    } catch (error) {
+      if (error instanceof RejectedInvocationAdmission) return error.decision;
+      throw error;
+    }
+  }
+
  /**
   * Decides one tool access request. Pass the same `cache` to every call of a
   * batch that shares one actor and run context (for example, one tools/list
@@ -1415,7 +1494,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
   */
  async function decide(
    input: ToolAccessDecisionInput,
-    options: { cache?: ToolAccessDecisionCache; admittedInvocationId?: string; sharedRateDecision?: ToolAccessDecision } = {},
+    options: { cache?: ToolAccessDecisionCache; admittedInvocationId?: string; admittedPreviewHash?: string; sharedRateDecision?: ToolAccessDecision } = {},
  ): Promise<ToolAccessDecision> {
    const consumedRateAdmissions: ConsumedRateAdmission[] = [];
    const result = await decideInternal(input, options, consumedRateAdmissions);
@@ -1426,7 +1505,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr

  async function decideInternal(
    input: ToolAccessDecisionInput,
-    options: { cache?: ToolAccessDecisionCache; admittedInvocationId?: string; sharedRateDecision?: ToolAccessDecision },
+    options: { cache?: ToolAccessDecisionCache; admittedInvocationId?: string; admittedPreviewHash?: string; sharedRateDecision?: ToolAccessDecision },
    consumedRateAdmissions: ConsumedRateAdmission[],
  ): Promise<ToolAccessDecision> {
    // A decision that consumes a rate limit can also record a trust-rule hit
@@ -1438,9 +1517,14 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
    // A final handoff rechecks current permissions for work already admitted by
    // the gateway. Its own last consumed slot cannot reject that same work.
    // Only durable consumption of an unchanged rule retains that slot; new or
-    // edited rules still apply. This option never approves or consumes a call.
-    const admittedRates = input.consumeRateLimit === false && options.admittedInvocationId
-      ? await admittedRateLimitAdmissions(ctx, options.admittedInvocationId) : [];
+    // edited rules still apply. admitInvocation owns atomic consuming rechecks;
+    // ordinary preflight and browser handoff callers remain read-only.
+    const admittedRates = options.admittedInvocationId
+      ? await admittedRateLimitAdmissions(ctx, options.admittedInvocationId, input.consumeRateLimit === true ? undefined : options.admittedPreviewHash) : [];
+    if (options.admittedInvocationId && input.consumeRateLimit === true && admittedRates === null) {
+      return decision("deny", "deny_policy_block", "Tool invocation no longer matches this admission.", [], []);
+    }
+    if (input.consumeRateLimit === true) consumedRateAdmissions.push(...(admittedRates ?? []));
    const profileState = await effectiveProfiles(ctx, cache);
    const effectiveProfileIds = profileState.profiles.map((profile) => profile.id);
    const permittedByProfile = profileState.profiles.some((profile) => {
@@ -1513,7 +1597,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
          );
        }
        const versionHash = ratePolicyVersionHash(policy);
-        if (admittedRates.some(admission => admission.policyId === policy.id && admission.versionHash === versionHash)) continue;
+        if (admittedRates?.some(admission => admission.policyId === policy.id && admission.versionHash === versionHash)) continue;
        // Paired caller/target checks for one delegated test share a slot only
        // when the exact policy version and counter bucket are identical.
        const shared = options.sharedRateDecision?.policyExplanation?.consumedRateAdmissions;
@@ -1548,7 +1632,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr
            { redactionPlan: redaction.redactionPlan, policyExplanation },
          );
        }
-        if (input.consumeRateLimit === true) {
+        if (input.consumeRateLimit === true && !options.admittedInvocationId) {
          await recordTrustRuleHit(policy, ctx, redaction);
        }
        return decision("allow", "allow_trust_rule", policy.description ?? "Tool access allowed by trust rule.", effectiveProfileIds, [policy.id], { redactionPlan: redaction.redactionPlan, policyExplanation });
@@ -2178,6 +2262,7 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr

  return {
    decide,
+    admitInvocation,
    writeAudit,
    recordInvocation,
    summarizeAndRedact,
@@ -0,0 +1,119 @@
+import { and, eq } from "drizzle-orm";
+import { toolActionRequests, toolInvocations, type Db } from "@paperclipai/db";
+import type { ToolRedactedValueSummary } from "@paperclipai/shared";
+import { notifyToolReviewChange } from "./tool-review-work-notifications.js";
+
+export type ToolApprovalClaim = { actionRequestId: string; invocationId: string; claimedAt: Date };
+export type ToolTestOutcome = {
+  status: "succeeded" | "failed" | "timed_out" | "rate_limited";
+  result?: ToolRedactedValueSummary;
+  errorCode?: string;
+  errorMessage?: string;
+};
+
+export async function claimToolTestApproval(db: Db, actionRequestId: string, invocationId: string): Promise<ToolApprovalClaim | null> {
+  return db.transaction(async tx => {
+    const [invocation] = await tx.select().from(toolInvocations)
+      .where(eq(toolInvocations.id, invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(and(eq(toolActionRequests.id, actionRequestId), eq(toolActionRequests.invocationId, invocationId))).for("update");
+    if (!invocation || request?.status !== "approved") return null;
+    await notifyToolReviewChange(tx, request.companyId);
+    const now = new Date();
+    if (invocation.status !== "awaiting_approval") {
+      if (["succeeded", "failed", "denied", "timed_out", "rate_limited"].includes(invocation.status)) {
+        await tx.update(toolActionRequests).set({ status: invocation.status === "succeeded" ? "executed" : "failed",
+          resolvedAt: invocation.completedAt ?? now, updatedAt: now }).where(eq(toolActionRequests.id, actionRequestId));
+      } else if (invocation.status === "executing") {
+        await tx.update(toolActionRequests).set({ status: "executing", updatedAt: invocation.startedAt ?? invocation.updatedAt })
+          .where(eq(toolActionRequests.id, actionRequestId));
+      }
+      return null;
+    }
+    await tx.update(toolActionRequests).set({ status: "executing", updatedAt: now })
+      .where(eq(toolActionRequests.id, actionRequestId));
+    return { actionRequestId, invocationId, claimedAt: now };
+  });
+}
+
+/** Return only this still-undispatched claim to its existing approval queue. */
+export async function releaseToolApproval(db: Db, claim: ToolApprovalClaim) {
+  return db.transaction(async tx => {
+    const [invocation] = await tx.select().from(toolInvocations)
+      .where(eq(toolInvocations.id, claim.invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(and(eq(toolActionRequests.id, claim.actionRequestId),
+        eq(toolActionRequests.invocationId, claim.invocationId))).for("update");
+    if (invocation?.status !== "awaiting_approval" || invocation.startedAt !== null ||
+      request?.status !== "executing" || request.companyId !== invocation.companyId ||
+      request.updatedAt.getTime() !== claim.claimedAt.getTime()) return false;
+    await notifyToolReviewChange(tx, request.companyId);
+    await tx.update(toolActionRequests).set({ status: "approved", updatedAt: new Date() })
+      .where(eq(toolActionRequests.id, request.id));
+    return true;
+  });
+}
+
+export async function bindVerifiedToolApprovalArguments(db: Db, claim: ToolApprovalClaim, hashes: { preview: string; verified: string }) {
+  return db.transaction(async tx => {
+    const [invocation] = await tx.select().from(toolInvocations)
+      .where(eq(toolInvocations.id, claim.invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(and(eq(toolActionRequests.id, claim.actionRequestId),
+        eq(toolActionRequests.invocationId, claim.invocationId))).for("update");
+    if (invocation?.status !== "awaiting_approval" || invocation.startedAt !== null ||
+      request?.status !== "executing" || request.companyId !== invocation.companyId ||
+      request.updatedAt.getTime() !== claim.claimedAt.getTime() ||
+      ![hashes.preview, hashes.verified].includes(request.canonicalArgumentsHash) ||
+      ![hashes.preview, hashes.verified].includes(invocation.argumentsHash ?? "")) return false;
+    if (invocation.argumentsHash !== hashes.verified) await tx.update(toolInvocations)
+      .set({ argumentsHash: hashes.verified, updatedAt: new Date() }).where(eq(toolInvocations.id, claim.invocationId));
+    return true;
+  });
+}
+
+export async function startToolInvocation(db: Db, invocationId: string, claim?: ToolApprovalClaim) {
+  const now = new Date();
+  if (!claim) {
+    await db.update(toolInvocations).set({ status: "executing", startedAt: now, updatedAt: now })
+      .where(eq(toolInvocations.id, invocationId));
+    return true;
+  }
+  return db.transaction(async tx => {
+    const [invocation] = await tx.select().from(toolInvocations)
+      .where(eq(toolInvocations.id, invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(eq(toolActionRequests.id, claim.actionRequestId)).for("update");
+    if (invocationId !== claim.invocationId || invocation?.status !== "awaiting_approval" || invocation.startedAt !== null ||
+      request?.status !== "executing" || request.invocationId !== invocationId || request.companyId !== invocation.companyId ||
+      request.updatedAt.getTime() !== claim.claimedAt.getTime()) return false;
+    await tx.update(toolInvocations).set({ status: "executing", approvalState: "approved", startedAt: now, updatedAt: now })
+      .where(eq(toolInvocations.id, invocationId));
+    return true;
+  });
+}
+
+export async function persistToolTestOutcome(db: Db, invocationId: string, outcome: ToolTestOutcome, claim?: ToolApprovalClaim) {
+  const now = new Date();
+  const values = { status: outcome.status, ...(outcome.result ? {
+    resultHash: outcome.result.sha256 ?? null, resultSummary: outcome.result,
+    resultSizeBytes: outcome.result.sizeBytes ?? null,
+  } : {}), errorCode: outcome.errorCode, errorMessage: outcome.errorMessage, completedAt: now, updatedAt: now };
+  if (!claim) {
+    await db.update(toolInvocations).set(values).where(eq(toolInvocations.id, invocationId));
+    return true;
+  }
+  return db.transaction(async tx => {
+    const [invocation] = await tx.select({ status: toolInvocations.status }).from(toolInvocations)
+      .where(eq(toolInvocations.id, invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(eq(toolActionRequests.id, claim.actionRequestId)).for("update");
+    if (!invocation || !["awaiting_approval", "executing"].includes(invocation.status) ||
+      request?.status !== "executing" || request.updatedAt.getTime() !== claim.claimedAt.getTime()) return false;
+    await notifyToolReviewChange(tx, request.companyId);
+    await tx.update(toolInvocations).set(values).where(eq(toolInvocations.id, invocationId));
+    await tx.update(toolActionRequests).set({ status: outcome.status === "succeeded" ? "executed" : "failed",
+      resolvedAt: now, updatedAt: now }).where(eq(toolActionRequests.id, claim.actionRequestId));
+    return true;
+  });
+}
@@ -0,0 +1,33 @@
+import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
+import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
+import { registerScopedDeadlineWork } from "./scoped-deadline-work.js";
+import type { createToolGatewayService } from "./tool-gateway.js";
+
+export function registerToolReviewRecoveryWork(
+  coordinator: ReturnType<typeof createDeliveryWorkCoordinator>,
+  gateway: Pick<ReturnType<typeof createToolGatewayService>, "recoverApprovedActionReviews" | "approvedActionReviewCompanies">,
+  canRun: () => boolean,
+) {
+  // Dispatch failures are reported by the domain owner, not thrown. Preserve
+  // their retry deadline when another write reloads the same company's work.
+  const retries = new Map<string, number>();
+  return registerScopedDeadlineWork(coordinator, {
+    queue: DELIVERY_QUEUES.toolReviewRecovery,
+    batchSize: 1,
+    decodeScope: hint => hint === "*" ? undefined : hint,
+    matches: (entry, companyId?: string) => companyId === undefined || entry.companyId === companyId,
+    load: async (companyId?: string) => {
+      const rows = await gateway.approvedActionReviewCompanies(companyId);
+      const retained = new Set(rows.map(row => row.companyId));
+      for (const id of retries.keys()) if ((companyId === undefined || id === companyId) && !retained.has(id)) retries.delete(id);
+      return rows.map(row => ({ ...row, id: row.companyId, dueAt: retries.get(row.companyId) ?? Date.now() }));
+    },
+    check: async entry => {
+      const result = await gateway.recoverApprovedActionReviews(entry.companyId);
+      if (!result.retry) { retries.delete(entry.companyId); return null; }
+      const dueAt = Date.now() + 30_000;
+      retries.set(entry.companyId, dueAt);
+      return { ...entry, dueAt };
+    },
+  }, canRun);
+}
@@ -0,0 +1,119 @@
+import { and, asc, eq, gt } from "drizzle-orm";
+import { approvals, toolActionRequests, toolInvocations, type Db } from "@paperclipai/db";
+
+import { readSignedToolArgumentsPayload } from "./tool-content-guards.js";
+import { notifyToolReviewChange } from "./tool-review-work-notifications.js";
+
+type ReviewProjection = {
+  actionRequestId: string;
+  status: "executed" | "executing" | "failed" | "cancelled";
+  errorCode?: string | null;
+  errorMessage?: string | null;
+  resultSummary?: string | null;
+};
+
+/** Reconcile saved history before dispatch; never infer a safe provider replay. */
+export async function prepareApprovedToolReview(db: Db, input: {
+  actionRequestId: string;
+  invocationId: string;
+  signingSecret?: string;
+}) {
+  return db.transaction(async tx => {
+    // Match Test-tab claims/outcome persistence. Recheck after the locks because
+    // a foreground approval may have claimed the row since the recovery scan.
+    const [invocation] = await tx.select().from(toolInvocations)
+      .where(eq(toolInvocations.id, input.invocationId)).for("update");
+    const [request] = await tx.select().from(toolActionRequests)
+      .where(and(eq(toolActionRequests.id, input.actionRequestId),
+        eq(toolActionRequests.invocationId, input.invocationId))).for("update");
+    if (!invocation || request?.status !== "approved") return { kind: "skip" } as const;
+    const now = new Date();
+
+    async function settle(projection: ReviewProjection, failure?: {
+      status: "failed" | "denied";
+      errorCode: string;
+      errorMessage: string;
+    }) {
+      await notifyToolReviewChange(tx, request.companyId);
+      if (failure) await tx.update(toolInvocations).set({ ...failure, completedAt: now, updatedAt: now })
+        .where(eq(toolInvocations.id, invocation.id));
+      await tx.update(toolActionRequests).set({ status: projection.status,
+        ...(projection.status === "executing"
+          ? { updatedAt: invocation.startedAt ?? invocation.updatedAt }
+          : { resolvedAt: invocation.completedAt ?? now, updatedAt: now }),
+      }).where(eq(toolActionRequests.id, request.id));
+      return { kind: "settled", projection } as const;
+    }
+    function fail(status: "failed" | "cancelled", invocationStatus: "failed" | "denied", errorCode: string, errorMessage: string) {
+      return settle({ actionRequestId: request.id, status, errorCode, errorMessage },
+        { status: invocationStatus, errorCode, errorMessage });
+    }
+    if (invocation.companyId !== request.companyId) {
+      // Quarantine the request without modifying another company's invocation.
+      return settle({ actionRequestId: request.id, status: "failed",
+        errorCode: "action_context_mismatch", errorMessage: "Tool action and invocation companies do not match." });
+    }
+    if (["succeeded", "failed", "denied", "cancelled", "timed_out", "rate_limited", "executing"].includes(invocation.status)) {
+      return settle({ actionRequestId: request.id,
+        status: invocation.status === "succeeded" ? "executed"
+          : invocation.status === "executing" ? "executing"
+          : invocation.status === "cancelled" ? "cancelled" : "failed",
+        errorCode: invocation.errorCode, errorMessage: invocation.errorMessage,
+        resultSummary: invocation.resultSummary?.summary });
+    }
+    if (invocation.status !== "awaiting_approval") {
+      return fail("failed", "failed", "tool_recovery_state_invalid",
+        "Approved action has inconsistent invocation history. Inspect the provider before retrying; recovery did not replay it.");
+    }
+
+    // Missing-key/configuration errors deliberately propagate as retryable;
+    // only a decoded invalid signature is a permanent request failure.
+    const payload = readSignedToolArgumentsPayload({ signedArguments: request.signedArguments,
+      invocationId: invocation.id, toolName: invocation.toolName, signingSecret: input.signingSecret });
+    if (!payload) return fail("cancelled", "failed", "action_request_invalidated",
+      "Stored tool arguments are no longer verifiable; submit a new action for review.");
+    if (request.approvalId) {
+      const [formal] = await tx.select({ status: approvals.status }).from(approvals)
+        .where(and(eq(approvals.id, request.approvalId), eq(approvals.companyId, request.companyId)));
+      if (!formal || formal.status === "rejected" || formal.status === "cancelled") {
+        return fail("cancelled", "denied", "formal_approval_unavailable",
+          "Required formal board approval is unavailable or was declined; submit a new action for review.");
+      }
+      // Human input has no deadline. The approval owner publishes a committed
+      // resolution/cancellation to this queue; startup also rediscovers the row.
+      if (formal.status === "pending" || formal.status === "revision_requested") {
+        return { kind: "skip" } as const;
+      }
+    }
+    return { kind: "ready", request, invocation, payload } as const;
+  });
+}
+
+export function approvedToolReviewCompanies(db: Db, companyId?: string) {
+  return db.select({ companyId: toolActionRequests.companyId }).from(toolActionRequests)
+    .innerJoin(toolInvocations, eq(toolInvocations.id, toolActionRequests.invocationId))
+    .where(and(eq(toolActionRequests.status, "approved"),
+      companyId ? eq(toolActionRequests.companyId, companyId) : undefined))
+    .groupBy(toolActionRequests.companyId);
+}
+
+export async function recoverApprovedToolReviews(db: Db, recover: (input: {
+  request: typeof toolActionRequests.$inferSelect;
+  invocation: typeof toolInvocations.$inferSelect;
+}) => Promise<boolean>, companyId?: string) {
+  let cursor: string | undefined;
+  let scanned = 0, retry = false;
+  for (;;) {
+    const rows = await db.select({ request: toolActionRequests, invocation: toolInvocations })
+      .from(toolActionRequests).innerJoin(toolInvocations, eq(toolInvocations.id, toolActionRequests.invocationId))
+      .where(and(eq(toolActionRequests.status, "approved"),
+        companyId ? eq(toolActionRequests.companyId, companyId) : undefined,
+        cursor ? gt(toolActionRequests.id, cursor) : undefined))
+      .orderBy(asc(toolActionRequests.id)).limit(100);
+    for (const row of rows) retry = await recover(row) || retry;
+    scanned += rows.length;
+    if (rows.length < 100) break;
+    cursor = rows[rows.length - 1].request.id;
+  }
+  return { scanned, retry };
+}
@@ -1,8 +1,9 @@
 import type { Db } from "@paperclipai/db";
 import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js";

-export function notifyToolReviewChange(transaction: object, companyId: string) {
-  return notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolReviewExpiry, companyId);
+export async function notifyToolReviewChange(transaction: object, companyId: string) {
+  await notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolReviewExpiry, companyId);
+  await notifyDeliveryWork(transaction, DELIVERY_QUEUES.toolReviewRecovery, companyId);
 }

 export function writeToolReviewChange<T>(db: Db, companyId: string, write: (transaction: Db) => Promise<T>): Promise<T> {
@@ -11,7 +11,7 @@ function minuteBucket(date: Date): Date {
 }

 export async function incrementToolRuntimeMetricCounter(
-  db: Db,
+  db: Pick<Db, "insert">,
  input: {
    companyId: string;
    metric: string;
@@ -42,7 +42,7 @@ export async function incrementToolRuntimeMetricCounter(
    });
 }

-export async function recordToolRuntimeAuditWriteFailure(db: Db, companyId: string) {
+export async function recordToolRuntimeAuditWriteFailure(db: Pick<Db, "insert">, companyId: string) {
  try {
    await incrementToolRuntimeMetricCounter(db, {
      companyId,