fix(server): separate tool outcome persistence from provider errors

Co-Authored-By: Paperclip <noreply@paperclip.ing>

doc/idle-approved-tool-recovery.md

@@ -75,8 +75,16 @@ Controlled Test-tab provider tests cover both foreground/recovery race winners,
 completed legacy outcomes, interrupted legacy calls, target changes, and a failed
 atomic outcome write followed by quarantine without another provider call.
 
-The service/startup selection passes 122 tests. The existing gateway acceptance
+The service/startup selection passes 123 tests. The existing gateway acceptance
 suite passes 92 tests. The strengthened paging and outcome-failure cases pass
 again after their final edits. Workspace typecheck and build pass. Full CI and
 review remain separate PR gates; no real external provider was invoked by these
 fixtures.
+
+Review found that a one-shot success-write failure could fall into the provider
+error handler and save a false failure on its second attempt. The added sequence-
+based PostgreSQL fault reproduces that bug before the fix. Provider execution,
+pre-dispatch validation, and outcome persistence now have separate error
+boundaries. Both persistent and one-shot result-write failures retain the claim
+for quarantine without repeating the call. The complete 123-test selection and
+92 gateway acceptance cases pass after the repair, as do typecheck and build.

doc/plans/2026-10-24-recurring-server-sleep-stack.md

@@ -179,3 +179,14 @@ login-cleanup timeout after all 16 local tests passed. #17453 has a Runner
 prepared-launch close failure. Seven focused local Runner cases also fail, so
 this is under investigation rather than assumed flaky. No retry of that run has
 been requested. Neither parent is reported green without fresh evidence.
