mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(agents): keep a managed instructions bundle managed when a restarted volume-free server saves another setting (#20997)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
1949ac6788
commit
faa78dd818
3 files changed
+110
No files matched your search
@@ -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
|
||||
|
||||
@@ -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<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let db: ReturnType<typeof createDb>;
|
||||
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<string, unknown>, 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");
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user