Repository navigation
feat(read-file): record read scope and report clipping separately (U4, #1375) #1913
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
27
commits into
Zoo-Code-Org:main
Choose a base branch
from
easonLiangWorldedtech:fws/u4-read-scope-recording
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 all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
d5f8a79
split unit U1 of PR 1833 (issue 1375)
aa0cdab
fix(file-safety): close the pre-merge findings on the publish primiti…
c4120b0
fix(file-safety): propagate a non-ENOENT lstat failure in resolvePubl…
97b599d
fix(file-safety): keep the rollback pair typed and the mock stand-ins…
435be8b
rebuild unit u2 on the fixed chain
625a976
chore(lint): prune the safeWriteJson suppression this unit earns
4e2de13
rebuild unit u3 on the fixed chain
60376ca
fix(task): declare the observation registry on Task in this unit
77eb0a4
rebuild unit u4 on the fixed chain
d7eab3d
chore(lint): prune the readFileTool.spec suppression this unit earns
823acfe
fix(file-safety): inherit unit 1 committed guard and exact rmdir asse…
347c56d
fix(tools): stop the source indentation leaking into the clipped-line…
18f5c12
fix(file-safety): keep the publish error message in RollbackFailureError
08f281d
test(file-safety): assert the exact staging directory removed after a…
a1b9823
test: re-trigger required checks - the queued runs were cancelled by …
a9bc6a4
fix(file-safety): give the Windows DACL dump a per-write name
c1e4169
fix(tools): the clipping notice must describe the slice it actually r…
e7a5580
fix(file-safety): keep the target present while publishing (durable c…
31ffa4b
feat(utils): let a caller confine a write to a directory
467cfda
fix(file-safety): do not read a failed target lstat as a missing target
03c0725
fix(file-safety): compare staging and target identity with bigint stats
1a59f51
test(file-safety): pin the bigint options in the staging-identity tests
1331a91
fix(file-safety): report a Windows replacement whose DACL was not pre…
e412fce
fix(file-safety): keep DACL warning delivery from failing the save
bcd178c
fix(utils): check confinement before taking the advisory lock
244f4b6
fix(file-safety): handle async warning sinks and confine before mkdir
6fc470c
test(file-safety,utils): cover the commit-rename failure and unmock w…
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
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,108 @@ | ||
| import { describe, it, expect, vi } from "vitest" | ||
|
|
||
| import { ObservationRegistry } from "../observationRegistry" | ||
|
|
||
| describe("ObservationRegistry", () => { | ||
| it("observe → get returns the recorded version and observedAt", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "1:2:300:4000000000:5000000000") | ||
|
|
||
| const obs = reg.get("/a/b/c.ts") | ||
| expect(obs).toBeDefined() | ||
| expect(obs!.version).toBe("1:2:300:4000000000:5000000000") | ||
| expect(typeof obs!.observedAt).toBe("number") | ||
| }) | ||
|
|
||
| it("re-observe replaces the entry with a fresh observedAt", () => { | ||
| vi.useFakeTimers() | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "v1") | ||
| const first = reg.get("/a/b/c.ts")! | ||
| expect(first.version).toBe("v1") | ||
|
|
||
| vi.advanceTimersByTime(50) | ||
| reg.observe("/a/b/c.ts", "v2") | ||
| const second = reg.get("/a/b/c.ts")! | ||
| expect(second.version).toBe("v2") | ||
| expect(second.observedAt).toBeGreaterThan(first.observedAt) | ||
|
|
||
| vi.useRealTimers() | ||
| }) | ||
|
|
||
| it("has returns true for observed paths, false otherwise", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/x.ts", "t1") | ||
| expect(reg.has("/x.ts")).toBe(true) | ||
| expect(reg.has("/y.ts")).toBe(false) | ||
| }) | ||
|
|
||
| it("size reflects the number of observed entries", () => { | ||
| const reg = new ObservationRegistry() | ||
| expect(reg.size).toBe(0) | ||
| reg.observe("/a.ts", "t1") | ||
| reg.observe("/b.ts", "t2") | ||
| expect(reg.size).toBe(2) | ||
| }) | ||
|
|
||
| it("clear removes all entries and resets size to 0", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a.ts", "t1") | ||
| reg.observe("/b.ts", "t2") | ||
| reg.clear() | ||
| expect(reg.size).toBe(0) | ||
| expect(reg.get("/a.ts")).toBeUndefined() | ||
| expect(reg.has("/b.ts")).toBe(false) | ||
| }) | ||
|
|
||
| it("get on empty registry returns undefined", () => { | ||
| const reg = new ObservationRegistry() | ||
| expect(reg.get("/any.ts")).toBeUndefined() | ||
| }) | ||
|
|
||
| it("separate instances are independent — observing in one does not appear in the other", () => { | ||
| const regA = new ObservationRegistry() | ||
| const regB = new ObservationRegistry() | ||
| regA.observe("/shared.ts", "v1") | ||
| expect(regA.get("/shared.ts")).toBeDefined() | ||
| expect(regB.get("/shared.ts")).toBeUndefined() | ||
| regB.observe("/shared.ts", "v2") | ||
| expect(regA.get("/shared.ts")!.version).toBe("v1") | ||
| expect(regB.get("/shared.ts")!.version).toBe("v2") | ||
| }) | ||
|
|
||
| describe("completeness scope (S4b follow-up #46)", () => { | ||
| it("defaults to a complete observation when the read scope is not given", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "v1") | ||
|
|
||
| expect(reg.get("/a/b/c.ts")!.complete).toBe(true) | ||
| }) | ||
|
|
||
| it("records a partial observation when the read only returned a view of the file", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "v1", false) | ||
|
|
||
| expect(reg.get("/a/b/c.ts")!.complete).toBe(false) | ||
| }) | ||
|
|
||
| it("re-observing replaces the entry's completeness with the new read's scope", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "v1", false) | ||
| reg.observe("/a/b/c.ts", "v2") | ||
|
|
||
| const obs = reg.get("/a/b/c.ts")! | ||
| expect(obs.version).toBe("v2") | ||
| expect(obs.complete).toBe(true) | ||
| }) | ||
|
|
||
| it("re-observing with a partial scope downgrades a previously complete entry", () => { | ||
| const reg = new ObservationRegistry() | ||
| reg.observe("/a/b/c.ts", "v1") | ||
| reg.observe("/a/b/c.ts", "v2", false) | ||
|
|
||
| const obs = reg.get("/a/b/c.ts")! | ||
| expect(obs.version).toBe("v2") | ||
| expect(obs.complete).toBe(false) | ||
| }) | ||
| }) | ||
| }) |
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,59 @@ | ||
| /** | ||
| * Per-task file observation registry (upstream epic #1375, phase A2). | ||
| * | ||
| * Each Task owns its own instance so parent and subtask observations are | ||
| * independent. The S4 guarded-write will compare these versions against the | ||
| * token recomputed pre-write to detect stale reads or file replacement. | ||
| * | ||
| * Pure in-memory — zero I/O, no dependencies. The S4 guarded-write consults | ||
| * these observations for the version check and for the completeness check that | ||
| * gates a full-file replacement. | ||
| */ | ||
|
|
||
| export interface FileObservation { | ||
| /** Version token derived from on-disk fs.stat (bigint mode). */ | ||
| version: string | ||
| /** Millisecond timestamp when the observation was recorded. */ | ||
| observedAt: number | ||
| /** | ||
| * Whether the read that produced this observation returned the complete | ||
| * file. A slice, line-range, truncated, or indentation-block read returns | ||
| * only a view of the file; such an observation authorizes targeted edits | ||
| * on the view the model saw, but never a full-file replacement. | ||
| */ | ||
| complete: boolean | ||
| } | ||
|
|
||
| export class ObservationRegistry { | ||
| private readonly entries = new Map<string, FileObservation>() | ||
|
|
||
| /** | ||
| * Record an observation for a file at its absolute path. | ||
| * | ||
| * Re-observing replaces the entry with a fresh observedAt timestamp, the | ||
| * new version token, and the read's completeness. `complete` defaults to | ||
| * true for callers that read the whole file themselves (spec doubles, | ||
| * WriteToFileTool). A caller whose read is internal to a targeted edit must | ||
| * carry the model's prior completeness instead, so the tool's own read cannot | ||
| * upgrade a partial read into authority for a full-file replacement. | ||
| */ | ||
| observe(absolutePath: string, version: string, complete: boolean = true): void { | ||
| this.entries.set(absolutePath, { version, observedAt: Date.now(), complete }) | ||
| } | ||
|
|
||
| get(absolutePath: string): FileObservation | undefined { | ||
| return this.entries.get(absolutePath) | ||
| } | ||
|
|
||
| has(absolutePath: string): boolean { | ||
| return this.entries.has(absolutePath) | ||
| } | ||
|
|
||
| clear(): void { | ||
| this.entries.clear() | ||
| } | ||
|
|
||
| get size(): number { | ||
| return this.entries.size | ||
| } | ||
| } |
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
Oops, something went wrong.
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.