From 77295faebc8fa0c3aa455523f2eb084974d8358d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:10:44 -0500 Subject: [PATCH] fix(server): recover approved tool actions from committed changes (#17454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- doc/idle-approved-tool-recovery.md | 381 ++++++++ ...2026-10-24-recurring-server-sleep-stack.md | 115 ++- packages/db/src/client.ts | 1 + .../migrations/0384_funny_adam_warlock.sql | 1 + ...{0379_snapshot.json => 0384_snapshot.json} | 85 +- packages/db/src/migrations/meta/_journal.json | 7 + packages/db/src/schema/tool_access.ts | 1 + .../connection-management.integration.test.ts | 91 +- .../__tests__/helpers/runner-api-server.ts | 1 + .../server-startup-feedback-export.test.ts | 2 + .../tool-access-policy-service.test.ts | 120 ++- .../src/__tests__/tool-access-service.test.ts | 8 +- .../__tests__/tool-gateway-service.test.ts | 733 ++++++++++++++- server/src/index.ts | 11 +- server/src/services/approvals.ts | 17 +- .../services/delivery-work-notifications.ts | 1 + server/src/services/tool-access-policy.ts | 115 ++- .../src/services/tool-approval-execution.ts | 119 +++ server/src/services/tool-gateway.ts | 849 +++++++++--------- .../src/services/tool-review-recovery-work.ts | 33 + server/src/services/tool-review-recovery.ts | 119 +++ .../tool-review-work-notifications.ts | 5 +- server/src/services/tool-runtime-metrics.ts | 4 +- 23 files changed, 2313 insertions(+), 506 deletions(-) create mode 100644 doc/idle-approved-tool-recovery.md create mode 100644 packages/db/src/migrations/0384_funny_adam_warlock.sql rename packages/db/src/migrations/meta/{0379_snapshot.json => 0384_snapshot.json} (99%) create mode 100644 server/src/services/tool-approval-execution.ts create mode 100644 server/src/services/tool-review-recovery-work.ts create mode 100644 server/src/services/tool-review-recovery.ts diff --git a/doc/idle-approved-tool-recovery.md b/doc/idle-approved-tool-recovery.md new file mode 100644 index 00000000000..dbb7a8682c7 --- /dev/null +++ b/doc/idle-approved-tool-recovery.md @@ -0,0 +1,381 @@ +# Approved tool-action recovery + +Approved tool actions recover at startup and after committed review changes. +The aggregate maintenance tick no longer scans approved requests. The existing +request and invocation rows remain authoritative; this change adds no queue table. + +## Work and quiet states + +The gateway owns the approved-work predicate. A signed agent action with +`executionOnApprove: true` owes execution through the normal approval path. +A legacy signed action without that flag remains deliberately inert. A Test-tab +approval can owe execution even when no agent run exists. Recovery uses the saved +origin and current authorization/target checks; the scheduler sees only whether +another attempt is needed. + +Startup discovers companies with saved approved requests. Committed review +signals carry the affected company ID; subsequent discovery and recovery reads +filter by that company. The shared scoped worker coalesces changes and dispatches +one company per pass with a one-second minimum spacing. A company writing reviews +does not scan or lock another company's approved backlog. + +Recovery pages that company's approved requests in ID order using a partial index. It does not +let a full batch of unavailable approvals starve later work. A failed read or +unresolved executable approval retains a 30-second retry. New writes preserve +that company's retry deadline, while other companies can run independently. +A company pass still drains all its pages; this change does not cap provider +calls within that pass or alter the provider's existing execution timeout. Once a pass leaves no +such obligation, there is no recurring approved-recovery query. An empty system, +configured idle tools, completed history, and deliberately inert approvals are +quiet after reconciliation. + +## Producer and transaction ownership + +The existing tool-review notification boundary now signals both pending expiry +and approved recovery. This covers human review transactions, gateway approval +and accepted-interaction inference, administrative interaction resolution, review +queue invalidation, and the gateway's approval claims and managed-argument +invalidation. The notification is awaited before SQL in the existing transaction; +caller-owned outer transactions retain commit/rollback ownership. + +A pending-only change can cause one empty company-scoped discovery read. It does not +create a periodic scan. No per-route recovery bookkeeping or database listener +is added. Signals use the existing database owner's single-active-writer model. +Independent SQL writers or another process need a separate notification contract. + +## Single execution and outcome ownership + +Agent actions retain their existing approved-to-executing claim. Test-tab +foreground approval and recovery now share an equivalent claim before provider +dispatch. The invocation then starts only if the request still holds that claim. +Provider calls and target discovery remain outside SQL transactions. Current rate-policy admission and its receipt commit together in a short database transaction. + +The Test-tab invocation outcome and terminal request status commit together. +Claim timestamps fence late settlement. A later audit failure cannot overwrite +an already committed outcome. Recovery repairs legacy approved Test-tab requests +from a saved terminal invocation without decoding old arguments or calling the +provider. A legacy executing invocation moves to the existing interrupted-work +path; it is not dispatched again. + +If the provider may have run but its result cannot be persisted, the executing +claim remains authoritative. The existing stale-execution recovery quarantines +unknown outcomes. This slice does not promise exactly-once external effects or +replay an uncertain provider call. + +## Lifecycle and remaining duties + +The shared deadline worker coalesces notifications, preserves changes during a +pass, and retains skipped work across idle drain/resume. Startup reconstructs +saved approvals. Company deletion invalidates cached recovery and retries through +the same transactional notification boundary. Shutdown stops its coordinator registration. With heartbeat +scheduling disabled, startup keeps the existing one-off recovery behavior. + +Pending expiry is owned by the preceding slice. Interrupted execution and +interaction projection repair remain in the old sweep until their own migrations. +The public combined sweep retains all duties for manual/recovery callers. +No Cloud eligibility or external-wake proof changes here; worker quiet does not +establish deployed database or host suspension. + +## Verification + +Real PostgreSQL service/startup tests cover producer commit and rollback, +startup reconstruction, empty/configured/history quiet states, concrete retries, +first-write drain/resume, legacy inert approvals, and multi-page recovery. +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 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. + +The existing tool-access service regression now expects a completed Test-tab +request to be executed and verifies exactly one provider call before asking for +a new approval. Its full 469-test suite passes. The initial local command skipped +its database cases; the suite now honors its supplied disposable database through +the standard test-database guard independently of embedded Postgres support. + +Company-scoping regression: a real signed/reviewed action in company A previously +caused an unfiltered approved-request SELECT over company B's inert legacy action. +The captured-SQL test fails before this repair and passes with scoped reads. Added +lifecycle cases check retry preservation across committed writes, independent +company progress, and disarming after the real company-deletion producer. These +checks supplement the earlier execution, transaction, and paging evidence above. + +After company scoping, the complete gateway-service and startup selection passes +149 tests against a disposable PostgreSQL database, and server typecheck passes. +Stack integration, workspace checks, and fresh remote CI remain separate gates. + +## Preparation failures retain an approved Test-tab action + +A real PostgreSQL policy-read timeout reproduced a permanent-failure bug after +claiming a Test-tab action: the provider had never been called, but both request +and invocation were saved as failed, so approved recovery could not retry them. + +The preparation boundary now distinguishes explicit validation/access rejections +from unexpected dependency errors and transient HTTP errors. The existing +`tool-approval-execution` owner releases only its exact claim: invocation and request +are locked in the established order, their link/company/claim timestamp must +match, and the invocation must still await approval with no recorded start. +The request returns to `approved` with a transaction-aware notification. Signed +arguments and the human approval remain unchanged; the error propagates so the +existing recovery worker retains its retry obligation. + +This boundary ends before `runTestToolInvocation`. Provider dispatch and outcome +persistence keep their existing conservative no-replay behavior. A changed claim, +started invocation or prior-dispatch evidence cannot be released by this path. +If the requeue transaction itself fails, the error propagates and no terminal +failure is fabricated; existing interrupted-execution reconciliation remains the +fallback. This change does not infer replay safety from a failed requeue. + +Before the fix, both foreground and recovery timeout regressions failed. They +now observe the committed claim under a real PostgreSQL table lock, wait for +`55P03`, verify unchanged approval and no provider call, then prove automatic +recovery invokes the provider once and becomes quiet. Additional real-row cases +cover changed ownership, mismatched invocation, a started call, prior-dispatch +history, and outer-transaction rollback. A released claim cannot start execution. +All 117 gateway-service cases pass on the owning slice, including existing target +invalidation, provider-outcome persistence failures and unknown-outcome no-replay +cases. Integrated consumer checks, workspace verification and remote PR gates +must still be recorded after propagation. + + +## Agent-origin preparation and dispatch ownership + +Agent-origin approvals use the same pre-dispatch retry contract as Test-tab +approvals. Previously, a policy or identity dependency failure after claiming an +approved action permanently failed it; earlier issue/signing failures could +leave the claim executing. Preparation now has one owning error boundary around +issue, signature, identity, target, policy, managed-argument, and content checks. +Validation and nontransient access rejections retain their terminal handling. +Unexpected dependency failures and transient HTTP errors release only the exact, +still-undispatched claim to the existing approved queue and publish its existing +transactional work signal. No table, maintenance scan, or retry wrapper per +producer is added. + +`tool-approval-execution.ts` owns the shared claim token, release, and dispatch +transition. Its Test-tab claim/outcome functions remain specific to that consumer. +Both origins use the same invocation-then-request locks and verify invocation, +company, request linkage, claim timestamp, awaiting-approval state, and absence +of an earlier start before dispatch. An agent that loses its claim during +preparation cannot call the provider or overwrite its successor. The Test-tab +unapproved direct-call path retains its original behavior. + +Only preparation is retryable here. Recording the invocation start, provider +execution, and outcome persistence remain outside that catch. A provider error, +unknown outcome, or failed durable release does not establish replay safety. +Existing interruption handling remains the conservative fallback. Closed-issue +expiry, signature/target invalidation, identity authority, and policy rejection +remain enforced, including the final issue check before recording dispatch. + +Real PostgreSQL lock-timeout cases cover foreground approval and recovery: +observe the committed executing claim, block the policy read, then require the +same signed request to remain approved with no invocation start and zero provider +calls. Recovery after releasing the table lock executes once and becomes quiet. +A separate contention case replaces the claim while preparation is blocked and +proves the old owner cannot dispatch; the surviving approval later executes once. +A provider-error case proves that an already-started call is not requeued as a +preparation failure. The existing Test-tab timeout and ownership cases exercise +the other consumer of the shared boundary. + + +## New or edited rate rules during approval + +A reviewed invocation already has durable `consumedRateAdmissions` in its policy +explanation. The original read-only recheck retained unchanged rules correctly, +but new or edited rules only inspected their counters. Two approved actions could +therefore both execute against one available slot. The real recovery regression +failed for both a newly added limit and an increased limit before this repair. + +The policy service now owns `admitInvocation`: lock the company-scoped invocation, +validate its exact actor/task/tool/argument binding, recheck current policy, spend +only admissions not already recorded for that policy generation, and save the +receipt atomically. The gateway supplies its execution claim; that claim is locked +and checked by ID, invocation, company, executing status and timestamp. This also +supports historical approved requests whose invocation still says pending, without +accepting a stale claimant. Existing claim/start/settlement owners still control +provider execution. No queue table, periodic query, or per-route wrapper is added. + +Counter updates preserve the number of already spent slots when the limit changes. +A rejection by a later policy or failure to save the receipt rolls back new charges. +A retry of a committed admission retains the saved receipt, including concurrent +rechecks. Read-only approval preflight and browser authority probes stay read-only; +trust-rule usage is not counted again during execution revalidation. The policy +service and its metric helper accept only the database capabilities they use, so +transaction clients do not require a cast to a root connection. + +The owning gateway and policy suites pass all 181 cases against disposable real +PostgreSQL databases. Coverage includes two competing invocations, concurrent +retries of one invocation, changed limits, failed receipt writes, rejection after +an earlier rate charge, unchanged admissions, historical approvals, scope mismatch, +claim loss, quiet recovery and unknown provider outcomes. Server typechecking +passes after preparing current runner dependencies. + +Earlier verification failures remain part of this evidence: the first full run +was stopped because an old claim-loss fixture held the policy table while trying +to revoke the newly locked invocation. It now pauses discovery before admission +and retains the no-provider-dispatch assertion. A new receipt assertion was fixed +to permit the intended additional receipt while preserving original fields. The +next full run exposed an overly strict historical-approval check; exact execution- +claim validation fixed it. Typechecking then exposed root-client-only signatures, +which were narrowed to their actual database capabilities. The final complete +suites and TypeScript check pass; stack integration and current-head CI remain +separate gates. This proves worker and admission behavior, not deployed host sleep. + + +## Managed-argument expiry ordering + +Latest-head CI exposed a preparation-order regression in +`tool-gateway.test.ts`: a valid historical Shopify approval whose managed UCP +arguments were obsolete became `failed` instead of `expired`. The new consuming +admission compared the current invocation identity before the existing managed +argument expiry owner ran. It prevented dispatch, but bypassed the correct saved +expiry outcome. + +Approved execution now verifies target/signed/managed arguments and content +before consuming admission. Exact claim fencing, fresh policy checks, atomic +rate receipts, and the final issue-open check remain before dispatch. No check +was removed and stale payloads are not permitted to execute. + +The existing regression reproduced the CI failure on this owner in +`/tmp/sleep-approved-managed-expiry-red.log`. All 92 gateway acceptance cases +pass after the ordering change (`/tmp/sleep-approved-managed-expiry-full.log`), +including the historical approval's expired request, failed invocation with +`approvalState: expired`, and zero provider requests. Server typecheck passes +in `/tmp/sleep-approved-managed-expiry-types.log`. Latest-head remote CI and +combined-stack acceptance still need their own results. + +## Delegated Test-tab admission after policy changes + +A delegated Test-tab call has an originating agent but no gateway token. The +named-gateway admission path therefore did not evaluate its caller policy. If a +rate rule was added while approval was pending, approval charged only the target. +A shared company/tool bucket then rejected the caller against that same charge; +a caller-specific rule instead allowed execution without recording its charge. + +The Test-tab owner now supplies its caller/target policy evaluation to the existing +claim-transaction admission API. Both decisions use the claimed invocation's +receipts. The target can share the caller's charge only when the rule and bucket +match. Both receipts commit together. A denial rolls back new charges and receipts. +Management reauthorization and credential identity checks remain with their owners, +and provider execution stays outside the transaction. This adds no queue or timer. + +Three actual HTTP/PostgreSQL cases cover a newly added shared rate rule, separate +caller and target rules, and a target denial after caller admission. The unchanged +implementation failed the first two cases: the shared rule rejected execution, +and separate rules left only one charge. The denial case already passed. After +the repair, all three pass, including duplicate approval, both saved receipts, +one provider execution, and rejection of a subsequent authorized call. + +The full connection-management suite passes all 78 cases against a fresh database. +All 92 existing gateway tests and the server production typecheck also pass. +Intermediate fixture failures remain accounted for: the new assertion initially +used a nonexistent counter count field instead of remaining; the subsequent-call +fixture first reused an ended run and then lacked responsible-user authority. +The final fixture uses a new live run with the original responsible user. A reused +full-suite database also retained a conflicting Slack bot identity; final acceptance +uses a fresh isolated database. These are local checks, not current-head CI or a +host-suspension qualification. + + +## Historical argument identity compatibility repair + +Earlier invocations stored the hash of their redacted argument preview. Modern +admission correctly requires the complete request identity, including a keyed hash +for sensitive arguments. The gateway already verified signed historical arguments, +but admission rejected their saved preview hash even with no rate policy. That +permanently failed an otherwise valid approval before the provider call. + +Both agent-origin and Test-tab preparation now bind the verified complete identity +for a historical invocation before current policy admission. Signature, reviewed +target and managed-argument checks still run first. The approval execution owner +performs the identity transition in an invocation-then-request lock transaction. +It requires the exact current claim, company and invocation relationship, an +undispatched awaiting-approval invocation, and the expected reviewed/preview hash. +Unrelated saved hashes are not accepted. Modern matching identities do not add +this transaction. Current policies and rate charges still apply afterward. + +Four real-PostgreSQL regressions reproduced the defect: valid sensitive historical +arguments, with and without a newly configured rate rule, through each of the two +origins. Four controls keep invalid signatures and unrelated invocation hashes +rejected without dispatch or identity changes. The provider receives the original +verified arguments exactly once, rather than their redacted preview. A separate +real-transaction case proves a replaced claim cannot change the saved identity. + +The first draft fixtures used a nonexistent request-level tool name and then +non-sensitive arguments whose preview and complete hashes were equal. Those +fixture failures are not credited as product regressions. The corrected four +positive cases fail before the repair and all eight parameter cases pass after. +The complete gateway, policy and startup selection passes 230 cases with no skips; +server types pass. The complete assembled-server connection-management suite also +passes all 78 cases, including delegated approval and rate-policy behavior. Fresh +remote CI and reviews remain required after publication. + + +The repair was then integrated with the master-based parent, including #17918's +tracing changes, at `81231824d773a79fc50c835eccf7d9b3977e79fc`. Server types and the +production server build pass. The gateway, policy, startup and preparation-timing +suites pass all 242 cases. The complete HTTP management suite passes all 78 cases +on a fresh dedicated database, for 320 passing cases across the accepted runs. + +The first combined integrated run passed 319 cases and failed the Slack setup +case because its reused HTTP database still held the previous run's bot identity. +The existing ownership guard correctly rejected the duplicate connection. A fresh +dedicated fixture resolved that environment collision; no product behavior or +assertion changed for the rerun. That failed run remains part of the evidence. + + +## Gateway admission uses its transaction connection + +Review of the master-targeted branch exposed a delegated gateway deadlock: the +admission transaction held the sole pool connection while issuer/token authority +reads used the root client. Foreground approval and approved recovery each failed +the real one-connection regression; a revoked-issuer control already passed. +The admission callback now receives its transaction and passes it through the +existing issuer authority checks. Both connection authorization checks and the +token read use that connection. Ordinary checks retain their root client, and +current authority, claim, company and rate enforcement remain in place. +The three constrained-pool cases pass, as do all 81 HTTP integration cases and +190 gateway, policy, startup and deadline consumer cases. The initial consumer +command selected an incorrect policy filename and omitted the gateway database +override; it was interrupted and is not counted as passing evidence. + +## Pending historical approvals retain their original admission + +A second review finding concerned the normal foreground approval gate, before +execution has claimed and rebound historical arguments. A real PostgreSQL +regression with signed sensitive arguments and an already-consumed limit of one +failed with `rate_limited`; the Test-tab and invalid-signature controls passed. +The gateway now supplies the preview digest only after verifying the signed +payload and matching either supported form of the review's canonical digest. Policy may match that digest +only in a read-only preflight, with every other invocation-context field and +stored rate-rule receipt still checked. The consuming admission continues to +require the complete keyed identity, rebound under the exact execution claim. +New or changed rate rules still apply. Historical recovery, invalid signatures, +foreign invocation hashes, stale claims, and provider argument preservation stay +covered. This adds no schema, recurring query or additional connection. + +The preceding CI head also failed E2E shard 3/8. Its failure is retained in the +validation record; local gateway checks do not erase it. Fresh-head CI and both +required automated reviews remain merge gates. + +The five full gateway/policy/startup/deadline suites passed 329 cases before the +additional mixed historical identity control. The first combined attempt is +retained: a Test-tab failure-result assertion incorrectly expected a thrown +error, and reusing the earlier HTTP database left formal-approval foreign keys +that prevented gateway acceptance setup. The assertion now checks the existing +failed-result contract, with unchanged no-provider, hash and counter checks; +the complete acceptance rerun used an independently migrated gateway database. +Server production typechecking and build passed. The earlier E2E failure was a +500 during the agent-chat project-plan fixture restoration, with a PostgreSQL +deadlock in cancellation; it remains a fresh-head CI gate. diff --git a/doc/plans/2026-10-24-recurring-server-sleep-stack.md b/doc/plans/2026-10-24-recurring-server-sleep-stack.md index 5bdacbcd130..b699f1b242d 100644 --- a/doc/plans/2026-10-24-recurring-server-sleep-stack.md +++ b/doc/plans/2026-10-24-recurring-server-sleep-stack.md @@ -18,7 +18,7 @@ Do not mark an item done from a timer removal or a green check alone. | 04 | Decision retention and notifications | Retention deadlines and notification delivery are separate reasons | Pending | | 05 | Decision expiry and interrupted effects | Expiry, effect recovery, continuation recovery | Pending | | 06 | Tool connection health | Configuration/use activation and defined proactive health deadlines | Pending | -| 07 | Secret proposals, tool reviews, gateway tokens | Separate expiry/scrub/recovery owners | Secret proposals: [#17446](https://github.com/paperclipai/paperclip/pull/17446), merged; gateway credentials: [#17450](https://github.com/paperclipai/paperclip/pull/17450), merged; pending tool-review expiry implemented on slice 04; other review duties pending | +| 07 | Secret proposals, tool reviews, gateway tokens | Separate expiry/scrub/recovery owners | Secret proposals: [#17446](https://github.com/paperclipai/paperclip/pull/17446), merged; gateway credentials: [#17450](https://github.com/paperclipai/paperclip/pull/17450), merged; pending tool-review expiry: [#17453](https://github.com/paperclipai/paperclip/pull/17453), merged; approved recovery: [#17454](https://github.com/paperclipai/paperclip/pull/17454), historical argument identity repair verified and awaiting fresh CI/review; other review duties pending | | 08 | GitHub connection events and continuity | Event discovery/replay and credential continuity separately | Pending | | 09 | Merged-PR confirmations | Candidate writes, webhook activation where supported, outstanding-candidate fallback | Pending | | 10 | External-object refresh | Configuration changes, refresh deadlines, source events | Pending | @@ -117,3 +117,116 @@ index, applies/replays its migration, and preserves approval-time rejection and separate approved/executing/projection recovery. Workspace typecheck/build and all 92 existing gateway acceptance tests also passed. The first acceptance command matched no files; the corrected `tool-gateway.test.ts` selection passed. Fresh PR CI/review remains required. + + +## Approved-review follow-up repairs (2026-10-26) + +Slice 05 preserves the rate slot admitted before a human review. Agent and +Test-tab foreground/recovery paths reuse only the exact invocation/context and +unchanged rate-policy admission while rechecking current policy. A one-call +limit must not reject the same approved operation against its own occupied slot. + +Recovery now locks and rereads invocation/request history before dispatch. +Finished records repair the request without provider replay; executing records +retain unknown-outcome recovery. Invalid signatures, unusable formal approvals, +and inconsistent invocation states settle once. Missing signing-key configuration +and temporary dependencies retain retries. Company mismatches never mutate the +foreign invocation. No new table or generic HTTP-error discard rule is introduced. + +Formal approvals awaiting human input have no recovery cadence. The existing +approval service publishes commit-aware review notifications for approve/reject +and cancellation. Its shared record owner retains the decision validation/CAS +contract; hire decisions remain with their lifecycle owner. Revision/resubmission +keeps the review parked, and startup rediscovers saved work. Budget approval writes +belong to budget incidents, not tool-gateway formal reviews. Company deletion +removes the dependent work. These exclusions come from the producer audit. + +Local evidence: the four rate-admission regressions, seven permanent-record +regressions, and four formal-approval commit/rollback regressions failed before +their respective repairs. The complete gateway, policy, approval-service and +signature suites pass (208 tests), and server typechecking passes. Configuration +failure, company boundaries, concurrent claims, provider-outcome persistence +failure, restart and paging remain covered. The paging fixture now has independent +invocations instead of 101 approvals sharing an already-expired invocation. +Full remote checks and downstream integration verification remain required; +this is not an all-stack acceptance or deployed-sleep claim. + +## Next owner audit: approved tool-action recovery + +The next branch is `codex/sleep-05-approved-tool-recovery`, based on pending +expiry PR #17453 at `ce6a260a83`. Keep this separate from interrupted execution +and terminal projection repair. The public combined sweep remains compatible. + +- `commitToolActionReview` and accepted-interaction inference in the gateway are + approved-state producers. Existing notification ownership from slice 04 can + carry the new duty without adding another wrapper around callers. +- Agent execute-on-approve uses an atomic approved-to-executing claim before + provider execution. Keep that claim and all current authorization, signed + argument, current-target, identity, and issue-state checks in the gateway. +- Approved history is not uniformly executable. Legacy signed arguments without + `executionOnApprove` are deliberately inert. The payload can be encrypted; + shared scheduling must not decode it or inspect private provider fields. +- Board Test-tab invocations are a separate execution origin. The normal fresh + approval calls `runApprovedTestInvocation`, but the already-approved branch + currently skips test-origin execution. Test execution can leave the request + approved while the invocation holds the completed result. Do not treat those + completed rows as perpetual unfinished work. +- Before driving test-origin recovery, establish the same single-execution + claim on both foreground and recovery paths. The existing test helper does + not claim the request before provider dispatch. Simply invoking it from a + new worker would risk duplicate effects during foreground/recovery races. +- Distinguish saved completed outcomes, never-dispatched approved work, and + interrupted execution with unknown outcomes. Known completed provider results + must not be converted to unknown-outcome failures because another projection + write was interrupted. Retain quarantine for actually uncertain calls. +- Concrete unresolved approval work may retain a retry; configured idle, + terminal invocation history, and deliberately inert legacy approvals must + not cause empty recurring scans. Preserve restart reconstruction and all + supported producer paths, including transitions that finish an approved row. + +This is an audit, not an implementation or readiness claim. Tests must exercise +real approval producers, foreground/recovery contention, provider failure, +commit/rollback, legacy inert approvals, completed Test-tab history, and actual +unfinished Test-tab recovery before the next PR is ready. + +Current parent CI note: #17450 at `c5c53fc43f` has Greptile 5/5. Its server shard +10/12 failed the unchanged login-cleanup consume test waiting for an idle hold. +All 16 login-cleanup tests passed locally on recheck. The run remains active; +an attempted single-job rerun was rejected as not rerunnable. Recheck after the +run finishes before requesting its one justified retry. Do not report it green. + + +Slice 05 implementation now separates approved recovery from the aggregate tick. +The existing review notification boundary wakes it after outer commit. Legacy +inert approvals and completed Test-tab history become quiet; unresolved work +retains a retry. Test-tab foreground and recovery share a claim and atomic outcome +settlement. No provider call occurs inside that transaction. See +[approved recovery](../idle-approved-tool-recovery.md). Local service/startup +verification passes 122 tests, gateway acceptance passes 92, and workspace +typecheck/build pass. Paging and outcome-failure tests passed again after their +final strengthening. Full CI/review is not yet complete. + +Parent CI follow-up: #17450's completed run was retried once for its unchanged +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. + +#17453 now has passing checks after its single Runner retry. #17454's initial +server shard found an old expected approved state for a completed Test-tab call; +the updated regression requires executed and one provider call. All 469 +tool-access service tests pass after enabling the existing external database +path independently of embedded support. Its initial browser shard failed during +unrelated plan-task fixture company deletion with a concurrent activity-log FK +violation. Inspect fresh-head CI before deciding whether that needs a repair. diff --git a/packages/db/src/client.ts b/packages/db/src/client.ts index df20aa973ab..b3d66e14855 100644 --- a/packages/db/src/client.ts +++ b/packages/db/src/client.ts @@ -503,6 +503,7 @@ const MIGRATIONS_WITH_TOP_LEVEL_STATEMENTS = new Set([ "0369_late_logan.sql", "0381_mean_scream.sql", "0382_narrow_the_executioner.sql", + "0384_funny_adam_warlock.sql", "0383_tool_review_company_expiry.sql", ]); diff --git a/packages/db/src/migrations/0384_funny_adam_warlock.sql b/packages/db/src/migrations/0384_funny_adam_warlock.sql new file mode 100644 index 00000000000..c4f62d14e82 --- /dev/null +++ b/packages/db/src/migrations/0384_funny_adam_warlock.sql @@ -0,0 +1 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS "tool_action_requests_approved_idx" ON "tool_action_requests" USING btree ("id") WHERE "tool_action_requests"."status" = 'approved'; diff --git a/packages/db/src/migrations/meta/0379_snapshot.json b/packages/db/src/migrations/meta/0384_snapshot.json similarity index 99% rename from packages/db/src/migrations/meta/0379_snapshot.json rename to packages/db/src/migrations/meta/0384_snapshot.json index 98067609cb0..bb2ed3c9595 100644 --- a/packages/db/src/migrations/meta/0379_snapshot.json +++ b/packages/db/src/migrations/meta/0384_snapshot.json @@ -1,6 +1,6 @@ { - "id": "2a9439e3-4a2b-4a20-ab2b-2aee3db8cb03", - "prevId": "a0fcd9b8-f125-4985-8795-f567466b9aef", + "id": "4f96f70a-fd19-4d35-ad8a-8d5559b83cbd", + "prevId": "410457fc-266b-4bd0-8bc0-2b14b0634d07", "version": "7", "dialect": "postgresql", "tables": { @@ -52812,6 +52812,66 @@ "method": "btree", "with": {} }, + "tool_action_requests_pending_expiry_idx": { + "name": "tool_action_requests_pending_expiry_idx", + "columns": [ + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "where": "\"tool_action_requests\".\"status\" = 'pending'", + "concurrently": false, + "method": "btree", + "with": {} + }, + "tool_action_requests_company_pending_expiry_idx": { + "name": "tool_action_requests_company_pending_expiry_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "where": "\"tool_action_requests\".\"status\" = 'pending'", + "concurrently": false, + "method": "btree", + "with": {} + }, + "tool_action_requests_approved_idx": { + "name": "tool_action_requests_approved_idx", + "columns": [ + { + "expression": "id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "where": "\"tool_action_requests\".\"status\" = 'approved'", + "concurrently": false, + "method": "btree", + "with": {} + }, "tool_action_requests_invocation_idx": { "name": "tool_action_requests_invocation_idx", "columns": [ @@ -55222,6 +55282,21 @@ "method": "btree", "with": {} }, + "tool_gateway_sessions_expiry_idx": { + "name": "tool_gateway_sessions_expiry_idx", + "columns": [ + { + "expression": "expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, "tool_gateway_sessions_run_idx": { "name": "tool_gateway_sessions_run_idx", "columns": [ @@ -55967,6 +56042,12 @@ "notNull": true, "default": "''" }, + "responsible_user_id": { + "name": "responsible_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, "owner_note": { "name": "owner_note", "type": "text", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 02023247485..6b79f74c167 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -2675,6 +2675,13 @@ "when": 1793060791727, "tag": "0383_tool_review_company_expiry", "breakpoints": true + }, + { + "idx": 384, + "version": "7", + "when": 1793060791728, + "tag": "0384_funny_adam_warlock", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/tool_access.ts b/packages/db/src/schema/tool_access.ts index e970c445b6c..f1e788626e3 100644 --- a/packages/db/src/schema/tool_access.ts +++ b/packages/db/src/schema/tool_access.ts @@ -836,6 +836,7 @@ export const toolActionRequests = pgTable( .where(sql`${table.status} = 'pending'`), index("tool_action_requests_company_pending_expiry_idx").on(table.companyId, table.expiresAt) .where(sql`${table.status} = 'pending'`), + index("tool_action_requests_approved_idx").on(table.id).where(sql`${table.status} = 'approved'`), index("tool_action_requests_invocation_idx").on(table.invocationId), index("tool_action_requests_issue_idx").on(table.companyId, table.issueId), ], diff --git a/server/src/__tests__/connection-management.integration.test.ts b/server/src/__tests__/connection-management.integration.test.ts index 8f091fa49d6..2a91b321306 100644 --- a/server/src/__tests__/connection-management.integration.test.ts +++ b/server/src/__tests__/connection-management.integration.test.ts @@ -1,10 +1,12 @@ +import { createToolGatewayService } from "../services/tool-gateway.js"; +import { commitToolActionReview } from "../services/tool-action-review.js"; import { googleWorkspaceSelectedScopes, defaultGitHubReviewPolicy, SLACK_CHAT_BOT_SCOPES, SLACK_BOT_TOOL_SCOPES } from "@paperclipai/shared"; import { randomUUID, generateKeyPairSync } from "node:crypto"; import { join } from "node:path"; import { createServer } from "node:http"; -import { and, eq } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import { beforeAll, afterAll, describe, expect, it, vi } from "vitest"; -import { activityLog, agentApiKeys, companies, companySecrets, companySecretVersions, chatConversations, chatDeliveries, chatEndpoints, chatExternalPrincipals, chatIdentityLinks, toolPolicies, connectionGrants, connectionGrantDelegations, toolAccessAuditEvents, toolOauthStates, issueThreadInteractions, connectionIntentDeliveries, agents, companyMemberships, heartbeatRuns, issues, principalPermissionGrants, projects, toolConnections, toolMcpGateways, toolMcpGatewayTokens, toolActionRequests, toolCatalogEntries, toolInvocations, toolCallEvents, toolProfileBindings, plugins, adapterAuthSessions, environments, routines, chatGitHubReviews } from "@paperclipai/db"; +import { createDb, activityLog, agentApiKeys, companies, companySecrets, companySecretVersions, chatConversations, chatDeliveries, chatEndpoints, chatExternalPrincipals, chatIdentityLinks, toolPolicies, toolRateLimitCounters, connectionGrants, connectionGrantDelegations, toolAccessAuditEvents, toolOauthStates, issueThreadInteractions, connectionIntentDeliveries, agents, companyMemberships, heartbeatRuns, issues, principalPermissionGrants, projects, toolConnections, toolMcpGateways, toolMcpGatewayTokens, toolActionRequests, toolCatalogEntries, toolInvocations, toolCallEvents, toolProfileBindings, plugins, adapterAuthSessions, environments, routines, chatGitHubReviews } from "@paperclipai/db"; import { findSatisfiedToolConnection, satisfiedConnectionIntentService } from "../services/satisfied-connection-intents.js"; import { connectionManagementService, continuingConnectionManagement } from "../services/connection-management.js"; import { redactRunnerApiPayloads } from "../services/native-runtime/runner-api-log-redaction.js"; @@ -234,7 +236,7 @@ describe("connection management with production default agents", () => { expect(bindings.map(binding => binding.targetId)).toEqual([f.agentId]); }); - it.each(["none", "other-agent", "other-agent-no-access", "other-agent-allow-policy", "other-agent-explicit-grant", "other-agent-caller-ask", "other-agent-caller-rate", "other-agent-shared-rate", "other-agent-credential", "other-agent-credential-none", "other-agent-caller-revoked", "private-status", "agent", "user", "cancelled", "project-block", "rate-limit", "agent-rate-approval", "authorized-policy-edit"] as const)("keeps delegated test policy and approval continuation bounded (%s)", async revocation => { + it.each(["none", "other-agent", "other-agent-no-access", "other-agent-allow-policy", "other-agent-explicit-grant", "other-agent-caller-ask", "other-agent-new-rate", "other-agent-new-shared-rate", "other-agent-new-rate-target-block", "other-agent-caller-rate", "other-agent-shared-rate", "other-agent-credential", "other-agent-credential-none", "other-agent-caller-revoked", "private-status", "agent", "user", "cancelled", "project-block", "rate-limit", "agent-rate-approval", "authorized-policy-edit"] as const)("keeps delegated test policy and approval continuation bounded (%s)", async revocation => { const f = await fixture(); let targetAgentId = f.agentId; if (revocation.startsWith("other-agent")) { @@ -247,7 +249,7 @@ describe("connection management with production default agents", () => { await lifecycle.process(targetAgentId); await vi.waitFor(async () => expect((await agentService(server.db).getById(targetAgentId))?.status).toBe("idle")); } - const allowed = revocation === "none" || revocation === "other-agent" || revocation === "other-agent-caller-ask"; + const allowed = revocation === "none" || revocation === "other-agent" || revocation === "other-agent-caller-ask" || ["other-agent-new-rate", "other-agent-new-shared-rate"].includes(revocation); let calls = 0; const remote = createServer(async (req, res) => { const chunks: Buffer[] = []; @@ -264,7 +266,7 @@ describe("connection management with production default agents", () => { const address = remote.address(); if (!address || typeof address === "string") throw new Error("Fixture did not listen"); const mcpUrl = `http://127.0.0.1:${address.port}/mcp`; - const freezeRateClock = ["other-agent-caller-rate", "other-agent-shared-rate", "rate-limit", "agent-rate-approval"].includes(revocation); + const freezeRateClock = revocation.startsWith("other-agent-new-") || ["other-agent-caller-rate", "other-agent-shared-rate", "rate-limit", "agent-rate-approval"].includes(revocation); if (freezeRateClock) vi.useFakeTimers({ toFake: ["Date"] }); try { const created = await f.request("POST", `/companies/${f.companyId}/tools/connections`, { name: "Governed test", transport: "mcp_remote", authKind: "none", transportConfig: { url: mcpUrl } }); @@ -381,6 +383,16 @@ describe("connection management with production default agents", () => { if (revocation === "cancelled") await server.db.update(issues).set({ status: "cancelled" }).where(eq(issues.id, f.issueId)); if (revocation === "other-agent-caller-revoked") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "Caller access revoked", policyType: "block", selectors: { connectionId: created.body.id, agentId: f.agentId }, priority: 0 }); if (revocation === "project-block") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New project block", policyType: "block", selectors: { connectionId: created.body.id, projectId: f.projectId }, priority: 0 }); + if (revocation.startsWith("other-agent-new-")) { + await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New delegated test admission", policyType: "rate_limit", + selectors: { connectionId: created.body.id, ...(revocation === "other-agent-new-shared-rate" ? {} : { agentId: f.agentId }) }, + priority: -2, config: { limit: 1, windowSeconds: 60, keyBy: revocation === "other-agent-new-shared-rate" ? ["company", "tool"] : ["agent", "tool"] } }); + if (revocation === "other-agent-new-rate") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New test target admission", policyType: "rate_limit", + selectors: { connectionId: created.body.id, agentId: targetAgentId }, priority: -2, + config: { limit: 1, windowSeconds: 60, keyBy: ["agent", "tool"] } }); + if (revocation === "other-agent-new-rate-target-block") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New test target block", policyType: "block", + selectors: { connectionId: created.body.id, agentId: targetAgentId }, priority: -1 }); + } const cookie = await server.humanSession(f.userId); const approve = () => fetch(server.apiUrl + "/api" + approvalPath, { method: "POST", headers: { cookie, origin: server.apiUrl, "Content-Type": "application/json" }, body: JSON.stringify({ companyId: f.companyId }) }); const response = await approve(); @@ -390,6 +402,20 @@ describe("connection management with production default agents", () => { else expect(settled, JSON.stringify(settled)).toMatchObject({ status: allowed ? "succeeded" : "failed" }); expect(calls).toBe(allowed ? 1 : 0); if (allowed) { expect((await approve()).status).toBe(200); expect(calls).toBe(1); } + if (revocation.startsWith("other-agent-new-")) { + const counters = await server.db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.companyId, f.companyId)); + expect(counters).toHaveLength(allowed ? revocation === "other-agent-new-shared-rate" ? 1 : 2 : 0); + expect(counters.every(counter => counter.remaining === 0)).toBe(true); + if (allowed) { + expect(settled.policyExplanation?.consumedRateAdmissions).toHaveLength(1); + expect(settled.policyExplanation?.delegatedCallerConsumedRateAdmissions).toHaveLength(1); + const [nextRun] = await server.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, + status: "running", responsibleUserId: f.userId, contextSnapshot: { issueId: f.issueId } }).returning(); + const nextJwt = createLocalAgentJwt(f.agentId, f.companyId, "paperclip_runner", nextRun.id, f.userId); + expect(await f.request("POST", path, body, nextJwt)).toMatchObject({ status: 200, body: { decision: "off", error: { reasonCode: "rate_limited" } } }); + expect(calls).toBe(1); + } + } } finally { if (freezeRateClock) vi.useRealTimers(); remote.closeAllConnections(); await new Promise((resolve, reject) => remote.close(error => error ? reject(error) : resolve())); } }); @@ -1229,7 +1255,7 @@ describe("connection management with production default agents", () => { expect((await use()).status).toBeGreaterThanOrEqual(400); }); - it.each(["block", "ask", "ask-board", "ask-revoked", "ask-rate", "ask-token-revoked", "ask-token-deleted", "ask-token-scope", "ask-grant-revoked", "rate", "shared-rate", "target-block", "credential"] as const)("intersects gateway issuer and target rules (%s)", async mode => { + it.each(["block", "ask", "ask-board", "ask-revoked", "ask-rate", "ask-new-rate", "ask-new-shared-rate", "ask-new-rate-target-block", "ask-token-revoked", "ask-token-deleted", "ask-token-scope", "ask-grant-revoked", "ask-one-connection", "ask-one-connection-recovery", "ask-one-connection-revoked", "rate", "shared-rate", "target-block", "credential"] as const)("intersects gateway issuer and target rules (%s)", async mode => { const f = await fixture(); const cookie = await server.humanSession(f.userId); const createdTarget = await fetch(`${server.apiUrl}/api/companies/${f.companyId}/agents`, { method: "POST", headers: { cookie, origin: server.apiUrl, "Content-Type": "application/json" }, @@ -1302,16 +1328,59 @@ describe("connection management with production default agents", () => { if (!mode.startsWith("ask")) return; const [action] = await server.db.select().from(toolActionRequests).where(eq(toolActionRequests.invocationId, invocation.id)); expect(action.requestedByAgentId).toBe(mode === "ask-board" ? null : f.agentId); + if (mode.startsWith("ask-new-")) { + await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New admission after review", policyType: "rate_limit", + selectors: { connectionId: connection.body.id, ...(mode === "ask-new-shared-rate" ? {} : { agentId: f.agentId }) }, + priority: -2, config: { limit: 1, windowSeconds: 60, keyBy: mode === "ask-new-shared-rate" ? ["company", "tool"] : ["agent", "tool"] } }); + if (mode === "ask-new-rate") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New target admission", policyType: "rate_limit", + selectors: { connectionId: connection.body.id, agentId: target.id }, priority: -1, + config: { limit: 1, windowSeconds: 60, keyBy: ["agent", "tool"] } }); + if (mode === "ask-new-rate-target-block") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "New target block", policyType: "block", + selectors: { connectionId: connection.body.id, agentId: target.id }, priority: -1 }); + } if (mode === "ask-revoked") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "Issuer blocked after review", policyType: "block", selectors: { connectionId: connection.body.id, agentId: f.agentId }, priority: -1 }); if (mode === "ask-token-revoked") await server.db.update(toolMcpGatewayTokens).set({ revokedAt: new Date() }).where(eq(toolMcpGatewayTokens.id, token.body.id)); if (mode === "ask-token-deleted") await server.db.delete(toolMcpGatewayTokens).where(eq(toolMcpGatewayTokens.id, token.body.id)); if (mode === "ask-token-scope") await server.db.update(toolMcpGatewayTokens).set({ allowedActions: ["tools/list"] }).where(eq(toolMcpGatewayTokens.id, token.body.id)); - if (mode === "ask-grant-revoked") await server.db.delete(principalPermissionGrants).where(and(eq(principalPermissionGrants.principalId, f.agentId), eq(principalPermissionGrants.permissionKey, "tools:manage_connections"))); - const approvalDenied = ["ask-revoked", "ask-token-revoked", "ask-token-deleted", "ask-token-scope", "ask-grant-revoked"].includes(mode); - const response = await fetch(`${server.apiUrl}/api/tool-gateway/action-requests/${action.id}/approve`, { method: "POST", headers: { cookie, origin: server.apiUrl, "Content-Type": "application/json" }, body: JSON.stringify({ companyId: f.companyId }) }); - expect(response.status, JSON.stringify(await response.json())).toBe(approvalDenied ? 403 : 200); + if (mode === "ask-grant-revoked" || mode === "ask-one-connection-revoked") await server.db.delete(principalPermissionGrants).where(and(eq(principalPermissionGrants.principalId, f.agentId), eq(principalPermissionGrants.permissionKey, "tools:manage_connections"))); + const approvalDenied = ["ask-revoked", "ask-new-rate-target-block", "ask-token-revoked", "ask-token-deleted", "ask-token-scope", "ask-grant-revoked", "ask-one-connection-revoked"].includes(mode); + if (mode.startsWith("ask-one-connection")) { + const limited = createDb(server.databaseUrl, { maxConnections: 1, idleTimeoutSeconds: 0, maxLifetimeSeconds: 0 }); + const service = createToolGatewayService(limited); + let timer: ReturnType | undefined; + let attempt: Promise | undefined; + try { + if (mode === "ask-one-connection-recovery") await commitToolActionReview(server.db, { + companyId: f.companyId, actionRequestId: action.id, decision: "approved", actor: { userId: f.userId }, + }); + attempt = mode === "ask-one-connection-recovery" ? service.recoverApprovedActionReviews(f.companyId) + : service.approveActionRequest({ companyId: f.companyId, actionRequestId: action.id, actor: { userId: f.userId } }); + void attempt.catch(() => undefined); + const completed = Promise.race([attempt, new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error("Single-connection gateway approval did not settle")), 5000); + })]); + if (approvalDenied) await expect(completed).rejects.toMatchObject({ status: 403 }); + else await completed; + expect((await limited.execute(sql`select 1 as available`))[0].available).toBe(1); + } finally { + clearTimeout(timer); await limited.$client.end({ timeout: 1 }); await attempt?.catch(() => undefined); + } + } else { + const response = await fetch(`${server.apiUrl}/api/tool-gateway/action-requests/${action.id}/approve`, { method: "POST", headers: { cookie, origin: server.apiUrl, "Content-Type": "application/json" }, body: JSON.stringify({ companyId: f.companyId }) }); + expect(response.status, JSON.stringify(await response.json())).toBe(approvalDenied ? 403 : 200); + } expect(calls).toBe(approvalDenied ? 0 : 1); - if (mode === "ask-rate") { + if (mode.startsWith("ask-new-")) { + const counters = await server.db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.companyId, f.companyId)); + expect(counters).toHaveLength(approvalDenied ? 0 : mode === "ask-new-shared-rate" ? 1 : 2); + expect(counters.every(counter => counter.remaining === 0)).toBe(true); + if (!approvalDenied) { + const [settled] = await server.db.select().from(toolInvocations).where(eq(toolInvocations.id, invocation.id)); + expect(settled.policyExplanation?.consumedRateAdmissions).toHaveLength(1); + expect(settled.policyExplanation?.delegatedCallerConsumedRateAdmissions).toHaveLength(1); + } + } + if (["ask-rate", "ask-new-rate", "ask-new-shared-rate"].includes(mode)) { await rpc("tools/call", { name: tool.name, arguments: { another: true } }); expect(calls).toBe(1); } diff --git a/server/src/__tests__/helpers/runner-api-server.ts b/server/src/__tests__/helpers/runner-api-server.ts index e59e1bb61fe..40cbabf896e 100644 --- a/server/src/__tests__/helpers/runner-api-server.ts +++ b/server/src/__tests__/helpers/runner-api-server.ts @@ -60,6 +60,7 @@ export async function startRunnerApiTestServer(options: { const apiUrl = `http://127.0.0.1:${address.port}`; setupRunnerPrpWebSocketServer(http, { apiUrl }); return { + databaseUrl: temporary.connectionString, db, root, apiUrl, storage, async humanSession(userId: string) { if (!auth) throw new Error("Enable humanSessions for this fixture"); diff --git a/server/src/__tests__/server-startup-feedback-export.test.ts b/server/src/__tests__/server-startup-feedback-export.test.ts index 2ede1c6c85f..658f5c418ff 100644 --- a/server/src/__tests__/server-startup-feedback-export.test.ts +++ b/server/src/__tests__/server-startup-feedback-export.test.ts @@ -69,6 +69,8 @@ const { stop: vi.fn(async () => {}), }, toolGateway: { + recoverApprovedActionReviews: vi.fn(async () => ({ scanned: 0, retry: false })), + approvedActionReviewCompanies: vi.fn(async () => []), sweepActionReviews: vi.fn(async () => ({ scanned: 0 })), sweepExpiredActionReviews: vi.fn(async () => ({ scanned: 0 })), nextActionReviewExpiryAt: vi.fn(async () => null), diff --git a/server/src/__tests__/tool-access-policy-service.test.ts b/server/src/__tests__/tool-access-policy-service.test.ts index e74fa60b9c9..adf2fd5a374 100644 --- a/server/src/__tests__/tool-access-policy-service.test.ts +++ b/server/src/__tests__/tool-access-policy-service.test.ts @@ -1,5 +1,5 @@ import { createHash, randomUUID } from "node:crypto"; -import { eq } from "drizzle-orm"; +import { eq, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { activityLog, @@ -34,8 +34,9 @@ import { toolAccessService } from "../services/tool-access.js"; import { createToolGatewayService, ToolGatewayHttpError } from "../services/tool-gateway.js"; import { canonicalToolArguments, signToolArguments } from "../services/tool-content-guards.js"; -const externalTestDatabaseUrl = process.env.PAPERCLIP_TOOL_POLICY_TEST_DATABASE_URL; -const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; +const externalDatabaseUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_TOOL_POLICY_TEST_DATABASE_URL"); +const embeddedPostgresSupport = externalDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; function legacySideEffectKey(invocation: typeof toolInvocations.$inferSelect, argumentsHash: string) { @@ -216,8 +217,11 @@ describeEmbeddedPostgres("tool access policy service", () => { let tempDb: Awaited> | null = null; beforeAll(async () => { - if (!externalTestDatabaseUrl) tempDb = await startEmbeddedPostgresTestDatabase("paperclip-tool-access-policy-"); - db = createDb(externalTestDatabaseUrl ?? tempDb!.connectionString); + if (externalDatabaseUrl) db = createDb(externalDatabaseUrl); + else { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-tool-access-policy-"); + db = createDb(tempDb.connectionString); + } }, 20_000); afterEach(async () => { @@ -1115,11 +1119,103 @@ describeEmbeddedPostgres("tool access policy service", () => { expect(await svc.decide(input, { admittedInvocationId: invocation.id })).toMatchObject({ allowed: true }); expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); expect((await db.select().from(toolRateLimitCounters))[0].remaining).toBe(0); - // Neither a new invocation nor an accidental consuming recheck can borrow - // another invocation's already-spent slot. + // A fresh invocation cannot borrow another invocation's already-spent slot. + // Execution revalidation retains its own durable admission. expect(await svc.decide(input, { admittedInvocationId: randomUUID() })).toMatchObject({ decision: "rate_limited" }); - expect(await svc.decide({ ...input, consumeRateLimit: true }, { admittedInvocationId: invocation.id })) - .toMatchObject({ decision: "rate_limited" }); + expect(await svc.admitInvocation(input, invocation.id)).toMatchObject({ allowed: true }); + expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + }); + + it("atomically charges a new rule once across concurrent retries of the same invocation", async () => { + const { company, input, invocation, svc } = await admittedRateLimitedInvocation(); + const added = await svc.createPolicy(company.id, { name: "New admission", policyType: "rate_limit", priority: 0, + selectors: { toolName: "send_email" }, config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + const oldCounters = await db.select().from(toolRateLimitCounters); + expect(await svc.decide(input, { admittedInvocationId: invocation.id })).toMatchObject({ allowed: true }); + expect(await db.select().from(toolRateLimitCounters)).toEqual(oldCounters); + const results = await Promise.all(Array.from({ length: 4 }, () => svc.admitInvocation(input, invocation.id))); + expect(results.every(result => result.allowed)).toBe(true); + expect(await db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.policyId, added.id))) + .toMatchObject([{ limit: 1, remaining: 0 }]); + const [saved] = await db.select().from(toolInvocations).where(eq(toolInvocations.id, invocation.id)); + expect(saved.policyExplanation?.consumedRateAdmissions).toHaveLength(2); + const { consumedRateAdmissions: oldAdmissions, ...oldExplanation } = invocation.policyExplanation!; + expect(saved.policyExplanation).toMatchObject(oldExplanation); + expect(saved.policyExplanation?.consumedRateAdmissions).toEqual(expect.arrayContaining(oldAdmissions as unknown[])); + }); + + it("allows only one of two admitted invocations to spend a newly added final slot", async () => { + const { company, input, invocation, svc } = await admittedRateLimitedInvocation(2); + const initial = await svc.decide({ ...input, consumeRateLimit: true }); + expect(initial.allowed).toBe(true); + const { invocation: second } = await svc.recordInvocation(input, initial); + await db.update(toolInvocations).set({ status: "executing" }).where(eq(toolInvocations.id, second.id)); + const added = await svc.createPolicy(company.id, { name: "One competing slot", policyType: "rate_limit", priority: 0, + selectors: { toolName: "send_email" }, config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + const results = await Promise.all([invocation.id, second.id].map(id => svc.admitInvocation(input, id))); + expect(results.filter(result => result.allowed)).toHaveLength(1); + expect(results.filter(result => result.decision === "rate_limited")).toHaveLength(1); + expect(await db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.policyId, added.id))) + .toMatchObject([{ limit: 1, remaining: 0 }]); + }); + + it("rolls back new rate charges when a later policy rejects the invocation", async () => { + const { company, input, invocation, svc } = await admittedRateLimitedInvocation(); + await svc.createPolicy(company.id, { name: "Before rejection", policyType: "rate_limit", priority: 0, + selectors: { toolName: "send_email" }, config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + await svc.createPolicy(company.id, { name: "Later rejection", policyType: "block", priority: 10, + selectors: { toolName: "send_email" } }); + const counters = await db.select().from(toolRateLimitCounters); + expect(await svc.admitInvocation(input, invocation.id)).toMatchObject({ decision: "deny" }); + expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, invocation.id)))[0].policyExplanation) + .toEqual(invocation.policyExplanation); + }); + + it("rolls back rate charges when saving the admission receipt fails and admits a retry once", async () => { + const { company, input, invocation, svc } = await admittedRateLimitedInvocation(); + const added = await svc.createPolicy(company.id, { name: "Receipt failure", policyType: "rate_limit", priority: 0, + selectors: { toolName: "send_email" }, config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + await db.execute(sql`create function sleep_admission_fail() returns trigger language plpgsql as $$ begin + raise exception 'receipt write unavailable'; end $$`); + await db.execute(sql`create trigger sleep_admission_fail before update of policy_explanation on tool_invocations + for each row execute function sleep_admission_fail()`); + try { + await expect(svc.admitInvocation(input, invocation.id)).rejects.toThrow(); + expect(await db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.policyId, added.id))).toEqual([]); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, invocation.id)))[0].policyExplanation) + .toEqual(invocation.policyExplanation); + } finally { + await db.execute(sql`drop trigger sleep_admission_fail on tool_invocations`); + await db.execute(sql`drop function sleep_admission_fail()`); + } + expect(await svc.admitInvocation(input, invocation.id)).toMatchObject({ allowed: true }); + expect(await svc.admitInvocation(input, invocation.id)).toMatchObject({ allowed: true }); + expect(await db.select().from(toolRateLimitCounters).where(eq(toolRateLimitCounters.policyId, added.id))) + .toMatchObject([{ limit: 1, remaining: 0 }]); + }); + + it.each([2, 4])("preserves spent slots when a rate limit changes from three to %s", async limit => { + const { company, input, ratePolicy, svc } = await admittedRateLimitedInvocation(3); + await svc.updatePolicy({ companyId: company.id, policyId: ratePolicy.id, + body: { config: { limit, windowSeconds: 3600, keyBy: ["agent", "tool"] } } }); + const results = await Promise.all(Array.from({ length: limit }, () => svc.decide({ ...input, consumeRateLimit: true }))); + expect(results.filter(result => result.allowed)).toHaveLength(limit - 1); + expect(results.filter(result => result.decision === "rate_limited")).toHaveLength(1); + expect(await db.select().from(toolRateLimitCounters)).toMatchObject([{ limit, remaining: 0 }]); + }); + + it.each(["pending", "approved"] as const)("retains a waiting invocation's consumed slot without bypassing %s approval policy", async approvalState => { + const { company, input, invocation, svc } = await admittedRateLimitedInvocation(); + await db.update(toolInvocations).set({ status: "awaiting_approval", policyDecision: "require_approval", approvalState }) + .where(eq(toolInvocations.id, invocation.id)); + await db.insert(toolPolicies).values({ companyId: company.id, name: "Review admitted sends", + policyType: "require_approval", priority: 2, selectors: { toolName: "send_email" } }); + const counters = await db.select().from(toolRateLimitCounters); + expect(await svc.decide(input, { admittedInvocationId: invocation.id })) + .toMatchObject({ allowed: false, decision: "require_approval" }); + expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + expect(await svc.decide(input)).toMatchObject({ decision: "rate_limited" }); }); it("rechecks a newly added rate policy rather than borrowing an older policy's consumed slot", async () => { @@ -1172,15 +1268,19 @@ describeEmbeddedPostgres("tool access policy service", () => { { actorType: "system" }, { actorId: randomUUID() }, { agentId: null }, { runId: null }, { issueId: null }, { gatewayId: null }, { applicationId: null }, { connectionId: null }, { catalogEntryId: null }, { toolName: "different_tool" }, { providerType: null }, { applicationKey: null }, { upstreamToolName: null }, { argumentsHash: "different" }, - { status: "authorized" }, { policyDecision: "deny" }, { policyDecision: "require_approval", approvalState: "pending" }, + { status: "authorized" }, { status: "succeeded" }, { status: "awaiting_approval" }, + { status: "awaiting_approval", policyDecision: "require_approval", approvalState: "rejected" }, { policyDecision: "deny" }, { policyDecision: "require_approval", approvalState: "pending" }, ]; for (const mutation of mutations) { await db.update(toolInvocations).set(mutation).where(eq(toolInvocations.id, invocation.id)); expect(await svc.decide(input, options), JSON.stringify(mutation)).toMatchObject({ decision: "rate_limited" }); + expect(await svc.admitInvocation(input, invocation.id), JSON.stringify(mutation)).toMatchObject({ decision: "deny" }); await db.update(toolInvocations).set({ ...invocation, status: "executing" }).where(eq(toolInvocations.id, invocation.id)); } expect(await svc.decide({ ...input, request: { ...input.request, arguments: { to: "another@example.test" } } }, options)) .toMatchObject({ decision: "rate_limited" }); + expect(await svc.admitInvocation({ ...input, request: { ...input.request, arguments: { to: "another@example.test" } } }, invocation.id)) + .toMatchObject({ decision: "deny" }); }); it("rejects unsupported policy semantics at create and update time", async () => { diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 6caafb2cb03..e5629ead55f 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -1,3 +1,4 @@ +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import * as fs from "node:fs/promises"; import { createHash, generateKeyPairSync, randomUUID } from "node:crypto"; import { createServer } from "node:http"; @@ -97,8 +98,8 @@ import { HttpError } from "../errors.js"; import type { VercelConnectClient } from "../services/vercel-connect.js"; import { invalidatePaperclipCloudConnectorCapabilities, PaperclipCloudConnectorError, type PaperclipCloudConnector } from "../services/paperclip-cloud-connector.js"; -const externalDatabaseUrl = process.env.PAPERCLIP_TOOL_ACCESS_TEST_DATABASE_URL?.trim(); -const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const externalDatabaseUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_TOOL_ACCESS_TEST_DATABASE_URL"); +const embeddedPostgresSupport = externalDatabaseUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -4119,8 +4120,9 @@ describeEmbeddedPostgres("tool access service", () => { .from(toolActionRequests) .where(eq(toolActionRequests.companyId, company.id)); expect(requests).toHaveLength(2); + expect(globalThis.fetch).toHaveBeenCalledTimes(1); expect(requests.map((row) => row.status).sort()).toEqual([ - "approved", + "executed", "pending", ]); }); diff --git a/server/src/__tests__/tool-gateway-service.test.ts b/server/src/__tests__/tool-gateway-service.test.ts index 3e9829670c6..4a014b22ab0 100644 --- a/server/src/__tests__/tool-gateway-service.test.ts +++ b/server/src/__tests__/tool-gateway-service.test.ts @@ -35,12 +35,15 @@ import { toolGatewaySessions, toolInvocations, toolPolicies, + toolRateLimitCounters, } from "@paperclipai/db"; import type { PluginToolDispatcher } from "../services/plugin-tool-dispatcher.js"; import type { VercelConnectClient } from "../services/vercel-connect.js"; import { initializeRunIdentity, reserveSteeredIdentity, reconcileSteeredIdentity } from "../services/run-identity.js"; import { issueService } from "../services/issues.js"; +import { approvalService } from "../services/approvals.js"; import { commitToolActionReview } from "../services/tool-action-review.js"; +import { bindVerifiedToolApprovalArguments, claimToolTestApproval, releaseToolApproval, startToolInvocation } from "../services/tool-approval-execution.js"; import { materializeNativeInteractionResponses } from "../services/native-runtime/native-interaction-bridge.js"; import { toolActionDeliveryService } from "../services/tool-action-delivery.js"; import { secretService } from "../services/secrets.js"; @@ -49,7 +52,7 @@ import { createToolGatewayService, ToolGatewayHttpError, } from "../services/tool-gateway.js"; -import { canonicalToolArguments, signToolArguments } from "../services/tool-content-guards.js"; +import { canonicalToolArguments, readSignedToolArgumentsPayload, signToolArguments, summarizeToolValue, toolRequestIdentityHash } from "../services/tool-content-guards.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, @@ -57,6 +60,7 @@ import { import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; +import { registerToolReviewRecoveryWork } from "../services/tool-review-recovery-work.js"; import { registerToolReviewExpiryWork } from "../services/tool-review-expiry-work.js"; import { idleWorkSnapshot } from "../services/task-admission.js"; @@ -210,7 +214,7 @@ function runSessionInput(company: IdRef, agent: IdRef, run: IdRef): Parameters { let db!: ReturnType; let tempDb: Awaited> | null = null; - let stopExpiryWorker: (() => Promise) | undefined; + let stopReviewWorker: (() => Promise) | undefined; beforeAll(async () => { if (!externalDatabaseUrl) tempDb = await startEmbeddedPostgresTestDatabase("paperclip-tool-gateway-"); @@ -221,7 +225,7 @@ describeEmbeddedPostgres("tool gateway service", () => { }, 20_000); afterEach(async () => { - await stopExpiryWorker?.(); stopExpiryWorker = undefined; + await stopReviewWorker?.(); stopReviewWorker = undefined; await clearFixtures(); }); @@ -268,7 +272,7 @@ describeEmbeddedPostgres("tool gateway service", () => { await db.insert(toolPolicies).values(reviewNotePolicyRow(fixture.company)); gateway = createTestToolGatewayService(db); draining = false; errors.length = 0; - stopExpiryWorker = stopWorker; + stopReviewWorker = stopWorker; vi.useFakeTimers({ toFake: ["Date", "setTimeout", "clearTimeout", "setInterval", "clearInterval"], shouldAdvanceTime: true }); }); async function stopWorker() { @@ -276,6 +280,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() { @@ -313,6 +318,710 @@ describeEmbeddedPostgres("tool gateway service", () => { expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toEqual([]); } + describe("approved review recovery worker", () => { + async function startRecovery() { + coordinator = createDeliveryWorkCoordinator({ owner: db, canRun: () => !draining, + canReconcile: () => true, onError: error => errors.push(error) }); + await registerToolReviewRecoveryWork(coordinator, gateway, () => !draining).ready; + } + const approve = (pending: Awaited>, source = db) => commitToolActionReview(source, { + companyId: fixture.company.id, actionRequestId: pending.id, decision: "approved", actor: { userId: "board" }, + }); + async function executed(pending: Awaited>) { + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("executed"); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0].status).toBe("succeeded"); + } + + it("is quiet while configured idle and executes a committed approval once", async () => { + await startRecovery(); await quiet(); + const pending = await request(); await approve(pending); await settle(); + await executed(pending); await quiet(); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(1); + }); + + it("does not scan another company's approved backlog after review writes", async () => { + const other = await createRunFixture(db); + await db.insert(toolPolicies).values(reviewNotePolicyRow(other.company)); + const foreign = await request(gateway, db, other); + await db.update(toolActionRequests).set({ signedArguments: signToolArguments({ + invocationId: foreign.invocationId, toolName: "mcp-remote-fixture:update_note", + canonicalArguments: canonicalToolArguments({ noteId: "legacy", body: "inert" }), + signingSecret: testToolActionSigningSecret, + }) }).where(eq(toolActionRequests.id, foreign.id)); + await commitToolActionReview(db, { companyId: other.company.id, actionRequestId: foreign.id, + decision: "approved", actor: { userId: "board" } }); + await startRecovery(); await quiet(); + const reads: Array<{ query: string; params: unknown[] }> = []; + const debug = db.$client.options.debug; + db.$client.options.debug = (_id, query, params) => { + if (/^select/i.test(query) && query.includes('"tool_action_requests"') && + query.includes('"tool_action_requests"."status" =') && params.includes("approved")) + reads.push({ query, params: [...params] }); + }; + try { + for (let index = 0; index < 3; index++) { + const pending = await request(); await settle(1500); + await approve(pending); await settle(1500); await executed(pending); + } + } finally { db.$client.options.debug = debug; } + expect(reads.length).toBeGreaterThan(0); + for (const read of reads) { + expect(read.query).toContain('"tool_action_requests"."company_id" ='); + expect(read.params).toContain(fixture.company.id); + expect(read.params).not.toContain(other.company.id); + } + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, foreign.id)))[0].status).toBe("approved"); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, foreign.invocationId)))[0].status).toBe("awaiting_approval"); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(3); + await quiet(); + }); + + it.each(["foreground", "recovery"] as const)("preserves the admitted rate slot for an agent action through %s approval", async mode => { + await db.insert(toolPolicies).values({ companyId: fixture.company.id, name: "One reviewed note", + policyType: "rate_limit", priority: 1, selectors: { toolName: "mcp-remote-fixture:update_note" }, + config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + const pending = await request(); + const counters = await db.select().from(toolRateLimitCounters); + expect(counters).toHaveLength(1); expect(counters[0].remaining).toBe(0); + if (mode === "foreground") { + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + await startRecovery(); + } else { await approve(pending); await startRecovery(); } + await settle(); await executed(pending); await quiet(); + expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(1); + }); + + it.each(["new", "increased"] as const)("spends the newly available slot of a %s rate rule during approved recovery", async change => { + const rate = { companyId: fixture.company.id, name: "Changed reviewed-note limit", + policyType: "rate_limit" as const, priority: 1, selectors: { toolName: "mcp-remote-fixture:update_note" }, + config: { limit: 2, windowSeconds: 3600, keyBy: ["agent", "tool"] } }; + const original = change === "increased" ? (await db.insert(toolPolicies).values(rate).returning())[0] : null; + const first = await request(), second = await request(); + if (original) { + await db.update(toolPolicies).set({ config: { ...rate.config, limit: 3 }, updatedAt: new Date() }) + .where(eq(toolPolicies.id, original.id)); + } else { + await db.insert(toolPolicies).values({ ...rate, config: { ...rate.config, limit: 1 } }); + } + await approve(first); await approve(second); + await startRecovery(); await settle(); await quiet(); + const requests = await db.select().from(toolActionRequests); + expect(requests.filter(row => row.status === "executed")).toHaveLength(1); + expect(requests.filter(row => row.status === "failed")).toHaveLength(1); + const invocations = await db.select().from(toolInvocations); + expect(invocations.filter(row => row.errorCode === "rate_limited")).toHaveLength(1); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(1); + const counters = await db.select().from(toolRateLimitCounters); + expect(counters).toHaveLength(1); + expect(counters[0]).toMatchObject({ limit: change === "increased" ? 3 : 1, remaining: 0 }); + }); + + it.each([ + ...["agent", "test-tab"].flatMap(origin => ["valid", "new-rate", "invalid-signature", "foreign-hash", "foreground-rate", "foreground-invalid-signature", "foreground-foreign-hash", "foreground-modern-review-hash"] + .map(state => ({ origin, state }))), + ])("recovers historical preview-hash approval ($origin, $state)", async ({ origin, state }) => { + const remoteCall = providerFixture(); remoteCall.release(); + const parameters = { value: "reviewed", apiKey: "synthetic-historical-approval-key" }; + const foreground = state.startsWith("foreground-"); + if (foreground) await db.insert(toolPolicies).values({ companyId: fixture.company.id, + name: "Already admitted historical action", policyType: "rate_limit", priority: 1, + selectors: {}, config: { limit: 1, windowSeconds: 3600, keyBy: ["company", "tool"] } }); + const pending = origin === "agent" ? await agentRequest(remoteCall.provider, parameters) + : (await testRequest(remoteCall.provider, false, parameters)).pending; + const [originalInvocation] = await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)); + const payload = readSignedToolArgumentsPayload({ signedArguments: pending.signedArguments, + invocationId: pending.invocationId, toolName: originalInvocation.toolName, signingSecret: testToolActionSigningSecret })!; + expect(payload).not.toBeNull(); + const legacyHash = summarizeToolValue(payload.arguments).sha256!; + const completeHash = toolRequestIdentityHash(payload.arguments, testToolActionSigningSecret); + expect(legacyHash).not.toBe(completeHash); + const storedHash = state.endsWith("foreign-hash") ? "e".repeat(64) : legacyHash; + await db.update(toolInvocations).set({ argumentsHash: storedHash }).where(eq(toolInvocations.id, pending.invocationId)); + await db.update(toolActionRequests).set({ canonicalArgumentsHash: state === "foreground-modern-review-hash" ? completeHash : legacyHash, + ...(["invalid-signature", "foreground-invalid-signature"].includes(state) ? { signedArguments: "unreadable-old-signature" } : {}), + }).where(eq(toolActionRequests.id, pending.id)); + if (state === "new-rate") await db.insert(toolPolicies).values({ companyId: fixture.company.id, + name: "New admission for historical approval", policyType: "rate_limit", priority: 1, + selectors: {}, config: { limit: 1, windowSeconds: 3600, keyBy: ["company", "tool"] } }); + const counters = foreground ? await db.select().from(toolRateLimitCounters) : []; + if (foreground) { + expect(counters).toEqual([expect.objectContaining({ remaining: 0 })]); + const approval = gateway.approveActionRequest({ companyId: fixture.company.id, + actionRequestId: pending.id, actor: { userId: "board" } }); + if (state === "foreground-invalid-signature") await expect(approval).rejects.toMatchObject({ reasonCode: "action_request_invalidated" }); + else if (state === "foreground-foreign-hash" && origin === "agent") await expect(approval).rejects.toMatchObject({ reasonCode: "rate_limited" }); + else await approval; + } else await approve(pending); + await startRecovery(); await settle(); + if (state === "valid" || state === "new-rate" || state === "foreground-rate" || state === "foreground-modern-review-hash") { + await executed(pending); + expect((await db.select().from(toolInvocations))[0].argumentsHash).toBe(completeHash); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + expect(JSON.parse(String(remoteCall.provider.mock.calls[0][1].body)).params.arguments).toEqual(parameters); + if (state === "new-rate") expect(await db.select().from(toolRateLimitCounters)) + .toEqual([expect.objectContaining({ limit: 1, remaining: 0 })]); + } else { + expect((await db.select().from(toolActionRequests))[0].status) + .toBe(["invalid-signature", "foreground-invalid-signature"].includes(state) ? "cancelled" + : state === "foreground-foreign-hash" && origin === "agent" ? "pending" : "failed"); + expect((await db.select().from(toolInvocations))[0].argumentsHash).toBe(storedHash); + expect(remoteCall.provider).not.toHaveBeenCalled(); + } + if (foreground) expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + await quiet(); + }); + + it("does not rebind historical arguments after the execution claim is replaced", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const pending = await agentRequest(remoteCall.provider, { value: "reviewed", apiKey: "synthetic-stale-claim-key" }); + const [invocation] = await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)); + const payload = readSignedToolArgumentsPayload({ signedArguments: pending.signedArguments, + invocationId: invocation.id, toolName: invocation.toolName, signingSecret: testToolActionSigningSecret })!; + const preview = summarizeToolValue(payload.arguments).sha256; + const verified = toolRequestIdentityHash(payload.arguments, testToolActionSigningSecret); + await db.update(toolInvocations).set({ argumentsHash: preview }).where(eq(toolInvocations.id, invocation.id)); + await db.update(toolActionRequests).set({ canonicalArgumentsHash: preview }).where(eq(toolActionRequests.id, pending.id)); + await approve(pending); + const claim = (await claimToolTestApproval(db, pending.id, invocation.id))!; + expect(claim).not.toBeNull(); + await db.update(toolActionRequests).set({ updatedAt: new Date(claim.claimedAt.getTime() + 1) }) + .where(eq(toolActionRequests.id, pending.id)); + expect(await bindVerifiedToolApprovalArguments(db, claim, { preview, verified })).toBe(false); + expect((await db.select().from(toolInvocations))[0].argumentsHash).toBe(preview); + expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + + it("reconstructs a saved approved action after restart", async () => { + const pending = await request(); await approve(pending); + await startRecovery(); await settle(); await executed(pending); await quiet(); + }); + + it("retries a failed approval dispatch only while the approved action remains unresolved", async () => { + const pending = await request(); await approve(pending); + const recover = vi.spyOn(gateway, "approveActionRequest").mockRejectedValue(new Error("Recovery dependency unavailable")); + await startRecovery(); + expect(recover).toHaveBeenCalledTimes(1); expect(coordinator!.nextWakeAt()).not.toBeNull(); + await settle(10_000); expect(recover).toHaveBeenCalledTimes(1); + recover.mockRestore(); await settle(); await executed(pending); await quiet(); + }); + + it("preserves a failed company's retry across writes while another company executes", async () => { + const pending = await request(); await approve(pending); + const original = gateway.approveActionRequest.bind(gateway); + const dispatch = vi.spyOn(gateway, "approveActionRequest").mockImplementation(async input => { + if (input.companyId === fixture.company.id) throw new Error("Company dependency unavailable"); + return original(input); + }); + await startRecovery(); + expect(dispatch).toHaveBeenCalledTimes(1); + const retryAt = coordinator!.nextWakeAt(); + expect(retryAt).not.toBeNull(); + const later = await request(); await approve(later); + const other = await createRunFixture(db); + await db.insert(toolPolicies).values(reviewNotePolicyRow(other.company)); + const foreign = await request(gateway, db, other); + await commitToolActionReview(db, { companyId: other.company.id, actionRequestId: foreign.id, + decision: "approved", actor: { userId: "board" } }); + await settle(5000); await executed(foreign); + expect(dispatch.mock.calls.filter(([input]) => input.companyId === fixture.company.id)).toHaveLength(1); + expect(coordinator!.nextWakeAt()).toBe(retryAt); + dispatch.mockRestore(); + await settle(); await executed(pending); await executed(later); await quiet(); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(3); + }); + + it("disarms a failed company's recovery after company deletion commits", async () => { + const pending = await request(); await approve(pending); + const dispatch = vi.spyOn(gateway, "approveActionRequest").mockRejectedValue(new Error("Company dependency unavailable")); + await startRecovery(); expect(coordinator!.nextWakeAt()).not.toBeNull(); + await db.update(heartbeatRuns).set({ status: "succeeded", finishedAt: new Date() }) + .where(eq(heartbeatRuns.id, fixture.run.id)); + await db.update(agents).set({ status: "terminated", lifecycleState: "terminated" }) + .where(eq(agents.id, fixture.agent.id)); + await deleteCompany(db, fixture.company.id); + await settle(1500); + expect(coordinator!.nextWakeAt()).toBeNull(); + expect(dispatch).toHaveBeenCalledTimes(1); await quiet(); + }); + + it.each([null, "unreadable-old-signature"])("settles an approved invalid signature (%s) without recurring recovery", async signedArguments => { + const pending = await request(); await approve(pending); + await db.update(toolActionRequests).set({ signedArguments }).where(eq(toolActionRequests.id, pending.id)); + const dispatch = vi.spyOn(gateway, "approveActionRequest"); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("cancelled"); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ + status: "failed", errorCode: "action_request_invalidated", + }); + expect(dispatch).not.toHaveBeenCalled(); await quiet(); + }); + + it.each(["rejected", "cancelled"])("settles a %s formal approval without recurring recovery", async status => { + const pending = await request(); await approve(pending); + const [formal] = await db.insert(approvals).values({ companyId: fixture.company.id, + type: "request_board_approval", status, payload: {} }).returning(); + await db.update(toolActionRequests).set({ approvalId: formal.id }).where(eq(toolActionRequests.id, pending.id)); + const dispatch = vi.spyOn(gateway, "approveActionRequest"); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("cancelled"); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ + status: "denied", errorCode: "formal_approval_unavailable", + }); + expect(dispatch).not.toHaveBeenCalled(); await quiet(); + }); + + it.each([false, true])("parks a formal approval until its decision commits (rollback %s)", async rollback => { + const pending = await request(); await approve(pending); + const [formal] = await db.insert(approvals).values({ companyId: fixture.company.id, + type: "request_board_approval", status: "pending", payload: {} }).returning(); + await db.update(toolActionRequests).set({ approvalId: formal.id }).where(eq(toolActionRequests.id, pending.id)); + await startRecovery(); await quiet(); + let release!: () => void, staged!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const ready = new Promise(resolve => { staged = resolve; }); + const aborted = new Error("formal approval rollback fixture"); + const writing = db.transaction(async tx => { + await approvalService(tx as unknown as typeof db).approve(formal.id, "board"); + staged(); await gate; if (rollback) throw aborted; + }).catch(error => { if (error !== aborted) throw error; }); + await Promise.race([ready, writing]); + try { + await vi.advanceTimersByTimeAsync(60_000); + expect((await db.select().from(toolActionRequests))[0].status).toBe("approved"); + expect((await db.select().from(toolInvocations))[0].status).toBe("awaiting_approval"); + } finally { release(); } + await writing; await settle(); + if (rollback) { + expect((await db.select().from(toolActionRequests))[0].status).toBe("approved"); await quiet(); + await approvalService(db).approve(formal.id, "board"); await settle(); + } + await executed(pending); await quiet(); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(1); + }); + + it.each(["reject", "cancel"] as const)("wakes a parked revision-requested formal approval on %s", async decision => { + const pending = await request(); await approve(pending); + const [formal] = await db.insert(approvals).values({ companyId: fixture.company.id, + type: "request_board_approval", status: "revision_requested", payload: {} }).returning(); + await db.update(toolActionRequests).set({ approvalId: formal.id }).where(eq(toolActionRequests.id, pending.id)); + await startRecovery(); await quiet(); + const service = approvalService(db); + if (decision === "cancel") await service.cancel(formal.id, "Cancelled by user"); + else await service.reject(formal.id, "board"); + await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("cancelled"); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ status: "denied", errorCode: "formal_approval_unavailable" }); + await quiet(); + }); + + it("retains approved work when the signing key is temporarily unavailable", async () => { + const pending = await request(); await approve(pending); + vi.stubEnv("PAPERCLIP_TOOL_ACTION_SIGNING_SECRET", ""); + vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "invalid-master-key"); + gateway = createToolGatewayService(db); + await startRecovery(); + expect(coordinator!.nextWakeAt()).not.toBeNull(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("approved"); + expect((await db.select().from(toolInvocations))[0].status).toBe("awaiting_approval"); + vi.stubEnv("PAPERCLIP_TOOL_ACTION_SIGNING_SECRET", testToolActionSigningSecret); + await settle(); await executed(pending); await quiet(); + }); + + it("does not accept another company's formal approval", async () => { + const pending = await request(); await approve(pending); + const other = await createRunFixture(db); + const [formal] = await db.insert(approvals).values({ companyId: other.company.id, + type: "request_board_approval", status: "approved", payload: {} }).returning(); + await db.update(toolActionRequests).set({ approvalId: formal.id }).where(eq(toolActionRequests.id, pending.id)); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("cancelled"); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ status: "denied", errorCode: "formal_approval_unavailable" }); + expect((await db.select().from(approvals))[0]).toEqual(formal); + await quiet(); + }); + + it("quarantines a mismatched request without modifying another company's invocation", async () => { + const pending = await request(); await approve(pending); + const [invocation] = await db.select().from(toolInvocations); + const other = await createRunFixture(db); + await db.update(toolActionRequests).set({ companyId: other.company.id }).where(eq(toolActionRequests.id, pending.id)); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("failed"); + expect((await db.select().from(toolInvocations))[0]).toEqual(invocation); + await quiet(); + }); + + it("repairs completed agent history without executing the action again", async () => { + const pending = await request(); + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + await executed(pending); + const [invocation] = await db.select().from(toolInvocations); + await db.update(toolActionRequests).set({ status: "approved", signedArguments: "unreadable-old-signature" }) + .where(eq(toolActionRequests.id, pending.id)); + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect((await db.select().from(toolInvocations))[0]).toEqual(invocation); + expect(await db.select().from(toolCallEvents).where(eq(toolCallEvents.reasonCode, "approved_action_executed"))).toHaveLength(1); + }); + + it("keeps deliberately inert legacy approvals quiet", async () => { + const pending = await request(); + await db.update(toolActionRequests).set({ signedArguments: signToolArguments({ + invocationId: pending.invocationId, toolName: "mcp-remote-fixture:update_note", + canonicalArguments: canonicalToolArguments({ noteId: "legacy", body: "inert" }), + signingSecret: testToolActionSigningSecret, + }) }).where(eq(toolActionRequests.id, pending.id)); + await approve(pending); await startRecovery(); await quiet(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("approved"); + expect((await db.select().from(toolInvocations))[0].status).toBe("awaiting_approval"); + }); + + it.each([false, true])("waits for the outer approval transaction (rollback %s)", async rollback => { + await startRecovery(); const pending = await request(); + let release!: () => void, staged!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const ready = new Promise(resolve => { staged = resolve; }); + const aborted = new Error("approval rollback fixture"); + const writing = db.transaction(async tx => { + await approve(pending, tx as unknown as typeof db); staged(); await gate; + if (rollback) throw aborted; + }).catch(error => { if (error !== aborted) throw error; }); + await Promise.race([ready, writing]); + try { + await vi.advanceTimersByTimeAsync(60_000); + expect((await db.select().from(toolActionRequests))[0].status).toBe("pending"); + } finally { release(); } + await writing; await settle(); + if (rollback) expect((await db.select().from(toolActionRequests))[0].status).toBe("pending"); + else await executed(pending); + await quiet(); + }); + + it("retains the first approval during drain without passive queries", async () => { + await startRecovery(); await quiet(); draining = true; + const pending = await request(); await approve(pending); await settle(); + const reads = vi.spyOn(db, "select"); + await settle(60 * 60_000); expect(reads).not.toHaveBeenCalled(); reads.mockRestore(); + draining = false; await settle(); await executed(pending); await quiet(); + }); + + async function testRequest(provider: NonNullable, rateLimited = false, parameters: Record = { value: "reviewed" }) { + await db.insert(companyMemberships).values({ companyId: fixture.company.id, principalType: "user", + principalId: "board", status: "active", membershipRole: "owner" }); + const remote = await createRemoteMcpToolFixture(db, fixture.company.id); + await db.update(toolCatalogEntries).set({ isReadOnly: false, riskLevel: "write" }) + .where(eq(toolCatalogEntries.id, remote.catalogEntry.id)); + await db.insert(toolPolicies).values({ companyId: fixture.company.id, name: "Review connected writes", + policyType: "require_approval", priority: 10, selectors: { connectionId: remote.connection.id } }); + if (rateLimited) await db.insert(toolPolicies).values({ companyId: fixture.company.id, name: "One reviewed test call", + policyType: "rate_limit", priority: 1, selectors: { connectionId: remote.connection.id }, + config: { limit: 1, windowSeconds: 3600, keyBy: ["agent", "tool"] } }); + gateway = createTestToolGatewayService(db, { remoteHttpRequest: provider }); + const result = await gateway.executeTestCall({ companyId: fixture.company.id, connectionId: remote.connection.id, + agentId: fixture.agent.id, userId: "board", toolName: "needs_input", parameters }); + expect(result.decision).toBe("ask_first"); + const [pending] = await db.select().from(toolActionRequests); + return { pending, remote }; + } + function providerFixture() { + let release!: () => void, entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { entered = resolve; }); + const provider = vi.fn(async (_url, init) => { + const payload = JSON.parse(String(init.body)); + expect(payload.method).toBe("tools/call"); + entered(); await gate; + return Response.json({ jsonrpc: "2.0", id: payload.id, + result: { content: [{ type: "text", text: "Saved once" }] } }); + }); + return { provider, release, started }; + } + + it.each(["foreground", "recovery"] as const)("retries a Test-tab %s approval after a real pre-dispatch database timeout", async mode => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); await approve(pending); + const preparationDb = createDb(externalDatabaseUrl ?? tempDb!.connectionString, { + maxConnections: 1, idleTimeoutSeconds: 0, maxLifetimeSeconds: 0, + }); + const preparingGateway = createTestToolGatewayService(preparationDb, { remoteHttpRequest: remoteCall.provider }); + let unlock!: () => void, locked!: () => void; + const gate = new Promise(resolve => { unlock = resolve; }); + const acquired = new Promise(resolve => { locked = resolve; }); + const lock = db.transaction(async tx => { + await tx.execute(sql`lock table tool_policies in access exclusive mode`); + locked(); await gate; + }); + await acquired; + try { + await preparationDb.execute(sql`set lock_timeout = '2s'`); + const attempting = mode === "foreground" + ? preparingGateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }) + : preparingGateway.recoverApprovedActionReviews(fixture.company.id); + // Observe the real committed claim while its subsequent policy read + // is blocked. Provider execution has not started. + void attempting.catch(() => undefined); + await vi.waitFor(async () => { + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("executing"); + }); + if (mode === "foreground") await expect(attempting).rejects.toMatchObject({ cause: { code: "55P03" } }); + else expect(await attempting).toEqual({ scanned: 1, retry: true }); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0]) + .toMatchObject({ status: "approved", signedArguments: pending.signedArguments }); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0]) + .toMatchObject({ status: "awaiting_approval", startedAt: null }); + expect(remoteCall.provider).not.toHaveBeenCalled(); + } finally { unlock(); await lock; await preparationDb.$client.end({ timeout: 1 }); } + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + }, 15_000); + + async function agentRequest(provider: NonNullable, parameters: Record = { value: "reviewed" }) { + const remote = await createRemoteMcpToolFixture(db, fixture.company.id); + await db.update(toolCatalogEntries).set({ isReadOnly: false, riskLevel: "write" }) + .where(eq(toolCatalogEntries.id, remote.catalogEntry.id)); + await db.insert(toolPolicies).values({ companyId: fixture.company.id, name: "Review agent writes", + policyType: "require_approval", selectors: { connectionId: remote.connection.id } }); + gateway = createTestToolGatewayService(db, { remoteHttpRequest: provider }); + const session = await gateway.createSession(runSessionInput(fixture.company, fixture.agent, fixture.run)); + const tool = (await gateway.listToolsForSession(session.token)).find(t => t.connectionId === remote.connection.id)!; + await expect(gateway.executeTool({ sessionToken: session.token, tool: tool.name, parameters })) + .rejects.toMatchObject({ reasonCode: "approval_required" }); + const [pending] = await db.select().from(toolActionRequests); + return pending; + } + + it.each(["foreground", "recovery"] as const)("retries an agent-origin %s approval after a real pre-dispatch database timeout", async mode => { + const remoteCall = providerFixture(); remoteCall.release(); + const pending = await agentRequest(remoteCall.provider); await approve(pending); + const preparationDb = createDb(externalDatabaseUrl ?? tempDb!.connectionString, { + maxConnections: 1, idleTimeoutSeconds: 0, maxLifetimeSeconds: 0, + }); + const preparingGateway = createTestToolGatewayService(preparationDb, { remoteHttpRequest: remoteCall.provider }); + let unlock!: () => void, locked!: () => void; + const gate = new Promise(resolve => { unlock = resolve; }); + const acquired = new Promise(resolve => { locked = resolve; }); + const lock = db.transaction(async tx => { + await tx.execute(sql`lock table tool_policies in access exclusive mode`); + locked(); await gate; + }); + await acquired; + try { + await preparationDb.execute(sql`set lock_timeout = '2s'`); + const attempting = mode === "foreground" + ? preparingGateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }) + : preparingGateway.recoverApprovedActionReviews(fixture.company.id); + // Observe the real committed claim while its subsequent policy read + // is blocked. Provider execution has not started. + void attempting.catch(() => undefined); + await vi.waitFor(async () => { + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("executing"); + }); + if (mode === "foreground") expect(await attempting).toMatchObject({ status: "approved" }); + else expect(await attempting).toEqual({ scanned: 1, retry: true }); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0]) + .toMatchObject({ status: "approved", signedArguments: pending.signedArguments }); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0]) + .toMatchObject({ status: "awaiting_approval", startedAt: null }); + expect(remoteCall.provider).not.toHaveBeenCalled(); + } finally { unlock(); await lock; await preparationDb.$client.end({ timeout: 1 }); } + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + }, 15_000); + + it("refuses agent dispatch after preparation loses its claim", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const pending = await agentRequest(remoteCall.provider); await approve(pending); + let unlock!: () => void, locked!: () => void; + const gate = new Promise(resolve => { unlock = resolve; }); + const acquired = new Promise(resolve => { locked = resolve; }); + const lock = db.transaction(async tx => { + // Pause tool discovery before admission locks the invocation. Revocation + // cannot legitimately take that lock from an in-flight admission. + await tx.execute(sql`lock table tool_catalog_entries in access exclusive mode`); + locked(); await gate; + }); + await acquired; + const attempting = gateway.approveActionRequest({ companyId: fixture.company.id, + actionRequestId: pending.id, actor: { userId: "board" } }); + void attempting.catch(() => undefined); + let successor!: NonNullable>>; + try { + await vi.waitFor(async () => { + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("executing"); + }); + const [claimed] = await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)); + expect(await releaseToolApproval(db, { actionRequestId: pending.id, + invocationId: pending.invocationId, claimedAt: claimed.updatedAt })).toBe(true); + await vi.advanceTimersByTimeAsync(10); + successor = (await claimToolTestApproval(db, pending.id, pending.invocationId))!; + expect(successor).not.toBeNull(); + expect(successor.claimedAt.getTime()).toBeGreaterThan(claimed.updatedAt.getTime()); + } finally { unlock(); await lock; } + expect(await attempting).toMatchObject({ status: "executing" }); + expect(remoteCall.provider).not.toHaveBeenCalled(); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0]) + .toMatchObject({ status: "awaiting_approval", startedAt: null }); + expect(await releaseToolApproval(db, successor)).toBe(true); + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + }); + + it("does not retry an agent provider error as a preparation failure", async () => { + const provider = vi.fn(async () => { throw new Error("Provider outcome is unknown"); }); + const pending = await agentRequest(provider); await approve(pending); + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("failed"); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0].startedAt).not.toBeNull(); + await startRecovery(); await quiet(); + expect(provider).toHaveBeenCalledOnce(); + }); + + it.each(["started", "reclaimed", "mismatched", "prior_dispatch"] as const)("refuses to release a Test-tab preparation claim with %s ownership evidence", async state => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); await approve(pending); + const claim = await claimToolTestApproval(db, pending.id, pending.invocationId); + expect(claim).not.toBeNull(); + if (state === "started") expect(await startToolInvocation(db, pending.invocationId, claim!)).toBe(true); + if (state === "reclaimed") await db.update(toolActionRequests) + .set({ updatedAt: new Date(claim!.claimedAt.getTime() + 1000) }).where(eq(toolActionRequests.id, pending.id)); + if (state === "prior_dispatch") await db.update(toolInvocations) + .set({ startedAt: new Date() }).where(eq(toolInvocations.id, pending.invocationId)); + const beforeRequest = (await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0]; + const beforeInvocation = (await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0]; + expect(await releaseToolApproval(db, state === "mismatched" ? { ...claim!, invocationId: randomUUID() } : claim!)).toBe(false); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0]).toEqual(beforeRequest); + expect((await db.select().from(toolInvocations).where(eq(toolInvocations.id, pending.invocationId)))[0]).toEqual(beforeInvocation); + expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + + it("rolls a Test-tab preparation release back with its transaction and invalidates the released claim", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); await approve(pending); + const claim = await claimToolTestApproval(db, pending.id, pending.invocationId); + expect(claim).not.toBeNull(); + await expect(db.transaction(async tx => { + expect(await releaseToolApproval(tx as unknown as typeof db, claim!)).toBe(true); + throw new Error("release rollback"); + })).rejects.toThrow("release rollback"); + expect((await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, pending.id)))[0].status).toBe("executing"); + expect(await releaseToolApproval(db, claim!)).toBe(true); + expect(await startToolInvocation(db, pending.invocationId, claim!)).toBe(false); + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + }); + + it.each(["foreground", "recovery"] as const)("claims a Test-tab action once when %s wins", async winner => { + const remoteCall = providerFixture(); + const { pending } = await testRequest(remoteCall.provider); + let running: Promise; + if (winner === "foreground") { + await startRecovery(); + running = gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + } else { + await approve(pending); + running = startRecovery(); + } + await Promise.race([remoteCall.started, running]); + try { + await vi.advanceTimersByTimeAsync(60_000); + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + expect(remoteCall.provider).toHaveBeenCalledTimes(1); + } finally { remoteCall.release(); } + await running; await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).toHaveBeenCalledTimes(1); + }); + + it.each(["foreground", "recovery"] as const)("preserves the admitted rate slot for a Test-tab action through %s approval", async mode => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider, true); + const counters = await db.select().from(toolRateLimitCounters); + expect(counters).toHaveLength(1); expect(counters[0].remaining).toBe(0); + if (mode === "foreground") { + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + await startRecovery(); + } else { await approve(pending); await startRecovery(); } + await settle(); await executed(pending); await quiet(); + expect(await db.select().from(toolRateLimitCounters)).toEqual(counters); + expect(remoteCall.provider).toHaveBeenCalledOnce(); + }); + + it("repairs completed Test-tab history without sending the request again", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); + await gateway.approveActionRequest({ companyId: fixture.company.id, actionRequestId: pending.id, actor: { userId: "board" } }); + await executed(pending); + await db.update(toolActionRequests).set({ status: "approved", signedArguments: "unreadable-old-signature" }) + .where(eq(toolActionRequests.id, pending.id)); + remoteCall.provider.mockClear(); + await startRecovery(); await settle(); await executed(pending); await quiet(); + expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + + it.each(["pending", "authorized", "cancelled"])("settles inconsistent Test-tab %s history without replay", async status => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); await approve(pending); + await db.update(toolInvocations).set({ status }).where(eq(toolInvocations.id, pending.invocationId)); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe(status === "cancelled" ? "cancelled" : "failed"); + expect((await db.select().from(toolInvocations))[0]).toMatchObject(status === "cancelled" + ? { status: "cancelled" } + : { status: "failed", errorCode: "tool_recovery_state_invalid" }); + await quiet(); expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + + it("does not replay an interrupted legacy Test-tab invocation", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending } = await testRequest(remoteCall.provider); + await approve(pending); + await db.update(toolInvocations).set({ status: "executing", startedAt: new Date(Date.now() - 11 * 60_000) }) + .where(eq(toolInvocations.id, pending.invocationId)); + await startRecovery(); await settle(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("executing"); + await gateway.sweepActionReviews({ includeApproved: false, includeExpiry: false }); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ status: "failed", errorCode: "tool_execution_outcome_unknown" }); + await quiet(); expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + + 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 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 { + await startRecovery(); + expect((await db.select().from(toolActionRequests))[0].status).toBe("executing"); + expect((await db.select().from(toolInvocations))[0].status).toBe("executing"); + expect(remoteCall.provider).toHaveBeenCalledTimes(1); + } finally { + await db.execute(sql`drop trigger sleep_review_fail on tool_action_requests`); + } + await settle(); + await vi.advanceTimersByTimeAsync(11 * 60_000); + await gateway.sweepActionReviews({ includeApproved: false, includeExpiry: false }); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ status: "failed", errorCode: "tool_execution_outcome_unknown" }); + await quiet(); expect(remoteCall.provider).toHaveBeenCalledTimes(1); + }); + + it("revalidates the reviewed Test-tab target before recovery dispatch", async () => { + const remoteCall = providerFixture(); remoteCall.release(); + const { pending, remote } = await testRequest(remoteCall.provider); + await approve(pending); + await db.update(toolCatalogEntries).set({ versionHash: "changed-after-review" }).where(eq(toolCatalogEntries.id, remote.catalogEntry.id)); + await startRecovery(); await settle(); + expect((await db.select().from(toolInvocations))[0]).toMatchObject({ status: "failed", errorCode: "approved_tool_target_changed" }); + await quiet(); expect(remoteCall.provider).not.toHaveBeenCalled(); + }); + }); + async function requestWithExpiry(expiresAt: Date, context = fixture) { return db.transaction(async tx => { const source = tx as unknown as typeof db; @@ -876,13 +1585,21 @@ describeEmbeddedPostgres("tool gateway service", () => { await expect(gateway.executeTool({ sessionToken: session.token, tool: "mcp-remote-fixture:update_note", parameters: { noteId: "expire" } })).rejects.toMatchObject({ reasonCode: "approval_required" }); const [request] = await db.select().from(toolActionRequests); await db.update(toolActionRequests).set({ expiresAt: new Date(Date.now() - 1000) }).where(eq(toolActionRequests.id, request.id)); - await db.insert(toolActionRequests).values(Array.from({ length: 100 }, (_, i) => ({ + const [originalInvocation] = await db.select().from(toolInvocations).where(eq(toolInvocations.id, request.invocationId)); + const blockedInvocations = Array.from({ length: 101 }, (_, i) => ({ ...originalInvocation, + id: `10000000-0000-4000-8000-${String(i).padStart(12, "0")}`, approvalState: "approved", idempotencyKey: null, + })); + await db.insert(toolInvocations).values(blockedInvocations); + await db.insert(toolActionRequests).values(blockedInvocations.map((invocation, i) => ({ ...request, id: `00000000-0000-4000-8000-${String(i).padStart(12, "0")}`, - interactionId: null, status: "approved" as const, + invocationId: invocation.id, interactionId: null, status: "approved" as const, + signedArguments: signToolArguments({ invocationId: invocation.id, toolName: invocation.toolName, + canonicalArguments: canonicalToolArguments({ noteId: "blocked" }), executionOnApprove: true, + signingSecret: testToolActionSigningSecret }), }))); const recover = vi.spyOn(gateway, "approveActionRequest").mockRejectedValue(new Error("Unavailable approval dependency")); - expect(await gateway.sweepActionReviews()).toEqual({ scanned: 101 }); - expect(recover).toHaveBeenCalledTimes(100); + expect(await gateway.sweepActionReviews()).toEqual({ scanned: 102 }); + expect(recover).toHaveBeenCalledTimes(101); const [settled] = await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, request.interactionId!)); expect(settled.status).toBe("expired"); }); diff --git a/server/src/index.ts b/server/src/index.ts index ea24010565f..e9fc24fcf0b 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -1,3 +1,4 @@ +import { registerToolReviewRecoveryWork } from "./services/tool-review-recovery-work.js"; import { registerToolReviewExpiryWork } from "./services/tool-review-expiry-work.js"; import { registerGatewayCredentialExpiryWork } from "./services/gateway-credential-expiry-work.js"; import { registerSecretProposalExpiryWork } from "./services/secret-proposal-expiry-work.js"; @@ -1472,7 +1473,13 @@ async function startServerWithDatabaseTeardown( } else { await app.locals.toolGateway.sweepExpiredActionReviews(); } - return app.locals.toolGateway.sweepActionReviews({ includeExpiry: false }); + if (config.heartbeatSchedulerEnabled) { + await registerToolReviewRecoveryWork(deliveryWork, app.locals.toolGateway, + () => !isIdleTaskDrainActive() && !heartbeatSchedulerStopped && !isWarmStandby()).ready; + } else { + await app.locals.toolGateway.recoverApprovedActionReviews(); + } + return app.locals.toolGateway.sweepActionReviews({ includeExpiry: false, includeApproved: false }); }).catch((err: unknown) => logger.error({ err }, "startup tool review recovery failed")); await timeStartupRecovery("gateway_sessions", () => config.heartbeatSchedulerEnabled ? registerGatewayCredentialExpiryWork(deliveryWork, app.locals.toolGateway, @@ -1903,7 +1910,7 @@ async function startServerWithDatabaseTeardown( logger.error({ err }, "periodic tool connection health sweep failed"); })); - trackHeartbeatSchedulerWork(app.locals.toolGateway.sweepActionReviews({ includeExpiry: false }).catch((err: unknown) => logger.error({ err }, "tool review recovery failed"))); + trackHeartbeatSchedulerWork(app.locals.toolGateway.sweepActionReviews({ includeExpiry: false, includeApproved: false }).catch((err: unknown) => logger.error({ err }, "tool review recovery failed"))); if (isIdleTaskDrainActive() || heartbeatSchedulerStopped) return; if (!(await heartbeat.resolveSchedulingSuppression()).suppressed) { diff --git a/server/src/services/approvals.ts b/server/src/services/approvals.ts index 290a59fdf4b..4f4149f773d 100644 --- a/server/src/services/approvals.ts +++ b/server/src/services/approvals.ts @@ -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(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; }, diff --git a/server/src/services/delivery-work-notifications.ts b/server/src/services/delivery-work-notifications.ts index 210c830f91a..923ae4b7979 100644 --- a/server/src/services/delivery-work-notifications.ts +++ b/server/src/services/delivery-work-notifications.ts @@ -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", diff --git a/server/src/services/tool-access-policy.ts b/server/src/services/tool-access-policy.ts index 16ac7f6fdce..cfd2f4478a6 100644 --- a/server/src/services/tool-access-policy.ts +++ b/server/src/services/tool-access-policy.ts @@ -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, companyId: string, policyId: string) { const [policy] = await db .select() .from(toolPolicies) @@ -785,7 +792,7 @@ function scopeAllowsTool(scope: Record | null, ctx: ToolAccessC return selectorMatches(selectors, ctx); } -export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecret?: string } = {}) { +export function toolAccessPolicyService(db: Pick, 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 { + async function admittedRateLimitAdmissions(ctx: ToolAccessContext, invocationId: string, previewHash?: string): Promise { 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, "decide" | "writeAudit">, + input: ToolAccessDecisionInput, + transaction: Db, + ) => Promise, + ): Promise { + 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 { 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 { // 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, diff --git a/server/src/services/tool-approval-execution.ts b/server/src/services/tool-approval-execution.ts new file mode 100644 index 00000000000..f5f4fb78abf --- /dev/null +++ b/server/src/services/tool-approval-execution.ts @@ -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 { + 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; + }); +} diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index 2ff322ad308..ff5e1028ffc 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -1,3 +1,5 @@ +import { bindVerifiedToolApprovalArguments, claimToolTestApproval, releaseToolApproval, persistToolTestOutcome, startToolInvocation, type ToolApprovalClaim } from "./tool-approval-execution.js"; +import { approvedToolReviewCompanies, prepareApprovedToolReview, recoverApprovedToolReviews } from "./tool-review-recovery.js"; import { toolReviewExpiryService } from "./tool-review-expiry.js"; import { writeToolReviewChange } from "./tool-review-work-notifications.js"; import { restConnectorFor } from "./connectors/rest-registry.js"; @@ -1245,46 +1247,56 @@ export function createToolGatewayService( const interactions = issueThreadInteractionService(db); const basePolicyService = toolAccessPolicyService(db, { toolActionSigningSecret: options.toolActionSigningSecret }); const issuerDecisionCaches = new WeakMap(); + async function decideGatewayPolicy( + input: ToolAccessDecisionInput, + decisionOptions: NonNullable[1]> & { requiredGatewayAction?: ToolMcpGatewayTokenAction } = {}, + policy: Pick = basePolicyService, + database: Db = db, + ): Promise { + const issuerId = input.runContext?.originatingAgentId; + if (!issuerId || !input.runContext?.gatewayId || input.actor.actorType !== "agent") return policy.decide(input, decisionOptions); + if (!input.runContext.gatewayTokenId) throw new ToolGatewayHttpError(403, "Delegated gateway token is no longer available", "gateway_token_run_context_invalid"); + let issuerCache: ToolAccessDecisionCache | undefined; + if (decisionOptions.cache) { + issuerCache = issuerDecisionCaches.get(decisionOptions.cache); + if (!issuerCache) { issuerCache = createToolAccessDecisionCache(); issuerDecisionCaches.set(decisionOptions.cache, issuerCache); } + } + const requiredAction = decisionOptions.requiredGatewayAction ?? (input.consumeRateLimit ? "tools/call" : undefined); + const authorityKey = `gateway-issuer:${input.companyId}:${input.runContext.gatewayTokenId}:${issuerId}:${requiredAction ?? "inspect"}`; + let authority = issuerCache?.get(authorityKey); + if (!authority) { + authority = assertGatewayIssuerAuthority(input, requiredAction, database); + issuerCache?.set(authorityKey, authority); + } + await authority; + if (issuerId === input.actor.agentId) { + const decision = await policy.decide(input, decisionOptions); + return { ...decision, policyExplanation: { ...decision.policyExplanation, delegatedGatewayIssuer: issuerId } }; + } + const issuerInput = { ...input, actor: { ...input.actor, agentId: issuerId } }; + const issuer = await policy.decide(issuerInput, { ...decisionOptions, cache: issuerCache }); + const target = await policy.decide(input, { ...decisionOptions, sharedRateDecision: issuer }); + const chosen = !issuer.allowed && issuer.decision !== "require_approval" ? issuer + : !target.allowed ? target : issuer.decision === "require_approval" ? issuer : target; + if (input.consumeRateLimit) await policy.writeAudit(issuerInput, issuer); + return { ...chosen, matchedPolicyIds: [...new Set([...issuer.matchedPolicyIds, ...target.matchedPolicyIds])], policyExplanation: { + ...chosen.policyExplanation, + consumedRateAdmissions: target.policyExplanation?.consumedRateAdmissions, + delegatedCallerConsumedRateAdmissions: issuer.policyExplanation?.consumedRateAdmissions, + delegatedGatewayIssuer: issuerId, + } }; + } const policyService = { ...basePolicyService, - async decide(input: ToolAccessDecisionInput, decisionOptions: NonNullable[1]> & { requiredGatewayAction?: ToolMcpGatewayTokenAction } = {}): Promise { - const issuerId = input.runContext?.originatingAgentId; - if (!issuerId || !input.runContext?.gatewayId || input.actor.actorType !== "agent") return basePolicyService.decide(input, decisionOptions); - if (!input.runContext.gatewayTokenId) throw new ToolGatewayHttpError(403, "Delegated gateway token is no longer available", "gateway_token_run_context_invalid"); - let issuerCache: ToolAccessDecisionCache | undefined; - if (decisionOptions.cache) { - issuerCache = issuerDecisionCaches.get(decisionOptions.cache); - if (!issuerCache) { issuerCache = createToolAccessDecisionCache(); issuerDecisionCaches.set(decisionOptions.cache, issuerCache); } - } - const requiredAction = decisionOptions.requiredGatewayAction ?? (input.consumeRateLimit ? "tools/call" : undefined); - const authorityKey = `gateway-issuer:${input.companyId}:${input.runContext.gatewayTokenId}:${issuerId}:${requiredAction ?? "inspect"}`; - let authority = issuerCache?.get(authorityKey); - if (!authority) { - authority = assertGatewayIssuerAuthority(input, requiredAction); - issuerCache?.set(authorityKey, authority); - } - await authority; - if (issuerId === input.actor.agentId) { - const decision = await basePolicyService.decide(input, decisionOptions); - return { ...decision, policyExplanation: { ...decision.policyExplanation, delegatedGatewayIssuer: issuerId } }; - } - const issuerInput = { ...input, actor: { ...input.actor, agentId: issuerId } }; - const issuer = await basePolicyService.decide(issuerInput, { ...decisionOptions, cache: issuerCache }); - const target = await basePolicyService.decide(input, { ...decisionOptions, sharedRateDecision: issuer }); - const chosen = !issuer.allowed && issuer.decision !== "require_approval" ? issuer - : !target.allowed ? target : issuer.decision === "require_approval" ? issuer : target; - if (input.consumeRateLimit) await basePolicyService.writeAudit(issuerInput, issuer); - return { ...chosen, matchedPolicyIds: [...new Set([...issuer.matchedPolicyIds, ...target.matchedPolicyIds])], policyExplanation: { - ...chosen.policyExplanation, - consumedRateAdmissions: target.policyExplanation?.consumedRateAdmissions, - delegatedCallerConsumedRateAdmissions: issuer.policyExplanation?.consumedRateAdmissions, - delegatedGatewayIssuer: issuerId, - } }; + decide: decideGatewayPolicy, + admitInvocation(input: ToolAccessDecisionInput, invocationId: string, claim?: ToolApprovalClaim) { + return basePolicyService.admitInvocation(input, invocationId, claim, (policy, admissionInput, transaction) => + decideGatewayPolicy(admissionInput, { admittedInvocationId: invocationId, requiredGatewayAction: "tools/call" }, policy, transaction)); }, }; - async function assertGatewayIssuerAuthority(input: ToolAccessDecisionInput, requiredAction?: ToolMcpGatewayTokenAction) { - const [row] = await db.select({ token: toolMcpGatewayTokens, gateway: toolMcpGateways }).from(toolMcpGatewayTokens) + async function assertGatewayIssuerAuthority(input: ToolAccessDecisionInput, requiredAction?: ToolMcpGatewayTokenAction, database: Db = db) { + const [row] = await database.select({ token: toolMcpGatewayTokens, gateway: toolMcpGateways }).from(toolMcpGatewayTokens) .innerJoin(toolMcpGateways, eq(toolMcpGateways.id, toolMcpGatewayTokens.gatewayId)) .where(and(eq(toolMcpGatewayTokens.id, input.runContext!.gatewayTokenId!), eq(toolMcpGatewayTokens.companyId, input.companyId))); const issuerId = input.runContext!.originatingAgentId!; @@ -1299,8 +1311,8 @@ export function createToolGatewayService( const resource = { type: "connection_management" as const, companyId: input.companyId, gatewayId: row.gateway.id, profileId: row.gateway.profileId, purpose: "tool" as const, operation: "gateway_token.use", targetAgentId: row.gateway.agentId, projectId: row.gateway.projectId, issueId: row.gateway.issueId }; - await connectionManagementService(db).authorize(actor, "connections:gateway", resource, { mutation: false }); - await connectionManagementService(db).authorize(actor, "connections:test", resource, { mutation: false }); + await connectionManagementService(database).authorize(actor, "connections:gateway", resource, { mutation: false }); + await connectionManagementService(database).authorize(actor, "connections:test", resource, { mutation: false }); } async function recordToolInvocation(input: ToolAccessDecisionInput, decision: ToolAccessDecision) { try { @@ -7548,6 +7560,7 @@ export function createToolGatewayService( matchedPolicyIds: string[]; timeoutMs?: number; signal?: AbortSignal; + approvedClaim?: ToolApprovalClaim; managementContext?: ConnectionManagementContext; }): Promise< | { decision: "allowed"; invocationId: string; result: unknown } @@ -7557,14 +7570,9 @@ export function createToolGatewayService( error: { message: string; reasonCode: string }; } > { - await db - .update(toolInvocations) - .set({ - status: "executing", - startedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, args.invocationId)); + if (!(await startToolInvocation(db, args.invocationId, args.approvedClaim))) { + throw new ToolGatewayHttpError(409, "Tool action execution claim is no longer current", "action_already_consumed"); + } await writeAudit({ session: args.session, companyId: args.companyId, @@ -7587,6 +7595,8 @@ export function createToolGatewayService( }); const startedAt = Date.now(); + let connectedMcpExecution: Awaited> | Awaited> | null; + let resultValidation: Awaited>; try { await args.managementContext?.reauthorize(); if (args.managementContext?.actor.type === "agent") { @@ -7598,7 +7608,7 @@ export function createToolGatewayService( } } 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, @@ -7629,7 +7639,7 @@ export function createToolGatewayService( ); } const result = connectedMcpExecution.result; - const resultValidation = await validateToolContent({ + resultValidation = await validateToolContent({ value: result, direction: "result", sensitiveMode: "redact", @@ -7640,69 +7650,6 @@ export function createToolGatewayService( throw new ToolGatewayHttpError(502, String(providerResult.content || providerResult.error), "tool_error", { execution: connectedMcpExecution.execution }); } - await db - .update(toolInvocations) - .set({ - status: "succeeded", - resultHash: resultValidation.summary.sha256 ?? null, - resultSummary: resultValidation.summary, - resultSizeBytes: resultValidation.summary.sizeBytes ?? null, - completedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, args.invocationId)); - 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: args.session.runId, - issueId: args.session.issueId, - actorType: args.session.actorType, - actorId: args.session.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, - }, - }); - // After the bookkeeping above: a throw there is caught below, overwrites - // the row to failed, and emits — an earlier success emit would make one - // execution report both outcomes. - 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 = @@ -7712,21 +7659,10 @@ export function createToolGatewayService( ? err.reasonCode : "tool_execution_failed"; const message = err instanceof Error ? err.message : String(err); - await db - .update(toolInvocations) - .set({ - status: - status === 504 - ? "timed_out" - : status === 429 - ? "rate_limited" - : "failed", - errorCode: reasonCode, - errorMessage: message, - completedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, args.invocationId)); + await persistToolTestOutcome(db, args.invocationId, { + status: status === 504 ? "timed_out" : status === 429 ? "rate_limited" : "failed", + errorCode: reasonCode, errorMessage: message, + }, args.approvedClaim); void emitConnectionInvoked(db, args.invocationId); await writeToolCallEvent({ invocationId: args.invocationId, @@ -7785,132 +7721,161 @@ 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: args.session.runId, + issueId: args.session.issueId, + actorType: args.session.actorType, + actorId: args.session.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, + }; + + } + + function isApprovalPreparationRejection(error: unknown) { + return error instanceof ToolContentValidationError || + (error instanceof ToolGatewayHttpError || error instanceof HttpError) && error.status >= 400 && error.status < 500 && + ![408, 425, 429].includes(error.status); + } + + async function bindVerifiedApprovalArguments(invocation: typeof toolInvocations.$inferSelect, claim: ToolApprovalClaim, parameters: unknown) { + const preview = summarizeToolValue(parameters).sha256; + const verified = toolRequestIdentityHash(parameters, options.toolActionSigningSecret); + if (invocation.argumentsHash === preview && invocation.argumentsHash !== verified && + !await bindVerifiedToolApprovalArguments(db, claim, { preview, verified })) { + throw new ToolGatewayHttpError(409, "Tool action execution claim is no longer current", "action_already_consumed"); + } } - /** - * 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, - parameters: unknown, + signedPayload: NonNullable>, actionRequestId: string, ): Promise { - const agentId = invocation.agentId; - if (!invocation.connectionId || !agentId) return; - const delegated = invocation.policyExplanation?.delegatedConnectionTest as { - agentId: string; responsibleUserId: string; runId: string | null; keyId: string | null; - resource: ConnectionManagementContext["resource"]; - } | undefined; - const userId = delegated?.responsibleUserId ?? invocation.actorId ?? "board"; - const session: ToolGatewaySession = { - id: "test-call", - token: "test-call", - companyId: invocation.companyId, - agentId, - runId: null, - issueId: null, - projectId: null, - actorType: delegated ? "agent" : "user", - actorId: delegated?.agentId ?? userId, - responsibleUserId: userId, - createdAt: new Date(), - expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), - }; - let managementContext: ConnectionManagementContext | undefined; - let tool: ToolGatewayDescriptor | undefined; - try { - tool = ( - await connectedMcpToolsForConnection( - invocation.companyId, - invocation.connectionId, - ) - ).find( - (candidate) => - candidate.name === invocation.toolName || - candidate.upstreamToolName === invocation.toolName, - ); - } catch { - tool = undefined; - } - if (!tool) { - await db - .update(toolInvocations) - .set({ - status: "failed", - errorCode: "tool_not_found", - errorMessage: `Tool "${invocation.toolName}" is no longer connected`, - completedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, invocation.id)); - void emitConnectionInvoked(db, invocation.id); - await reflectToolActionInteractionLifecycle({ - actionRequestId, - status: "failed", - errorCode: "tool_not_found", - errorMessage: `Tool "${invocation.toolName}" is no longer connected`, - }); - return; - } - const argumentsSummary = (await validateToolContent({ - value: parameters, - direction: "arguments", - sensitiveMode: "redact", - promptInjectionMode: "ignore", - })).summary; + const claim = await claimToolTestApproval(db, actionRequestId, invocation.id); + if (!claim) return; + let execution: Parameters[0]; try { + const agentId = invocation.agentId; + if (!invocation.connectionId || !agentId) { + throw new ToolGatewayHttpError(409, "Test action context is no longer available", "action_origin_invalid"); + } + const delegated = invocation.policyExplanation?.delegatedConnectionTest as { + agentId: string; responsibleUserId: string; runId: string | null; keyId: string | null; + resource: ConnectionManagementContext["resource"]; + } | undefined; + const userId = delegated?.responsibleUserId ?? invocation.actorId ?? "board"; + const session: ToolGatewaySession = { + id: "test-call", token: "test-call", companyId: invocation.companyId, agentId, + runId: null, issueId: null, projectId: null, actorType: delegated ? "agent" : "user", actorId: delegated?.agentId ?? userId, + responsibleUserId: userId, createdAt: new Date(), expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), + }; + let managementContext: ConnectionManagementContext | undefined; if (delegated) { managementContext = await connectionManagementService(db).authorize({ type: "agent", companyId: invocation.companyId, agentId: delegated.agentId, onBehalfOfUserId: delegated.responsibleUserId, runId: delegated.runId, keyId: delegated.keyId, }, "connections:test", delegated.resource, { targetAgentIds: [agentId], afterHumanAction: true }); Object.assign(session, managementContext.workContext, { originatingAgentId: delegated.agentId, approvedInvocationId: invocation.id }); - const current = await policyService.decide(policyInputForTool({ session, tool, parameters }), { admittedInvocationId: invocation.id }); - if (!current.allowed && current.decision !== "require_approval") throw new ToolGatewayHttpError(403, current.explanation, current.reasonCode); } - const outcome = await runTestToolInvocation({ - session, - managementContext, - tool, - parameters, - invocationId: invocation.id, - companyId: invocation.companyId, - agentId, - userId, - argumentsSummary, - reasonCode: "approval_granted", - matchedPolicyIds: invocation.matchedPolicyIds ?? [], - }); - await reflectToolActionInteractionLifecycle({ - actionRequestId, - status: "error" in outcome ? "failed" : "executed", - ...("error" in outcome ? { errorCode: outcome.error.reasonCode, errorMessage: outcome.error.message } : {}), - }); + const tool = (await connectedMcpToolsForConnection(invocation.companyId, invocation.connectionId)) + .find(candidate => candidate.name === invocation.toolName || candidate.upstreamToolName === invocation.toolName); + if (!tool) throw new ToolGatewayHttpError(404, `Tool "${invocation.toolName}" is no longer connected`, "tool_not_found"); + if (!approvalSnapshotsMatch(signedPayload.approvalSnapshot, await connectedRemoteApprovalSnapshot(session, tool))) { + throw new ToolGatewayHttpError(409, "Tool definition or connection changed; request a new review", "approved_tool_target_changed"); + } + if (!(await approvedManagedArgumentsRemainCurrent(session, tool, signedPayload.arguments))) { + throw new ToolGatewayHttpError(409, "Approved tool action managed arguments changed after review", "approved_tool_managed_arguments_changed"); + } + await bindVerifiedApprovalArguments(invocation, claim, signedPayload.arguments); + const access = await basePolicyService.admitInvocation( + policyInputForTool({ session, tool, parameters: signedPayload.arguments, consumeRateLimit: false }), + invocation.id, + claim, + async (policy, admissionInput) => { + const callerSession = delegatedTestCallerSession(session); + const callerInput = callerSession ? policyInputForTool({ session: callerSession, tool, + parameters: signedPayload.arguments, consumeRateLimit: true }) : null; + const options = { admittedInvocationId: invocation.id }; + const caller = callerInput ? await policy.decide(callerInput, options) : null; + const target = await policy.decide(admissionInput, { ...options, sharedRateDecision: caller ?? undefined }); + const chosen = caller && !caller.allowed && caller.decision !== "require_approval" ? caller + : !target.allowed ? target : caller?.decision === "require_approval" ? caller : target; + if (callerInput && caller) await policy.writeAudit(callerInput, caller); + return { ...chosen, policyExplanation: { + ...chosen.policyExplanation, + consumedRateAdmissions: target.policyExplanation?.consumedRateAdmissions, + delegatedCallerConsumedRateAdmissions: caller?.policyExplanation?.consumedRateAdmissions, + } }; + }, + ); + if (!access.allowed && access.decision !== "require_approval") { + throw new ToolGatewayHttpError(403, access.explanation, access.reasonCode); + } + const argumentsSummary = (await validateToolContent({ value: signedPayload.arguments, + direction: "arguments", sensitiveMode: "redact", promptInjectionMode: "ignore" })).summary; + execution = { session, managementContext, tool, parameters: signedPayload.arguments, invocationId: invocation.id, + companyId: invocation.companyId, agentId, userId, argumentsSummary, reasonCode: "approval_granted", + approvedClaim: claim, matchedPolicyIds: invocation.matchedPolicyIds ?? [] }; } catch (err) { + if (!isApprovalPreparationRejection(err)) { + // Preparation has not entered runTestToolInvocation. Preserve approval + // through dependency failures; the claim owner still verifies no dispatch. + await releaseToolApproval(db, claim); + throw err; + } const message = err instanceof Error ? err.message : String(err); - await db - .update(toolInvocations) - .set({ - status: "failed", - errorCode: "tool_execution_failed", - errorMessage: message, - completedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, invocation.id)); - void emitConnectionInvoked(db, invocation.id); - await reflectToolActionInteractionLifecycle({ - actionRequestId, - status: "failed", - errorCode: "tool_execution_failed", - errorMessage: message, - }); + 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) { @@ -8080,6 +8045,7 @@ export function createToolGatewayService( } async function expireApprovedActionForManagedArgumentDrift(input: { + companyId: string; actionRequestId: string; invocationId: string; toolName: string; @@ -8097,7 +8063,7 @@ export function createToolGatewayService( ); const now = new Date(); await options.beforeManagedArgumentDriftExpiry?.(); - const [expired] = await db + const [expired] = await writeToolReviewChange(db, input.companyId, tx => tx .update(toolActionRequests) .set({ status: "expired", resolvedAt: now, updatedAt: now }) .where( @@ -8108,7 +8074,7 @@ export function createToolGatewayService( : eq(toolActionRequests.status, "approved"), ), ) - .returning({ id: toolActionRequests.id }); + .returning({ id: toolActionRequests.id })); if (!expired) return error; await db .update(toolInvocations) @@ -8227,7 +8193,7 @@ export function createToolGatewayService( ); } - const [claimed] = await db + const [claimed] = await writeToolReviewChange(db, actionRequest.companyId, tx => tx .update(toolActionRequests) .set({ status: "executing", updatedAt: new Date() }) .where( @@ -8236,7 +8202,7 @@ export function createToolGatewayService( eq(toolActionRequests.status, "approved"), ), ) - .returning(); + .returning()); if (!claimed) { const settled = await waitForActionRequestExecution(actionRequest.id); const [settledInvocation] = await db @@ -8262,92 +8228,141 @@ export function createToolGatewayService( ); } - // Terminal-issue expiry revokes pending/approved requests, but a claim that - // committed just before the issue closed slips past that revocation. Recheck - // the issue after winning the claim so a governed action never runs external - // side effects for an issue that is already done or cancelled. - const issue = await assertIssueOpenForApprovedAction({ - claimed, - invocation, - }); - - const signedPayload = readSignedToolArgumentsPayload({ - signedArguments: claimed.signedArguments, - invocationId: invocation.id, - toolName: invocation.toolName, - signingSecret: options.toolActionSigningSecret, - }); - if (!signedPayload) { - const error = new ToolGatewayHttpError( - 409, - "Approved tool action arguments signature is invalid", - "signed_arguments_invalid", - ); - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); - throw error; - } - if (signedPayload.executionOnApprove !== true) { - const error = new ToolGatewayHttpError( - 409, - "This approval predates execute-on-approve and must remain inert", - "legacy_approved_action_inert", - ); - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); - throw error; - } - - const session: ToolGatewaySession = { - id: `approved-action:${claimed.id}`, - token: "", - companyId: invocation.companyId, - agentId: invocation.agentId, - runId: invocation.runId, - issueId: invocation.issueId, - projectId: issue?.projectId ?? null, - gatewayId: invocation.gatewayId, - gatewayPublicId: invocation.gatewayPublicId, - gatewayTokenId: invocation.gatewayTokenId, - actorType: "agent", - actorId: invocation.actorType === "agent" ? invocation.actorId ?? invocation.agentId : invocation.agentId, - originatingAgentId: typeof invocation.policyExplanation?.delegatedGatewayIssuer === "string" ? invocation.policyExplanation.delegatedGatewayIssuer : invocation.actorType === "agent" && invocation.gatewayId && !invocation.runId && !invocation.gatewayTokenId ? invocation.actorId : null, - createdAt: new Date(), - expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), + const claim: ToolApprovalClaim = { + actionRequestId: claimed.id, invocationId: invocation.id, claimedAt: claimed.updatedAt, }; - let tool: ToolGatewayDescriptor; - let liveApprovalSnapshot: Awaited< - ReturnType - >; - try { + const agentId = invocation.agentId; + async function prepareExecution() { + // Terminal-issue expiry revokes pending/approved requests, but a claim that + // committed just before the issue closed slips past that revocation. Recheck + // the issue after winning the claim so a governed action never runs external + // side effects for an issue that is already done or cancelled. + const issue = await assertIssueOpenForApprovedAction({ + claimed, + invocation, + }); + + const signedPayload = readSignedToolArgumentsPayload({ + signedArguments: claimed.signedArguments, + invocationId: invocation.id, + toolName: invocation.toolName, + signingSecret: options.toolActionSigningSecret, + }); + if (!signedPayload) { + const error = new ToolGatewayHttpError( + 409, + "Approved tool action arguments signature is invalid", + "signed_arguments_invalid", + ); + throw error; + } + if (signedPayload.executionOnApprove !== true) { + const error = new ToolGatewayHttpError( + 409, + "This approval predates execute-on-approve and must remain inert", + "legacy_approved_action_inert", + ); + throw error; + } + + const session: ToolGatewaySession = { + id: `approved-action:${claimed.id}`, + token: "", + companyId: invocation.companyId, + agentId, + runId: invocation.runId, + issueId: invocation.issueId, + projectId: issue?.projectId ?? null, + gatewayId: invocation.gatewayId, + gatewayPublicId: invocation.gatewayPublicId, + gatewayTokenId: invocation.gatewayTokenId, + actorType: invocation.actorType as ToolGatewaySession["actorType"], + actorId: invocation.actorId, + originatingAgentId: typeof invocation.policyExplanation?.delegatedGatewayIssuer === "string" ? invocation.policyExplanation.delegatedGatewayIssuer : invocation.actorType === "agent" && invocation.gatewayId && !invocation.runId && !invocation.gatewayTokenId ? invocation.actorId : null, + createdAt: new Date(), + expiresAt: new Date(Date.now() + DEFAULT_SESSION_TTL_MS), + }; await restoreApprovedActionIdentity( session, signedPayload.identityContextId, ); session.approvedInvocationId = invocation.id; session.approvedSlackInvocationId = invocation.id; - tool = await findToolForSession(session, invocation.toolName); - liveApprovalSnapshot = await connectedRemoteApprovalSnapshot( + const tool = await findToolForSession(session, invocation.toolName); + const liveApprovalSnapshot = await connectedRemoteApprovalSnapshot( session, tool, ); - const currentAccess = await policyService.decide( + if ( + !approvalSnapshotsMatch( + signedPayload.approvalSnapshot, + liveApprovalSnapshot, + ) + ) { + const error = new ToolGatewayHttpError( + 409, + "Approved tool action target changed after review", + "approved_tool_target_changed", + ); + throw error; + } + const parameters = signedPayload.arguments; + const canonicalArguments = canonicalToolArguments(parameters); + if ( + ![ + toolRequestIdentityHash(parameters, options.toolActionSigningSecret), + // Historical approvals used the redacted preview hash. Their signature + // still binds the complete canonical arguments checked below. + summarizeToolValue(parameters).sha256, + ].includes(claimed.canonicalArgumentsHash) || + !verifyToolArgumentsSignature({ + signedArguments: claimed.signedArguments, + invocationId: invocation.id, + toolName: invocation.toolName, + canonicalArguments, + approvalSnapshot: signedPayload.approvalSnapshot, + executionOnApprove: true, + identityContextId: signedPayload.identityContextId, + signingSecret: options.toolActionSigningSecret, + }) + ) { + const error = new ToolGatewayHttpError( + 409, + "Approved tool action arguments do not match reviewed hash", + "signed_arguments_mismatch", + ); + throw error; + } + const managedArgumentsRemainCurrent = + await approvedManagedArgumentsRemainCurrent(session, tool, parameters); + if (!managedArgumentsRemainCurrent) { + throw await expireApprovedActionForManagedArgumentDrift({ + companyId: claimed.companyId, + actionRequestId: claimed.id, + invocationId: invocation.id, + toolName: invocation.toolName, + ownsExecutingClaim: true, + }); + } + + const argumentsSummary = (await validateToolContent({ + value: parameters, + direction: "arguments", + sensitiveMode: "redact", + promptInjectionMode: "ignore", + })).summary; + // Validate the signed and managed payload before consuming admission. + // Historical payload drift belongs to the expiry owner above. + await bindVerifiedApprovalArguments(invocation, claim, signedPayload.arguments); + const currentAccess = await policyService.admitInvocation( policyInputForTool({ session, tool, parameters: signedPayload.arguments, + consumeRateLimit: false, }), - { admittedInvocationId: invocation.id, requiredGatewayAction: "tools/call" }, + invocation.id, + claim, ); if ( !currentAccess.allowed && @@ -8358,120 +8373,43 @@ export function createToolGatewayService( currentAccess.explanation, currentAccess.reasonCode, ); - } catch (error) { - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); - throw error; + // Final recheck at the last DB write before dispatch: tool and snapshot + // resolution above involve network calls, so re-verify the issue is still + // open now that only the provider call remains. A close that commits after + // this read has raced an execution that was approved, claimed, and verified + // while the issue was open; that instant is irreducible for an external + // side effect gated by DB state (holding a DB lock across a remote provider + // call is not an option), and the accepted linearization is that the + // execution wins — its result still lands on the expired card via + // reflectToolActionInteractionLifecycle. + await assertIssueOpenForApprovedAction({ claimed, invocation }); + return { session, tool, parameters, argumentsSummary }; } - if ( - !approvalSnapshotsMatch( - signedPayload.approvalSnapshot, - liveApprovalSnapshot, - ) - ) { - const error = new ToolGatewayHttpError( - 409, - "Approved tool action target changed after review", - "approved_tool_target_changed", - ); - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); - throw error; - } - const parameters = signedPayload.arguments; - const canonicalArguments = canonicalToolArguments(parameters); - if ( - ![ - toolRequestIdentityHash(parameters, options.toolActionSigningSecret), - // Historical approvals used the redacted preview hash. Their signature - // still binds the complete canonical arguments checked below. - summarizeToolValue(parameters).sha256, - ].includes(claimed.canonicalArgumentsHash) || - !verifyToolArgumentsSignature({ - signedArguments: claimed.signedArguments, - invocationId: invocation.id, - toolName: invocation.toolName, - canonicalArguments, - approvalSnapshot: signedPayload.approvalSnapshot, - executionOnApprove: true, - identityContextId: signedPayload.identityContextId, - signingSecret: options.toolActionSigningSecret, - }) - ) { - const error = new ToolGatewayHttpError( - 409, - "Approved tool action arguments do not match reviewed hash", - "signed_arguments_mismatch", - ); - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); - throw error; - } - let managedArgumentsRemainCurrent: boolean; + let prepared: Awaited>; try { - managedArgumentsRemainCurrent = - await approvedManagedArgumentsRemainCurrent(session, tool, parameters); + prepared = await prepareExecution(); } catch (error) { - await markApprovedActionFailed({ - actionRequestId: claimed.id, - invocationId: invocation.id, - claimUpdatedAt: claimed.updatedAt, - expectedInvocationStatus: "awaiting_approval", - error, - }); + if (isApprovalPreparationRejection(error)) { + await markApprovedActionFailed({ + actionRequestId: claimed.id, + invocationId: invocation.id, + claimUpdatedAt: claimed.updatedAt, + expectedInvocationStatus: "awaiting_approval", + error, + }); + } else { + await releaseToolApproval(db, claim); + } throw error; } - if (!managedArgumentsRemainCurrent) { - throw await expireApprovedActionForManagedArgumentDrift({ - actionRequestId: claimed.id, - invocationId: invocation.id, - toolName: invocation.toolName, - ownsExecutingClaim: true, - }); - } - - const argumentsSummary = (await validateToolContent({ - value: parameters, - direction: "arguments", - sensitiveMode: "redact", - promptInjectionMode: "ignore", - })).summary; - // Final recheck at the last DB write before dispatch: tool and snapshot - // resolution above involve network calls, so re-verify the issue is still - // open now that only the provider call remains. A close that commits after - // this read has raced an execution that was approved, claimed, and verified - // while the issue was open; that instant is irreducible for an external - // side effect gated by DB state (holding a DB lock across a remote provider - // call is not an option), and the accepted linearization is that the - // execution wins — its result still lands on the expired card via - // reflectToolActionInteractionLifecycle. - await assertIssueOpenForApprovedAction({ claimed, invocation }); + const { session, tool, parameters, argumentsSummary } = prepared; const startedAt = Date.now(); - await db - .update(toolInvocations) - .set({ - status: "executing", - approvalState: "approved", - startedAt: new Date(), - updatedAt: new Date(), - }) - .where(eq(toolInvocations.id, invocation.id)); + // Keep dispatch and outcome persistence outside the preparation retry + // boundary. Only the still-current claim may record the provider start. + if (!(await startToolInvocation(db, invocation.id, claim))) { + throw new ToolGatewayHttpError(409, "Tool action execution claim is no longer current", "action_already_consumed"); + } await reflectToolActionInteractionLifecycle({ actionRequestId: claimed.id, status: "executing", @@ -9977,7 +9915,33 @@ export function createToolGatewayService( actionReviewExpiryDeadlines: reviewExpiry.deadlines, sweepExpiredActionReviewsForCompany: reviewExpiry.sweepForCompany, - async sweepActionReviews(input: { includeExpiry?: boolean } = {}) { + approvedActionReviewCompanies: (companyId?: string) => approvedToolReviewCompanies(db, companyId), + + async recoverApprovedActionReviews(companyId?: string) { + return recoverApprovedToolReviews(db, async row => { + try { + const prepared = await prepareApprovedToolReview(db, { actionRequestId: row.request.id, + invocationId: row.invocation.id, signingSecret: options.toolActionSigningSecret }); + if (prepared.kind === "skip") return false; + if (prepared.kind === "settled") { + await reflectToolActionInteractionLifecycle(prepared.projection); + return false; + } + const { request, invocation, payload } = prepared; + if (!isTestOriginInvocation(invocation) && payload.executionOnApprove !== true) return false; + await this.approveActionRequest({ companyId: request.companyId, actionRequestId: request.id, + actor: { userId: request.decidedByUserId ?? request.resolvedByUserId } }); + const [remaining] = await db.select({ status: toolActionRequests.status }).from(toolActionRequests) + .where(eq(toolActionRequests.id, request.id)); + return remaining?.status === "approved"; + } catch (err) { + logger.warn({ err, actionRequestId: row.request.id }, "Could not recover approved tool action"); + return true; + } + }, companyId); + }, + + async sweepActionReviews(input: { includeExpiry?: boolean; includeApproved?: boolean } = {}) { // A process may stop between persisting a provider outcome and updating the // feed projection. Reconcile from authoritative rows before delivering it. const unreflected = await db @@ -10016,38 +9980,21 @@ export function createToolGatewayService( const staleAt = new Date(now.getTime() - 10 * 60_000); let cursor: string | undefined; let scanned = input.includeExpiry === false ? 0 : (await reviewExpiry.sweep(now)).scanned; + if (input.includeApproved !== false) scanned += (await this.recoverApprovedActionReviews()).scanned; for (;;) { const rows = await db .select() .from(toolActionRequests) .where( and( - or( - eq(toolActionRequests.status, "approved"), - and( - eq(toolActionRequests.status, "executing"), - lte(toolActionRequests.updatedAt, staleAt), - ), - ), + 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) { - if (row.status === "approved") { - await this.approveActionRequest({ - companyId: row.companyId, - actionRequestId: row.id, - actor: { userId: row.decidedByUserId ?? row.resolvedByUserId }, - }).catch((error) => - logger.warn( - { err: error, actionRequestId: row.id }, - "Could not recover approved tool action", - ), - ); - continue; - } const status = "failed"; const errorCode = "tool_execution_outcome_unknown"; const errorMessage = "Execution was interrupted; the external outcome is unknown. Inspect the provider before retrying."; @@ -10239,10 +10186,12 @@ export function createToolGatewayService( actionRequestId: actionRequest.id, status: "approved", }); - if ( - !isTestOriginInvocation(invocation) && - signedPayload.executionOnApprove === true - ) { + if (isTestOriginInvocation(invocation)) { + await runApprovedTestInvocation(invocation, signedPayload, actionRequest.id); + const [settled] = await db.select().from(toolActionRequests).where(eq(toolActionRequests.id, actionRequest.id)); + return actionRequestResolution(settled ?? actionRequest); + } + if (signedPayload.executionOnApprove === true) { try { await executeApprovedAgentInvocation({ actionRequest, invocation }); } catch { @@ -10318,13 +10267,18 @@ export function createToolGatewayService( "Tool definition or connection changed; request a new review", "approved_tool_target_changed", ); + const previewHash = summarizeToolValue(signedPayload.arguments).sha256; const access = await policyService.decide( policyInputForTool({ session, tool, parameters: signedPayload.arguments, + consumeRateLimit: false, }), - { admittedInvocationId: invocation.id, requiredGatewayAction: "tools/call" }, + { admittedInvocationId: invocation.id, requiredGatewayAction: "tools/call", + admittedPreviewHash: [previewHash, toolRequestIdentityHash(signedPayload.arguments, options.toolActionSigningSecret)] + .includes(actionRequest.canonicalArgumentsHash) ? previewHash : undefined, + }, ); if (!access.allowed && access.decision !== "require_approval") throw new ToolGatewayHttpError( @@ -10347,7 +10301,7 @@ export function createToolGatewayService( if (isTestOriginInvocation(invocation)) { await runApprovedTestInvocation( { ...invocation, approvalState: "approved" }, - signedPayload.arguments, + signedPayload, updated.id, ); } else if (signedPayload.executionOnApprove === true) { @@ -10911,7 +10865,7 @@ export function createToolGatewayService( ); await options.beforeLegacyApprovedActionClaim?.(); const claimedAt = new Date(); - const [claimed] = await db + const [claimed] = await writeToolReviewChange(db, actionRequest.companyId, tx => tx .update(toolActionRequests) .set({ status: "executing", @@ -10924,7 +10878,7 @@ export function createToolGatewayService( eq(toolActionRequests.status, "approved"), ), ) - .returning(); + .returning()); if (!claimed) { throw new ToolGatewayHttpError( 409, @@ -11004,13 +10958,14 @@ export function createToolGatewayService( )) ) { throw await expireApprovedActionForManagedArgumentDrift({ + companyId: actionRequest.companyId, actionRequestId: actionRequest.id, invocationId: storedInvocation.id, toolName: storedInvocation.toolName, }); } const claimedAt = new Date(); - const [claimed] = await db + const [claimed] = await writeToolReviewChange(db, actionRequest.companyId, tx => tx .update(toolActionRequests) .set({ status: "executing", @@ -11023,7 +10978,7 @@ export function createToolGatewayService( eq(toolActionRequests.status, "approved"), ), ) - .returning(); + .returning()); if (!claimed) { throw new ToolGatewayHttpError( 409, diff --git a/server/src/services/tool-review-recovery-work.ts b/server/src/services/tool-review-recovery-work.ts new file mode 100644 index 00000000000..5611f5cf155 --- /dev/null +++ b/server/src/services/tool-review-recovery-work.ts @@ -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, + gateway: Pick, "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(); + 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); +} diff --git a/server/src/services/tool-review-recovery.ts b/server/src/services/tool-review-recovery.ts new file mode 100644 index 00000000000..69aa98c76f0 --- /dev/null +++ b/server/src/services/tool-review-recovery.ts @@ -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, 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 }; +} diff --git a/server/src/services/tool-review-work-notifications.ts b/server/src/services/tool-review-work-notifications.ts index 19c8e433d75..1fc52d02b56 100644 --- a/server/src/services/tool-review-work-notifications.ts +++ b/server/src/services/tool-review-work-notifications.ts @@ -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(db: Db, companyId: string, write: (transaction: Db) => Promise): Promise { diff --git a/server/src/services/tool-runtime-metrics.ts b/server/src/services/tool-runtime-metrics.ts index 3d877f0dee9..c984ba24915 100644 --- a/server/src/services/tool-runtime-metrics.ts +++ b/server/src/services/tool-runtime-metrics.ts @@ -11,7 +11,7 @@ function minuteBucket(date: Date): Date { } export async function incrementToolRuntimeMetricCounter( - db: Db, + db: Pick, 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, companyId: string) { try { await incrementToolRuntimeMetricCounter(db, { companyId,