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 <buruguo2000@163.com>

* chore: keep changelog release-owned

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
gucasbrg
2026-07-11 00:57:25 +08:00
committed by GitHub
parent 0b9c2e6f95
commit 2e742e3f8e
2 changed files with 103 additions and 56 deletions

View File

@@ -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 () => {});

View File

@@ -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<boolean> {
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,