mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
Pause agents automatically when moving to a plugin environment (#20375)
## Thinking Path > - Paperclip manages AI agents and their execution environments. > - Core owns agent lifecycle state and controls when work can run. > - A plugin environment can need a computer that belongs to the agent. > - Moving a ready agent to that environment currently requires manual pause and resume actions. > - Saving the environment is sufficient intent to perform those routine steps. > - This change records the pause with the environment update and uses the existing lifecycle worker. > - Work can resume only after execution stops, the plugin starts its resources, and verification passes. ## Linked Issues or Issue Description Related: #19173 introduced environment-scoped plugin selection and the manual-pause requirement. I searched for related pull requests and found no duplicate fix. No separate issue is linked. **What happened?** Saving a ready agent with an environment whose lifecycle plugin is not selected returns a conflict. The error tells the user to pause the agent, change its environment, and resume it. **Expected behavior** Saving the environment change starts the required pause, resource setup, and verification automatically. Existing manual, budget, and company holds still block resume. **Steps to reproduce** 1. Use an agent in `ready` on an environment without an agent-owned plugin. 2. Select an agent-owned plugin environment in the agent configuration. 3. Save without first pausing the agent. Before this fix, the save fails. With this fix, the agent moves through `pausing`, `resuming`, and `verifying` before `ready`. ## What Changed - Save the new environment and automatic pause in the same transaction. Keep the existing plugin selection during stop. - Use the existing completion of a pause without holds to resume automatically. Set the saved resume target to `verifying` so the new environment must pass its test. - Preserve independent pause holds, lifecycle version checks, notifications, resource status events, and worker recovery. - Add migration tests for idle and running agents, real process-stop proof across a worker restart, added holds, failed verification, and route authorization. - Update the lifecycle and development docs. Keep the feature maps unchanged, as required for a small behavior fix. ## Verification - Current revision: `300f57539bce9b198406c3793ab3c2de46492659`. - Lifecycle and case API discovery suites passed after updating the branch to master. The final environment-plugin suite passed all 22 tests, including four real-route authorization cases for users and agents. Coverage includes real process-stop proof across worker replacement, independent holds, and failed migration verification. - Server typecheck: passed on the current base. All workspace typechecks and `pnpm build` passed before the base update; fresh CI covers the updated base. - `node scripts/check-module-boundaries.mjs`, `node scripts/check-maps.mjs`, and `git diff --check`: passed. - Reviewed the diff for secrets, personal data, private addresses, and run artifacts before each push. - [Current CI](https://github.com/paperclipai/paperclip/actions/runs/42714733533): passed on `300f57539bce9b198406c3793ab3c2de46492659` (52 successful jobs; the optional test-selection optimization was skipped). Standard Greptile: 5/5 on the current revision, with no unresolved findings. The security scanner accepted the documented configuration-authority policy. - Earlier CI failed the `case-history-api-discovery` assertion for the query `case for task`. It also failed locally. The branch now includes master's fix #20382, and that test passes. The migration implementation did not change. The superseded CI attempt was cancelled after recording the failure. - Full local `pnpm test:run`: incomplete; stopped after two passing chat partitions. The focused suites passed, and required CI supplies broad coverage, as specified in the repository instructions. - Local target: isolated source checkout on macOS ARM64, Node 26, disposable PostgreSQL, fixture lifecycle plugin. Initial database startup failed because the bundled macOS PostgreSQL package lacked dylib aliases. Temporary dependency aliases repaired startup; the focused database tests then passed. The installed Rust toolchain also needed to be added to PATH. - The pending-stop test retains a live process ID in the cancelled run. The real host stop check blocks plugin setup. After stop acknowledgment and worker replacement, the saved migration reaches readiness. ## Risks - Saving this environment change can stop current execution. The system performs that pause as part of the requested migration. - A failed stop, plugin call, or verification keeps the agent blocked and uses existing retry behavior. - Editing a shared environment or the instance default still uses its existing conflict checks. This change covers the individual agent environment update. - Configuration permission authorizes the temporary system pause needed for the resource move. Separate manual-pause authority is not required for this configuration operation. Real route tests cover authorized and denied users and agents. - No schema migration or deployment is included. A fixture plugin does not prove live computer provisioning or model execution. ## Model Used OpenAI Codex, based on GPT-6, with code execution and tool use. The exact runtime model ID and context limit 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 `#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 - [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>
This commit is contained in:
1 parent
ec75547a2e
commit
1838de8c6c
4 files changed
+124
-14
No files matched your search
+14
-3
@@ -198,11 +198,22 @@ plugins are selected for all agents, including a sandbox provider that declares
|
||||
`requiresAgent: true`.
|
||||
|
||||
When an agent moves to such an environment, the host adds the plugin at the next
|
||||
`preparing` or `resuming` step. An agent in `ready` cannot move to it until it is
|
||||
paused, because the plugin must finish its work before the agent uses that
|
||||
environment. An agent in `verifying` returns to `preparing`. An agent that moves away keeps the selection until it is terminated,
|
||||
`preparing` or `resuming` step. Moving a `ready` agent to an environment that
|
||||
needs a new plugin automatically pauses execution, waits for the host and
|
||||
existing plugins to stop, resumes with the new plugin, and verifies the saved
|
||||
configuration before admitting work. The change and pause request commit
|
||||
together; retries and server restarts retain the pending operation. Manual,
|
||||
budget, and company holds still prevent automatic resume. An agent in
|
||||
`verifying` returns to `preparing`. An agent that moves away keeps the selection until it is terminated,
|
||||
so the plugin keeps its resources until then.
|
||||
|
||||
The existing agent configuration permission authorizes this move, including its
|
||||
temporary system pause. That permission already controls environment selection
|
||||
and resource allocation. Separate permission to issue a manual pause is not
|
||||
required for the migration. Configuration and company access checks still run
|
||||
before the change; an unauthorized request cannot save the environment or pause
|
||||
the agent.
|
||||
|
||||
Saving an environment or the instance default that gives such a plugin to `ready`
|
||||
or `verifying` agents without that selection returns a conflict. Pause those agents,
|
||||
save the change, and then resume them.
|
||||
|
||||
@@ -3189,6 +3189,10 @@ Saved OpenCode agents keep their runner and runtime policies.
|
||||
Plugin environment drivers can declare `requiresAgent: true` when their leases
|
||||
need an agent-owned resource. The host selects that plugin's lifecycle work only for
|
||||
agents on that environment. See [Required plugin work](./AGENT-LIFECYCLE.md#required-plugin-work).
|
||||
Moving a ready agent to an environment that needs a new plugin automatically
|
||||
pauses execution, waits for stop completion, starts the required resources, and
|
||||
verifies the saved configuration before allowing work. Existing pause holds
|
||||
still apply; the user does not need to pause or resume the agent for this move.
|
||||
Setup saves the draft and environment, runs its
|
||||
existing lifecycle preparation, and waits in `verifying` with the `setup` hold.
|
||||
The worker becomes idle there until the user finishes hiring. Connection tests
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { agents, companies, environments, instanceSettings, plugins, createDb, type Db } from "@paperclipai/db";
|
||||
import { agents, companies, companyMemberships, environments, heartbeatRuns, instanceSettings, plugins, principalPermissionGrants, createDb, type Db } from "@paperclipai/db";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { agentRoutes } from "../../../routes/agents.js";
|
||||
import { errorHandler } from "../../../middleware/error-handler.js";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../../__tests__/helpers/embedded-postgres.js";
|
||||
import { configureAgentLifecycle, createAgentLifecycle, createAgentLifecycleEffects } from "../../../services/agent-lifecycle.js";
|
||||
@@ -8,6 +12,7 @@ import { createLifecycleDriver } from "../../../services/agent-lifecycle-driver.
|
||||
import { agentService as agentConfiguration } from "../../../services/agents.js";
|
||||
import { assertAgentsReadyForEnvironmentChange } from "../../../services/plugin-environment-driver.js";
|
||||
import { createLifecycleStore } from "../adapters/postgres.js";
|
||||
import type { LifecycleAgent, LifecycleDriver } from "../application/ports.js";
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
(support.supported ? describe : describe.skip)("environment-bound lifecycle plugins", () => {
|
||||
@@ -38,8 +43,8 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
]);
|
||||
});
|
||||
|
||||
function worker() {
|
||||
const work = configureAgentLifecycle(db, { ...createLifecycleDriver(db, { call } as never), runHost: async () => "complete" as const });
|
||||
function worker(runHost: LifecycleDriver["runHost"] = async () => "complete") {
|
||||
const work = configureAgentLifecycle(db, { ...createLifecycleDriver(db, { call } as never), runHost });
|
||||
workers.push(work);
|
||||
return work;
|
||||
}
|
||||
@@ -49,6 +54,11 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
return id;
|
||||
}
|
||||
async function current(id: string) { return (await db.select().from(agents).where(eq(agents.id, id)))[0]!; }
|
||||
function app(actor: Express.Request["actor"]) {
|
||||
const value = express(); value.use(express.json());
|
||||
value.use((req, _res, next) => { req.actor = actor; next(); });
|
||||
value.use("/api", agentRoutes(db)); value.use(errorHandler); return value;
|
||||
}
|
||||
async function selection(id: string) {
|
||||
return createLifecycleDriver(db, { call } as never).requiredPluginIds((await createLifecycleStore(db, createAgentLifecycleEffects()).get(id))!);
|
||||
}
|
||||
@@ -147,10 +157,91 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
expect((await current(id)).lifecycleState).toBe("verifying");
|
||||
});
|
||||
|
||||
it("refuses to move a ready agent to an environment whose plugin it has not selected", async () => {
|
||||
it.each(["idle", "running"])("automatically pauses an %s agent, prepares its new environment, and verifies before readiness", async status => {
|
||||
const id = await agent(otherEnvironmentId, { status, lifecycleRequiredPluginIds: [] });
|
||||
const saved = await agentConfiguration(db).update(id, { defaultEnvironmentId: boundEnvironmentId });
|
||||
expect(saved).toMatchObject({ defaultEnvironmentId: boundEnvironmentId, lifecycleState: "pausing", status: "paused" });
|
||||
expect(await current(id)).toMatchObject({ lifecycleHolds: [], lifecycleOperation: { resumeState: "verifying" } });
|
||||
|
||||
const phases: string[] = [];
|
||||
const runHost = vi.fn(async (row: LifecycleAgent) => { phases.push(row.lifecycleState); return "complete" as const; });
|
||||
await worker(runHost).process(id);
|
||||
expect(phases).toEqual(["pausing", "resuming", "verifying"]);
|
||||
expect(call.mock.calls.map(args => args[2].phase)).toEqual(["resuming", "verifying"]);
|
||||
expect(await current(id)).toMatchObject({ lifecycleState: "ready", status: "idle", lifecycleRequiredPluginIds: [boundPluginId] });
|
||||
});
|
||||
|
||||
it.each(["board", "agent"] as const)("lets a configuration-authorized %s migrate without separate manual-pause authority", async type => {
|
||||
const id = await agent(otherEnvironmentId, { status: "running", adapterType: "paperclip_runner",
|
||||
adapterConfig: { provider: "codex" }, lifecycleRequiredPluginIds: [] });
|
||||
const principalId = type === "agent" ? await agent(otherEnvironmentId) : randomUUID();
|
||||
await db.insert(companyMemberships).values({ companyId, principalType: type === "board" ? "user" : "agent",
|
||||
principalId, membershipRole: "member", status: "active" });
|
||||
await db.insert(principalPermissionGrants).values({ companyId, principalType: type === "board" ? "user" : "agent",
|
||||
principalId, permissionKey: "agents:configure" });
|
||||
const actor: Express.Request["actor"] = type === "board"
|
||||
? { type, source: "session", userId: principalId, companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: "member", status: "active" }] }
|
||||
: { type, source: "agent_key", agentId: principalId, companyId };
|
||||
await request(app(actor)).post(`/api/agents/${id}/pause`).send({}).expect(403);
|
||||
const response = await request(app(actor)).patch(`/api/agents/${id}`).send({ defaultEnvironmentId: boundEnvironmentId });
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(200);
|
||||
expect(await current(id)).toMatchObject({ defaultEnvironmentId: boundEnvironmentId, lifecycleState: "pausing", status: "paused" });
|
||||
});
|
||||
|
||||
it.each(["board", "agent"] as const)("rejects a %s without configuration permission before saving the environment or pausing", async type => {
|
||||
const id = await agent(otherEnvironmentId, { status: "running", adapterType: "paperclip_runner",
|
||||
adapterConfig: { provider: "codex" }, lifecycleRequiredPluginIds: [] });
|
||||
const principalId = type === "agent" ? await agent(otherEnvironmentId) : randomUUID();
|
||||
await db.insert(companyMemberships).values({ companyId, principalType: type === "board" ? "user" : "agent", principalId, membershipRole: "member", status: "active" });
|
||||
const actor: Express.Request["actor"] = type === "board"
|
||||
? { type, source: "session", userId: principalId, companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: "member", status: "active" }] }
|
||||
: { type, source: "agent_key", agentId: principalId, companyId };
|
||||
const before = await current(id);
|
||||
await request(app(actor)).patch(`/api/agents/${id}`).send({ defaultEnvironmentId: boundEnvironmentId }).expect(403);
|
||||
expect(await current(id)).toMatchObject({ defaultEnvironmentId: otherEnvironmentId, lifecycleState: "ready", status: "running",
|
||||
lifecycleVersion: before.lifecycleVersion, lifecycleOperation: before.lifecycleOperation });
|
||||
expect(call).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retains the migration across a pending stop and worker restart without starting the new plugin early", async () => {
|
||||
const id = await agent(otherEnvironmentId, { status: "running", lifecycleRequiredPluginIds: [] });
|
||||
const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId: id,
|
||||
status: "cancelled", invocationSource: "on_demand", runtimeMode: "legacy",
|
||||
startedAt: new Date(), finishedAt: new Date(), processPid: process.pid }).returning();
|
||||
await agentConfiguration(db).update(id, { defaultEnvironmentId: boundEnvironmentId });
|
||||
const driver = createLifecycleDriver(db, { call } as never);
|
||||
const stopping = worker(row => row.lifecycleState === "pausing" ? driver.runHost(row) : Promise.resolve("complete"));
|
||||
await stopping.process(id);
|
||||
expect((await current(id)).lifecycleState).toBe("pausing");
|
||||
expect(call).not.toHaveBeenCalled();
|
||||
await stopping.stop();
|
||||
await db.update(heartbeatRuns).set({ processPid: null,
|
||||
resultJson: { executionCancellation: { state: "acknowledged" } } }).where(eq(heartbeatRuns.id, run.id));
|
||||
await createAgentLifecycle(db).retry(id);
|
||||
await worker().process(id);
|
||||
expect(await current(id)).toMatchObject({ lifecycleState: "ready", lifecycleRequiredPluginIds: [boundPluginId] });
|
||||
});
|
||||
|
||||
it.each(["manual", "budget", "company_paused"])("preserves a %s hold added while migration is pausing", async reason => {
|
||||
const id = await agent(otherEnvironmentId, { lifecycleRequiredPluginIds: [] });
|
||||
await expect(agentConfiguration(db).update(id, { defaultEnvironmentId: boundEnvironmentId })).rejects.toThrow("Pause the agent");
|
||||
expect((await current(id)).defaultEnvironmentId).toBe(otherEnvironmentId);
|
||||
await agentConfiguration(db).update(id, { defaultEnvironmentId: boundEnvironmentId });
|
||||
await createAgentLifecycle(db).pauseAgent(id, reason);
|
||||
await worker().process(id);
|
||||
expect(await current(id)).toMatchObject({ lifecycleState: "paused", status: "paused", lifecycleHolds: [reason] });
|
||||
expect(call).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps the agent blocked when verification of the new environment fails", async () => {
|
||||
const id = await agent(otherEnvironmentId, { lifecycleRequiredPluginIds: [] });
|
||||
await agentConfiguration(db).update(id, { defaultEnvironmentId: boundEnvironmentId });
|
||||
await worker(vi.fn(async (row: LifecycleAgent) => {
|
||||
if (row.lifecycleState === "verifying") throw Object.assign(new Error("Fixture verification failed"), { code: "harness_test_failed" });
|
||||
return "complete" as const;
|
||||
})).process(id);
|
||||
expect(await current(id)).toMatchObject({ lifecycleState: "verifying", status: "paused", lifecycleRequiredPluginIds: [boundPluginId] });
|
||||
expect((await current(id)).lifecycleError).toBeTruthy();
|
||||
});
|
||||
|
||||
it("lets a ready agent move when the plugin is selected, and keeps the selection when it leaves", async () => {
|
||||
|
||||
@@ -29,9 +29,7 @@ export async function updateAgentConfiguration(tx: Db, effects: LifecycleEffects
|
||||
&& Object.prototype.hasOwnProperty.call(patch, "defaultEnvironmentId")
|
||||
&& patch.defaultEnvironmentId !== current.defaultEnvironmentId
|
||||
&& await effects.environmentRequiresNewPlugin(tx, patch.defaultEnvironmentId, current.lifecycleRequiredPluginIds);
|
||||
if (needsNewPlugin && current.lifecycleState === "ready") {
|
||||
throw new AgentLifecycleConflict("This environment needs setup for the agent. Pause the agent, change its environment, and then resume it.");
|
||||
}
|
||||
const pauseForEnvironment = needsNewPlugin && current.lifecycleState === "ready";
|
||||
const prepareEnvironment = needsNewPlugin && !current.lifecycleHolds.includes("setup");
|
||||
const prepareSetup = changed && current.lifecycleHolds.length === 1 && current.lifecycleHolds[0] === "setup"
|
||||
&& ["paused", "preparing", "verifying"].includes(current.lifecycleState)
|
||||
@@ -39,19 +37,25 @@ export async function updateAgentConfiguration(tx: Db, effects: LifecycleEffects
|
||||
Object.prototype.hasOwnProperty.call(patch, "defaultEnvironmentId") ? patch.defaultEnvironmentId : current.defaultEnvironmentId,
|
||||
{ adapterType: patch.adapterType ?? current.adapterType,
|
||||
adapterConfig: Object.prototype.hasOwnProperty.call(patch, "adapterConfig") ? patch.adapterConfig : current.adapterConfig });
|
||||
const invalidate = changed && (["preparing", "verifying", "resuming"].includes(current.lifecycleState) || prepareSetup);
|
||||
const invalidate = changed && (["preparing", "verifying", "resuming"].includes(current.lifecycleState) || prepareSetup || pauseForEnvironment);
|
||||
if(prepareSetup&&!canConfigureAgentConnection(current))await effects.beforeConnectionEligibilityRestore?.(tx,current.companyId,current.id);
|
||||
if (pauseForEnvironment) await effects.beforeStatusChange(tx, current.companyId, id, current.status, "paused");
|
||||
if (invalidate) await effects.notifyWork(tx);
|
||||
const [updated] = await tx.update(agents).set({ ...patch, updatedAt: new Date(), ...(invalidate ? {
|
||||
lifecycleVersion: current.lifecycleVersion + 1, lifecycleError: null,
|
||||
...(prepareSetup || prepareEnvironment ? { lifecycleState: "preparing" as const } : {}),
|
||||
...(pauseForEnvironment ? { lifecycleState: "pausing" as const, status: "paused", pausedAt: new Date() } : {}),
|
||||
...(!prepareSetup && current.lifecycleHolds.includes("setup") && ["preparing", "verifying"].includes(current.lifecycleState)
|
||||
? { lifecycleState: "pausing" as const } : {}),
|
||||
lifecycleOperation: { ...current.lifecycleOperation!, id: randomUUID(), hostComplete: false,
|
||||
completedPluginIds: [], attempts: 0, leaseOwner: undefined, leaseUntil: undefined, leaseHolder: undefined,
|
||||
retryAt: undefined, retryRequired: undefined, verificationFailures: undefined,
|
||||
waitingForSignal: undefined, recheckRequested: undefined },
|
||||
waitingForSignal: undefined, recheckRequested: undefined,
|
||||
// With no new hold, completed pause work resumes automatically, then verifies the new environment.
|
||||
...(pauseForEnvironment ? { resumeState: "verifying" as const,
|
||||
pauseCause: { holds: current.lifecycleHolds, companyPauseReason: null } } : {}) },
|
||||
} : {}) }).where(eq(agents.id, id)).returning();
|
||||
if (pauseForEnvironment) await effects.recordStatus(tx, current.companyId, id, "idle", "paused");
|
||||
await effects.completeConfiguration(tx, current, updated, patch, options, publications);
|
||||
return agentRecordQueries(tx).getById(id);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user