mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(recovery): settle a dead controller's result lease for a run a pause cancelled (#20113)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A budget hard stop pauses the agent, which cancels its running work and waits until every execution has provably stopped > - The shared ownership reconciler (#19933) settles an accepted result's executor lease when its controller is proven dead, so finalization does not wait out the 20-minute lease > - In the restart-chaos H1 wave, the pause cancelled a run 0.36 s after its result was accepted, and a hard kill then took the controller > - The settle only acted on runs that were still `running`, so the dead lease held finalization, the agent stayed in `pausing`, and every later task for it was stranded > - This pull request settles that lease for a cancelled run too, and records the budget pause with its reason > - The benefit is that a pause crossed by a restart completes at once and resume continues normally ## Linked Issues or Issue Description Refs #19933, #20101 **What happened?** Restart-chaos wave w-20261108T210118-s9802 (scenario H1). An agent-scope budget hard stop paused agent "Chaos claude". The pause cancelled run `dfc22298` at 21:02:27.818, 0.36 s after its result was accepted (finalization phase `workspace_finalizing`, executor lease until 21:21:55, controller PID 78157). A K4 SIGKILL followed. After the board's `raise_budget_and_resume`, the agent stayed `status=paused`, `lifecycle_state=pausing`, `lifecycle_holds=[]`, with its lifecycle operation at `attempts: 90`, `hostComplete: false` and no error. The pause host step (`stopInvocationsForAgents` → `agentExecutionsHaveStopped`) returns `pending` while any coordinator holds an owner lease with an accepted result. `settleNativeAcceptedResultLease` required `heartbeat_runs.status = 'running'`, so it returned `lease_no_longer_held` for the cancelled run. The pause also wrote no activity entry. **Expected behavior** A proven-dead controller's accepted-result lease is settled whatever stopped the run, so finalization finishes and the pause completes. The budget pause is logged with its reason. **Steps to reproduce** Pause an agent right after its run's result is accepted, then SIGKILL the server. After restart, the agent stays in `pausing` until the lease expires (up to 20 minutes). ## What Changed - `native-accepted-result-lease-settlement.ts`: settle the lease for a `running` or `cancelled` run. All other guards are unchanged: exact owner compare-and-set, owner proven gone by `proveOwnerGone`, remote copyback excluded, another host never proof. - `agent-lifecycle.ts` `applyAgentBudgetHoldInTransaction`: when the budget hold is added or removed, records `agent.paused` / `agent.resumed` with `reason: "budget"`, the lifecycle state and the holds, in the transaction that applies the hold. Applying the same hold again records nothing. - `budgets-service.test.ts`: its activity-log mock now also stubs `persistActivity`. Not changed, by design: resume still waits for an incomplete pause to finish before resuming. The pause's host step proves the agent's executions stopped before new work starts. In H1 it was blocked only by the dead lease, which is now settled at restart. ## Verification - `heartbeat-native-restart-suspension.test.ts`: new "finalizes an accepted result whose run a pause cancelled before a hard kill took its controller". It fails on master (lease still 20 minutes out) and passes with the fix (lease owner cleared, finalization past `workspace_finalizing`). Whole file: 41 passed. - New `agent-budget-hold-activity.test.ts`: the pause and release entries, with the reason, written once each. - `budgets-service.test.ts` (19), `budgets-history.test.ts`, `company-portability-routes.test.ts`, `agent-lifecycle/__tests__/lifecycle.test.ts`: passed. - `server` typecheck (`scripts/typecheck-layers.mjs`, low-memory): no errors. - Not run: the H1 chaos rerun (Phase 3 schedules it after merge). ## Risks - A cancelled run's accepted result is now finalized promptly after a hard kill instead of after the lease term. The finalizer already handles a run cancelled during finalization; the settle only shortens the wait. - New activity entries on budget pause and release. Run-log and activity only; no Telemetry. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, test execution). Extended reasoning enabled. ## 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:
1 parent
89265f7312
commit
77168d3367
6 files changed
+106
-10
No files matched your search
@@ -27,6 +27,7 @@ const mockLogActivity = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../services/activity-log.js", () => ({
|
||||
logActivity: mockLogActivity,
|
||||
persistActivity: vi.fn(async () => ({ activity: null, publication: null })),
|
||||
}));
|
||||
|
||||
// Budget behavior is exercised against PostgreSQL below and in
|
||||
|
||||
@@ -372,6 +372,22 @@ describeEmbeddedPostgres("heartbeat native recovery interrupted by graceful shut
|
||||
.where(eq(heartbeatRuns.nativeIssueId, f.issueId))).toHaveLength(1);
|
||||
}, 90_000);
|
||||
|
||||
// H1 bug A: a budget pause cancelled the run 0.36 s after its result was
|
||||
// accepted, then a hard kill took the controller. The dead controller's
|
||||
// result lease held finalization, and with it the agent's pause, for the
|
||||
// lease's 20 minutes.
|
||||
it("finalizes an accepted result whose run a pause cancelled before a hard kill took its controller", async () => {
|
||||
const f = await seedAcceptedResult("ACCEPTED-CANCELLED-THEN-SIGKILL", (runner) => `${runner}:${randomUUID()}`);
|
||||
await killedController(f, deadController(currentProcessSpaceHash()));
|
||||
await f.db.update(heartbeatRuns).set({ status: "cancelled", finishedAt: new Date(), errorCode: "cancelled" })
|
||||
.where(eq(heartbeatRuns.id, f.runId));
|
||||
const { owner } = await restart(f);
|
||||
// The agent's pause waits only while an accepted result stays leased
|
||||
// (agentExecutionsHaveStopped); finalization has now released it.
|
||||
expect(owner.leaseOwner).toBeNull();
|
||||
expect(owner.phase).not.toBe("workspace_finalizing");
|
||||
}, 90_000);
|
||||
|
||||
it("releases a dead controller's lease whose expiry Postgres stored with microsecond precision", async () => {
|
||||
// The controller writes its lease with SQL time arithmetic, so the stored
|
||||
// expiry carries microseconds that a JavaScript Date cannot round-trip.
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { activityLog, agents, companies, createDb } from "@paperclipai/db";
|
||||
import { applyAgentBudgetHoldInTransaction } from "./agent-lifecycle.js";
|
||||
import {
|
||||
EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase,
|
||||
} from "../__tests__/helpers/embedded-postgres.js";
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
|
||||
// H1 bug A: a budget hard stop paused the agent with no activity entry and
|
||||
// no recorded reason. A governance stop must be explainable from the log.
|
||||
describe.skipIf(!support.supported)("agent budget hold activity", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-agent-budget-hold-activity-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
afterAll(async () => {
|
||||
await db?.$client.end();
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
it("records the budget pause and its release with the reason", async () => {
|
||||
const [company] = await db.insert(companies).values({ name: "Hold", issuePrefix: "HLD" }).returning();
|
||||
const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Worker" }).returning();
|
||||
const publications: unknown[] = [];
|
||||
await db.transaction(tx => applyAgentBudgetHoldInTransaction(tx as never, company.id, agent.id, true, publications as never));
|
||||
// The entry is handed to the caller's post-commit publications.
|
||||
expect(publications).toEqual([expect.objectContaining({ companyId: company.id })]);
|
||||
const [paused] = await db.select().from(agents).where(eq(agents.id, agent.id));
|
||||
expect(paused).toMatchObject({ pauseReason: "budget", lifecycleHolds: ["budget"] });
|
||||
const entries = () => db.select({ action: activityLog.action, details: activityLog.details }).from(activityLog)
|
||||
.where(and(eq(activityLog.companyId, company.id), eq(activityLog.entityId, agent.id)));
|
||||
expect(await entries()).toEqual([{ action: "agent.paused",
|
||||
details: expect.objectContaining({ reason: "budget", holds: ["budget"] }) }]);
|
||||
// Applying the same hold again records nothing new.
|
||||
await db.transaction(tx => applyAgentBudgetHoldInTransaction(tx as never, company.id, agent.id, true));
|
||||
expect(await entries()).toHaveLength(1);
|
||||
await db.transaction(tx => applyAgentBudgetHoldInTransaction(tx as never, company.id, agent.id, false));
|
||||
expect((await entries()).map(entry => entry.action)).toEqual(["agent.paused", "agent.resumed"]);
|
||||
});
|
||||
|
||||
it("records a budget hold release, not a resume, while another hold keeps the agent paused", async () => {
|
||||
const [company] = await db.insert(companies).values({ name: "Held", issuePrefix: "HLE" }).returning();
|
||||
const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Worker" }).returning();
|
||||
await db.transaction(tx => applyAgentBudgetHoldInTransaction(tx as never, company.id, agent.id, true));
|
||||
await db.update(agents).set({ lifecycleHolds: ["budget", "manual"] }).where(eq(agents.id, agent.id));
|
||||
await db.transaction(tx => applyAgentBudgetHoldInTransaction(tx as never, company.id, agent.id, false));
|
||||
const actions = (await db.select({ action: activityLog.action }).from(activityLog)
|
||||
.where(and(eq(activityLog.companyId, company.id), eq(activityLog.entityId, agent.id)))).map(row => row.action);
|
||||
expect(actions).toEqual(["agent.paused", "agent.budget_hold_released"]);
|
||||
});
|
||||
});
|
||||
@@ -9,8 +9,9 @@ import { isAgentStatusInvokable } from "@paperclipai/shared";
|
||||
import { notifyNativeResponsePresentation } from "./native-response-presentation-notifications.js";
|
||||
import { notifyChatInboundWakeNotices } from "./chat-inbound-wake-notice-notifications.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { companies, type Db } from "@paperclipai/db";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { agents, companies, type Db } from "@paperclipai/db";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { persistActivity, type ActivityPublication } from "./activity-log.js";
|
||||
import * as lifecycle from "../modules/agent-lifecycle/index.js";
|
||||
import { withAccountingTransaction } from "./accounting-transaction.js";
|
||||
import { budgetServiceInTransaction, policyBlocks, type BudgetServiceHooks } from "./budgets.js";
|
||||
@@ -110,8 +111,27 @@ export function startAgentLifecycle(db: Db, driver: lifecycle.LifecycleDriver, c
|
||||
}
|
||||
|
||||
/** Budget evaluation owns the transaction and supplies the aggregate agent decision. */
|
||||
export function applyAgentBudgetHoldInTransaction(tx: Db, companyId: string, agentId: string, blocked: boolean) {
|
||||
return applyAgentPolicyHoldsInTransaction(tx, createAgentLifecycleEffects(), companyId, agentId, { budget: blocked });
|
||||
/** Pass the transaction's post-commit `publications` so the entry reaches live
|
||||
* subscribers after commit; without them it publishes nothing until reload. */
|
||||
export async function applyAgentBudgetHoldInTransaction(tx: Db, companyId: string, agentId: string, blocked: boolean,
|
||||
publications?: ActivityPublication[]) {
|
||||
const [before] = await tx.select({ holds: agents.lifecycleHolds }).from(agents)
|
||||
.where(and(eq(agents.id, agentId), eq(agents.companyId, companyId)));
|
||||
const updated = await applyAgentPolicyHoldsInTransaction(tx, createAgentLifecycleEffects(), companyId, agentId, { budget: blocked });
|
||||
// A budget pause is a governance stop: record it, and its release, with the
|
||||
// reason, in the transaction that applies it.
|
||||
const had = before?.holds?.includes("budget") ?? false;
|
||||
const has = updated?.lifecycleHolds?.includes("budget") ?? had;
|
||||
if (updated && had !== has) {
|
||||
const { publication } = await persistActivity(tx, { companyId, actorType: "system", actorId: "budget",
|
||||
// Releasing the budget hold resumes the agent only when no other hold
|
||||
// (a manual or company pause) still keeps it paused.
|
||||
agentId, action: has ? "agent.paused" : updated.lifecycleHolds.length === 0 ? "agent.resumed" : "agent.budget_hold_released",
|
||||
entityType: "agent", entityId: agentId,
|
||||
details: { reason: "budget", lifecycleState: updated.lifecycleState, holds: updated.lifecycleHolds } });
|
||||
if (publication) publications?.push(publication);
|
||||
}
|
||||
return updated;
|
||||
}
|
||||
|
||||
export function fenceActiveCompanyAgentStopsInTransaction(tx: Db, companyId: string, status: "paused" | "archived") {
|
||||
|
||||
@@ -349,7 +349,7 @@ export function budgetServiceInTransaction(db: Db, publications: ActivityPublica
|
||||
const now = new Date();
|
||||
if (policy.scopeType === "agent") {
|
||||
const { applyAgentBudgetHoldInTransaction } = await import("./agent-lifecycle.js");
|
||||
await applyAgentBudgetHoldInTransaction(db, policy.companyId, policy.scopeId, true);
|
||||
await applyAgentBudgetHoldInTransaction(db, policy.companyId, policy.scopeId, true, publications);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -420,7 +420,7 @@ export function budgetServiceInTransaction(db: Db, publications: ActivityPublica
|
||||
await promoteDeferredWorkIntents(db, { companyId: policy.companyId, trigger: "pause_lifted" });
|
||||
if (policy.scopeType === "agent") {
|
||||
const { applyAgentBudgetHoldInTransaction } = await import("./agent-lifecycle.js");
|
||||
await applyAgentBudgetHoldInTransaction(db, policy.companyId, policy.scopeId, false);
|
||||
await applyAgentBudgetHoldInTransaction(db, policy.companyId, policy.scopeId, false, publications);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { and, eq, gt, isNull, notLike, sql } from "drizzle-orm";
|
||||
import { and, eq, gt, inArray, isNull, notLike, sql } from "drizzle-orm";
|
||||
import { heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db";
|
||||
import { notifyLiveRunRecovery } from "../live-run-recovery-notifications.js";
|
||||
import { notifyNativeFinalizationWork } from "../native-finalization-notifications.js";
|
||||
@@ -19,8 +19,9 @@ import { recordLegacyHostFallbackProof } from "./native-restart-recovery.js";
|
||||
*
|
||||
* The shared reconciler calls this only after `proveOwnerGone` proved the
|
||||
* recorded controller gone. Settle expires exactly that lease: a lease taken
|
||||
* over since the proof, a run no longer running, and remote copyback (fenced
|
||||
* by its own controller lease) are left alone. It settles ownership only; the
|
||||
* over since the proof, a run that already finished (other than a run cancelled
|
||||
* after its result was accepted), and remote copyback (fenced by its own
|
||||
* controller lease) are left alone. It settles ownership only; the
|
||||
* accepted result is finalized by the finalizer that the notifications wake.
|
||||
*/
|
||||
export async function settleNativeAcceptedResultLease(db: Db, record: InProgressRecord,
|
||||
@@ -43,7 +44,10 @@ export async function settleNativeAcceptedResultLease(db: Db, record: InProgress
|
||||
eq(heartbeatRuns.companyId, nativeRunFinalizations.companyId),
|
||||
eq(heartbeatRuns.nativeIssueId, nativeRunFinalizations.issueId)))
|
||||
.where(and(eq(nativeRunFinalizations.runId, record.recordId), eq(heartbeatRuns.runtimeMode, "native"),
|
||||
eq(heartbeatRuns.status, "running"), gt(nativeRunFinalizations.leaseExpiresAt, now),
|
||||
// A run cancelled after its result was accepted (for example by a
|
||||
// budget pause) still owes finalization, and the dead lease would
|
||||
// hold it, and the agent's pause, for the lease's full term.
|
||||
inArray(heartbeatRuns.status, ["running", "cancelled"]), gt(nativeRunFinalizations.leaseExpiresAt, now),
|
||||
notLike(nativeRunFinalizations.leaseOwner, "native-finalizer:%"),
|
||||
notLike(nativeRunFinalizations.leaseOwner, "native-cleanup:%")))
|
||||
.for("update", { of: nativeRunFinalizations });
|
||||
|
||||
Reference in new issue
Block a user