mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 12:04:41 +01:00
fix: avoid cancelling in-flight tool approvals
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
c1c3faf4d1
commit
79efb23338
2 files changed
+26
-3
No files matched your search
@@ -4148,7 +4148,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
schemaHash: "s1",
|
||||
}).returning();
|
||||
const canonicalArguments = canonicalToolArguments({ key: "alpha", value: "one" });
|
||||
const invocationValues = [1, 2, 3].map(() => ({
|
||||
const invocationValues = [1, 2, 3, 4].map(() => ({
|
||||
companyId: company.id,
|
||||
applicationId: application.id,
|
||||
connectionId: connection.id,
|
||||
@@ -4160,7 +4160,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
approvalState: "pending" as const,
|
||||
status: "awaiting_approval" as const,
|
||||
}));
|
||||
const [validInvocation, missingSignatureInvocation, oldSecretInvocation] =
|
||||
const [validInvocation, missingSignatureInvocation, oldSecretInvocation, inFlightInvocation] =
|
||||
await db.insert(toolInvocations).values(invocationValues).returning();
|
||||
const validSignedArguments = signToolArguments({
|
||||
invocationId: validInvocation.id,
|
||||
@@ -4174,7 +4174,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
canonicalArguments,
|
||||
signingSecret: "old-secret",
|
||||
});
|
||||
const [validRequest, missingSignatureRequest, oldSecretRequest] = await db.insert(toolActionRequests).values([
|
||||
const [validRequest, missingSignatureRequest, oldSecretRequest, inFlightRequest] = await db.insert(toolActionRequests).values([
|
||||
{
|
||||
companyId: company.id,
|
||||
invocationId: validInvocation.id,
|
||||
@@ -4190,6 +4190,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
canonicalArgumentsHash: "args-hash",
|
||||
canonicalArgumentsSummary: { summary: canonicalArguments, sha256: "args-hash", sizeBytes: canonicalArguments.length },
|
||||
signedArguments: null,
|
||||
createdAt: new Date(Date.now() - 60_000),
|
||||
},
|
||||
{
|
||||
companyId: company.id,
|
||||
@@ -4199,6 +4200,14 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
canonicalArgumentsSummary: { summary: canonicalArguments, sha256: "args-hash", sizeBytes: canonicalArguments.length },
|
||||
signedArguments: oldSecretSignedArguments,
|
||||
},
|
||||
{
|
||||
companyId: company.id,
|
||||
invocationId: inFlightInvocation.id,
|
||||
status: "pending",
|
||||
canonicalArgumentsHash: "args-hash",
|
||||
canonicalArgumentsSummary: { summary: canonicalArguments, sha256: "args-hash", sizeBytes: canonicalArguments.length },
|
||||
signedArguments: null,
|
||||
},
|
||||
]).returning();
|
||||
|
||||
const list = await toolAccessService(db).listActionRequests(company.id, "pending");
|
||||
@@ -4209,6 +4218,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
expect(statusById.get(validRequest.id)).toBe("pending");
|
||||
expect(statusById.get(missingSignatureRequest.id)).toBe("cancelled");
|
||||
expect(statusById.get(oldSecretRequest.id)).toBe("cancelled");
|
||||
expect(statusById.get(inFlightRequest.id)).toBe("pending");
|
||||
});
|
||||
|
||||
it("tracks new profile tools, reviews mixed allow/block decisions, and clears pending counts", async () => {
|
||||
|
||||
@@ -147,6 +147,7 @@ type OAuthProviderEndpoints = {
|
||||
};
|
||||
|
||||
const oauthRegistrationFlights = new Map<string, Promise<unknown>>();
|
||||
const PENDING_ACTION_REQUEST_SIGNING_GRACE_MS = 5_000;
|
||||
|
||||
async function oauthSingleFlight<T>(
|
||||
flights: Map<string, Promise<unknown>>,
|
||||
@@ -6749,10 +6750,19 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
const invocationById = new Map(invocations.map((invocation) => [invocation.id, invocation]));
|
||||
let visibleRequests = requests;
|
||||
if (status === "pending") {
|
||||
const hiddenInFlightRequestIds = new Set<string>();
|
||||
const signingGraceDeadline = now().getTime() - PENDING_ACTION_REQUEST_SIGNING_GRACE_MS;
|
||||
const invalidRequestIds = requests
|
||||
.filter((request) => {
|
||||
const invocation = invocationById.get(request.invocationId);
|
||||
if (!invocation) return true;
|
||||
if (!request.signedArguments && request.createdAt.getTime() > signingGraceDeadline) {
|
||||
// recordInvocation inserts the pending row before executeTestCall adds
|
||||
// its signed payload. A concurrent review-queue poll must not cancel
|
||||
// that short-lived intermediate state.
|
||||
hiddenInFlightRequestIds.add(request.id);
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
return !readSignedToolArgumentsPayload({
|
||||
signedArguments: request.signedArguments,
|
||||
@@ -6776,6 +6786,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
const invalidIds = new Set(invalidRequestIds);
|
||||
visibleRequests = requests.filter((request) => !invalidIds.has(request.id));
|
||||
}
|
||||
if (hiddenInFlightRequestIds.size > 0) {
|
||||
visibleRequests = visibleRequests.filter((request) => !hiddenInFlightRequestIds.has(request.id));
|
||||
}
|
||||
}
|
||||
if (visibleRequests.length === 0) return [];
|
||||
|
||||
|
||||
Reference in new issue
Block a user