mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
fix(instructions): refresh canonical content at safe run boundaries
Use the committed database snapshot for native instruction bundles when disk projection is stale. Restage completed stopped working copies from the current head, while preserving live warm owners and original CAS fences for uncollected edits. Cover native stale projection, repeated stopped retries, resolved candidates, and no silent rebasing. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
98c4425e5b
commit
ca29ebb284
5 files changed
+134
-12
No files matched your search
@@ -89,6 +89,50 @@ describe("registered run instruction copies", () => {
|
||||
expect(unchangedRun.status).toBe(status);
|
||||
});
|
||||
|
||||
it.each(["saved", "unchanged"])("refreshes a stopped %s copy from the current canonical revision before retry", async (state) => {
|
||||
const copy = await run();
|
||||
if (state === "saved") await fs.writeFile(path.join(copy.localRoot, entryFile), "previous saved turn");
|
||||
expect((await copies.collectStopped({ companyId, runId: copy.runId }))?.state).toBe(state);
|
||||
const before = (await revisions.readCurrent(target(), board()))!;
|
||||
const current = await revisions.commit({ ...target(), entryFile, baseRevisionId: before.revision.id, content: "new board instructions", source: "api" }, board());
|
||||
const retried = (await copies.prepare({ ...target(), runId: copy.runId, cwd: home }))!;
|
||||
expect(retried).toMatchObject({ state: "prepared", baseRevisionId: current.revision.id, baseHash: current.revision.contentHash, processStoppedAt: null });
|
||||
expect(await fs.readFile(path.join(retried.localRoot, entryFile), "utf8")).toBe("new board instructions");
|
||||
await fs.writeFile(path.join(retried.localRoot, entryFile), "retry adds a change");
|
||||
expect((await copies.collectStopped({ companyId, runId: copy.runId }))?.state).toBe("saved");
|
||||
});
|
||||
|
||||
it("never rebases uncollected private bytes or a live warm copy onto a newer head", async () => {
|
||||
const stopped = await run();
|
||||
await copies.collectStopped({ companyId, runId: stopped.runId });
|
||||
await fs.writeFile(path.join(stopped.localRoot, entryFile), "uncollected private edit");
|
||||
const warm = await run();
|
||||
expect(await copies.hasChanges({ companyId, runId: warm.runId })).toBe(false);
|
||||
const prior = (await revisions.readCurrent(target(), board()))!;
|
||||
await revisions.commit({ ...target(), entryFile, baseRevisionId: prior.revision.id, content: "board change", source: "api" }, board());
|
||||
for (const copy of [stopped, warm]) {
|
||||
const retried = (await copies.prepare({ ...target(), runId: copy.runId, cwd: home }))!;
|
||||
expect(retried.baseRevisionId).toBe(copy.baseRevisionId);
|
||||
}
|
||||
expect(await fs.readFile(path.join(stopped.localRoot, entryFile), "utf8")).toBe("uncollected private edit");
|
||||
expect(await fs.readFile(path.join(warm.localRoot, entryFile), "utf8")).toBe(initial);
|
||||
expect((await copies.collectStopped({ companyId, runId: stopped.runId }))?.state).toBe("conflict");
|
||||
expect((await revisions.readCurrent(target(), board()))?.content).toBe("board change");
|
||||
});
|
||||
|
||||
it("reads an existing canonical runtime snapshot without seeding or changing instruction bytes", async () => {
|
||||
expect(await revisions.readCommittedForRuntime(target())).toBeNull();
|
||||
const copy = await run();
|
||||
await fs.writeFile(path.join(copy.localRoot, entryFile), "saved runtime content");
|
||||
await copies.collectStopped({ companyId, runId: copy.runId });
|
||||
const current = (await revisions.readCurrent(target(), board()))!;
|
||||
await fs.writeFile(path.join(root, entryFile), "stale disk projection");
|
||||
expect(await revisions.readCommittedForRuntime(target())).toEqual(current);
|
||||
expect(await fs.readFile(path.join(root, entryFile), "utf8")).toBe("stale disk projection");
|
||||
expect((await revisions.history({ ...target(), entryFile }, board())).revisions).toHaveLength(2);
|
||||
await expect(revisions.readCommittedForRuntime({ companyId: randomUUID(), agentId })).rejects.toThrow("Agent not found");
|
||||
});
|
||||
|
||||
it("preserves a competing stale candidate and requires explicit resolution against the new head", async () => {
|
||||
const first = await run();
|
||||
const second = await run();
|
||||
@@ -105,6 +149,10 @@ describe("registered run instruction copies", () => {
|
||||
await copies.resolve({ ...target(), runId: second.runId, baseRevisionId: current.revision.id, content: "explicitly combined" }, board());
|
||||
expect((await revisions.readCurrent(target(), board()))?.content).toBe("explicitly combined");
|
||||
expect((await copies.collectStopped({ companyId, runId: second.runId }))?.state).toBe("resolved");
|
||||
const resumed = (await copies.prepare({ ...target(), runId: second.runId, cwd: home }))!;
|
||||
const resolved = (await revisions.readCurrent(target(), board()))!;
|
||||
expect(resumed.baseRevisionId).toBe(resolved.revision.id);
|
||||
expect(await fs.readFile(path.join(resumed.localRoot, entryFile), "utf8")).toBe("explicitly combined");
|
||||
});
|
||||
|
||||
it("rechecks responsible-user permission at collection and preserves denied edits", async () => {
|
||||
|
||||
@@ -243,6 +243,15 @@ export function agentInstructionRevisionService(db: Db) {
|
||||
return row ? snapshot(row) : null;
|
||||
});
|
||||
}
|
||||
/** Trusted orchestration read for the already authorized run target. This
|
||||
* never seeds or writes, and is not exposed through HTTP or model tools. */
|
||||
async function readCommittedForRuntime(target: InstructionTarget): Promise<AgentInstructionSnapshot | null> {
|
||||
return db.transaction(async (tx) => {
|
||||
const state = await lockTarget(tx, target);
|
||||
const current = await head(tx, target, state.entryFile);
|
||||
return current ? snapshot(current) : null;
|
||||
});
|
||||
}
|
||||
/** Rebuild disk from the current committed head under the same lock as commits. Safe after restart. */
|
||||
async function materializeCurrent(target: InstructionTarget): Promise<void> {
|
||||
await db.transaction(async (tx) => {
|
||||
@@ -449,6 +458,7 @@ export function agentInstructionRevisionService(db: Db) {
|
||||
};
|
||||
}
|
||||
return {
|
||||
readCommittedForRuntime,
|
||||
readCurrent,
|
||||
readRevision,
|
||||
history,
|
||||
|
||||
@@ -90,6 +90,7 @@ export function agentInstructionWorkingCopyService(db: Db) {
|
||||
}
|
||||
async function prepare(input: { companyId: string; agentId: string; runId: string; target?: AdapterExecutionTarget | null; cwd: string }) {
|
||||
const existing = await get(input.companyId, input.runId);
|
||||
let refreshStoppedCopy = false;
|
||||
const workspace = await fs.realpath(input.cwd);
|
||||
const expectedLocalRoot = path.join(workspace, ".paperclip-runtime", `instruction-edits-${input.runId}`, "instructions");
|
||||
if (existing && existing.localRoot !== expectedLocalRoot) throw conflict("The registered instruction copy belongs to a different run workspace");
|
||||
@@ -97,25 +98,33 @@ export function agentInstructionWorkingCopyService(db: Db) {
|
||||
if (existing && (existing.agentId !== input.agentId || existing.location !== location)) {
|
||||
throw conflict("The registered instruction copy belongs to a different run environment");
|
||||
}
|
||||
// Same-run retries preserve the working bytes and original CAS baseline.
|
||||
// Preserve in-flight bytes and their CAS baseline. A completed, stopped copy
|
||||
// can start a fresh lifecycle only after its recorded bytes are accounted for.
|
||||
if (existing && existing.state !== "preparing") {
|
||||
if (["conflict", "pending_commit", "unavailable"].includes(existing.state)) {
|
||||
throw conflict("Resolve this run's preserved instruction candidate before editing its working copy again");
|
||||
}
|
||||
if (input.target) liveTargets.set(targetKey(input.companyId, input.runId), input.target);
|
||||
if (completed.has(existing.state) || existing.state === "unchanged_turn") {
|
||||
const revision = existing.receipt?.revision as { id?: string; contentHash?: string } | undefined;
|
||||
return patch(existing, { state: "prepared", baseRevisionId: revision?.id ?? existing.baseRevisionId,
|
||||
baseHash: revision?.contentHash ?? existing.baseHash, candidateBase64: null, candidateHash: null,
|
||||
receipt: null, processStoppedAt: null, attempts: 0, nextAttemptAt: null });
|
||||
if (completed.has(existing.state) && existing.processStoppedAt) {
|
||||
const privateBytes = await readCandidate(existing, input.target);
|
||||
// Refresh only bytes whose previous lifecycle is complete. A changed
|
||||
// private file must keep its old CAS fence; never silently rebase edits.
|
||||
refreshStoppedCopy = privateBytes === null || hash(privateBytes) === (existing.candidateHash ?? existing.baseHash);
|
||||
}
|
||||
if (!refreshStoppedCopy) {
|
||||
if (completed.has(existing.state) || existing.state === "unchanged_turn") {
|
||||
// unchanged_turn is a live warm-owner observation, not stop proof.
|
||||
return patch(existing, { state: "prepared", candidateBase64: null, candidateHash: null,
|
||||
receipt: null, processStoppedAt: null, attempts: 0, nextAttemptAt: null });
|
||||
}
|
||||
return existing;
|
||||
}
|
||||
return existing;
|
||||
}
|
||||
const [agent] = await db.select().from(agents).where(and(eq(agents.id, input.agentId), eq(agents.companyId, input.companyId)));
|
||||
if (!agent) throw notFound("Agent not found");
|
||||
if (agentInstructionsBundleMode(agent) !== "managed") return null;
|
||||
const bound = await resolveInstructionActor(db, { type: "agent", companyId: input.companyId, agentId: input.agentId, runId: input.runId });
|
||||
const baseline = existing?.baseRevisionId
|
||||
const baseline = existing?.baseRevisionId && !refreshStoppedCopy
|
||||
? await revisions.readRevision({ companyId: input.companyId, agentId: input.agentId, entryFile: existing.entryFile, revisionId: existing.baseRevisionId }, bound)
|
||||
: await revisions.readCurrent({ companyId: input.companyId, agentId: input.agentId }, bound);
|
||||
if (!baseline) return null;
|
||||
@@ -135,7 +144,12 @@ export function agentInstructionWorkingCopyService(db: Db) {
|
||||
localRoot, executionRoot, location, state: "preparing",
|
||||
});
|
||||
}
|
||||
const row = (await get(input.companyId, input.runId))!;
|
||||
let row = (await get(input.companyId, input.runId))!;
|
||||
if (refreshStoppedCopy) {
|
||||
row = await patch(row, { state: "preparing", entryFile, baseRevisionId: baseline.revision.id,
|
||||
baseHash: hash(content), candidateBase64: null, candidateHash: null, receipt: null,
|
||||
processStoppedAt: null, attempts: 0, nextAttemptAt: null, errorCode: null, errorMessage: null });
|
||||
}
|
||||
await assertInstructionPathSafe(localRoot, entryFile);
|
||||
await execFile(process.execPath, ["-e", instructionGitExcludeProgram, workspace], { timeout: 15_000 });
|
||||
await fs.mkdir(localRoot, { recursive: true, mode: 0o700 });
|
||||
|
||||
@@ -10,6 +10,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const serviceMocks = vi.hoisted(() => ({
|
||||
exportFiles: vi.fn(),
|
||||
readCommittedForRuntime: vi.fn(),
|
||||
getEffectiveProfilesForAgent: vi.fn(),
|
||||
githubBotConnectionIdsForRun: vi.fn(),
|
||||
}));
|
||||
@@ -20,6 +21,11 @@ vi.mock("../chat-github-tools.js", () => ({
|
||||
|
||||
vi.mock("../agent-instructions.js", () => ({
|
||||
agentInstructionsService: () => ({ exportFiles: serviceMocks.exportFiles }),
|
||||
agentInstructionsBundleMode: (agent: { adapterConfig?: { instructionsBundleMode?: string } }) => agent.adapterConfig?.instructionsBundleMode ?? null,
|
||||
}));
|
||||
|
||||
vi.mock("../agent-instruction-revisions.js", () => ({
|
||||
agentInstructionRevisionService: () => ({ readCommittedForRuntime: serviceMocks.readCommittedForRuntime }),
|
||||
}));
|
||||
|
||||
vi.mock("../tool-access.js", () => ({
|
||||
@@ -50,6 +56,7 @@ async function makeTreeWritable(target: string): Promise<void> {
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
serviceMocks.readCommittedForRuntime.mockResolvedValue(null);
|
||||
serviceMocks.githubBotConnectionIdsForRun.mockResolvedValue(new Set());
|
||||
previousPaperclipHome = process.env.PAPERCLIP_HOME;
|
||||
previousInstanceId = process.env.PAPERCLIP_INSTANCE_ID;
|
||||
@@ -90,6 +97,38 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("buildNativeRuntimeContext", () => {
|
||||
it("uses committed instructions when the disk projection is stale", async () => {
|
||||
serviceMocks.exportFiles.mockResolvedValue({
|
||||
entryFile: "AGENTS.md",
|
||||
files: { "AGENTS.md": "Old disk projection\n", "reference.md": "Sibling reference\n" },
|
||||
});
|
||||
serviceMocks.readCommittedForRuntime.mockResolvedValue({
|
||||
revision: { entryFile: "AGENTS.md" }, content: "Saved canonical instructions\n",
|
||||
});
|
||||
const context = await buildNativeRuntimeContext({
|
||||
db: {} as Db,
|
||||
agent: { id: "agent-1", companyId: "company-1", name: "Reviewer", adapterType: "paperclip_runner", adapterConfig: { instructionsBundleMode: "managed" } },
|
||||
runId: "run-1", runtimeConfig: {}, runtimeSkillEntries: [],
|
||||
});
|
||||
expect(await readFile(path.join(context.instructions.bundle.rootPath, context.instructions.entryPath), "utf8"))
|
||||
.toBe("Saved canonical instructions\n");
|
||||
expect(await readFile(path.join(context.instructions.bundle.rootPath, "reference.md"), "utf8"))
|
||||
.toBe("Sibling reference\n");
|
||||
expect(serviceMocks.readCommittedForRuntime).toHaveBeenCalledWith({ companyId: "company-1", agentId: "agent-1" });
|
||||
});
|
||||
|
||||
it("does not substitute canonical managed instructions into an external bundle", async () => {
|
||||
serviceMocks.exportFiles.mockResolvedValue({ entryFile: "AGENTS.md", files: { "AGENTS.md": "External instructions\n" } });
|
||||
const context = await buildNativeRuntimeContext({
|
||||
db: {} as Db,
|
||||
agent: { id: "agent-1", companyId: "company-1", name: "Reviewer", adapterConfig: { instructionsBundleMode: "external" } },
|
||||
runId: "run-1", runtimeConfig: {}, runtimeSkillEntries: [],
|
||||
});
|
||||
expect(await readFile(path.join(context.instructions.bundle.rootPath, context.instructions.entryPath), "utf8"))
|
||||
.toBe("External instructions\n");
|
||||
expect(serviceMocks.readCommittedForRuntime).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps the pinned prompt immutable while exposing the registered editable instruction copy", async () => {
|
||||
serviceMocks.exportFiles.mockResolvedValue({
|
||||
entryFile: "instructions/CHARter.md",
|
||||
|
||||
@@ -21,7 +21,8 @@ import {
|
||||
type NativeRuntimeContextSnapshot,
|
||||
} from "../../vendor/paperclip-runner/index.js";
|
||||
import { resolvePaperclipInstanceRoot } from "../../home-paths.js";
|
||||
import { agentInstructionsService } from "../agent-instructions.js";
|
||||
import { agentInstructionsService, agentInstructionsBundleMode } from "../agent-instructions.js";
|
||||
import { agentInstructionRevisionService } from "../agent-instruction-revisions.js";
|
||||
import { toolAccessService } from "../tool-access.js";
|
||||
import { filterResolvedGitHubConnectionsForRun } from "../git-credentials.js";
|
||||
|
||||
@@ -145,8 +146,18 @@ export async function materializeAsset(files: AssetFile[]): Promise<NativeRuntim
|
||||
return { schema: NATIVE_RUNTIME_ASSET_SCHEMA, digest: assetDigest, manifestDigest, rootPath, fileCount: manifestFiles.length, totalBytes };
|
||||
}
|
||||
|
||||
async function materializeInstructionBundle(agent: RuntimeAgent) {
|
||||
async function materializeInstructionBundle(db: Db, agent: RuntimeAgent) {
|
||||
const exported = await agentInstructionsService().exportFiles(agent, { rejectSymlinks: true });
|
||||
if (agentInstructionsBundleMode(agent) === "managed") {
|
||||
// The database head remains authoritative when a saved revision could not
|
||||
// update its disk projection. Overlay the exact committed snapshot directly;
|
||||
// a repair followed by a separate export can race another pending commit.
|
||||
const current = await agentInstructionRevisionService(db).readCommittedForRuntime({ companyId: agent.companyId, agentId: agent.id });
|
||||
if (current) {
|
||||
exported.entryFile = current.revision.entryFile;
|
||||
exported.files[current.revision.entryFile] = current.content;
|
||||
}
|
||||
}
|
||||
const entryPath = safeRelativePath(exported.entryFile, "instruction entry path");
|
||||
if (!(entryPath in exported.files)) throw new Error(`configured instruction entry is missing: ${entryPath}`);
|
||||
const files = Object.entries(exported.files).map(([relativePath, content]) => ({ path: safeRelativePath(relativePath, "instruction path"), content: Buffer.from(content, "utf8"), mode: 0o444 }));
|
||||
@@ -224,7 +235,7 @@ export async function resolveNativeRuntimeMcpSnapshot(input: { db: Db; agent: Pi
|
||||
|
||||
export async function buildNativeRuntimeContext(input: { db: Db; agent: RuntimeAgent; runId: string; runtimeConfig: Record<string, unknown>; runtimeSkillEntries: PaperclipSkillEntry[]; instructionWorkingCopy?: { rootPath: string; entryPath: string } }): Promise<NativeRuntimeContextSnapshot> {
|
||||
const [instructions, skills, mcp] = await Promise.all([
|
||||
materializeInstructionBundle(input.agent),
|
||||
materializeInstructionBundle(input.db, input.agent),
|
||||
materializeSelectedSkills(input.runtimeConfig, input.runtimeSkillEntries, input.agent.adapterType === "paperclip_runner"),
|
||||
resolveNativeRuntimeMcpSnapshot({ db: input.db, agent: input.agent, runId: input.runId }),
|
||||
]);
|
||||
|
||||
Reference in new issue
Block a user