From d612ebaef15cf2ac1e5182489262d16b3ddfc6ee Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Fri, 26 Jun 2026 18:14:30 +0800 Subject: [PATCH] fix: address codex review round 3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the freshness token observed AFTER the read (re-stat post-read, falling back to the routing stat if the file vanished) so the version returned/recorded matches the bytes returned — a writer racing between the routing stat and the read can no longer make a follow-up edit spuriously stale. Stream reads when the backend reports no size, so a size-less backend never buffers a large file whole. Update the cordis-catalog link map to the current filesystem API symbols (FileContextExec/FileReadRequest/FileReadOutcome/FsInfo/FsWriteExpectation). --- docs/cordis-catalog/events-and-services.md | 4 +- packages/fs/file-context/src/index.ts | 19 ++++++-- packages/fs/file-context/tests/policy.spec.ts | 46 ++++++++++++++++++- scripts/gen-cordis-catalog.ts | 9 ++-- 4 files changed, 66 insertions(+), 12 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index c4896ec0ad..78f9f44324 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -351,7 +351,7 @@ async write(target: FsTarget, content: string, exec?: FileContextExec, signal?: async edit(target: FsTarget, edit: FsEditRequest, exec?: FileContextExec, signal?: AbortSignal): Promise ``` -Types: [FsEditOutcome](../core-data-structures/filesystem.md) · [FsEditRequest](../core-data-structures/filesystem.md) · [FsTarget](../core-data-structures/filesystem.md) · [FsWriteOutcome](../core-data-structures/filesystem.md) +Types: [FileContextExec](../core-data-structures/filesystem.md) · [FileReadOutcome](../core-data-structures/filesystem.md) · [FileReadRequest](../core-data-structures/filesystem.md) · [FsEditOutcome](../core-data-structures/filesystem.md) · [FsEditRequest](../core-data-structures/filesystem.md) · [FsTarget](../core-data-structures/filesystem.md) · [FsWriteOutcome](../core-data-structures/filesystem.md) Source: [`packages/fs/file-context/src/index.ts:65`](../../packages/fs/file-context/src/index.ts) @@ -376,7 +376,7 @@ abstract writeText(target: FsTarget, content: string, expected: FsWriteExpectati abstract editText(target: FsTarget, edit: FsEditRequest, expected: { version: FsVersion }, signal?: AbortSignal): Promise ``` -Types: [FsEditOutcome](../core-data-structures/filesystem.md) · [FsEditRequest](../core-data-structures/filesystem.md) · [FsTarget](../core-data-structures/filesystem.md) · [FsVersion](../core-data-structures/filesystem.md) · [FsWriteOutcome](../core-data-structures/filesystem.md) +Types: [FsEditOutcome](../core-data-structures/filesystem.md) · [FsEditRequest](../core-data-structures/filesystem.md) · [FsInfo](../core-data-structures/filesystem.md) · [FsTarget](../core-data-structures/filesystem.md) · [FsVersion](../core-data-structures/filesystem.md) · [FsWriteExpectation](../core-data-structures/filesystem.md) · [FsWriteOutcome](../core-data-structures/filesystem.md) Source: [`packages/fs/fs/src/index.ts:90`](../../packages/fs/fs/src/index.ts) diff --git a/packages/fs/file-context/src/index.ts b/packages/fs/file-context/src/index.ts index 6dbb33d1d2..c8ed44dde1 100644 --- a/packages/fs/file-context/src/index.ts +++ b/packages/fs/file-context/src/index.ts @@ -118,27 +118,36 @@ export class FileContext extends Service { /** * Read a bounded line window from a target. Stats first (rejecting an absent * target with `FS_NOT_FOUND` and a non-regular one with `FS_NOT_REGULAR_FILE`), - * chooses `readText` vs `streamText` by size, builds the window, and — when an - * owner is derivable — records the version so a later write/edit is authorized. + * chooses `readText` vs `streamText` by size — streaming when the size is + * large OR unknown so a size-less backend never buffers an arbitrarily large + * file — builds the window, then records the version observed AFTER the read + * so the recorded freshness token corresponds to the bytes actually returned + * (a writer racing between the routing stat and the read can't make a + * follow-up edit spuriously stale against a pre-read version). */ async read(target: FsTarget, request: FileReadRequest, exec?: FileContextExec, signal?: AbortSignal): Promise { const info = await this.ctx.fs.stat(target, signal) if (!info) throw new FsError(`cannot read "${target.displayPath}": not found`, 'FS_NOT_FOUND') if (info.type !== 'file') throw new FsError(`cannot read "${target.displayPath}": not a regular file`, 'FS_NOT_REGULAR_FILE') - const chunks = info.size !== undefined && info.size >= STREAM_MIN_SIZE + const chunks = info.size === undefined || info.size >= STREAM_MIN_SIZE ? await this.ctx.fs.streamText(target, signal) : [await this.ctx.fs.readText(target, signal)] const window = await buildWindow(chunks, request, target.displayPath) + // The version that matches the bytes just read: a stat taken after the read + // (falling back to the routing stat if the file vanished in the interim). + const after = await this.ctx.fs.stat(target, signal) + const version = after?.version ?? info.version + const owner = this.owner(exec) - if (owner) this.record(owner, target.targetKey, info.version) + if (owner) this.record(owner, target.targetKey, version) return { offset: request.offset, limit: request.limit, lines: window.lines, totalLines: window.totalLines, - version: info.version, + version, ...window.truncatedByBytes ? { truncatedByBytes: true } : {}, } } diff --git a/packages/fs/file-context/tests/policy.spec.ts b/packages/fs/file-context/tests/policy.spec.ts index e15398eb54..317776d5b3 100644 --- a/packages/fs/file-context/tests/policy.spec.ts +++ b/packages/fs/file-context/tests/policy.spec.ts @@ -27,6 +27,8 @@ class FakeFs extends FileSystem { versions = new Map() /** Size to report from stat (lets a test push read onto the streaming path). */ reportSize?: number + /** When true, stat omits `size` entirely (a size-less backend). */ + omitSize = false /** Whether streamText was used for the last read (vs readText). */ lastReadStreamed = false writeExpectations: FsWriteExpectation[] = [] @@ -47,7 +49,7 @@ class FakeFs extends FileSystem { override async stat(target: FsTarget): Promise { const content = this.files.get(target.targetKey) if (content === undefined) return undefined - return { version: this.ver(target.targetKey), type: 'file', size: this.reportSize ?? content.length } + return { version: this.ver(target.targetKey), type: 'file', ...this.omitSize ? {} : { size: this.reportSize ?? content.length } } } override async readText(target: FsTarget): Promise { this.lastReadStreamed = false @@ -154,6 +156,48 @@ describe('read', () => { expect(fs.lastReadStreamed).toBe(true) }) + it('streams when the backend reports no size (never buffers a size-less file)', async () => { + const { fs, fileContext } = await setup() + fs.files.set('a.txt', 'one\ntwo') + fs.omitSize = true + await fileContext.read(await fs.resolve('a.txt'), READ_ALL) + expect(fs.lastReadStreamed).toBe(true) + }) + + it('records the version observed after the read, not the routing stat', async () => { + const { fs, fileContext } = await setup() + const exec = ownerExec({}) + fs.files.set('a.txt', 'hello') + fs.versions.set('a.txt', 1) + const target = await fs.resolve('a.txt') + // A writer bumps the version after the routing stat but before the post-read stat. + const realReadText = fs.readText.bind(fs) + fs.readText = async (t) => { + const text = await realReadText(t) + fs.versions.set('a.txt', 5) // file changed during the read + return text + } + const outcome = await fileContext.read(target, READ_ALL, exec) + expect(outcome.version).toBe('v5') + // The recorded (post-read) version authorizes an edit without going stale. + await fileContext.edit(target, { oldString: 'hello', newString: 'bye', replaceAll: false }, exec) + expect(fs.editExpectedVersions).toEqual(['v5']) + }) + + it('falls back to the routing-stat version if the file vanishes after the read', async () => { + const { fs, fileContext } = await setup() + fs.files.set('a.txt', 'hello') + const target = await fs.resolve('a.txt') + const realReadText = fs.readText.bind(fs) + fs.readText = async (t) => { + const text = await realReadText(t) + fs.files.delete('a.txt') // vanishes → post-read stat returns undefined + return text + } + const outcome = await fileContext.read(target, READ_ALL) + expect(outcome.version).toBe('v0') // the routing-stat version + }) + it('surfaces truncatedByBytes when the window hits the byte cap', async () => { const { fs, fileContext } = await setup() fs.files.set('big.txt', Array.from({ length: 2000 }, () => 'y'.repeat(100)).join('\n')) diff --git a/scripts/gen-cordis-catalog.ts b/scripts/gen-cordis-catalog.ts index 41f8830335..4d2598155b 100644 --- a/scripts/gen-cordis-catalog.ts +++ b/scripts/gen-cordis-catalog.ts @@ -77,13 +77,14 @@ const LINK_MAP: Record = { BashTaskRead: 'bash.md', FsEditOutcome: 'filesystem.md', FsEditRequest: 'filesystem.md', - FsExecContext: 'filesystem.md', - FsExpectation: 'filesystem.md', - FsReadOutcome: 'filesystem.md', - FsReadRequest: 'filesystem.md', + FsInfo: 'filesystem.md', FsTarget: 'filesystem.md', FsVersion: 'filesystem.md', + FsWriteExpectation: 'filesystem.md', FsWriteOutcome: 'filesystem.md', + FileContextExec: 'filesystem.md', + FileReadRequest: 'filesystem.md', + FileReadOutcome: 'filesystem.md', } /** One harness event, extracted from an `interface Events` block. */