Files
PaperClipAI/server/src/__tests__/plugin-task-credential-writeback.test.ts
T
Nicky LeachandClaude Opus 5.5 990de1e0bf perf(environments): release a stopped plugin task once (#20203)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Plugin environment drivers with `supportsTasks` run agents as
provider tasks, and warm runs keep a parked task for the next run
> - A follow-up run can find that its parked task already stopped. Core
then retires that task before it acquires a new one
> - Core sent `stop`, `status`, and `complete` in the settle step. Then
the plugin release hook sent `stop`, `status`, and `complete` again for
the same task. Every call was on the critical path of the new run
> - This pull request removes the repeated calls and keeps every call
that does work no other call does
> - The benefit is a faster start for a run whose parked task already
stopped

## Linked Issues or Issue Description

No issue exists. This follows the enhancement template.

**What existing behavior does this improve?**

The release of a plugin task lease that has already stopped. This occurs
when a follow-up run finds that its parked warm task stopped.

**Subsystem affected**

Server: plugin environment drivers and the plugin Runner task lifecycle.

**Current behavior**

Core sends `stop`, `status`, and `complete` in the settle step. Then the
plugin release hook sends `stop`, `status`, and `complete` again for the
same task. Core also sends `stop` after resume already saw that the task
stopped. On a hosted test deployment, 8 of 40 follow-up runs took this
path. From the resume to the new acquire, these runs spent P50 1014 ms
and P90 1080 ms. These 8 runs are the full overall P90 (1275 ms).

**Proposed behavior**

Core does not call the release hook when settle confirmed the stop and
completed the same task. After a resume with no lease, Core sends
`status` first, and sends `stop` only when `status` does not confirm the
stop.

**Reason and benefit**

The repeated calls do no new work. Removing them saves about 355 ms,
plus the `stop` share of about 187 ms, for each such run. These numbers
are modeled from trace timings.

**Breaking changes**

None for the plugin API. A task driver must release task-scoped
resources in its `status` or `complete` handling. The doc now says this.

**Additional context**

The settle `status` stays. Providers release task storage there, and it
records the stop that gates the credential writeback and `complete`.

## What Changed

- `settleDispatchedEnvironmentTask` now returns `"completed"` when it
confirmed the stop and completed the task in this call, `"stopped"` when
the lease was already settled, and `false` otherwise.
- The plugin driver release does not call `environmentReleaseLease` when
settle returned `"completed"`. In all other cases it calls the hook as
before.
- When `environmentResumeLease` returns no provider lease, the release
sends `status` first. It sends `stop` (and a second `status`) only when
that `status` does not confirm `executionStopped: true`. Startup and
startup-cleanup leases keep the old `stop` first order.
- The cleanup sweep (`retryPendingSandboxTeardown`) and
`destroyRunLease` do not change. A settle failure still leaves the lease
in `pending_cleanup`, and the sweep runs `stop`, `status`, `complete`,
and the release hook.
- `doc/plugins/PLUGIN_RUNNER_EXECUTION.md` tells drivers about the new
order and that task-scoped resources must be released in `status` or
`complete`.

### Dispatch count for a stopped parked task (resume to new acquire)

| | Calls |
| --- | --- |
| Before | resume, stop, status, (credentials), complete, release hook
(stop, status, complete), acquire: 9 provider calls (10 with Codex
writeback) |
| After | resume, status, (credentials), complete, acquire: 4 provider
calls (5 with Codex writeback) |

The modeled saving is about 540 ms for each such run: about 355 ms for
the repeated release, and about 187 ms for the settle `stop` and
`status`. This pull request keeps the settle `status`, so the real
saving is about 355 ms plus the `stop` share of the 187 ms. These
numbers are modeled from trace timings. They are not measured on this
branch.

### What covers each removed call

- Release hook `stop`, `status`, `complete`: settle already sent `stop`
and `status` and checked `executionStopped: true` for the same task id
(`lease.providerLeaseId`), then sent `complete`. The skip uses settle's
success as its condition, not the code path.
- Settle `stop` after a resume with no lease: the next `status` must
report `executionStopped: true`. If it does not, Core sends `stop` and
`status` as before.

### What each kept call protects

- Settle `status`: providers release task storage and cancel
preparations here. It also writes `runnerTaskExecutionStopped`, which
the credential writeback and the `complete` bookkeeping need.
- Credential writeback: Codex can rotate its token. The writeback must
land before the new task installs credentials. It still runs inside
`complete`, before the new acquire.
- Settle `complete`: it retires every run lease that owned the task and
clears the dispatch marker.
- Release hook when settle did not complete a dispatched task in this
call: the provider may still hold the lease.

## Verification

- `pnpm --filter @paperclipai/shared --filter @paperclipai/plugin-sdk
build`
- `pnpm --filter @paperclipai/server typecheck`
- `npx vitest run src/__tests__/plugin-warm-tasks.integration.test.ts
src/__tests__/plugin-task-credential-writeback.test.ts
src/__tests__/environment-task-runtime.test.ts`
- Related suites: `environment-runtime`,
`plugin-runner-task.integration`, `task-creation-recovery.integration`,
`heartbeat-plugin-environment`, `native-startup-cleanup-recovery`,
`plugin-environment-driver-ready-recovery`,
`environment-task-authority.integration`, `native-workspace-warm-reuse`,
`remote-workspaces.integration`, `legacy-workspace-restore-recovery`,
`plugin-task-source-recovery.integration`,
`managed-plugin-environments`.

Locally, all of these pass except `heartbeat-plugin-environment`
"inherits the instance default environment across companies…". That test
also fails locally with the `origin/master` service files, so this
change does not cause it.

The existing `plugin-runner-task.integration` same-run replacement test
now expects no release hook after a completed settle.

New tests:

- The stopped parked path sends exactly resume, `status`, `complete`,
acquire. The `status` goes to the old task.
- A resume with no lease for a task that still runs sends `status`,
`stop`, `status`, `complete`.
- A failure at `status`, `complete`, or `stop` leaves the lease in
`pending_cleanup`, and the sweep then sends `stop`, `status`,
`complete`, and the release hook.
- With an observed stop, the Codex writeback runs before `complete`. A
writeback failure leaves the lease for cleanup, and the retry writes
back and completes.
- A startup-cleanup lease still sends `stop` first.

## Risks

- A third-party task driver that releases resources only in
`onEnvironmentReleaseLease` would not get that call after a completed
settle. The doc now states the requirement. The cleanup sweep still
calls the hook after any failure.
- A driver that returns no lease from resume while the task still runs
costs one extra `status`. The behavior stays correct.
- No migration. No API change for plugins.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and
code execution.

## 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
- [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

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-08 23:00:49 -08:00

164 lines
12 KiB
TypeScript

import { randomUUID } from "node:crypto";
import { mkdtemp, realpath, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { and, eq, sql } from "drizzle-orm";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import { agents, companies, companyMemberships, createDb, environmentLeases, environments, heartbeatRuns, plugins } from "@paperclipai/db";
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
import { aiConnectionService } from "../services/ai-connections.js";
import { prepareManagedAiRuntime } from "../services/ai-connection-runtime.js";
import { registerPluginTaskCredentialHandoff, settlePluginTaskCredentialHandoff } from "../services/plugin-task-credential-writeback.js";
import { executeEnvironmentTask, settleDispatchedEnvironmentTask } from "../services/environment-task-runtime.js";
import { secretService } from "../services/secrets.js";
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
vi.mock("../services/subscription-refresh.js", () => ({ refreshSubscriptionConnection: async () => {} }));
const support = await getEmbeddedPostgresTestSupport();
const suite = support.supported ? describe : describe.skip;
const auth = (token: string, year = 2020) => JSON.stringify({ tokens: { account_id: "fixture-account", access_token: "fixture-access", refresh_token: token }, last_refresh: `${year}-01-01T00:00:00Z` });
suite("durable plugin task credential handoff", () => {
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>, db: ReturnType<typeof createDb>, home: string;
beforeAll(async () => {
home = await realpath(await mkdtemp(join(tmpdir(), "plugin-task-secret-")));
vi.stubEnv("PAPERCLIP_HOME", home); vi.stubEnv("PAPERCLIP_INSTANCE_ID", "plugin-task-fixture");
database = await startEmbeddedPostgresTestDatabase("plugin-task-credential-"); db = createDb(database.connectionString);
}, 90_000);
afterAll(async () => { await database?.cleanup(); vi.unstubAllEnvs(); if (home) await rm(home, { recursive: true, force: true }); });
async function fixture(dispatched = true) {
const companyId = randomUUID(), agentId = randomUUID(), runId = randomUUID(), pluginId = randomUUID(), environmentId = randomUUID(), leaseId = randomUUID();
const owner = `fixture-${randomUUID()}`;
await db.insert(companies).values({ id: companyId, name: "Task credential fixture", issuePrefix: `C${companyId.slice(0, 6)}` });
await db.insert(agents).values({ id: agentId, companyId, name: "Fixture", adapterType: "paperclip_runner" });
await db.insert(companyMemberships).values({ companyId, principalId: owner, principalType: "user", status: "active", membershipRole: "member" });
const service = aiConnectionService(db);
await service.save(companyId, owner, { provider: "openai", method: "subscription", ownership: "personal", name: "Fixture Codex", loginSessionId: "fixture", agentIds: [], allAgents: true }, auth("fixture-original"));
const [selection] = await service.quotaAccounts(companyId, owner);
const managed = await prepareManagedAiRuntime(db, { companyId, agentId, responsibleUserId: owner, adapterType: "paperclip_runner",
binding: { mode: "responsible_user", provider: "openai", method: "subscription" }, config: { provider: "codex" } });
expect(managed.credentialWriteback).toBeDefined();
await db.insert(environments).values({ id: environmentId, name: `Fixture ${environmentId}`, driver: "plugin", status: "active", config: { pluginKey: pluginId, driverKey: "tasks", driverConfig: {} } });
await db.insert(plugins).values({ id: pluginId, pluginKey: pluginId, packageName: "fixture-tasks", version: "1.0.0", status: "ready", manifestJson: {
capabilities: ["environment.drivers.register"], environmentDrivers: [{ driverKey: "tasks", supportsTasks: true }],
} as never });
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, invocationSource: "manual", status: "running" });
await db.insert(environmentLeases).values({ id: leaseId, companyId, environmentId, heartbeatRunId: runId, provider: `plugin:${pluginId}:tasks`, providerLeaseId: "fixture-task", status: "active",
metadata: { driver: "plugin", pluginId, driverKey: "tasks", runnerTaskLease: true, runnerTaskDispatched: dispatched, runnerTaskExecutionStopped: false } });
await registerPluginTaskCredentialHandoff(db, { companyId, leaseId }, managed.sessionIdentity, managed.credentialWriteback!);
// Recovery must not depend on the original controller or its private home.
await rm(managed.home!, { recursive: true, force: true });
const calls: string[] = [];
let failExport = false;
const workers = { getWorker: () => ({ supportedMethods: ["environmentTask"] }),
call: async (_plugin: string, _method: string, params: any) => {
calls.push(params.operation.kind);
expect(params.companyId).toBe(companyId); expect(params.taskId).toBe("fixture-task");
if (params.operation.kind === "status") return { kind: "status", taskId: "fixture-task", phase: "cancelled", executionStopped: true };
if (params.operation.kind === "credentials") {
expect(params.operation.accountIdentity).toBe(managed.sessionIdentity);
if (failExport) throw new Error("fixture export unavailable");
return { kind: "credentials", taskId: "fixture-task", credential: { type: "codex-auth-json", authJson: auth("fixture-refreshed", 2021) } };
}
return { kind: "accepted", taskId: "fixture-task" };
} } as unknown as PluginWorkerManager;
const lease = () => db.select().from(environmentLeases).where(and(eq(environmentLeases.id, leaseId), eq(environmentLeases.companyId, companyId))).then(rows => rows[0]!);
return { companyId, agentId, runId, environmentId, leaseId, owner, selection: selection!, managed, workers, calls, lease,
failExport(value: boolean) { failExport = value; }, credential: () => service.credential(selection!),
settle: (executionStopObserved = false) => settleDispatchedEnvironmentTask(db, workers, { companyId, id: leaseId }, { executionStopObserved }) };
}
it("persists refreshed credentials before completing a stopped task, after controller files disappear", async () => {
const f = await fixture();
expect(await f.settle()).toBe("completed");
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
expect(f.calls.indexOf("credentials")).toBeLessThan(f.calls.indexOf("complete"));
const saved = await f.lease();
expect(saved.metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "complete" });
expect(JSON.stringify(saved.metadata)).not.toContain("fixture-refreshed");
expect(JSON.stringify(saved.metadata)).not.toContain(f.managed.home!);
});
it("writes back credentials before completing a task whose stop was already observed, without stopping it again", async () => {
const f = await fixture();
expect(await f.settle(true)).toBe("completed");
expect(f.calls).toEqual(["status", "credentials", "complete"]);
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "complete" });
});
it("leaves an observed-stop task for cleanup when credential writeback fails", async () => {
const f = await fixture(); f.failExport(true);
await expect(f.settle(true)).rejects.toThrow("termination is unconfirmed");
expect(f.calls).toEqual(["status", "credentials"]);
expect(await f.lease()).toMatchObject({ status: "pending_cleanup", cleanupStatus: "failed" });
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "pending" });
f.failExport(false);
expect(await f.settle()).toBe("completed");
expect(f.calls.slice(2)).toEqual(["stop", "status", "credentials", "complete"]);
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
});
it("keeps a failed transfer pending and recovers through a fresh cleanup after run and environment deletion", async () => {
const f = await fixture(); f.failExport(true);
await expect(f.settle()).rejects.toThrow("termination is unconfirmed");
expect(f.calls).not.toContain("complete");
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "pending" });
expect(await f.credential()).toBe(auth("fixture-original"));
await db.delete(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
await db.delete(environments).where(eq(environments.id, f.environmentId));
f.failExport(false);
expect(await f.settle()).toBe("completed");
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
expect(f.calls.at(-1)).toBe("complete");
});
it("retains the Runner credential after a database save failure until a fresh receiver commits", async () => {
const f = await fixture();
await db.execute(sql.raw(`create function reject_runner_credential_save() returns trigger language plpgsql as $$ begin
if new.secret_id = '${f.managed.credentialWriteback!.secretId}'::uuid then raise exception 'fixture receiver save unavailable'; end if; return new; end $$;
create constraint trigger reject_runner_credential_save after insert on company_secret_versions
deferrable initially deferred for each row execute function reject_runner_credential_save();`));
try {
await expect(f.settle()).rejects.toThrow();
expect(f.calls).toContain("credentials");
expect(f.calls).not.toContain("complete");
expect(await f.credential()).toBe(auth("fixture-original"));
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "pending" });
} finally { await db.execute(sql`drop trigger reject_runner_credential_save on company_secret_versions; drop function reject_runner_credential_save()`); }
const receiver = createDb(database.connectionString);
expect(await settleDispatchedEnvironmentTask(receiver, f.workers, { companyId: f.companyId, id: f.leaseId })).toBe("completed");
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
expect(f.calls.filter(call => call === "credentials")).toHaveLength(2);
expect(f.calls.at(-1)).toBe("complete");
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "complete" });
});
it("does not overwrite an explicit account replacement", async () => {
const f = await fixture();
await secretService(db).rotate(f.managed.credentialWriteback!.secretId, { value: auth("fixture-reconnected", 2022) }, { userId: f.owner });
expect(await f.settle()).toBe("completed");
expect(f.calls).not.toContain("credentials");
expect(await f.credential()).toBe(auth("fixture-reconnected", 2022));
});
it("allows concurrent cleanup to complete the same durable handoff", async () => {
const f = await fixture();
expect(await Promise.all([f.settle(), f.settle()])).not.toContain(false);
expect(await f.credential()).toBe(auth("fixture-refreshed", 2021));
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "complete" });
});
it("does not mark a replacement task's identical credential handoff complete", async () => {
const f = await fixture(false), prior = await f.lease();
await db.update(environmentLeases).set({ providerLeaseId: "replacement-task" }).where(eq(environmentLeases.id, f.leaseId));
await expect(settlePluginTaskCredentialHandoff(db, { companyId: f.companyId, leaseId: f.leaseId, expectedTaskId: "fixture-task" },
prior.metadata!, async () => { throw new Error("Undispatched task must not export credentials"); })).rejects.toThrow("handoff changed");
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "pending" });
});
it("retires an undispatched task without exporting credentials and blocks a late submit", async () => {
const f = await fixture(false);
expect(await f.settle()).toBe(false);
expect(f.calls).toEqual([]);
expect((await f.lease()).metadata?.runnerTaskCredentialHandoff).toMatchObject({ state: "complete" });
await expect(executeEnvironmentTask(db, f.workers, { companyId: f.companyId, leaseId: f.leaseId, operation: {
kind: "submit", projectIds: [], bootstrapTicket: "fixture-ticket", runner: { harness: "codex", protocolMin: 1, protocolMax: 3,
runnerId: "runner", leaseId: f.leaseId, runId: f.runId, sessionId: "session", turnId: "turn", itemId: "item" },
} })).rejects.toThrow("not active");
expect(f.calls).toEqual([]);
});
});