mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(connections): retire requests for already available tools (#15856)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connection cards let users provide the access an agent needs. > - Users can complete setup in Apps while an earlier card stays pending. > - A stale card can then cancel recovery despite valid existing access. > - This change verifies existing access and expires the obsolete request. > - It records a system outcome and sends one durable continuation. > - Approval, identity, permission, and workspace recovery gates remain in place. ## Linked Issues or Issue Description **What happened?** An ordinary tool connection request stayed pending after the addressed user connected the service in Apps. A later conversation retry was cancelled with `issue_waiting_for_response`. **Expected behavior** A request expires when the same user and requesting agent already have usable access. The agent gets a durable tool refresh. The system records no new human approval. **Steps to reproduce** 1. Have an agent request a tool connection on an active task. 2. Connect the service through Apps with the addressed user's existing grant, agent install, and allowed tools. 3. Let a conversation run stop and retry. The pending card still blocks the retry on the base branch. Related: #12345 introduced the connection request flow. This fix uses its existing access checks and delivery queue. ## What Changed - Verify company, task ownership, user identity, membership, health, grants, installs, and task tool policy before expiring an ordinary request. - Preserve AI account, channel, additional permission, and upstream provider requests on their existing paths. - Save the system outcome, audit event, and continuation in one transaction. Notify an idle delivery worker when a request commits. - Recheck access and other questions or approvals before delivery, retry, final dispatch, and native projection. Refresh system observations for an authorized replacement account. - Rearm delivery when access changes before provider work starts. Verify the host's admission receipt against execution evidence before allowing retry without a repair incident. - Require the current delivery claim when marking an attempt complete. An old completion cannot overwrite a rearmed continuation. - Add embedded-Postgres regressions for permissions, policy, concurrency, restart, replacement accounts, both dispatch gates, and the delivery race. Keep genuine workspace repair holds blocking. ## Verification - Latest-head connection, wake-context, dispatch, and legacy recovery suites: 124 tests passed. The connection suite includes 44 real-Postgres cases. - Server typecheck passed. Full `pnpm -r typecheck` and `pnpm build` passed on the latest head. - `pnpm test:run` was run locally. Its general-server group passed 18,583 tests and failed 20. That long run started before the review fixes, then loaded new test cases against modules transformed before those fixes. The affected latest-head suites pass in a fresh process. - One full-run failure also reproduces on unchanged base commitde9ab8e7e0: native-session-resume's damaged-prior-epoch replacement case. It is outside this change. Full-run groups after general-server did not run because that command stopped on failure. - Apex reviewed commit96b96297f4at 5/5. All review threads are resolved. All latest-head CI gates passed. The branch has no merge conflicts. ## Risks A false readiness result could retire a request too early. The fix reuses the runtime access selection rules, makes no new grants, and rechecks access at delivery. Ordinary expired requests do not authorize continuation. A failed workspace restore still requires repair through the existing gate. No schema migration is needed. ## Model Used OpenAI GPT-6 through Codex, with repository inspection, code editing, shell execution, and test tools. The session identifies the model family as GPT-6. It does not expose a more specific runtime model ID or context window size. ## 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 (focused suites; full local limitations are listed above) - [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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e733bddc45
commit
5847f047c7
12 files changed
+703
-34
No files matched your search
@@ -6,9 +6,7 @@ import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
agentWakeupRequests,
|
||||
agents,
|
||||
approvals,
|
||||
issueApprovals,
|
||||
issueThreadInteractions,
|
||||
connectionIntentDeliveries,
|
||||
heartbeatRuns,
|
||||
issueRecoveryActions,
|
||||
issueComments,
|
||||
@@ -22,6 +20,8 @@ import { evaluateAgentInvokabilityFromDb } from "../../../services/agent-invokab
|
||||
import { budgetService, budgetServiceInTransaction } from "../../../services/budgets.js";
|
||||
import { isCompletedOnboardingHandoffWake } from "../../../services/chat-completion-delivery.js";
|
||||
import { isHeartbeatWakeOnDemandEnabled } from "../../../services/heartbeat-policy.js";
|
||||
import { DELIVERY_QUEUES, notifyDeliveryWork } from "../../../services/delivery-work-notifications.js";
|
||||
import { connectionContinuationPendingResponse } from "../../../services/satisfied-connection-intents.js";
|
||||
import { collectDispositionRepairSourceState } from "../../../services/recovery/disposition-repair.js";
|
||||
import { legacyDispositionEpisode, legacyDispositionFingerprint } from "../../../services/recovery/legacy-continuation.js";
|
||||
import { appendHeartbeatRunEvent } from "../../../services/heartbeat-run-events.js";
|
||||
@@ -72,6 +72,7 @@ import { RunDispatchApplicationError } from "../application/types.js";
|
||||
type HeartbeatRun = typeof heartbeatRuns.$inferSelect;
|
||||
type LoadGateFactsInput = {
|
||||
conversationContinuation: boolean;
|
||||
responsibleUserId: string | null;
|
||||
runId: string;
|
||||
companyId: string;
|
||||
agentId: string;
|
||||
@@ -84,6 +85,8 @@ type LoadGateFactsResult =
|
||||
| { agentFound: true; facts: ScheduledRetryFacts }
|
||||
| { agentFound: false; issueId: string | null };
|
||||
type LoadStalenessFactsInput = {
|
||||
responsibleUserId: string | null;
|
||||
conversationContinuation: boolean;
|
||||
runId: string;
|
||||
companyId: string;
|
||||
agentId: string;
|
||||
@@ -337,6 +340,7 @@ export function createPostgresRunDispatchAdapter(
|
||||
.select({
|
||||
id: issues.id,
|
||||
companyId: issues.companyId,
|
||||
projectId: issues.projectId,
|
||||
status: issues.status,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
assigneeUserId: issues.assigneeUserId,
|
||||
@@ -364,19 +368,8 @@ export function createPostgresRunDispatchAdapter(
|
||||
facts.issueExecutionRunId = issue.executionRunId;
|
||||
facts.issueCheckoutRunId = issue.checkoutRunId;
|
||||
if (input.conversationContinuation) {
|
||||
const [interactions, linkedApprovals] = await Promise.all([
|
||||
dbOrTx.select({ id: issueThreadInteractions.id }).from(issueThreadInteractions).where(and(
|
||||
eq(issueThreadInteractions.companyId, input.companyId),
|
||||
eq(issueThreadInteractions.issueId, issueId), eq(issueThreadInteractions.status, "pending"),
|
||||
)).limit(1),
|
||||
dbOrTx.select({ id: approvals.id }).from(issueApprovals).innerJoin(approvals, and(
|
||||
eq(approvals.id, issueApprovals.approvalId), eq(approvals.companyId, issueApprovals.companyId),
|
||||
)).where(and(
|
||||
eq(issueApprovals.companyId, input.companyId), eq(issueApprovals.issueId, issueId),
|
||||
inArray(approvals.status, ["pending", "revision_requested"]),
|
||||
)).limit(1),
|
||||
]);
|
||||
facts.pendingResponse = interactions.length > 0 ? "interaction" : linkedApprovals.length > 0 ? "approval" : null;
|
||||
facts.pendingResponse = await connectionContinuationPendingResponse(dbOrTx as unknown as Db,
|
||||
issue, input.responsibleUserId, input.runId);
|
||||
}
|
||||
facts.reviewParticipant = await readNativeReviewParticipantFacts(dbOrTx, {
|
||||
companyId: input.companyId, issueId, agentId: input.agentId,
|
||||
@@ -464,6 +457,7 @@ export function createPostgresRunDispatchAdapter(
|
||||
companyId: run.companyId,
|
||||
agentId: run.agentId,
|
||||
conversationContinuation: run.runtimeMode === "legacy" && hasConversationContinuationPolicy(run.resultJson),
|
||||
responsibleUserId: run.responsibleUserId,
|
||||
contextSnapshot: parseObject(run.contextSnapshot),
|
||||
scheduledRetryReason: run.scheduledRetryReason,
|
||||
retryReasonOverride: input.retryReasonOverride,
|
||||
@@ -519,6 +513,9 @@ export function createPostgresRunDispatchAdapter(
|
||||
const issueQuery = dbOrTx
|
||||
.select({
|
||||
id: issues.id,
|
||||
companyId: issues.companyId,
|
||||
projectId: issues.projectId,
|
||||
assigneeUserId: issues.assigneeUserId,
|
||||
status: issues.status,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
executionRunId: issues.executionRunId,
|
||||
@@ -600,6 +597,9 @@ export function createPostgresRunDispatchAdapter(
|
||||
unresolvedBlockerIssueIds: readiness.unresolvedBlockerIssueIds,
|
||||
unresolvedBlockerCount: readiness.unresolvedBlockerCount,
|
||||
} : null,
|
||||
pendingResponse: issue && (input.conversationContinuation || context.connectionIntentResolution === "existing_connection")
|
||||
? await connectionContinuationPendingResponse(dbOrTx as unknown as Db, issue, input.responsibleUserId, input.runId)
|
||||
: null,
|
||||
issueFound: issue !== null,
|
||||
issueStatus: issue?.status ?? null,
|
||||
issueAssigneeAgentId: issue?.assigneeAgentId ?? null,
|
||||
@@ -770,6 +770,7 @@ export function createPostgresRunDispatchAdapter(
|
||||
companyId: run.companyId,
|
||||
agentId: run.agentId,
|
||||
conversationContinuation: run.runtimeMode === "legacy" && hasConversationContinuationPolicy(run.resultJson),
|
||||
responsibleUserId: run.responsibleUserId,
|
||||
contextSnapshot: parseObject(run.contextSnapshot),
|
||||
scheduledRetryReason: run.scheduledRetryReason,
|
||||
retryReasonOverride: run.scheduledRetryReason,
|
||||
@@ -881,6 +882,7 @@ export function createPostgresRunDispatchAdapter(
|
||||
decision: Extract<ReturnType<typeof decideQueuedRunStaleness>, { stale: true }>,
|
||||
expectedStatus: "queued" | "running",
|
||||
now: Date,
|
||||
providerDispatchNotStarted = false,
|
||||
): Promise<CancelStaleQueuedRunOutcome> {
|
||||
const [row] = await tx
|
||||
.update(heartbeatRuns)
|
||||
@@ -892,6 +894,9 @@ export function createPostgresRunDispatchAdapter(
|
||||
resultJson: {
|
||||
...parseObject(run.resultJson),
|
||||
stopReason: decision.errorCode,
|
||||
...(providerDispatchNotStarted && decision.errorCode === "issue_waiting_for_response"
|
||||
? { preDispatchResponseWait: { version: 1, providerWorkStarted: false } }
|
||||
: {}),
|
||||
...(decision.errorCode === "execution_reconciliation_required"
|
||||
? { executionWait: decision.details }
|
||||
: {}),
|
||||
@@ -927,6 +932,20 @@ export function createPostgresRunDispatchAdapter(
|
||||
);
|
||||
}
|
||||
|
||||
const context = parseObject(row.contextSnapshot);
|
||||
if (decision.errorCode === "issue_waiting_for_response" && context.connectionIntentResolution === "existing_connection"
|
||||
&& context.source === "connection_intent.resolved" && context.interactionKind === "connection_intent"
|
||||
&& typeof context.interactionId === "string") {
|
||||
// The partial wake-key uniqueness fence permits another delivery after
|
||||
// this skipped receipt. Rearm atomically so restored access or a later
|
||||
// answer can resume the untouched request after a restart.
|
||||
await tx.update(connectionIntentDeliveries).set({ deliveredAt: null, nextAttemptAt: now }).where(and(
|
||||
eq(connectionIntentDeliveries.companyId, row.companyId),
|
||||
eq(connectionIntentDeliveries.interactionId, context.interactionId),
|
||||
));
|
||||
await notifyDeliveryWork(tx, DELIVERY_QUEUES.connection);
|
||||
}
|
||||
|
||||
await tx
|
||||
.update(issues)
|
||||
.set({
|
||||
@@ -976,6 +995,8 @@ export function createPostgresRunDispatchAdapter(
|
||||
runId: run.id,
|
||||
companyId: run.companyId,
|
||||
agentId: run.agentId,
|
||||
responsibleUserId: run.responsibleUserId,
|
||||
conversationContinuation: run.runtimeMode === "legacy" && hasConversationContinuationPolicy(run.resultJson),
|
||||
issueId,
|
||||
contextSnapshot,
|
||||
scheduledRetryReason: run.scheduledRetryReason,
|
||||
@@ -1129,6 +1150,7 @@ export function createPostgresRunDispatchAdapter(
|
||||
decision,
|
||||
input.expectedStatus,
|
||||
input.now,
|
||||
true, // This locked gate has not handed off to the provider callback.
|
||||
);
|
||||
return { dispatched: false as const, cancellation };
|
||||
}
|
||||
|
||||
@@ -108,6 +108,7 @@ export type ScheduledRetryFacts = {
|
||||
};
|
||||
|
||||
export type QueuedRunStalenessErrorCode =
|
||||
| "issue_waiting_for_response"
|
||||
| "execution_reconciliation_required"
|
||||
| "issue_dependencies_blocked"
|
||||
| "issue_not_found"
|
||||
@@ -129,6 +130,7 @@ export type StalenessDecision =
|
||||
};
|
||||
|
||||
export type QueuedRunFacts = {
|
||||
pendingResponse?: "interaction" | "approval" | null;
|
||||
/** Rechecked for automatic native replacements immediately before dispatch. */
|
||||
dependenciesBlocked?: DependencyBlockFacts | null;
|
||||
runId: string;
|
||||
@@ -546,6 +548,12 @@ export function decideQueuedRunStaleness(
|
||||
}
|
||||
}
|
||||
|
||||
if (facts.pendingResponse) {
|
||||
return { stale: true, errorCode: "issue_waiting_for_response",
|
||||
reason: "Cancelled because the continuation is still waiting for an authorized response",
|
||||
details: { issueId: facts.issueId, pendingResponse: facts.pendingResponse } };
|
||||
}
|
||||
|
||||
if (facts.continuationParkApplies && facts.continuationParksExecutor) {
|
||||
return {
|
||||
stale: true,
|
||||
|
||||
@@ -9,6 +9,13 @@ import {
|
||||
} from "./wake-context.js";
|
||||
|
||||
describe("wake context", () => {
|
||||
it("recognizes a retired available connection without accepting ordinary expiry", () => {
|
||||
const context = { interactionId: "connection", interactionStatus: "expired", interactionKind: "connection_intent",
|
||||
connectionIntentResolution: "existing_connection", mutation: "interaction", wakeReason: "issue_commented" };
|
||||
expect(isResolvedInteractionContinuationWakeContext(context)).toBe(true);
|
||||
expect(isResolvedInteractionContinuationWakeContext({ ...context, interactionKind: "request_confirmation" })).toBe(false);
|
||||
expect(isResolvedInteractionContinuationWakeContext({ ...context, connectionIntentResolution: undefined })).toBe(false);
|
||||
});
|
||||
it("requires the subscription-specific non-assignee receipt for a subscription wait", () => {
|
||||
expect(isNonAssigneeWorkspaceBusyRetry("ai_connection_busy", {
|
||||
aiConnectionBusyDeferredWhileAssignee: false,
|
||||
|
||||
@@ -95,7 +95,10 @@ export function isResolvedInteractionContinuationWakeContext(contextSnapshot: un
|
||||
const interactionId = readNonEmptyString(context.interactionId);
|
||||
const interactionStatus = readNonEmptyString(context.interactionStatus);
|
||||
if (!interactionId || !interactionStatus) return false;
|
||||
if (!RESOLVED_INTERACTION_CONTINUATION_STATUSES.has(interactionStatus)) return false;
|
||||
const retiredConnection = interactionStatus === "expired"
|
||||
&& context.interactionKind === "connection_intent"
|
||||
&& context.connectionIntentResolution === "existing_connection";
|
||||
if (!retiredConnection && !RESOLVED_INTERACTION_CONTINUATION_STATUSES.has(interactionStatus)) return false;
|
||||
|
||||
const mutation = readNonEmptyString(context.mutation);
|
||||
const wakeReason = readNonEmptyString(context.wakeReason);
|
||||
|
||||
Reference in new issue
Block a user