mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 11:20:42 +01:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Users can stop a run and send another message on the same task.
> - Remote runners need evidence from their sandbox provider that
execution stopped.
> - Local process checks cannot prove that a remote process exited.
> - This pull request records provider stop receipts and uses them for
conversation admission.
> - New user messages can proceed after confirmed cleanup without
repeating interrupted actions.
## Linked Issues or Issue Description
Refs #13237 and #13239. Related: #13163 covers app-restart recovery;
this change covers an explicit stop followed by a new user message.
**What happened?**
A stopped remote Claude ACP task kept its execution hold after Daytona
cleanup succeeded. Native runners also rejected remote process
identities and retained stale session cleanup gates. A message sent
during cleanup could stay deferred after the sandbox stopped.
**Expected behavior**
After the provider confirms that the old execution stopped, a new user
message starts a fresh turn. Pending user messages must not need another
message to trigger admission. Prior action outcomes remain recorded.
**Steps to reproduce**
1. Start a long-running task in Daytona with a legacy Claude ACP or
native ACP runner.
2. Cancel the run while its tool is active.
3. Send a new message immediately, or after cleanup completes.
4. Observe the execution hold despite the old sandbox having stopped.
**Paperclip version or commit**
Reproduced on master at 7b829efdf6. The
branch is rebased on current master.
## What Changed
- Add optional provider stop receipts to sandbox release and destroy
hooks. Old plugins remain compatible.
- Bind receipts to the company, run, lease, and provider resource.
Failed cleanup cannot supply stop authority.
- Acknowledge legacy remote cancellation after confirmed termination.
- Admit native user continuations using remote receipts instead of host
process checks.
- Retire only the settled cleanup owner matching the stopped company,
run, and provider resource. Isolate cleanup gates between remote
sandboxes, including two sandboxes owned by one run.
- Reconsider user messages deferred during remote cleanup through normal
admission, including successful later cleanup retries.
- Preserve receipts through cleanup retries and inline cleanup after
failed startup.
- Permit provider destruction after a terminal remote checkpoint
failure. Busy ownership still blocks destruction.
- Add regression tests and update execution semantics.
- Give the real preview fixture ten seconds for cold startup. Reuse
release-mode Rust artifacts for the filtered parity checks, avoiding a
duplicate debug test build that exhausted CI disk twice; test filters
and assertions are unchanged.
## Verification
- Recursive typecheck and build passed.
- Targeted server, Daytona plugin, and native runtime tests passed. They
cover missing or mismatched receipts, failed cleanup, local process
protection, exact cleanup ownership, and a message sent during cleanup.
- Live Daytona tests passed for legacy Claude ACP, native per-turn,
native warm, and a newly created native runner using disposable
sandboxes. Each original run was cancelled; its explicit follow-up
completed with no execution hold. The disposable case confirmed a new
sandbox after deletion.
- Native tests used the provider's Opus 5 selector, `opus[1m]`, and the
Linux runner bundle from the sandbox image. Existing checkpoint/sync
finalization warnings remained visible before the native follow-ups
reached committed success. This change does not repair those separate
warnings or guarantee recovery of uncheckpointed files.
- A direct live provider test also passed with the final delete-wait
change: the destroy hook returned its receipt only after Daytona
reported the sandbox destroyed. The final Daytona plugin suite passed
all 153 tests; its build passed.
- After rebasing on master, 284 targeted server/plugin tests and 313
native executor tests passed. The native session runtime suite passed
all 128 tests.
- The review regression passed all 145 tests across the continuation,
environment runtime, and pending-cleanup sweep suites. Recursive
typecheck and build passed again after that fix.
- The security review's exact-resource finding is fixed. Cleanup
completion requires the same provider-resource scope used during session
creation. All 128 native runtime tests, 61 server continuation/cleanup
tests, recursive typecheck, and build passed after this fix. The
two-sandboxes-in-one-run regression proves one receipt cannot retire the
other quarantine.
- Greptile reviewed the final commit at 5/5, the security scan passed,
and no review threads remain unresolved. The final native executor suite
also passed all 313 tests.
- The full local suite reached 10,616 passing tests before stopping on
four failures. All four now pass in focused reruns: the final-code
continuation/teardown suites, the attachment suite, and the native
session test after building its required fake-provider binary. The
original full run overlapped source edits and did not reach the
remaining groups; it is not counted as a full-suite pass.
- The preview exposure suite passed all 25 applicable tests (three
Linux-only tests skipped locally) after the startup allowance change.
Both release-mode Rust parity commands passed locally. All final-head PR
checks passed, including full server/workspace/browser test groups, full
native runner verification, build, typecheck, canary dry-run, and the
aggregate gates.
## Risks
- A provider must return a receipt only after confirmed termination.
Incorrect provider claims could permit overlapping execution.
- Older providers without receipts retain the existing hold. Missing
evidence, failed cleanup, active ownership, pauses, approvals, and
budgets still block admission.
- This change preserves unknown action outcomes and old checkpoints. It
does not authorize replay or alter historical runs.
- No database migration or telemetry contract change is required.
## Model Used
OpenAI GPT-6 through Codex, with repository inspection, code execution,
and browser tools. The exact backend revision and context-window size
are not exposed in this session.
## 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 references)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run targeted 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>
149 lines
8.7 KiB
TypeScript
149 lines
8.7 KiB
TypeScript
import { completeTerminatedRemoteNativeSessionCleanup } from "../vendor/paperclip-runner/index.js";
|
|
import { hasRemoteTerminationReceipt, remoteLeaseCleanupScope } from "./remote-execution-termination.js";
|
|
import { z } from "zod";
|
|
import { and, eq, inArray, isNull, ne, or, sql } from "drizzle-orm";
|
|
import {
|
|
approvals, issueApprovals, issueThreadInteractions,
|
|
environmentLeases, heartbeatRuns, issueComments, issueRecoveryActions,
|
|
issues, nativeRunFinalizations, type Db,
|
|
} from "@paperclipai/db";
|
|
import { executionBlockerPredicate, getExecutionBlocker } from "./execution-blocker.js";
|
|
import { buildExecutionContinuation } from "./execution-continuation.js";
|
|
import { adapterExecutionControls } from "./adapter-execution-control.js";
|
|
import { persistActivity } from "./activity-log.js";
|
|
|
|
type Run = typeof heartbeatRuns.$inferSelect;
|
|
const terminal = ["failed", "interrupted", "timed_out", "cancelled"];
|
|
|
|
function processStopped(pid: number): boolean {
|
|
try { process.kill(pid, 0); return false; }
|
|
catch (error) { return (error as NodeJS.ErrnoException).code === "ESRCH"; }
|
|
}
|
|
|
|
/** Called under the issue lock, in the transaction that creates the new turn.
|
|
* A user request authorizes a new conversation, not replay of the failed run.
|
|
* Unknown action outcomes and all prior records stay intact.
|
|
*/
|
|
export async function admitExplicitNativeContinuation(input: {
|
|
db: Db; companyId: string; issueId: string; agentId: string;
|
|
actorType: string | null | undefined; actorId: string | null | undefined;
|
|
reason: string | null; commentId: string | null; successorRunId: string;
|
|
dryRun?: boolean;
|
|
}): Promise<{ previousRunId: string; commentId: string } | null> {
|
|
const { db, companyId, issueId, agentId, actorId, commentId } = input;
|
|
if (input.actorType !== "user" || !actorId || !commentId ||
|
|
!["issue_commented", "issue_reopened_via_comment"].includes(input.reason ?? "")) return null;
|
|
if (!z.string().guid().safeParse(commentId).success) return null;
|
|
const [task] = await db.select().from(issues).where(and(
|
|
eq(issues.companyId, companyId), eq(issues.id, issueId),
|
|
));
|
|
if (!task || task.assigneeAgentId !== agentId || ["done", "cancelled"].includes(task.status)) return null;
|
|
const [comment] = await db.select().from(issueComments).where(and(
|
|
eq(issueComments.companyId, companyId), eq(issueComments.issueId, issueId),
|
|
eq(issueComments.id, commentId), eq(issueComments.authorType, "user"),
|
|
eq(issueComments.authorUserId, actorId), isNull(issueComments.createdByRunId),
|
|
isNull(issueComments.deletedAt),
|
|
));
|
|
if (!comment?.body.trim()) return null;
|
|
const actions = await db.select().from(issueRecoveryActions).where(and(
|
|
eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, issueId),
|
|
executionBlockerPredicate(),
|
|
)).for("update");
|
|
if (!actions.length) return null;
|
|
const blocker = await getExecutionBlocker(db, companyId, issueId);
|
|
if (blocker && blocker.recoveryActionId === null) return null;
|
|
const [pendingInteraction] = await db.select({ id: issueThreadInteractions.id }).from(issueThreadInteractions).where(and(
|
|
eq(issueThreadInteractions.companyId, companyId), eq(issueThreadInteractions.issueId, issueId),
|
|
eq(issueThreadInteractions.status, "pending"),
|
|
)).limit(1);
|
|
const [pendingApproval] = await db.select({ id: approvals.id }).from(issueApprovals).innerJoin(approvals, and(
|
|
eq(approvals.id, issueApprovals.approvalId), eq(approvals.companyId, companyId),
|
|
)).where(and(eq(issueApprovals.companyId, companyId), eq(issueApprovals.issueId, issueId),
|
|
inArray(approvals.status, ["pending", "revision_requested"]))).limit(1);
|
|
if (pendingInteraction || pendingApproval) return null;
|
|
|
|
const sources: Run[] = [];
|
|
for (const action of actions) {
|
|
const runId = action.evidence.runId ?? action.evidence.sourceRunId;
|
|
if (typeof runId !== "string") return null;
|
|
// Text comparison keeps malformed historical evidence a hold, not a UUID cast error.
|
|
const [run] = await db.select().from(heartbeatRuns).where(and(
|
|
eq(heartbeatRuns.companyId, companyId), sql`${heartbeatRuns.id}::text = ${runId}`,
|
|
));
|
|
if (!run || run.agentId !== agentId || !terminal.includes(run.status) ||
|
|
(run.nativeIssueId ?? run.contextSnapshot?.issueId) !== issueId ||
|
|
!run.finishedAt || comment.createdAt <= run.finishedAt) return null;
|
|
if (adapterExecutionControls.has(run.id)) return null;
|
|
const unusedAdmission = run.status === "cancelled" && !run.startedAt &&
|
|
run.errorCode === "execution_reconciliation_required" &&
|
|
!run.processPid && !run.processGroupId && !run.nativeSessionId;
|
|
if (run.runtimeMode !== "native" && !unusedAdmission) return null;
|
|
const [coordinator] = await db.select().from(nativeRunFinalizations).where(and(
|
|
eq(nativeRunFinalizations.companyId, companyId), eq(nativeRunFinalizations.runId, run.id),
|
|
)).for("update");
|
|
if (coordinator && (coordinator.phase !== "terminal_failure" || coordinator.leaseOwner ||
|
|
coordinator.resultId || coordinator.failureDetail?.successorRunId)) return null;
|
|
const leases = await db.select()
|
|
.from(environmentLeases).where(and(
|
|
eq(environmentLeases.companyId, companyId), eq(environmentLeases.heartbeatRunId, run.id),
|
|
));
|
|
const remote = leases.some(lease => lease.provider !== "local");
|
|
if (remote) {
|
|
// Never interpret remote PIDs using the control-plane host's process table.
|
|
if (!leases.every(hasRemoteTerminationReceipt)) return null;
|
|
if (!input.dryRun && !leases.every(lease => completeTerminatedRemoteNativeSessionCleanup({
|
|
companyId, runId: run.id, remoteCleanupScope: remoteLeaseCleanupScope(lease)!,
|
|
}))) return null;
|
|
} else {
|
|
if (leases.some(lease => !lease.releasedAt || lease.cleanupStatus === "failed")) return null;
|
|
if (!unusedAdmission) {
|
|
// A missing process identity is not evidence that a provider exited.
|
|
if (!run.processPid && !run.processGroupId) return null;
|
|
if (run.processPid && !processStopped(run.processPid)) return null;
|
|
if (run.processGroupId && !processStopped(-run.processGroupId)) return null;
|
|
}
|
|
}
|
|
sources.push(run);
|
|
}
|
|
const nativeSources = sources.filter(run => run.runtimeMode === "native");
|
|
if (!nativeSources.length) return null;
|
|
const [active] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and(
|
|
eq(heartbeatRuns.companyId, companyId),
|
|
or(eq(heartbeatRuns.nativeIssueId, issueId), sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueId}`),
|
|
inArray(heartbeatRuns.status, ["running", "queued", "scheduled_retry"]),
|
|
ne(heartbeatRuns.id, input.successorRunId),
|
|
)).limit(1);
|
|
if (active) return null;
|
|
const previous = nativeSources.sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime())[0]!;
|
|
// Prove required task history is available before retiring any hold.
|
|
await buildExecutionContinuation({ db, companyId, issueId, agentId,
|
|
context: { previousRunId: previous.id, wakeCommentId: commentId },
|
|
summary: null, exposeLowTrustRaw: false });
|
|
if (input.dryRun) return { previousRunId: previous.id, commentId };
|
|
const authorization = { actorId, commentId, runId: input.successorRunId,
|
|
previousRunId: previous.id, recordedAt: new Date().toISOString() };
|
|
await db.update(nativeRunFinalizations).set({
|
|
failureDetail: sql`coalesce(${nativeRunFinalizations.failureDetail}, '{}'::jsonb) || ${JSON.stringify({ replacementDenied: "explicit_user_continuation" })}::jsonb`,
|
|
updatedAt: new Date(),
|
|
}).where(and(eq(nativeRunFinalizations.companyId, companyId), inArray(nativeRunFinalizations.runId, nativeSources.map(run => run.id))));
|
|
for (const action of actions) {
|
|
await db.update(issueRecoveryActions).set({
|
|
status: "resolved", outcome: "cancelled", resolvedAt: new Date(), updatedAt: new Date(),
|
|
nextAction: "A new user message starts a fresh conversation turn. Prior action outcomes remain recorded.",
|
|
resolutionNote: "The user continued after the prior execution stopped. No action outcomes were inferred.",
|
|
wakePolicy: null, monitorPolicy: null,
|
|
evidence: { ...action.evidence, explicitUserContinuation: authorization,
|
|
...(action.evidence.automaticRecovery ? { automaticRecovery: {
|
|
...(action.evidence.automaticRecovery as Record<string, unknown>), replay: "explicit_user_continuation",
|
|
} } : {}),
|
|
},
|
|
}).where(eq(issueRecoveryActions.id, action.id));
|
|
}
|
|
await persistActivity(db, { companyId, actorType: "user", actorId,
|
|
action: "issue.execution_recovery_settled", entityType: "issue", entityId: issueId,
|
|
details: { continuation: "explicit_user_message", ...authorization,
|
|
recoveryActionIds: actions.map(action => action.id), previousRunIds: sources.map(run => run.id) },
|
|
});
|
|
return { previousRunId: previous.id, commentId };
|
|
}
|