mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
fix(native): let a follow-up wait out the previous turn's warm release instead of failing busy (#20068)
## Thinking Path - With a warm lifecycle, one session serves consecutive turns on a task. A follow-up comment wakes the next run, which claims the warm session's instruction copy and reuses the session. - #20048 makes warm the default on remote sandboxes. Its merge condition includes "no restart or follow-up ever leaves a task needing Retry". - In the #20048 Daytona chaos wave, task t12's follow-up run was created 43 ms after the previous run settled. The previous run was still releasing its warm session, so `claimWarmNativeInstructionCopy` saw it busy and failed the run with `native_session_supervisor_busy`. The task then needed a Retry. - The release takes seconds and always finishes, so a follow-up should wait for it rather than fail. A session busy because another run is preparing or executing a turn is a genuine conflict and must still fail at once, as existing tests require. ## Linked Issues or Issue Description Refs #20048 **What happened?** Wave w-20261108T142906-s9501, t12 (claude, warm): run 5b839da4 succeeded at 14:46:36.224. The follow-up run f2bf2f05 was created at 36.267 and failed immediately with `adapter_failed` / `native_session_supervisor_busy`, from `claimWarmNativeInstructionCopy` called by `instruction-working-copy.prepare`. Its automatic retry also failed, and only a user Retry completed the task. **Steps to reproduce** 1. Run a warm native session turn to completion. 2. While that run is still releasing its warm session (post-turn instruction check, release, hand-back), admit the next run on the same session. 3. The next run's instruction-copy claim throws `native_session_supervisor_busy`. **Expected behavior** The follow-up waits for the previous run's release (bounded) and reuses the session. A session held by a run that is still preparing or executing fails admission at once, as before. ## What Changed - `server/src/services/native-runtime/native-session-executor.ts`: - When the provider turn ends, the executor records the session scope as settling (`settlingNativeSessionScopes`). The outer `finally` clears the mark once the run has handed every scope back. - `claimWarmNativeInstructionCopy` waits, polling every 25 ms for at most 30 s (`warmSessionReleaseWait`), while the warm entry is busy and its scope is settling. It then re-reads the entry and runs the existing checks. If the entry is still busy at the deadline, or busy for any other reason, it throws `native_session_supervisor_busy` as before. - No wait happens on the normal path (entry idle), so turn admission adds no latency. - `server/src/services/native-runtime/native-session-executor.test.ts`: two cases. - A follow-up claimed while the previous turn's post-turn check is held: it waits, then succeeds once the release finishes. It fails on master with the wave's error. - The release outlasts the bound: it fails busy. **Previous-release state:** the change touches only in-memory admission state, so no saved run or session state changes. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-session-executor.test.ts src/services/heartbeat/instruction-working-copy.test.ts`: 1301/1301 pass. - With the wait removed (behaviour on master), the "release finishes" case fails with `native_session_supervisor_busy`. - The existing concurrency tests ("fences owned instruction preparation: concurrent", and the settled-release ownership tests) still fail admission at once. - Not yet covered: the Daytona warm wave rerun for #20048. ## Risks - **A wait that never ends.** The wait is bounded at 30 s and happens only while the owner is past its turn, which always reaches the outer `finally`. Every caller of the inner executor runs inside that `try`, and attachment staging keeps the same session scope. - **Hiding a real conflict.** Only a settling owner is waited for. A preparing or executing owner fails at once, as the existing tests assert. - **Scope.** I tried a similar wait at execution admission, but it conflicted with deliberate fences that reject a run during a settled release (the existing "releases settled codex only under its current owner" tests). It is not part of this PR. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`, Anthropic), 1M context window, tool use and code execution, via 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>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed
1 parent
c3c226d165
commit
d007afbf86
2 files changed
+244
-73
No files matched your search
@@ -738,6 +738,26 @@ export function readNativeSessionIdleWork(): IdleLocalWork {
|
||||
}
|
||||
registerIdleLocalWorkProbe(readNativeSessionIdleWork);
|
||||
|
||||
/**
|
||||
* How long a new run's admission waits for the warm session to stop being busy.
|
||||
* A follow-up comment can wake the next run within milliseconds of the previous
|
||||
* run settling, while that run's warm release is still finishing. Waiting out
|
||||
* the release lets the follow-up reuse the session instead of failing. A
|
||||
* session still busy at the deadline is a real concurrent owner and fails as
|
||||
* before.
|
||||
*/
|
||||
export const warmSessionReleaseWait = { timeoutMs: 30_000, pollMs: 25 };
|
||||
|
||||
async function waitForWarmNativeSessionSettled(scope: string): Promise<void> {
|
||||
const deadline = Date.now() + warmSessionReleaseWait.timeoutMs;
|
||||
// Wait while the owner's turn is over and it is releasing the session, until
|
||||
// it has handed every scope back (its execution's final cleanup), so the
|
||||
// follow-up's own admission does not meet the old owner's scopes either.
|
||||
while (settlingNativeSessionScopes.has(scope) && Date.now() < deadline) {
|
||||
await new Promise(resolve => setTimeout(resolve, warmSessionReleaseWait.pollMs));
|
||||
}
|
||||
}
|
||||
|
||||
/** Reserve the existing physical copy before heartbeat composes AGENT_HOME and
|
||||
* task guidance. The final execution/configuration fence still owns admission. */
|
||||
export async function claimWarmNativeInstructionCopy(input: {
|
||||
@@ -759,6 +779,9 @@ export async function claimWarmNativeInstructionCopy(input: {
|
||||
binding: { ...prior.binding, runId: input.runId, executionWorkspaceId: input.executionWorkspaceId }, workspace: input.workspace,
|
||||
})) || prior.workspace.cwd !== input.workspace.cwd) return null;
|
||||
const scope = nativeSessionScopeKey(prior);
|
||||
const busyEntry = warmNativeSessions.get(scope);
|
||||
if (busyEntry?.busy || settlingNativeSessionScopes.has(scope)) await waitForWarmNativeSessionSettled(scope);
|
||||
// Re-read after the wait: the release may have retired or replaced the session.
|
||||
const entry = warmNativeSessions.get(scope);
|
||||
if (!entry || !entry.instructionWorkingCopy?.runId || entry.environmentId !== input.environmentId) return null;
|
||||
if (entry.busy) throw new Error("native_session_supervisor_busy");
|
||||
@@ -1982,6 +2005,9 @@ const initializingSessionToolAuthorities = new Set<string>();
|
||||
const executingRunnerdSessionScopes = new Map<string, string>();
|
||||
const executingNativeOwnerScopes = new Map<string, symbol>();
|
||||
const executingNativeIngressAllocations = new Map<string, symbol>();
|
||||
/** Session scope -> the run releasing it after its turn (warm release and
|
||||
* hand-back), which a follow-up's instruction claim may wait for. */
|
||||
const settlingNativeSessionScopes = new Map<string, string>();
|
||||
|
||||
export function nativeSessionScopeDigestIsBusy(digest: string): boolean {
|
||||
const scopes = [
|
||||
@@ -9366,84 +9392,90 @@ export async function executePaperclipNativeSession(input: {
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
startup.providerPackSourceAbort.abort(new Error("native_execution_startup_completed"));
|
||||
for (const lease of diagnosticLeases) lease.retire();
|
||||
let startupUnsettled = false;
|
||||
if (startup.executionContext) {
|
||||
if (startup.executionContext.active) startup.executionContext.abort(new Error("native_execution_startup_completed"));
|
||||
startupUnsettled = !(await startup.executionContext.drain());
|
||||
startup.executionContext.finish();
|
||||
}
|
||||
// Join preparation cleanup before release; keep cancellation reachable
|
||||
// until the exact retained owner has finished the handover.
|
||||
await startup.executionLeaseRenewal?.stop().catch(() => undefined);
|
||||
if (startup.unstartedClaim && !nativeRunsDetachingForRestart.has(runId)) {
|
||||
await releaseUnstartedNativeExecutionClaim({ db: input.db, runId, companyId: input.execution.binding.companyId,
|
||||
issueId: input.execution.binding.issueId, ...startup.unstartedClaim }).catch(error => {
|
||||
logger.warn({ err: error, runId }, "Native preparation claim release failed; the control deadline will settle it");
|
||||
});
|
||||
}
|
||||
let stagedCleanupFailed = false;
|
||||
try {
|
||||
await cleanupStagedAttachments();
|
||||
} catch (cleanupError) {
|
||||
stagedCleanupFailed = true;
|
||||
// Cleanup is a confidentiality incident, but it occurs after the native
|
||||
// provider may already have completed the turn. Reclassifying that turn
|
||||
// as failed could replay provider side effects. Emit a private runtime
|
||||
// health event while preserving the provider result/error disposition.
|
||||
await input
|
||||
.onEvent?.({
|
||||
eventType: "native.attachment_staging_cleanup_failed",
|
||||
level: "error",
|
||||
message: "Native inbound attachment staging cleanup failed.",
|
||||
payload: {
|
||||
runId: input.execution.binding.runId,
|
||||
issueId: input.execution.binding.issueId,
|
||||
executionAlreadyFailed: executionFailure !== undefined,
|
||||
},
|
||||
})
|
||||
.catch(() => undefined);
|
||||
}
|
||||
releaseRetainedNativeSessionCleanup(input.db, {
|
||||
companyId: input.execution.binding.companyId, agentId: input.execution.binding.agentId,
|
||||
}, { companyId: input.execution.binding.companyId, runId: input.execution.binding.runId,
|
||||
issueId: input.execution.binding.issueId });
|
||||
const pendingRelease = ownsSessionScope && sessionScopeId
|
||||
&& executingRunnerdSessionScopes.get(sessionScopeId) === runId
|
||||
? warmNativeSessions.get(sessionScopeId) : undefined;
|
||||
try {
|
||||
if (pendingRelease?.runId === runId && pendingRelease.finishRelease) {
|
||||
if (executionFailure !== undefined || startupUnsettled || stagedCleanupFailed) {
|
||||
pendingRelease.closeOnReleaseReason = "warm native session final cleanup failed";
|
||||
startup.providerPackSourceAbort.abort(new Error("native_execution_startup_completed"));
|
||||
for (const lease of diagnosticLeases) lease.retire();
|
||||
let startupUnsettled = false;
|
||||
if (startup.executionContext) {
|
||||
if (startup.executionContext.active) startup.executionContext.abort(new Error("native_execution_startup_completed"));
|
||||
startupUnsettled = !(await startup.executionContext.drain());
|
||||
startup.executionContext.finish();
|
||||
}
|
||||
// Join preparation cleanup before release; keep cancellation reachable
|
||||
// until the exact retained owner has finished the handover.
|
||||
await startup.executionLeaseRenewal?.stop().catch(() => undefined);
|
||||
if (startup.unstartedClaim && !nativeRunsDetachingForRestart.has(runId)) {
|
||||
await releaseUnstartedNativeExecutionClaim({ db: input.db, runId, companyId: input.execution.binding.companyId,
|
||||
issueId: input.execution.binding.issueId, ...startup.unstartedClaim }).catch(error => {
|
||||
logger.warn({ err: error, runId }, "Native preparation claim release failed; the control deadline will settle it");
|
||||
});
|
||||
}
|
||||
let stagedCleanupFailed = false;
|
||||
try {
|
||||
await cleanupStagedAttachments();
|
||||
} catch (cleanupError) {
|
||||
stagedCleanupFailed = true;
|
||||
// Cleanup is a confidentiality incident, but it occurs after the native
|
||||
// provider may already have completed the turn. Reclassifying that turn
|
||||
// as failed could replay provider side effects. Emit a private runtime
|
||||
// health event while preserving the provider result/error disposition.
|
||||
await input
|
||||
.onEvent?.({
|
||||
eventType: "native.attachment_staging_cleanup_failed",
|
||||
level: "error",
|
||||
message: "Native inbound attachment staging cleanup failed.",
|
||||
payload: {
|
||||
runId: input.execution.binding.runId,
|
||||
issueId: input.execution.binding.issueId,
|
||||
executionAlreadyFailed: executionFailure !== undefined,
|
||||
},
|
||||
})
|
||||
.catch(() => undefined);
|
||||
}
|
||||
releaseRetainedNativeSessionCleanup(input.db, {
|
||||
companyId: input.execution.binding.companyId, agentId: input.execution.binding.agentId,
|
||||
}, { companyId: input.execution.binding.companyId, runId: input.execution.binding.runId,
|
||||
issueId: input.execution.binding.issueId });
|
||||
const pendingRelease = ownsSessionScope && sessionScopeId
|
||||
&& executingRunnerdSessionScopes.get(sessionScopeId) === runId
|
||||
? warmNativeSessions.get(sessionScopeId) : undefined;
|
||||
try {
|
||||
if (pendingRelease?.runId === runId && pendingRelease.finishRelease) {
|
||||
if (executionFailure !== undefined || startupUnsettled || stagedCleanupFailed) {
|
||||
pendingRelease.closeOnReleaseReason = "warm native session final cleanup failed";
|
||||
}
|
||||
await pendingRelease.finishRelease();
|
||||
}
|
||||
} finally {
|
||||
if (ownsPhysicalIngress && physicalIngressKey && executingNativeIngressAllocations.get(physicalIngressKey) === ownerToken) {
|
||||
executingNativeIngressAllocations.delete(physicalIngressKey);
|
||||
}
|
||||
if (executingNativeOwnerScopes.get(ownerScope) === ownerToken) {
|
||||
executingNativeOwnerScopes.delete(ownerScope);
|
||||
}
|
||||
if (
|
||||
ownsSessionScope &&
|
||||
sessionScopeId !== null &&
|
||||
executingRunnerdSessionScopes.get(sessionScopeId) ===
|
||||
input.execution.binding.runId
|
||||
) {
|
||||
executingRunnerdSessionScopes.delete(sessionScopeId);
|
||||
}
|
||||
startup.resolve(null);
|
||||
startup.settle();
|
||||
if (nativeSessionStartups.get(runId) === startup) {
|
||||
nativeSessionStartups.delete(runId);
|
||||
setNativeExecutionActive(runId, false);
|
||||
}
|
||||
await pendingRelease.finishRelease();
|
||||
}
|
||||
if (startupUnsettled) throw new NativeStartupCleanupPendingError(startup.executionContext!.id,
|
||||
startup.executionContext!.deadlineAtMs, startup.leaseOwner!, startup.executionContext!.pluginSupported,
|
||||
input.runnerExecutionTarget?.kind === "remote" ? input.runnerExecutionTarget.leaseId ?? undefined : undefined);
|
||||
} finally {
|
||||
if (ownsPhysicalIngress && physicalIngressKey && executingNativeIngressAllocations.get(physicalIngressKey) === ownerToken) {
|
||||
executingNativeIngressAllocations.delete(physicalIngressKey);
|
||||
}
|
||||
if (executingNativeOwnerScopes.get(ownerScope) === ownerToken) {
|
||||
executingNativeOwnerScopes.delete(ownerScope);
|
||||
}
|
||||
if (
|
||||
ownsSessionScope &&
|
||||
sessionScopeId !== null &&
|
||||
executingRunnerdSessionScopes.get(sessionScopeId) ===
|
||||
input.execution.binding.runId
|
||||
) {
|
||||
executingRunnerdSessionScopes.delete(sessionScopeId);
|
||||
}
|
||||
startup.resolve(null);
|
||||
startup.settle();
|
||||
if (nativeSessionStartups.get(runId) === startup) {
|
||||
nativeSessionStartups.delete(runId);
|
||||
setNativeExecutionActive(runId, false);
|
||||
}
|
||||
// Always hand the settling mark back, even if a cleanup step throws.
|
||||
const settledScope = nativeSessionScopeKey(input.execution);
|
||||
if (settlingNativeSessionScopes.get(settledScope) === runId) settlingNativeSessionScopes.delete(settledScope);
|
||||
}
|
||||
if (startupUnsettled) throw new NativeStartupCleanupPendingError(startup.executionContext!.id,
|
||||
startup.executionContext!.deadlineAtMs, startup.leaseOwner!, startup.executionContext!.pluginSupported,
|
||||
input.runnerExecutionTarget?.kind === "remote" ? input.runnerExecutionTarget.leaseId ?? undefined : undefined);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11362,6 +11394,9 @@ async function executePaperclipNativeSessionWithinScope(
|
||||
clearSteeringDeliveries(input.execution.binding.runId);
|
||||
clearNativeRuntimeRequestResolutions(input.execution.binding.runId);
|
||||
} catch (error) {
|
||||
// A failed turn releases its warm session here, before the finally below:
|
||||
// mark the scope settling first so a follow-up waits for that release too.
|
||||
settlingNativeSessionScopes.set(nativeSessionScopeKey(input.execution), input.execution.binding.runId);
|
||||
executionContext?.abort(error instanceof Error ? error : new Error("native_execution_startup_failed"));
|
||||
const startupUnsettled = executionContext ? !(await executionContext.drain()) : false;
|
||||
if (startupUnsettled) throw new NativeStartupCleanupPendingError(executionContext!.id, executionContext!.deadlineAtMs,
|
||||
@@ -12024,6 +12059,10 @@ async function executePaperclipNativeSessionWithinScope(
|
||||
if (protocolIntegrityFailure !== null) throw protocolIntegrityFailure;
|
||||
}
|
||||
} finally {
|
||||
// The provider turn is over, whether it succeeded or threw: what follows is
|
||||
// release and hand-back, which a follow-up's admission may wait for
|
||||
// (`warmSessionReleaseWait`). The outer execution's finally clears it.
|
||||
settlingNativeSessionScopes.set(nativeSessionScopeKey(input.execution), input.execution.binding.runId);
|
||||
releaseGitHubRun?.();
|
||||
if (githubAccess && (!warmSessionId || warmNativeSessions.get(warmSessionId)?.githubAccess !== githubAccess)) {
|
||||
await timeController("github_stop", () => githubAccess!.stop());
|
||||
|
||||
Reference in new issue
Block a user