mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 22:56:58 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Two runs must never write one workspace at once. A run that finds a workspace held (a remote workspace's writer run, or a running holder of a shared local tree) waits behind it > - The holder counted as live only while it produced output: after 60 minutes of silence it was "stale" > - For the shared local tree, that meant the waiter went ahead while the silent holder might still be working, which is a second writer. > - For a remote workspace, the waiter failed instead of waiting. > - Silence is not proof a run stopped. A long tool call can be silent and alive > - The durable ownership plan proves ownership by identity or by a held fence, never by age > - This pull request treats a holder whose controller still renews its lease (the legacy controller lease or the native coordinator lease) as live, however silent. The 60-minute rule applies only once that fence lapses > - The benefit is that a silent but live holder is never overtaken. A dead holder is still settled by the run reapers, which wake the waiters ## Linked Issues or Issue Description Refs #19831 (plan, Phase 2 sections A and C, and Phase 3 E), #19838 (inventory: "Remote workspace writer lane", and the "Released only by TTL/age" list: "Remote writer lane: 60 min"). **What happened?** `WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS` (60 min of output silence) decided whether a running holder still held a workspace. This applied in four places: - the remote reservation deferral; - the parked busy-workspace wait; - `findSharedWorkspaceHolder`, which stopped counting the holder at all; - `findExecutionWorkspaceHolder`, which flagged it stale. **Expected behavior** A running holder whose controller lease is held is live by fence. Only a holder with no held fence is judged by the silence cutoff, which keeps the visible failure instead of parking forever. The check is database-only (`runOwnerFenceHeldSql`): no process probe on the workspace-preparation path before a turn starts. ## What Changed - `remote-workspace-writer-hold.ts`: new `runOwnerFenceHeldSql`. It is true while `heartbeat_runs.controller_lease_expires_at > now()` or the run's native coordinator lease is held. `RemoteWorkspaceWriter.ownerFenceHeld` is reported for a running writer. - `heartbeat/workspace-preparation.ts`: the reservation keeps deferring behind a silent writer whose fence is held. - `run-dispatch/adapters/workspace-busy-wait.ts`: a parked wait behind a running holder with a held fence stays parked and re-checks at the parked cadence. - `heartbeat/retries.ts`: `findSharedWorkspaceHolder` counts a silent holder whose fence is held. `findExecutionWorkspaceHolder` flags a holder stale only when it is silent and its fence lapsed. ## Verification - `heartbeat/workspace-preparation.test.ts`: 138 pass. The extended "waits for the live writer of an inherited remote … at reservation" (both modes) fails on `master` and passes here: a silent writer with a held lease defers, and once the lease lapses it is the 409 as before. - `heartbeat-workspace-busy.test.ts`: 41 pass. The new "still defers behind a silent holder whose controller renews its lease" fails on `master` and passes here. The existing "does not defer when the only holder has been silent past the staleness threshold" (no lease) still passes. - The server package typecheck passes. ## Risks - A silent holder whose controller is alive now blocks waiters for as long as it holds its lease. That is the safe direction. A hung but live run is the active-run silence watchdog's to handle, and a dead controller's lease lapses within a minute. - The extra predicate is an indexed lookup of the run's own row and its coordinator row. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
87 lines
6.4 KiB
TypeScript
87 lines
6.4 KiB
TypeScript
import { and, eq, sql } from "drizzle-orm";
|
|
import { environmentLeases, heartbeatRuns, issueRecoveryActions, issues, nativeRunFinalizations, type Db } from "@paperclipai/db";
|
|
import type { RemoteWorkspaceAuthority } from "@paperclipai/shared";
|
|
import { hasRecoveredCleanupStopReceipt, hasRemoteTerminationReceipt } from "./remote-execution-termination.js";
|
|
import { canonicalExecutionUuid } from "./execution-identity.js";
|
|
import { REMOTE_WORKSPACE_PREPARATION_TIMEOUT_CODE } from "./remote-workspace-transport.js";
|
|
|
|
const object = (value: unknown): Record<string, unknown> =>
|
|
typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
|
|
|
export type RemoteWorkspaceWriter = {
|
|
runId: string; agentId: string; issueId: string; issueIdentifier: string | null; lastActivityAt: Date;
|
|
/** The writer run ended; its workspace save or sandbox release has not settled. */
|
|
ended?: boolean;
|
|
ownerFenceHeld?: boolean;
|
|
};
|
|
|
|
export function runOwnerFenceHeldSql() {
|
|
return sql<boolean>`(coalesce(${heartbeatRuns.controllerLeaseExpiresAt} > now(), false) or exists (
|
|
select 1 from ${nativeRunFinalizations} where ${nativeRunFinalizations.runId} = ${heartbeatRuns.id}
|
|
and ${nativeRunFinalizations.leaseOwner} is not null and ${nativeRunFinalizations.leaseExpiresAt} > now()))`;
|
|
}
|
|
|
|
/**
|
|
* Who still holds a remote workspace through its recorded writer run, other
|
|
* than `runId`. A running writer holds it. An ended writer keeps holding it
|
|
* until its save and sandbox release settle. `unattributed` is a hold whose
|
|
* writer cannot be named (no run or task); `free` means another run may
|
|
* become the writer. Workspace reservation and the parked workspace wait read
|
|
* this one rule, so a wait never ends before reservation would succeed.
|
|
*/
|
|
export async function readRemoteWorkspaceWriterHold(tx: Db, input: {
|
|
companyId: string; authority: RemoteWorkspaceAuthority; runId?: string | null;
|
|
}): Promise<{ kind: "free" } | { kind: "unattributed" } | { kind: "held"; writer: RemoteWorkspaceWriter }> {
|
|
const { authority, companyId } = input;
|
|
if (!authority.writerRunId || authority.writerRunId === input.runId) return { kind: "free" };
|
|
const [prior] = await tx.select({ status: heartbeatRuns.status, agentId: heartbeatRuns.agentId,
|
|
issueId: heartbeatRuns.issueId, nativeIssueId: heartbeatRuns.nativeIssueId,
|
|
runtimeMode: heartbeatRuns.runtimeMode, nativeSessionId: heartbeatRuns.nativeSessionId, runnerInstanceId: heartbeatRuns.runnerInstanceId,
|
|
errorCode: heartbeatRuns.errorCode, executionRecovery: sql<unknown>`${heartbeatRuns.resultJson}->'executionRecovery'`,
|
|
contextIssueId: sql<string | null>`coalesce(${heartbeatRuns.contextSnapshot}->>'issueId', ${heartbeatRuns.contextSnapshot}->>'taskId')`,
|
|
lastOutputAt: heartbeatRuns.lastOutputAt, startedAt: heartbeatRuns.startedAt, createdAt: heartbeatRuns.createdAt,
|
|
finishedAt: heartbeatRuns.finishedAt, ownerFenceHeld: runOwnerFenceHeldSql(),
|
|
}).from(heartbeatRuns).where(and(
|
|
eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.id, authority.writerRunId)));
|
|
const writerIssueId = prior ? prior.nativeIssueId ?? prior.issueId ?? prior.contextIssueId : null;
|
|
const [writerIssue] = writerIssueId ? await tx.select({ id: issues.id, identifier: issues.identifier }).from(issues)
|
|
.where(and(eq(issues.companyId, companyId), eq(issues.id, canonicalExecutionUuid(sql`${writerIssueId}::text`)))) : [];
|
|
if (prior?.status === "running" && writerIssue) return { kind: "held", writer: {
|
|
runId: authority.writerRunId, agentId: prior.agentId, issueId: writerIssue.id, issueIdentifier: writerIssue.identifier,
|
|
lastActivityAt: prior.lastOutputAt ?? prior.startedAt ?? prior.createdAt, ownerFenceHeld: prior.ownerFenceHeld === true,
|
|
} };
|
|
const leases = await tx.select().from(environmentLeases).where(and(eq(environmentLeases.companyId, companyId),
|
|
eq(environmentLeases.heartbeatRunId, authority.writerRunId)));
|
|
const bootstrap = object(prior?.executionRecovery);
|
|
const pristinePreparation = !authority.location && !authority.snapshotId && prior?.status === "failed"
|
|
&& prior.runtimeMode === "legacy" && prior.nativeSessionId === null && prior.runnerInstanceId === null
|
|
&& prior.errorCode === REMOTE_WORKSPACE_PREPARATION_TIMEOUT_CODE
|
|
&& bootstrap.kind === "bootstrap" && bootstrap.providerWorkStarted === false;
|
|
const unusedReservation = !authority.dirty && prior && ["failed", "cancelled", "interrupted", "timed_out", "succeeded"].includes(prior.status)
|
|
&& leases.every(lease => hasRemoteTerminationReceipt(lease) || hasRecoveredCleanupStopReceipt(lease) || (pristinePreparation
|
|
&& lease.releasedAt && lease.cleanupStatus === "success" && ["released", "expired", "failed"].includes(lease.status)));
|
|
if (unusedReservation) return { kind: "free" };
|
|
// An ended writer still holds the workspace until its save and sandbox
|
|
// release settle, within the same bounded cutoff as a live writer. While
|
|
// Paperclip is still saving the writer's retained files, that save's last
|
|
// progress is the writer's activity, so a waiter keeps waiting instead of
|
|
// failing setup. The save's own retry budget bounds it; a completed or
|
|
// board-handed-off save no longer extends the cutoff.
|
|
if (prior && writerIssue) {
|
|
const ended = new Date(prior.finishedAt ?? prior.lastOutputAt ?? prior.startedAt ?? prior.createdAt);
|
|
const [save] = await tx.select({ progressedAt: sql<string | null>`max(${issueRecoveryActions.updatedAt})` })
|
|
.from(issueRecoveryActions).where(and(
|
|
eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, writerIssue.id),
|
|
sql`${issueRecoveryActions.evidence}->>'runId' = ${authority.writerRunId}`,
|
|
sql`${issueRecoveryActions.evidence}->'workspaceRestoreRecovery'->>'schema' = 'paperclip.workspace-restore-recovery.v1'`,
|
|
sql`coalesce(${issueRecoveryActions.evidence}#>>'{systemRecovery,state}', 'scheduled') in ('scheduled', 'running', 'awaiting_evidence')`,
|
|
sql`${issueRecoveryActions.evidence}#>>'{workspaceRecovery,status}' is distinct from 'recovered'`));
|
|
const saving = save?.progressedAt ? new Date(save.progressedAt) : null;
|
|
return { kind: "held", writer: {
|
|
runId: authority.writerRunId, agentId: prior.agentId, issueId: writerIssue.id, issueIdentifier: writerIssue.identifier,
|
|
lastActivityAt: saving && saving.getTime() > ended.getTime() ? saving : ended, ended: true,
|
|
} };
|
|
}
|
|
return { kind: "unattributed" };
|
|
}
|