mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path > - Paperclip manages AI agents and their execution. > - Recovery must keep old execution ownership visible before new work starts. > - Ownership reads filter large run contexts through a combined text expression. > - Recovery and status delivery also cast indexed UUID columns to text. > - These reads can scan history before finding the requested execution. > - This pull request preserves identity and ownership rules while using existing indexes. > - The benefit is less database work during recovery and task polling. ## Linked Issues or Issue Description Refs #14406. Related community report: #15275. This is an independent implementation. It does not use that pull request's code or commits. It covers the reported execution joins and the additional ownership query hotspot. **What happened?** Accumulated staging query statistics showed 11,782 ownership reads at 51 ms each. A paired recovery-action read averaged 22 ms. Both queried large run JSON. Text casts also prevented indexed UUID lookups in recovery and status delivery. **What did you expect to happen?** Recovery must find the same execution evidence through existing indexes. Malformed identifiers must match nothing without a cast error. Native issue bindings must retain precedence. Missing termination evidence must continue to block new work. **Steps to reproduce** Run the disposable benchmark below. It loads the original service modules from the baseline revision. It compares their exact ownership, blocker action and status-delivery SQL with the current modules on 1,400 synthetic runs, 350 historical recovery actions and 192 KiB contexts. ## What Changed - Convert saved text to UUID only after checking its canonical lowercase form. - Keep indexed run and task UUID columns unchanged in execution joins. - Split native and legacy issue predicates so existing indexes can filter ownership candidates. - Preserve company scope, native precedence, process and lease holds, recovery eligibility and delivery receipts. - Add real PostgreSQL identity and ownership regressions, a reproducible benchmark and database documentation. ## Verification - Local service SQL benchmark: ownership 78.30 ms to 1.14 ms, with the same two complete rows. Blocker action read 76.94 ms to 22.55 ms, with the same complete row. These are synthetic SQL timings, excluding process probes, provider calls and HTTP. - Status delivery returned the same 100 rows. Timing was neutral on the small-task fixture: 7.85 ms before and 8.30 ms after. No speedup is claimed for that case. - 231 local tests passed across six server suites. They cover malformed identifiers, canonical spelling, company scope, native precedence, retained processes, workspace restore, native replacement, conversations and delivery. - `pnpm -r typecheck` and `pnpm build` passed. - `git diff --check` passed. - All current-head CI and security checks passed, including the full general and serialized suites, browser shards, Runner verification and build. Greptile reviewed the exact commit at 5/5 with no actionable findings. - The older full local sweep in the companion worktree was stopped after final-commit complete CI passed. It is not claimed as a completed local pass. The 231 focused tests above passed locally on this branch. ```sh pnpm --filter @paperclipai/server exec vitest run src/services/conversation-continuation.test.ts src/services/legacy-execution-recovery.test.ts src/__tests__/legacy-workspace-restore-recovery.test.ts src/__tests__/chat-completion-delivery.test.ts src/__tests__/agent-conversations.test.ts src/services/native-runtime/native-safe-replacement.test.ts pnpm --filter @paperclipai/server exec tsx ../scripts/benchmark-recovery-queries.ts --baseline-ref=1e731520bb ``` ## Risks - PostgreSQL accepts more UUID spellings than the old text joins. The explicit guard preserves the old lowercase spelling requirement. - Query plans depend on the data distribution. Small task sets can show neutral status-delivery timing. The benchmark is not a production latency claim. - Recovery ownership checks remain conservative. Process, lease, workspace and no-replay rules are unchanged. - No migration, new index, cache, cleanup change or lock removal is included. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository inspection, shell tools and local code execution. The runtime does not expose a more specific backend 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 - [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>
167 lines
9.3 KiB
TypeScript
167 lines
9.3 KiB
TypeScript
import { hasRequiredWorkspaceRecovery } from "./workspace-restore-recovery-state.js";
|
|
import { and, desc, eq, inArray, isNotNull, isNull, or, sql } from "drizzle-orm";
|
|
import { environmentLeases, heartbeatRunEvents, heartbeatRuns, issueRecoveryActions, type Db } from "@paperclipai/db";
|
|
import { canonicalExecutionUuid } from "./execution-identity.js";
|
|
import { readProcessStartedAt } from "./hot-restart.js";
|
|
import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js";
|
|
|
|
// These adapters accept a conversation turn. Retrying a process or webhook can
|
|
// replay the action itself, so those adapters retain their recovery contract.
|
|
export const CONVERSATION_ADAPTER_TYPES = [
|
|
"claude_local", "codex_local", "cursor", "gemini_local", "opencode_local",
|
|
"pi_local", "grok_local", "kimi_local", "hermes_local",
|
|
] as const;
|
|
|
|
export function isConversationAdapter(adapterType: string): boolean {
|
|
return (CONVERSATION_ADAPTER_TYPES as readonly string[]).includes(adapterType);
|
|
}
|
|
|
|
export const CONVERSATION_CONTINUATION_POLICY = "continue_conversation_v1";
|
|
|
|
export function hasConversationContinuationPolicy(result: Record<string, unknown> | null | undefined): boolean {
|
|
return result?.workspaceRestoreFailure !== "restore_unsafe_archive" && !hasRequiredWorkspaceRecovery(result) && result?.conversationContinuation === CONVERSATION_CONTINUATION_POLICY;
|
|
}
|
|
|
|
/** Persisted by the server when it claims the run, before remote provisioning. */
|
|
export function claimedAdapterType(run: Pick<typeof heartbeatRuns.$inferSelect, "runnerProfileJson">): string | null {
|
|
const dispatch = run.runnerProfileJson?.adapterDispatch as Record<string, unknown> | undefined;
|
|
return typeof dispatch?.adapterType === "string" ? dispatch.adapterType : null;
|
|
}
|
|
|
|
function conversationRunPredicate() {
|
|
return or(
|
|
inArray(sql`${heartbeatRuns.runnerProfileJson}->'adapterDispatch'->>'adapterType'`, [...CONVERSATION_ADAPTER_TYPES]),
|
|
sql`${heartbeatRuns.resultJson}->>'conversationContinuation' = ${CONVERSATION_CONTINUATION_POLICY}`,
|
|
sql`exists (
|
|
select 1 from ${heartbeatRunEvents}
|
|
where ${heartbeatRunEvents.companyId} = ${heartbeatRuns.companyId}
|
|
and ${heartbeatRunEvents.runId} = ${heartbeatRuns.id}
|
|
and ${heartbeatRunEvents.eventType} = 'adapter.invoke'
|
|
and ${inArray(sql`${heartbeatRunEvents.payload}->>'adapterType'`, [...CONVERSATION_ADAPTER_TYPES])}
|
|
)`,
|
|
);
|
|
}
|
|
|
|
/** Recovery must not infer the old adapter from the agent's mutable settings. */
|
|
export async function historicalAdapterType(db: Db, run: typeof heartbeatRuns.$inferSelect): Promise<string | null> {
|
|
const selected = claimedAdapterType(run);
|
|
if (selected) return selected;
|
|
const [invocation] = await db.select({ payload: heartbeatRunEvents.payload }).from(heartbeatRunEvents)
|
|
.where(and(eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id),
|
|
eq(heartbeatRunEvents.eventType, "adapter.invoke")))
|
|
.orderBy(desc(heartbeatRunEvents.seq)).limit(1);
|
|
const adapterType = invocation?.payload?.adapterType;
|
|
return typeof adapterType === "string" ? adapterType : null;
|
|
}
|
|
|
|
export async function runUsedConversationAdapter(db: Db, run: typeof heartbeatRuns.$inferSelect): Promise<boolean> {
|
|
if (run.resultJson?.workspaceRestoreFailure === "restore_unsafe_archive" || hasRequiredWorkspaceRecovery(run.resultJson)) return false;
|
|
if (hasConversationContinuationPolicy(run.resultJson)) return true;
|
|
const adapterType = await historicalAdapterType(db, run);
|
|
return adapterType !== null && isConversationAdapter(adapterType);
|
|
}
|
|
|
|
/** Only immutable run evidence can retire a historical conversation hold.
|
|
* An agent's current adapter can differ from the one that executed this run.
|
|
* Missing evidence retains the hold; the current agent is never a fallback.
|
|
*/
|
|
export function conversationRecoveryActionPredicate() {
|
|
return and(
|
|
eq(issueRecoveryActions.cause, "legacy_execution_requires_reconciliation"),
|
|
sql`exists (
|
|
select 1 from ${heartbeatRuns}
|
|
where ${heartbeatRuns.companyId} = ${issueRecoveryActions.companyId}
|
|
and ${heartbeatRuns.id} = ${canonicalExecutionUuid(sql`${issueRecoveryActions.evidence}->>'runId'`)}
|
|
and ${or(
|
|
eq(heartbeatRuns.nativeIssueId, issueRecoveryActions.sourceIssueId),
|
|
and(isNull(heartbeatRuns.nativeIssueId), sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueRecoveryActions.sourceIssueId}::text`),
|
|
)}
|
|
and ${heartbeatRuns.runtimeMode} = 'legacy'
|
|
and coalesce(${heartbeatRuns.resultJson}->>'workspaceRestoreFailure', '') <> 'restore_unsafe_archive'
|
|
and coalesce(${heartbeatRuns.resultJson}->'workspaceRestoreRecovery'->>'schema', '') <> 'paperclip.workspace-restore-recovery.v1'
|
|
and ${inArray(heartbeatRuns.status, ['failed', 'timed_out', 'interrupted', 'cancelled'])}
|
|
and ${conversationRunPredicate()}
|
|
and ${or(
|
|
sql`${heartbeatRuns.resultJson}->>'conversationContinuation' = ${CONVERSATION_CONTINUATION_POLICY}`,
|
|
eq(heartbeatRuns.status, "interrupted"),
|
|
inArray(heartbeatRuns.errorCode, ["process_lost", "server_shutdown_interrupted", "execution_reconciliation_required"]),
|
|
and(eq(heartbeatRuns.status, "cancelled"), sql`${heartbeatRuns.resultJson}->'executionCancellation'->>'state' = 'acknowledged'`),
|
|
)}
|
|
)`,
|
|
);
|
|
}
|
|
|
|
/** OS liveness probes do not signal or stop the process. Unknown ownership holds. */
|
|
function processMayBeAlive(pid: number): boolean {
|
|
try {
|
|
process.kill(pid, 0);
|
|
return true;
|
|
} catch (error) {
|
|
return (error as NodeJS.ErrnoException).code !== "ESRCH";
|
|
}
|
|
}
|
|
|
|
/** A terminal conversation row does not prove that its execution authority ended.
|
|
* Other adapters keep their existing bootstrap and ownership protocols.
|
|
*/
|
|
export async function getConversationOwnershipBlocker(db: Db, companyId: string, issueId: string) {
|
|
const activeLease = sql`exists (select 1 from ${environmentLeases}
|
|
where ${environmentLeases.companyId} = "heartbeat_runs"."company_id"
|
|
and ${environmentLeases.heartbeatRunId} = "heartbeat_runs"."id"
|
|
and (${environmentLeases.releasedAt} is null
|
|
or ${environmentLeases.status} = 'pending_cleanup'
|
|
or ${environmentLeases.cleanupStatus} = 'failed'))`;
|
|
const candidates = await db.select({ run: heartbeatRuns, activeLease }).from(heartbeatRuns)
|
|
.where(and(
|
|
eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.runtimeMode, "legacy"),
|
|
conversationRunPredicate(),
|
|
or(
|
|
eq(heartbeatRuns.nativeIssueId, canonicalExecutionUuid(sql`${issueId}::text`)),
|
|
and(isNull(heartbeatRuns.nativeIssueId), sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueId}`),
|
|
),
|
|
inArray(heartbeatRuns.status, ["failed", "timed_out", "interrupted", "cancelled"]),
|
|
or(isNotNull(heartbeatRuns.processPid), isNotNull(heartbeatRuns.processGroupId), activeLease,
|
|
sql`${heartbeatRuns.resultJson}->'workspaceRestoreRecovery'->>'schema' = 'paperclip.workspace-restore-recovery.v1'`),
|
|
)).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id));
|
|
for (const { run, activeLease: leaseHeld } of candidates) {
|
|
if (hasRequiredWorkspaceRecovery(run.resultJson)) {
|
|
const leases = await db.select().from(environmentLeases).where(and(
|
|
eq(environmentLeases.companyId, companyId), eq(environmentLeases.heartbeatRunId, run.id),
|
|
));
|
|
const retainedIds = (run.resultJson!.workspaceRestoreRecovery as { leaseIds?: unknown }).leaseIds;
|
|
const remoteLeases = leases.filter(lease => lease.provider !== "local");
|
|
const remoteStopped = Array.isArray(retainedIds) && retainedIds.length > 0
|
|
&& retainedIds.every(id => typeof id === "string" && remoteLeases.some(lease =>
|
|
lease.id === id && hasRemoteTerminationReceipt(lease)))
|
|
&& remoteLeases.every(hasRemoteTerminationReceipt);
|
|
if (!remoteStopped) return {
|
|
runId: run.id, agentId: run.agentId, cause: "execution_owner_active",
|
|
nextAction: "The retained workspace environment has not confirmed that it stopped. Wait for cleanup before continuing this task.",
|
|
};
|
|
// These PIDs came from the sandbox. A matching process on this host is
|
|
// unrelated and must not hide the repair action or block a repaired task.
|
|
// A separate local lease keeps the existing host process checks below.
|
|
if (leases.every(lease => lease.provider !== "local")) continue;
|
|
}
|
|
let pidAlive = run.processPid !== null && processMayBeAlive(run.processPid);
|
|
if (pidAlive && run.processStartedAt) {
|
|
// A recycled PID cannot keep an old task blocked. An unreadable identity
|
|
// stays conservative; the original process may still own execution.
|
|
const observed = await readProcessStartedAt(run.processPid!).catch(() => null);
|
|
if (observed && new Date(observed).getTime() !== run.processStartedAt.getTime()) pidAlive = false;
|
|
}
|
|
const groupAlive = run.processGroupId !== null && processMayBeAlive(-run.processGroupId);
|
|
if (pidAlive || groupAlive || leaseHeld) {
|
|
return {
|
|
runId: run.id,
|
|
agentId: run.agentId,
|
|
cause: "execution_owner_active",
|
|
nextAction: pidAlive || groupAlive
|
|
? "The previous provider process is still running. Stop it before continuing this task."
|
|
: "The previous execution has not released its environment lease. Wait for cleanup before continuing this task.",
|
|
};
|
|
}
|
|
}
|
|
return null;
|
|
}
|