From 2d72f2c4c0b2f119db3b94ea75bd8ee8b70ed959 Mon Sep 17 00:00:00 2001 From: "commitperclip[bot]" <284812584+commitperclip[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:55:37 -0700 Subject: [PATCH] fix: Serialize ACPX session record saves per record to stop overlapping renames (EPERM on Windows) (#16925) > [!NOTE] > Opened by Prospector's automated issue-fix pipeline. A maintainer reviews and decides; nothing here merges on its own. ## Thinking Path - Issue #15346 reports a defect in paperclipai/paperclip. - Root cause, as the fixing agent read it: ACPX saves one session record from several writers (live checkpoint timer, turn end, finalize, close) with no coordination; each save renames a temp file onto the same record file, and on Windows overlapping renames onto one file fail with EPERM. All of these saves go through Paperclip's session-store wrapper, which forwarded them concurrently. - This pull request: Serialize ACPX session record saves per record to stop overlapping renames (EPERM on Windows) ## What Changed - `packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts`: Every ACPX save in Paperclip passes through this wrapper, so it now puts all saves of one record id in a single queue (a promise chain shared by all wrappers in the process). Saves of one record never overlap and keep their order; a failed save still returns its own error and does not block the next one; saves of different records still run in parallel. ## Verification - Reproducing test(s), added in this PR: `packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts` - They fail on the base commit and pass with the fix, each twice in a fresh sandbox. - 3 independent attempts wrote 3 reproductions; this fix passes every one of them. - The repository's compile command passes with the fix applied. - 3 existing test file(s) near the change pass. - The full test suite passes except for the 246 file(s) that already fail on the base commit. ## Risks - Written by an automated pipeline, not a person: review it as you would an outside contribution. - Risk tier of the touched paths: 2. - The scope reviewer found it also changes, beyond the report: save() called when no earlier save of that record is pending - The scope reviewer found it also changes, beyond the report: persistedStore.save throwing synchronously - The scope reviewer found it also changes, beyond the report: Records with missing/undefined acpxRecordId - The scope reviewer found it also changes, beyond the report: save() for different record ids ## Model Used - opus, sonnet (Anthropic Claude, via Claude Code), writing and checking the change in an isolated sandbox. ## Linked Issues Fixes #15346 Other pull requests that name #15346: - #15370 (closed): fix(acpx): serialize session file saves and clean failed temporary writes ## 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) - [ ] 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) - [ ] 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 - [ ] 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 - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: commitperclip[bot] <284812584+commitperclip[bot]@users.noreply.github.com> --- .../ephemeral-session-environment.test.ts | 41 +++++++++++++++++++ .../ephemeral-session-environment.ts | 19 ++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts b/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts index 40d13e7b403..ef0e834aa8b 100644 --- a/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts +++ b/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.test.ts @@ -162,4 +162,45 @@ describe("ephemeral ACPX session environment", () => { const input = freeze(record()); await expect(wrapper.save(input)).rejects.toBe(failure); expect(input.acpx?.session_options?.env).toEqual(currentEnvironment); }); + + it("never overlaps two saves of one record, even across wrappers, and keeps their order", async () => { + let active = 0, maxActive = 0; + const order: string[] = []; + const failure = new Error("fixture rename failure"); + const backing = stubStore(undefined); + backing.save.mockImplementation(async (saved) => { + active += 1; maxActive = Math.max(maxActive, active); + await new Promise(resolve => setTimeout(resolve, 5)); + order.push(saved.lastUsedAt); + active -= 1; + if (saved.lastUsedAt === "checkpoint") throw failure; + }); + const turnStore = createEphemeralSessionEnvironmentStore(backing, currentEnvironment); + const closeStore = createEphemeralSessionEnvironmentStore(backing, currentEnvironment); + const results = await Promise.allSettled([ + turnStore.save({ ...record(), lastUsedAt: "checkpoint" }), + turnStore.save({ ...record(), lastUsedAt: "turn-end" }), + closeStore.save({ ...record(), lastUsedAt: "close" }), + ]); + expect(maxActive).toBe(1); + expect(order).toEqual(["checkpoint", "turn-end", "close"]); + expect(results.map(result => result.status)).toEqual(["rejected", "fulfilled", "fulfilled"]); + expect((results[0] as PromiseRejectedResult).reason).toBe(failure); + }); + + it("does not serialize saves of different records", async () => { + let active = 0, maxActive = 0; + const backing = stubStore(undefined); + backing.save.mockImplementation(async () => { + active += 1; maxActive = Math.max(maxActive, active); + await new Promise(resolve => setTimeout(resolve, 5)); + active -= 1; + }); + const wrapper = createEphemeralSessionEnvironmentStore(backing, currentEnvironment); + await Promise.all([ + wrapper.save({ ...record(), acpxRecordId: "fixture-record-a" }), + wrapper.save({ ...record(), acpxRecordId: "fixture-record-b" }), + ]); + expect(maxActive).toBe(2); + }); }); diff --git a/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts b/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts index 3bd59942ec9..a4d068f9f6c 100644 --- a/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts +++ b/packages/adapter-utils/src/acpx-engine/ephemeral-session-environment.ts @@ -1,5 +1,22 @@ import type { AcpSessionStore } from "acpx/runtime"; +// ACPX saves one record from several writers (live checkpoint, turn end, +// finalize, close) with no lock. Each save renames a temp file onto the record +// file, and on Windows two overlapping renames onto one file fail with EPERM. +// Chain every save of one record so they never overlap. +const pendingSaves = new Map>(); + +function saveInOrder(recordId: string, save: () => Promise): Promise { + const previous = pendingSaves.get(recordId) ?? Promise.resolve(); + const current = previous.catch(() => undefined).then(save); + const settled = current.catch(() => undefined); + pendingSaves.set(recordId, settled); + void settled.then(() => { + if (pendingSaves.get(recordId) === settled) pendingSaves.delete(recordId); + }); + return current; +} + /** Keep the current run's environment in memory, outside persisted conversation state. */ export function createEphemeralSessionEnvironmentStore( persistedStore: AcpSessionStore, @@ -26,7 +43,7 @@ export function createEphemeralSessionEnvironmentStore( const { env: _environment, ...sessionOptions } = record.acpx.session_options; persisted.acpx = { ...record.acpx, session_options: sessionOptions }; } - return persistedStore.save(persisted); + return saveInOrder(record.acpxRecordId, () => persistedStore.save(persisted)); }, }; }