Files
PaperClipAI/server/src/services/tool-review-interruption.ts
T
DottaandPaperclip f6bc653f7f fix(server): schedule interrupted tool claims by their deadlines (#17458)
## Thinking Path

> - Paperclip governs tool execution and keeps durable approval records.
> - An interrupted execution may have produced an external effect
without a saved final request state.
> - The maintenance tick currently scans executing requests even when
none exist.
> - Every executing claim already has a ten-minute stale deadline.
> - This PR schedules that deadline from committed claim changes and
clears it after settlement.
> - Recovery preserves saved provider outcomes and quarantines unknown
outcomes without another provider call.

## Linked Issues or Issue Description

Refs #17454. This is the sixth recurring-server sleep slice, stacked on
`codex/sleep-05-approved-tool-recovery`.

**Problem**
The aggregate maintenance tick repeatedly searches for stale executing
tool requests. Its settlement can overwrite a saved successful
invocation with an unknown-outcome failure after a crash between
invocation and request writes.

**Proposed Solution**
Use the existing review notification boundary and shared scheduler. Read
the earliest executing claim deadline through a partial index. At that
deadline, lock invocation then request, recheck claim identity, and
atomically settle from the saved outcome or quarantine the unknown
outcome.

**Alternatives Considered**
A longer polling interval still queries empty work. A new queue would
duplicate existing records. Replaying an interrupted call can duplicate
an external effect. Treating every stale request as unknown discards
already saved outcomes.

## What Changed

- Coalesce sequential review notifications for up to 30 seconds while
preserving already loaded recovery deadlines. Do not schedule scans when
no notification or execution deadline remains.

- Extract interrupted-execution deadline lookup and settlement into the
owning tool-review service.
- Register its worker at startup. Use committed claim/result changes to
update the existing ten-minute deadline; retry failed settlement after
30 seconds.
- Preserve terminal invocation results, reasons, and completion times
when repairing request state.
- Settle uncertain request/invocation state atomically with the existing
unknown-outcome reason, without dispatching a provider call.
- Notify all gateway claim and settlement paths through the existing
review transaction helper. Keep framework wiring and the shared
scheduler free of provider-specific behavior.
- Add an idempotent concurrent partial index for executing deadlines.
Preserve the combined manual sweep and scheduler-disabled startup pass.
- Remove interrupted scanning from the aggregate tick. Interaction
projection repair remains the next separate duty.

## Verification

Current head: `648d8a9a64ded15deeeb331545b92a12d69e277d`. The final
commit changes no files and requests CI after the master retarget. A
preceding run appeared while that trigger was being published; it was
observational delay, not an established CI outage. No test rerun or
bypass was used to turn a failure green. Source remains identical to the
198-test passing integration, apart from the already recorded ledger
update. Fresh checks and both automated reviews must qualify this
current head before merge.


Current master-targeted head:
`d55e44b376555e4926bb2c2a0e4dcfb431324162`. Parent #17454 landed as
`77295faebc8`; a normal merge reconciles that squash. Every conflicted
master file was checked against the already-integrated parent, and all
matched byte-for-byte. Each interruption-only hunk was retained; the
complete integrated source tree matched the 198-test passing
`848f1b87b3` exactly before the sole final documentation update. All
master migration journal entries are unchanged, with only this PR's
executing-deadline index appended. Server types pass. Fresh CI and
current-head Greptile/Ponytail are required before the next authorized
merge.


Latest published integration:
`848f1b87b3a6646ae4e56e8ab142f64889d7a69c`, normally merging
approved-recovery parent `23868c1751`. All 198
gateway/interruption/startup/deadline cases and server production
typechecking pass. Both parent approval fixes and all interruption tests
are retained. The two merge conflicts preserved the interruption import
alongside the parent's claim-bound hash helper and preserved the
parameterized historical test helpers. This remains stacked on #17454;
fresh CI and both automated review gates are required after integration.


- Review documentation correction at
`c985eb9d1b48a481909de6a1c2d89d7af11bfc03`: the focused real-PostgreSQL
interruption-worker selection passes all 16 cases; 124 unrelated cases
were filtered out. The docs now correctly name 16 cases and six saved
terminal statuses. No production behavior changed in the correction.
Fresh remote CI/review is pending after integrating the verified parent.

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/f6a5949fb726f71c247950714adecb275655284f/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: `f6a5949fb726f71c247950714adecb275655284f`. This remains a
single-duty PR stacked on #17454.

- 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/eaac8d343e5ef4d2c13cdf7cb7607cb5a498e9a2/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

- Signals cover the existing single-active-writer database owner.
Independent writers and replicas need an explicit wake contract.
- The ten-minute policy is unchanged. A provider call that outlives its
claim remains an uncertain outcome; this worker never retries the
external call.
- Interaction projection follows authoritative settlement outside its
transaction. Its separate existing repair duty still handles an
interrupted projection.
- Non-executing review writes can cause one empty reconciliation.
Without an executing claim or failed attempt, no recurring deadline
remains.
- Cloud eligibility and external-alarm coverage remain conservative.
Worker quiet does not prove actual host or database 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-27 16:33:30 -05:00

62 lines
3.2 KiB
TypeScript

import { and, asc, eq, gt, lte } from "drizzle-orm";
import { toolActionRequests, toolInvocations, type Db } from "@paperclipai/db";
import { writeToolReviewChange } from "./tool-review-work-notifications.js";
const STALE_EXECUTION_MS = 10 * 60_000;
export function toolReviewInterruptionService(db: Db, onSettled: (input: {
actionRequestId: string;
invocationId: string;
status: "executed" | "failed";
errorCode: string | null;
errorMessage: string | null;
resultSummary?: string;
}) => Promise<void>) {
return {
async next(): Promise<number | null> {
const [row] = await db.select({ at: toolActionRequests.updatedAt }).from(toolActionRequests)
.where(eq(toolActionRequests.status, "executing")).orderBy(asc(toolActionRequests.updatedAt)).limit(1);
return row ? row.at.getTime() + STALE_EXECUTION_MS : null;
},
async sweep(now = new Date()) {
const staleAt = new Date(now.getTime() - STALE_EXECUTION_MS);
let cursor: string | undefined;
let scanned = 0;
for (;;) {
const rows = await db.select().from(toolActionRequests).where(and(
eq(toolActionRequests.status, "executing"), lte(toolActionRequests.updatedAt, staleAt),
cursor ? gt(toolActionRequests.id, cursor) : undefined,
)).orderBy(asc(toolActionRequests.id)).limit(100);
for (const row of rows) {
const settled = await writeToolReviewChange(db, row.companyId, async tx => {
const [invocation] = await tx.select().from(toolInvocations)
.where(eq(toolInvocations.id, row.invocationId)).for("update");
const [request] = await tx.select().from(toolActionRequests)
.where(eq(toolActionRequests.id, row.id)).for("update");
if (!invocation || request?.status !== "executing" ||
request.updatedAt.getTime() !== row.updatedAt.getTime()) return null;
const completed = ["succeeded", "failed", "denied", "timed_out", "rate_limited", "cancelled"].includes(invocation.status);
const status = invocation.status === "succeeded" ? "executed" as const : "failed" as const;
const errorCode = completed ? invocation.errorCode : "tool_execution_outcome_unknown";
const errorMessage = completed ? invocation.errorMessage
: "Execution was interrupted; the external outcome is unknown. Inspect the provider before retrying.";
if (!completed) {
await tx.update(toolInvocations).set({ status: "failed", errorCode, errorMessage,
completedAt: now, updatedAt: now }).where(eq(toolInvocations.id, row.invocationId));
}
await tx.update(toolActionRequests).set({ status, resolvedAt: invocation.completedAt ?? now, updatedAt: now })
.where(eq(toolActionRequests.id, row.id));
return { actionRequestId: row.id, invocationId: row.invocationId, status,
errorCode, errorMessage, resultSummary: invocation.resultSummary?.summary };
});
if (settled) await onSettled(settled);
}
scanned += rows.length;
if (rows.length < 100) break;
cursor = rows[rows.length - 1].id;
}
return { scanned };
},
};
}