From faa78dd818151ac2c5d38d096214177051b1fa72 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:06:51 -0600 Subject: [PATCH] fix(agents): keep a managed instructions bundle managed when a restarted volume-free server saves another setting (#20997) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent settings are saved through `PATCH /api/agents/:id`. On a volume-free server, a save is refused when the agent's instructions are external, because files outside the managed root do not survive a restart > - A managed bundle saves its instruction paths as absolute paths under the home of the server that wrote them. A replacement volume-free server can start with a new home > - After such a restart, any `adapterConfig` change re-derived the bundle mode from the saved `instructionsFilePath`. That path is outside the new managed root, so the bundle was read as `external` and the save failed with 422 "Volume-free servers require managed agent instructions" > - This pull request keeps a managed bundle managed when the save keeps its saved `instructionsFilePath`, and rewrites its paths under the current managed root > - The benefit is that a board user can change one setting (for example `idleTimeoutMs`) after a restart, and a real switch to external instructions is still refused ## Linked Issues or Issue Description **What happened?** In restart-chaos wave `w-20261112T012841-s7000` (volume-free mode, Daytona, Codex, restarts K1 and K15 with a fresh server home), the board sent `PATCH /api/agents/:id` with `{"adapterConfig":{"idleTimeoutMs":301001}}`. All 3 tasks of the W2C workload got 422 "Volume-free servers require managed agent instructions". **Expected behavior** A partial `adapterConfig` change of an agent with managed instructions succeeds on a restarted volume-free server. **Steps to reproduce** 1. Create an agent with a managed instructions bundle on a volume-free server. 2. Restart the server with a new `PAPERCLIP_HOME`. 3. Send `PATCH /api/agents/:id` with `{"adapterConfig":{"idleTimeoutMs":301001}}`. 4. The server returns 422. **Why Codex looked different from Claude** It is not the adapter. The Claude wave that passed (`w-20261112T010036-s6801`) had no faults, so its PATCH ran on the server that wrote the bundle. The Codex wave sent its PATCH after K1 and K15 restarts with a new home. The new test fails the same way for a Claude agent after a fresh-home restart, and passes for both adapters on the original server. **The PATCH contract** `adapterConfig` merges into the saved config unless the request sets `replaceAdapterConfig: true` (validator in `packages/shared/src/validators/agent.ts`, route in `server/src/routes/agents.ts`). The UI full-config save sends `replaceAdapterConfig: true` with the whole config. The merge was already correct. The fault was in `syncInstructionsBundleConfigFromFilePath`, which the route applies to the effective (merged) config: it classified the bundle only from the absolute file path and the current managed root. The contract does not change. ## What Changed - `server/src/services/agent-instructions.ts` (`syncInstructionsBundleConfigFromFilePath`): when the saved bundle is managed and the effective config keeps the saved `instructionsFilePath`, keep it managed with the root that `deriveBundleState` already resolves (the current server's managed root) and the saved entry file. A different `instructionsFilePath` is classified as before. - `server/src/__tests__/agent-volume-free-config-patch.test.ts`: route tests with embedded Postgres for a Codex agent and a Claude agent: a one-setting change after a fresh-home restart, the same change on the original server, and a real switch to an external path, which is still refused. - `doc/DEVELOPING.md`: describe how a volume-free server keeps a managed bundle across a new home. ## Verification - Red on `origin/master` (`9dfdeaad86`), `cd server && npx vitest run src/__tests__/agent-volume-free-config-patch.test.ts`: - `× changes one codex setting after a fresh-home restart and keeps managed instructions` - `× changes one claude setting after a fresh-home restart and keeps managed instructions` - `AssertionError: {"error":"Volume-free servers require managed agent instructions"}: expected 422 to be 200` - The 4 other cases passed (the original server, and the refused external switch). - Green with the fix: the same file passes 6/6, together with `agent-instructions-service`, `agent-instructions-routes`, `ephemeral-server-contract` and `agent-configuration-save` (65 tests). - The restart-chaos W2C workload (`tests/restart-chaos/rewake.ts`) already sends this PATCH after K1/K15 restarts and caught the fault as `T:config_changed_before_wake`. No new chaos cell is necessary. ## Risks - Low risk. The change applies only when the saved bundle is managed and the save keeps the saved `instructionsFilePath`. Then the paths are rewritten under the current managed root, which is the root that bundle reads already use. - The volume-free guard is unchanged. A save that sets an `instructionsFilePath` outside the managed root is still refused (tested for both adapters). ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, test runs). ## 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 --- doc/DEVELOPING.md | 9 ++ .../agent-volume-free-config-patch.test.ts | 97 +++++++++++++++++++ server/src/services/agent-instructions.ts | 4 + 3 files changed, 110 insertions(+) create mode 100644 server/src/__tests__/agent-volume-free-config-patch.test.ts diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index afc36029247..3a5b5aa1440 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -5168,6 +5168,15 @@ count is kept on the run's context so a resume claim does not reset it, and a Stop, a lost lease or a moved task leave it to normal settlement. A busy lock met while collecting files after a turn is not deferred this way. +Agent configuration writes refuse external instructions on a volume-free +server. An agent's managed instruction paths are saved as absolute paths under +the home of the server that wrote them, and each replacement server can have a +new home. When a configuration change keeps the saved `instructionsFilePath` +of a managed bundle, the bundle stays managed and its paths are rewritten under +the current server's managed root. A change that sets a different +`instructionsFilePath` is classified from that path and is still refused when +it points outside the managed root. + ### Runner controller state on a restarted volume-free server A volume-free server keeps Runner controller files under diff --git a/server/src/__tests__/agent-volume-free-config-patch.test.ts b/server/src/__tests__/agent-volume-free-config-patch.test.ts new file mode 100644 index 00000000000..5485d799af9 --- /dev/null +++ b/server/src/__tests__/agent-volume-free-config-patch.test.ts @@ -0,0 +1,97 @@ +import { randomUUID } from "node:crypto"; +import { mkdtemp, realpath, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import express from "express"; +import request from "supertest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { eq } from "drizzle-orm"; +import { agents, companies, companyMemberships, createDb } from "@paperclipai/db"; +import { agentRoutes } from "../routes/agents.js"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; + +const support = await getEmbeddedPostgresTestSupport(); +const describePostgres = support.supported ? describe : describe.skip; + +const adapters = [ + { name: "codex", adapterConfig: { provider: "codex", model: "gpt-5.6-sol" } }, + { name: "claude", adapterConfig: { provider: "acpx", acpxAgent: "claude", model: "claude-opus-5-5" } }, +] as const; + +describePostgres("volume-free agent configuration patch", () => { + let database: Awaited>; + let db: ReturnType; + let home: string; + let fixtureNumber = 0; + const userId = randomUUID(); + + beforeAll(async () => { + home = await realpath(await mkdtemp(path.join(os.tmpdir(), "paperclip-volume-free-patch-"))); + database = await startEmbeddedPostgresTestDatabase("paperclip-volume-free-patch-db-"); + db = createDb(database.connectionString); + }, 90_000); + afterEach(() => { vi.unstubAllEnvs(); vi.restoreAllMocks(); }); + afterAll(async () => { + await database?.cleanup(); + if (home) await rm(home, { recursive: true, force: true }); + }); + + function serveAs(companyId: string) { + const app = express(); app.use(express.json()); + app.use((req, _res, next) => { req.actor = { type: "board", source: "local_implicit", userId, companyIds: [companyId] }; next(); }); + app.use("/api", agentRoutes(db)); + app.use((error: { status?: number; message: string }, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + res.status(error.status ?? 500).json({ error: error.message }); + }); + return app; + } + + function useServerHome(generation: string) { + vi.stubEnv("PAPERCLIP_HOME", path.join(home, generation)); + vi.stubEnv("PAPERCLIP_INSTANCE_ID", "default"); + vi.stubEnv("PAPERCLIP_EPHEMERAL_SERVER", "true"); + } + + async function managedAgent(adapterConfig: Record, generation: string) { + const companyId = randomUUID(); + const agentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Volume-free patch", issuePrefix: `VF${++fixtureNumber}` }); + await db.insert(companyMemberships).values({ companyId, principalId: userId, principalType: "user", status: "active", membershipRole: "member" }); + const root = path.join(home, generation, "instances", "default", "companies", companyId, "agents", agentId, "instructions"); + const [agent] = await db.insert(agents).values({ id: agentId, companyId, name: "Volume-free agent", adapterType: "paperclip_runner", + adapterConfig: { ...adapterConfig, instructionsBundleMode: "managed", instructionsRootPath: root, + instructionsEntryFile: "AGENTS.md", instructionsFilePath: path.join(root, "AGENTS.md") } }).returning(); + return agent; + } + + const readConfig = async (id: string) => (await db.select().from(agents).where(eq(agents.id, id)))[0].adapterConfig; + + it.each(adapters)("changes one $name setting after a fresh-home restart and keeps managed instructions", async ({ adapterConfig }) => { + const agent = await managedAgent(adapterConfig, "gen-1"); + useServerHome("gen-2"); + const response = await request(serveAs(agent.companyId)).patch(`/api/agents/${agent.id}`).send({ adapterConfig: { idleTimeoutMs: 301_001 } }); + expect(response.status, JSON.stringify(response.body)).toBe(200); + const saved = await readConfig(agent.id); + const currentRoot = path.join(home, "gen-2", "instances", "default", "companies", agent.companyId, "agents", agent.id, "instructions"); + expect(saved).toMatchObject({ ...adapterConfig, idleTimeoutMs: 301_001, instructionsBundleMode: "managed", + instructionsRootPath: currentRoot, instructionsFilePath: path.join(currentRoot, "AGENTS.md") }); + }); + + it.each(adapters)("changes one $name setting on the server that wrote the bundle", async ({ adapterConfig }) => { + const agent = await managedAgent(adapterConfig, "gen-1"); + useServerHome("gen-1"); + const response = await request(serveAs(agent.companyId)).patch(`/api/agents/${agent.id}`).send({ adapterConfig: { idleTimeoutMs: 301_002 } }); + expect(response.status, JSON.stringify(response.body)).toBe(200); + expect(await readConfig(agent.id)).toMatchObject({ idleTimeoutMs: 301_002, instructionsBundleMode: "managed" }); + }); + + it.each(adapters)("still refuses a real switch of a $name agent to external instructions", async ({ adapterConfig }) => { + const agent = await managedAgent(adapterConfig, "gen-1"); + useServerHome("gen-2"); + const response = await request(serveAs(agent.companyId)).patch(`/api/agents/${agent.id}`) + .send({ adapterConfig: { instructionsFilePath: path.join(home, "outside", "AGENTS.md") } }); + expect(response.status, JSON.stringify(response.body)).toBe(422); + expect(response.body.error).toBe("Volume-free servers require managed agent instructions"); + expect((await readConfig(agent.id)).instructionsBundleMode).toBe("managed"); + }); +}); diff --git a/server/src/services/agent-instructions.ts b/server/src/services/agent-instructions.ts index 7403f547d3a..8606c4bc4da 100644 --- a/server/src/services/agent-instructions.ts +++ b/server/src/services/agent-instructions.ts @@ -508,6 +508,10 @@ export function syncInstructionsBundleConfigFromFilePath( delete next[ENTRY_KEY]; return next; } + const saved = deriveBundleState(agent); + if (saved.mode === "managed" && saved.rootPath && instructionsFilePath === asString(saved.config[FILE_KEY])) { + return applyBundleConfig(next, { mode: "managed", rootPath: saved.rootPath, entryFile: saved.entryFile }); + } const resolvedPath = resolveLegacyInstructionsPath(instructionsFilePath, adapterConfig); const managedRoot = resolveManagedInstructionsRoot(agent); const relative = path.relative(managedRoot, resolvedPath);