+
+
+Slice 05 is [#17454](https://github.com/paperclipai/paperclip/pull/17454).
+Review identified a real one-shot outcome-write error path. A new regression
+failed before the repair; result persistence now runs outside provider-error
+handling. The repaired head passes 123 service/startup tests, 92 acceptance
+tests, workspace typecheck and build. Fresh CI/review remains required.
+#17450 now has passing checks after its single retry. The local Runner
+reproduction for #17453 was missing the fake-provider binary: its durable state
+showed ENOENT hidden by close failure. After building the fixture all seven
+cases pass; one CI retry is running.

server/src/__tests__/tool-gateway-service.test.ts

@@ -276,6 +276,7 @@ describeEmbeddedPostgres("tool gateway service", () => {
       vi.useRealTimers(); vi.restoreAllMocks();
       await db.execute(sql`drop trigger if exists sleep_review_fail on tool_invocations`);
       await db.execute(sql`drop function if exists sleep_review_fail()`);
+      await db.execute(sql`drop sequence if exists sleep_review_failure_attempt`);
       expect(errors).toEqual([]);
     }
     async function start() {
@@ -461,12 +462,18 @@ describeEmbeddedPostgres("tool gateway service", () => {
         await quiet(); expect(remoteCall.provider).not.toHaveBeenCalled();
       });
 
-      it("does not replay a Test-tab call when persisting its external result fails", async () => {
+      it.each(["always", "once"])("does not replay a Test-tab call when persisting its external result fails %s", async mode => {
         const remoteCall = providerFixture(); remoteCall.release();
         const { pending } = await testRequest(remoteCall.provider);
         await approve(pending);
-        await db.execute(sql`create function sleep_review_fail() returns trigger language plpgsql as $$
-          begin raise exception 'approval result fixture failure'; end $$`);
+        await db.execute(sql`create sequence sleep_review_failure_attempt`);
+        await db.execute(sql.raw(`create function sleep_review_fail() returns trigger language plpgsql as $$
+          begin
+            if ${mode === "always" ? "true" : "nextval('sleep_review_failure_attempt') = 1"} then
+              raise exception 'approval result fixture failure';
+            end if;
+            return new;
+          end $$`));
         await db.execute(sql`create trigger sleep_review_fail before update on tool_action_requests
           for each row when (new.status in ('executed', 'failed')) execute function sleep_review_fail()`);
         try {

server/src/services/tool-gateway.ts

@@ -7452,9 +7452,11 @@ export function createToolGatewayService(
     });
 
     const startedAt = Date.now();
+    let connectedMcpExecution: Awaited<ReturnType<typeof executeRemoteHttpTool>> | Awaited<ReturnType<typeof executeLocalStdioTool>> | null;
+    let resultValidation: Awaited<ReturnType<typeof validateToolContent>>;
     try {
       const executionTimeoutMs = timeoutMs(args.timeoutMs, args.tool);
-      const connectedMcpExecution =
+      connectedMcpExecution =
         (args.tool.providerType === "mcp_remote_http" || args.tool.providerType === "provider_rest")
           ? await executeRemoteHttpTool(
               args.session,
@@ -7485,7 +7487,7 @@ export function createToolGatewayService(
         );
       }
       const result = connectedMcpExecution.result;
-      const resultValidation = await validateToolContent({
+      resultValidation = await validateToolContent({
         value: result,
         direction: "result",
         sensitiveMode: "redact",
@@ -7496,58 +7498,6 @@ export function createToolGatewayService(
         throw new ToolGatewayHttpError(502, String(providerResult.content || providerResult.error),
           "tool_error", { execution: connectedMcpExecution.execution });
       }
-      await persistToolTestOutcome(db, args.invocationId, {
-        status: "succeeded", result: resultValidation.summary,
-      }, args.approvedClaim);
-      await writeToolCallEvent({
-        invocationId: args.invocationId,
-        session: args.session,
-        eventType: "call_completed",
-        outcome: "success",
-        toolName: args.tool.name,
-        policyDecision: "allow",
-        reasonCode: "tool_completed",
-        argumentsSummary: args.argumentsSummary,
-        resultSummary: resultValidation.summary,
-        detectorMetadata: resultValidation.detectorMetadata,
-        metadata: {
-          source: "test",
-          headerSummary: connectedMcpExecution.headerSummary ?? undefined,
-          execution: connectedMcpExecution.execution,
-        },
-        tool: args.tool,
-      });
-      await writeAudit({
-        session: args.session,
-        companyId: args.companyId,
-        agentId: args.agentId,
-        runId: null,
-        issueId: null,
-        actorType: "user",
-        actorId: args.userId,
-        action: "tool_gateway.call_completed",
-        details: {
-          source: "test",
-          invocationId: args.invocationId,
-          decision: "allow",
-          reasonCode: "tool_completed",
-          tool: args.tool.name,
-          ...toolAuditMetadata(args.tool),
-          durationMs: Date.now() - startedAt,
-          argumentsSummary: args.argumentsSummary,
-          result: summarizeResult(resultValidation.value),
-          resultSummary: resultValidation.summary,
-          promptInjection: resultValidation.detectorMetadata,
-          headerSummary: connectedMcpExecution.headerSummary ?? undefined,
-          execution: connectedMcpExecution.execution,
-        },
-      });
-      void emitConnectionInvoked(db, args.invocationId);
-      return {
-        decision: "allowed" as const,
-        invocationId: args.invocationId,
-        result: resultValidation.value,
-      };
     } catch (err) {
       const status = err instanceof ToolGatewayHttpError ? err.status : 502;
       const reasonCode =
@@ -7619,16 +7569,61 @@ export function createToolGatewayService(
           : {}),
       };
     }
+    await persistToolTestOutcome(db, args.invocationId, {
+      status: "succeeded", result: resultValidation.summary,
+    }, args.approvedClaim);
+    await writeToolCallEvent({
+      invocationId: args.invocationId,
+      session: args.session,
+      eventType: "call_completed",
+      outcome: "success",
+      toolName: args.tool.name,
+      policyDecision: "allow",
+      reasonCode: "tool_completed",
+      argumentsSummary: args.argumentsSummary,
+      resultSummary: resultValidation.summary,
+      detectorMetadata: resultValidation.detectorMetadata,
+      metadata: {
+        source: "test",
+        headerSummary: connectedMcpExecution.headerSummary ?? undefined,
+        execution: connectedMcpExecution.execution,
+      },
+      tool: args.tool,
+    });
+    await writeAudit({
+      session: args.session,
+      companyId: args.companyId,
+      agentId: args.agentId,
+      runId: null,
+      issueId: null,
+      actorType: "user",
+      actorId: args.userId,
+      action: "tool_gateway.call_completed",
+      details: {
+        source: "test",
+        invocationId: args.invocationId,
+        decision: "allow",
+        reasonCode: "tool_completed",
+        tool: args.tool.name,
+        ...toolAuditMetadata(args.tool),
+        durationMs: Date.now() - startedAt,
+        argumentsSummary: args.argumentsSummary,
+        result: summarizeResult(resultValidation.value),
+        resultSummary: resultValidation.summary,
+        promptInjection: resultValidation.detectorMetadata,
+        headerSummary: connectedMcpExecution.headerSummary ?? undefined,
+        execution: connectedMcpExecution.execution,
+      },
+    });
+    void emitConnectionInvoked(db, args.invocationId);
+    return {
+      decision: "allowed" as const,
+      invocationId: args.invocationId,
+      result: resultValidation.value,
+    };
+
   }
 
-  /**
-   * Drive a freshly-approved test-origin ask-first request to completion. The
-   * Test tab has no agent run to re-invoke the parked call, so approving it in
-   * the Review tab is what executes it — the live status panel then surfaces the
-   * real result. Reconstructs the impersonated test session and the signed
-   * arguments, and never throws: any failure is recorded on the invocation so it
-   * shows up as an error in the panel rather than rolling back the approval.
-   */
   async function runApprovedTestInvocation(
     invocation: typeof toolInvocations.$inferSelect,
     signedPayload: NonNullable<ReturnType<typeof readSignedToolArgumentsPayload>>,
@@ -7636,6 +7631,7 @@ export function createToolGatewayService(
   ): Promise<void> {
     const claim = await claimToolTestApproval(db, actionRequestId, invocation.id);
     if (!claim) return;
+    let execution: Parameters<typeof runTestToolInvocation>[0];
     try {
       const agentId = invocation.agentId;
       if (!invocation.connectionId || !agentId) {
@@ -7662,16 +7658,18 @@ export function createToolGatewayService(
       }
       const argumentsSummary = (await validateToolContent({ value: signedPayload.arguments,
         direction: "arguments", sensitiveMode: "redact", promptInjectionMode: "ignore" })).summary;
-      await runTestToolInvocation({ session, tool, parameters: signedPayload.arguments, invocationId: invocation.id,
+      execution = { session, tool, parameters: signedPayload.arguments, invocationId: invocation.id,
         companyId: invocation.companyId, agentId, userId, argumentsSummary, reasonCode: "approval_granted",
-        approvedClaim: claim, matchedPolicyIds: invocation.matchedPolicyIds ?? [] });
+        approvedClaim: claim, matchedPolicyIds: invocation.matchedPolicyIds ?? [] };
     } catch (err) {
       const message = err instanceof Error ? err.message : String(err);
       const errorCode = err instanceof ToolGatewayHttpError || err instanceof ToolContentValidationError
         ? err.reasonCode : "tool_execution_failed";
       const settled = await persistToolTestOutcome(db, invocation.id, { status: "failed", errorCode, errorMessage: message }, claim);
       if (settled) void emitConnectionInvoked(db, invocation.id);
+      return;
     }
+    await runTestToolInvocation(execution);
   }
 
   async function waitForActionRequestExecution(actionRequestId: string) {