-
Notifications
You must be signed in to change notification settings - Fork 267
feat(file-safety): file version token for the guarded-write path (A1, #1375) #1383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
easonLiangWorldedtech
wants to merge
4
commits into
Zoo-Code-Org:main
Choose a base branch
from
easonLiangWorldedtech:feat/version-token-s1
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
131d18d
feat(file-safety): add version token for the guarded-write path (A1, …
easonliang28 13188d2
docs(file-safety): correct ino precision bounds in version token (A1,…
easonliang28 2c1582b
fix(file-safety): derive the version token from exact BigInt stats (A…
easonliang28 2a42f88
chore(ci): empty commit — re-trigger CI and the CodeRabbit current-he…
easonliang28 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import * as fs from "fs/promises" | ||
| import * as os from "os" | ||
| import * as path from "path" | ||
| import type { Stats } from "fs" | ||
| import { afterEach, beforeEach, describe, expect, it } from "vitest" | ||
|
|
||
| import { computeVersionToken, versionTokenOfStat } from "../versionToken" | ||
|
|
||
| // Stats is a class-backed interface without a public constructor, so a plain-object | ||
| // test double is the only practical way to pin the token format without real files. | ||
| // Last-resort double assertion (test-local, per AGENTS.md). | ||
| function makeStats(overrides: Partial<Stats> = {}): Stats { | ||
| const base: Partial<Stats> = { | ||
| dev: 7, | ||
| ino: 4242, | ||
| size: 1234, | ||
| atimeMs: 1_700_000_000_000, | ||
| mtimeMs: 1_700_000_000_123.456, | ||
| ctimeMs: 1_700_000_000_789.999, | ||
| birthtimeMs: 1_700_000_000_000, | ||
| } | ||
| return { ...base, ...overrides } as unknown as Stats | ||
| } | ||
|
|
||
| describe("versionTokenOfStat (A1, epic #1375)", () => { | ||
| it("is deterministic for an identical stat", () => { | ||
| expect(versionTokenOfStat(makeStats())).toBe(versionTokenOfStat(makeStats())) | ||
| }) | ||
|
|
||
| it("matches the documented dev:ino:size:mtimeNs:ctimeNs format", () => { | ||
| const expected = [ | ||
| "7", | ||
| "4242", | ||
| "1234", | ||
| Math.round(1_700_000_000_123.456 * 1e6).toString(), | ||
| Math.round(1_700_000_000_789.999 * 1e6).toString(), | ||
| ].join(":") | ||
| expect(versionTokenOfStat(makeStats())).toBe(expected) | ||
| }) | ||
|
|
||
| it("distinguishes size changes at identical timestamps", () => { | ||
| expect(versionTokenOfStat(makeStats({ size: 1235 }))).not.toBe(versionTokenOfStat(makeStats())) | ||
| }) | ||
|
|
||
| it("distinguishes mtime changes at identical size", () => { | ||
| expect(versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_124 }))).not.toBe(versionTokenOfStat(makeStats())) | ||
| }) | ||
|
|
||
| it("distinguishes a replaced file (dev/ino change) with identical content state", () => { | ||
| const replaced = makeStats({ dev: 8, ino: 999 }) | ||
| expect(versionTokenOfStat(replaced)).not.toBe(versionTokenOfStat(makeStats())) | ||
| }) | ||
|
|
||
| it("preserves sub-ms mtime resolution in the ns field", () => { | ||
| const wholeMs = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123 })) | ||
| const halfMsLater = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123.5 })) | ||
| expect(halfMsLater).not.toBe(wholeMs) | ||
| // 0.5 ms = 500_000 ns. The float-derived ns field is quantized (~256 ns at | ||
| // this epoch), so allow a bounded drift instead of asserting an exact value. | ||
| const diff = Number(halfMsLater.split(":")[3]) - Number(wholeMs.split(":")[3]) | ||
| expect(Math.abs(diff - 500_000)).toBeLessThanOrEqual(512) | ||
| }) | ||
|
|
||
| it("handles sizes beyond 32 bits without precision loss", () => { | ||
| const size = 5_000_000_000 // > 2^32 | ||
| const token = versionTokenOfStat(makeStats({ size })) | ||
| expect(token).toContain(`:4242:${size}:`) | ||
| }) | ||
| }) | ||
|
|
||
| describe("computeVersionToken (A1, epic #1375)", () => { | ||
| let tmpDir: string | ||
| let file: string | ||
|
|
||
| beforeEach(async () => { | ||
| tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "version-token-")) | ||
| file = path.join(tmpDir, "seed.txt") | ||
| await fs.writeFile(file, "seed content", "utf8") | ||
| }) | ||
|
|
||
| afterEach(async () => { | ||
| await fs.rm(tmpDir, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| it("derives the token from the on-disk state (single stat)", async () => { | ||
| const token = await computeVersionToken(file) | ||
| expect(token).toBe(versionTokenOfStat(await fs.stat(file))) | ||
| }) | ||
|
|
||
| it("changes when the file content changes", async () => { | ||
| const before = await computeVersionToken(file) | ||
| // Different size + a new mtime — both must move the token. | ||
| await fs.writeFile(file, "seed content, extended", "utf8") | ||
| await new Promise((resolve) => setTimeout(resolve, 5)) | ||
| expect(await computeVersionToken(file)).not.toBe(before) | ||
| }) | ||
|
|
||
| it("rejects with ENOENT for an absent file", async () => { | ||
| await expect(computeVersionToken(path.join(tmpDir, "absent.txt"))).rejects.toMatchObject({ | ||
| code: "ENOENT", | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { stat } from "fs/promises" | ||
| import type { Stats } from "fs" | ||
|
|
||
| /** | ||
| * Version token for the compare-and-swap write guard (upstream epic #1375, phase A1). | ||
| * | ||
| * A token is a pure function of a file's on-disk state, derived from a single | ||
| * `fs.stat`, so every process that observes the same file state (a second VS Code | ||
| * window, the CLI, the user's own editor tooling) computes the same token. The | ||
| * downstream guard phases (A2/A3) compare the token observed at read time with the | ||
| * token recomputed just before a write to detect "the file changed since the read" | ||
| * (stale) or "the file was replaced by a different file" (dev/ino change). | ||
| * | ||
| * Format: `dev:ino:size:mtimeNs:ctimeNs` | ||
| * | ||
| * Resolution note: Node exposes modification/change times as float milliseconds, | ||
| * so the ns fields are derived as `Math.round(mtimeMs * 1e6)`. The integer-to-double | ||
| * conversion is correctly rounded, so the derivation is deterministic across | ||
| * processes, but it is quantized by double precision (~256 ns at the current epoch). | ||
| * Two file states whose timestamps differ by less than the quantum derive the same | ||
| * ns field; in practice distinct states differ by at least the OS clock resolution | ||
| * (and no write workload produces mtimes closer than that), so the guard contract | ||
| * holds: same disk state → same token; changed state → a different token in all | ||
| * realistic cases. `dev` and `size` are exact integers. `ino` is Node's | ||
| * `number` (float64): exact for small POSIX inode numbers, but on modern Windows | ||
| * the underlying file ID exceeds 2^53, so Node's own value is already rounded — | ||
| * still deterministic per file (same file → same token), but not guaranteed | ||
| * injective across distinct files. Change detection therefore rests on size + | ||
| * mtime/ctime: any size change is always detected regardless of the timestamp | ||
| * quantum, and a replacement whose size and timestamps are indistinguishable is | ||
| * undetectable by any scheme reading the same Stats — the detect-and-reread | ||
| * stance (no lockfile) accepts that. | ||
| */ | ||
|
|
||
| /** Derive an ns-scale field from Node's float milliseconds (see module docs). */ | ||
| function nsFromMs(ms: number): string { | ||
| return Math.round(ms * 1e6).toString() | ||
| } | ||
|
|
||
| /** | ||
| * Build the version token from an already-fetched `Stats` — no I/O. | ||
| * | ||
| * Exported separately from {@link computeVersionToken} so tests can pin the exact | ||
| * format against synthetic stats. | ||
| */ | ||
| export function versionTokenOfStat(stats: Stats): string { | ||
| return [ | ||
| stats.dev.toString(), | ||
| stats.ino.toString(), | ||
| stats.size.toString(), | ||
| nsFromMs(stats.mtimeMs), | ||
| nsFromMs(stats.ctimeMs), | ||
| ].join(":") | ||
| } | ||
|
|
||
| /** | ||
| * Compute the version token for a file (one `fs.stat`). | ||
| * | ||
| * Rejects with the underlying ENOENT (or equivalent) error when the file is absent; | ||
| * how an unobservable target is treated is decided by the guard layer (A3). | ||
| */ | ||
| export async function computeVersionToken(filePath: string): Promise<string> { | ||
| return versionTokenOfStat(await stat(filePath)) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.