diff --git a/doc/AGENT-LIFECYCLE.md b/doc/AGENT-LIFECYCLE.md index 99601bea647..8da664e1a35 100644 --- a/doc/AGENT-LIFECYCLE.md +++ b/doc/AGENT-LIFECYCLE.md @@ -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. diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 8318e22e2c4..5727800977e 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -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 diff --git a/server/src/modules/agent-lifecycle/__tests__/environment-plugins.test.ts b/server/src/modules/agent-lifecycle/__tests__/environment-plugins.test.ts index c4b48cedfde..96a87845273 100644 --- a/server/src/modules/agent-lifecycle/__tests__/environment-plugins.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/environment-plugins.test.ts @@ -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 () => { diff --git a/server/src/modules/agent-lifecycle/adapters/configuration.ts b/server/src/modules/agent-lifecycle/adapters/configuration.ts index 38af7db7013..0af6a7ab986 100644 --- a/server/src/modules/agent-lifecycle/adapters/configuration.ts +++ b/server/src/modules/agent-lifecycle/adapters/configuration.ts @@ -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); }