From 2e742e3f8ec3e38c2be552b5ee07bc13b161767a Mon Sep 17 00:00:00 2001 From: gucasbrg Date: Sat, 11 Jul 2026 00:57:25 +0800 Subject: [PATCH] fix(embedded-agent-runner): treat the run's own no-op in-place session rewrite as benign (#91797) * fix(agents): accept byte-identical session rewrites Co-authored-by: gucasbrg * chore: keep changelog release-owned --------- Co-authored-by: Peter Steinberger --- .../run/attempt.session-lock.test.ts | 122 ++++++++++++------ .../run/attempt.session-lock.ts | 37 +++--- 2 files changed, 103 insertions(+), 56 deletions(-) diff --git a/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts b/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts index 71bd3042d9b2..a11e84ff6d4b 100644 --- a/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts +++ b/src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts @@ -2016,6 +2016,36 @@ describe("embedded attempt session lock lifecycle", () => { expect(release).toHaveBeenCalledTimes(2); }); + it("allows a byte-identical in-place rewrite while the prompt lock is released", async () => { + const sessionFile = await createTempSessionFile(); + const oldTime = new Date("2020-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, oldTime, oldTime); + const before = await fs.stat(sessionFile, { bigint: true }); + const originalBytes = await fs.readFile(sessionFile); + const release = vi.fn(async () => {}); + const acquireSessionWriteLockLocalIdenticalRewrite = vi.fn(async () => ({ release })); + const controller = await createEmbeddedAttemptSessionLockController({ + acquireSessionWriteLock: acquireSessionWriteLockLocalIdenticalRewrite, + lockOptions: { ...lockOptions, sessionFile }, + }); + + await controller.releaseForPrompt(); + await fs.writeFile(sessionFile, originalBytes); + const after = await fs.stat(sessionFile, { bigint: true }); + + expect(after.dev).toBe(before.dev); + expect(after.ino).toBe(before.ino); + expect(after.size).toBe(before.size); + expect(after.mtimeNs).not.toBe(before.mtimeNs); + expect(await fs.readFile(sessionFile)).toEqual(originalBytes); + await expect(controller.reacquireAfterPrompt()).resolves.toBeUndefined(); + expect(controller.hasSessionTakeover()).toBe(false); + expect(acquireSessionWriteLockLocalIdenticalRewrite).toHaveBeenCalledTimes(2); + expect(release).toHaveBeenCalledTimes(1); + await controller.dispose(); + expect(release).toHaveBeenCalledTimes(2); + }); + it("trusts owned writes after accepting ctime-only fingerprint drift", async () => { const sessionFile = await createTempSessionFile(); const release = vi.fn(async () => {}); @@ -2064,39 +2094,38 @@ describe("embedded attempt session lock lifecycle", () => { expect(release).toHaveBeenCalledTimes(3); }); - it("allows ctime-only fingerprint drift for large transcript snapshots", async () => { + it("allows metadata drift for digest-backed transcript snapshots", async () => { const sessionFile = await createTempSessionFile(); await fs.writeFile(sessionFile, Buffer.alloc(8 * 1024 * 1024 + 1, "x")); + const oldTime = new Date("2020-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, oldTime, oldTime); + const before = await fs.stat(sessionFile, { bigint: true }); const release = vi.fn(async () => {}); - const acquireSessionWriteLockLocalLargeCtimeDrift = vi.fn(async () => ({ release })); + const acquireSessionWriteLockLocalDigestDrift = vi.fn(async () => ({ release })); const controller = await createEmbeddedAttemptSessionLockController({ - acquireSessionWriteLock: acquireSessionWriteLockLocalLargeCtimeDrift, + acquireSessionWriteLock: acquireSessionWriteLockLocalDigestDrift, lockOptions: { ...lockOptions, sessionFile }, }); await controller.releaseForPrompt(); + const newTime = new Date("2021-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, newTime, newTime); + const after = await fs.stat(sessionFile, { bigint: true }); - const stableStat = await fs.stat(sessionFile, { bigint: true }); - const driftedStat = cloneBigIntStatWith(stableStat, { - ctimeNs: stableStat.ctimeNs + 1_000_000n, - }); - const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (target, options) => { - if (target === sessionFile && options?.bigint === true) { - return driftedStat; - } - throw new Error(`unexpected stat call for ${String(target)}`); - }); - - try { - await expect(controller.withSessionWriteLock(() => "finalize")).resolves.toBe("finalize"); - } finally { - statSpy.mockRestore(); - } + expect(after.dev).toBe(before.dev); + expect(after.ino).toBe(before.ino); + expect(after.size).toBe(before.size); + expect(after.mtimeNs).not.toBe(before.mtimeNs); + await expect(controller.reacquireAfterPrompt()).resolves.toBeUndefined(); expect(controller.hasSessionTakeover()).toBe(false); + await controller.dispose(); }); - it("rejects same-size transcript rewrites with restored mtime", async () => { + it("rejects same-inode, same-size transcript rewrites when one byte changes", async () => { const sessionFile = await createTempSessionFile(); + const oldTime = new Date("2020-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, oldTime, oldTime); + const before = await fs.stat(sessionFile, { bigint: true }); const release = vi.fn(async () => {}); const acquireSessionWriteLockLocalSameSizeRewrite = vi.fn(async () => ({ release })); const controller = await createEmbeddedAttemptSessionLockController({ @@ -2105,31 +2134,48 @@ describe("embedded attempt session lock lifecycle", () => { }); await controller.releaseForPrompt(); - - const stableStat = await fs.stat(sessionFile, { bigint: true }); await fs.writeFile(sessionFile, '{"type":"sessioN"}\n', "utf8"); - const driftedStat = cloneBigIntStatWith(stableStat, { - ctimeNs: stableStat.ctimeNs + 1_000_000n, - }); - const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (target, options) => { - if (target === sessionFile && options?.bigint === true) { - return driftedStat; - } - throw new Error(`unexpected stat call for ${String(target)}`); - }); + const after = await fs.stat(sessionFile, { bigint: true }); - try { - await expect(controller.withSessionWriteLock(() => "finalize")).rejects.toBeInstanceOf( - EmbeddedAttemptSessionTakeoverError, - ); - } finally { - statSpy.mockRestore(); - } + expect(after.dev).toBe(before.dev); + expect(after.ino).toBe(before.ino); + expect(after.size).toBe(before.size); + await expect(controller.reacquireAfterPrompt()).rejects.toBeInstanceOf( + EmbeddedAttemptSessionTakeoverError, + ); expect(controller.hasSessionTakeover()).toBe(true); expect(acquireSessionWriteLockLocalSameSizeRewrite).toHaveBeenCalledTimes(2); expect(release).toHaveBeenCalledTimes(2); }); + it("fails closed when a metadata-only snapshot is too large to verify", async () => { + const sessionFile = await createTempSessionFile(); + await fs.truncate(sessionFile, 32 * 1024 * 1024 + 1); + const oldTime = new Date("2020-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, oldTime, oldTime); + const before = await fs.stat(sessionFile, { bigint: true }); + const release = vi.fn(async () => {}); + const acquireSessionWriteLockLocalOversizeDrift = vi.fn(async () => ({ release })); + const controller = await createEmbeddedAttemptSessionLockController({ + acquireSessionWriteLock: acquireSessionWriteLockLocalOversizeDrift, + lockOptions: { ...lockOptions, sessionFile }, + }); + + await controller.releaseForPrompt(); + const newTime = new Date("2021-01-01T00:00:00.000Z"); + await fs.utimes(sessionFile, newTime, newTime); + const after = await fs.stat(sessionFile, { bigint: true }); + + expect(after.dev).toBe(before.dev); + expect(after.ino).toBe(before.ino); + expect(after.size).toBe(before.size); + expect(after.mtimeNs).not.toBe(before.mtimeNs); + await expect(controller.reacquireAfterPrompt()).rejects.toBeInstanceOf( + EmbeddedAttemptSessionTakeoverError, + ); + expect(controller.hasSessionTakeover()).toBe(true); + }); + it("still rejects external edits after the prompt stream lock is reacquired", async () => { const sessionFile = await createTempSessionFile(); const release = vi.fn(async () => {}); diff --git a/src/agents/embedded-agent-runner/run/attempt.session-lock.ts b/src/agents/embedded-agent-runner/run/attempt.session-lock.ts index e7e4ce9d7316..ddfe635625b0 100644 --- a/src/agents/embedded-agent-runner/run/attempt.session-lock.ts +++ b/src/agents/embedded-agent-runner/run/attempt.session-lock.ts @@ -132,13 +132,13 @@ const MAX_BENIGN_SESSION_FENCE_ADVANCE_BYTES = 1024 * 1024; const MAX_BENIGN_SESSION_FENCE_REWRITE_BYTES = 8 * 1024 * 1024; const MAX_BENIGN_SESSION_FENCE_REWRITE_RESULT_BYTES = MAX_BENIGN_SESSION_FENCE_REWRITE_BYTES + MAX_BENIGN_SESSION_FENCE_ADVANCE_BYTES; -const MAX_BENIGN_SESSION_FENCE_CTIME_DIGEST_BYTES = 32 * 1024 * 1024; +const MAX_BENIGN_SESSION_FENCE_CONTENT_DIGEST_BYTES = 32 * 1024 * 1024; const MAX_SAFE_FILE_OFFSET = BigInt(Number.MAX_SAFE_INTEGER); type SessionFileFenceSnapshot = { fingerprint: SessionFileFingerprint; + bytes?: Buffer; digest?: string; - text?: string; }; function sameSessionFileFingerprint( @@ -167,7 +167,7 @@ function sameSessionFileIdentity( return Boolean(left?.exists && right.exists && left.dev === right.dev && left.ino === right.ino); } -function sameSessionFileContentMetadata( +function sameSessionFileIdentityAndSize( left: SessionFileFingerprint | undefined, right: SessionFileFingerprint, ): boolean { @@ -176,8 +176,7 @@ function sameSessionFileContentMetadata( right.exists && left.dev === right.dev && left.ino === right.ino && - left.size === right.size && - left.mtimeNs === right.mtimeNs, + left.size === right.size, ); } @@ -614,13 +613,13 @@ async function readSessionFileFenceSnapshot( try { return { fingerprint, - text: await fs.readFile(sessionFile, "utf8"), + bytes: await fs.readFile(sessionFile), }; } catch { return { fingerprint }; } } - if (fingerprint.size > BigInt(MAX_BENIGN_SESSION_FENCE_CTIME_DIGEST_BYTES)) { + if (fingerprint.size > BigInt(MAX_BENIGN_SESSION_FENCE_CONTENT_DIGEST_BYTES)) { return { fingerprint }; } return { @@ -729,28 +728,30 @@ async function classifyOwnedSessionFileInitialization(params: { : resolvedChange; } -async function sessionFenceCtimeDriftIsBenign(params: { +async function sessionFenceByteIdenticalRewriteIsBenign(params: { sessionFile: string; previous: SessionFileFenceSnapshot | undefined; current: SessionFileFingerprint; }): Promise { + const previous = params.previous; if ( - !sameSessionFileContentMetadata(params.previous?.fingerprint, params.current) || - params.previous?.fingerprint.exists !== true || + previous?.fingerprint.exists !== true || !params.current.exists || - params.previous.fingerprint.ctimeNs === params.current.ctimeNs + !sameSessionFileIdentityAndSize(previous.fingerprint, params.current) ) { return false; } - if (params.previous.text === undefined) { - if (params.previous.digest === undefined) { + // Truncate-and-rewrite keeps inode and size while advancing timestamps. + // Only exact bytes may make that metadata-only fence drift trustworthy. + if (previous.bytes === undefined) { + if (previous.digest === undefined) { return false; } const currentDigest = await readSessionFileDigest(params.sessionFile); - return currentDigest !== undefined && currentDigest === params.previous.digest; + return currentDigest !== undefined && currentDigest === previous.digest; } try { - return (await fs.readFile(params.sessionFile, "utf8")) === params.previous.text; + return previous.bytes.equals(await fs.readFile(params.sessionFile)); } catch { return false; } @@ -766,7 +767,7 @@ async function classifySessionFenceRewrite(params: { if ( !params.previous?.fingerprint.exists || !params.current.exists || - !params.previous.text || + params.previous.bytes === undefined || !sameSessionFileIdentity(params.previous.fingerprint, params.current) || (!params.allowAnyMessage && params.current.size > BigInt(MAX_BENIGN_SESSION_FENCE_REWRITE_RESULT_BYTES)) || @@ -783,7 +784,7 @@ async function classifySessionFenceRewrite(params: { if (!currentText.endsWith("\n")) { return undefined; } - const previousLines = splitSessionFileLines(params.previous.text); + const previousLines = splitSessionFileLines(params.previous.bytes.toString("utf8")); const currentLines = splitSessionFileLines(currentText); if (currentLines.length <= previousLines.length) { return undefined; @@ -1492,7 +1493,7 @@ export async function createEmbeddedAttemptSessionLockController(params: { } if ( - await sessionFenceCtimeDriftIsBenign({ + await sessionFenceByteIdenticalRewriteIsBenign({ sessionFile: params.lockOptions.sessionFile, previous: fenceSnapshot, current,