-
Notifications
You must be signed in to change notification settings - Fork 254
feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375) #1408
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
8
commits into
Zoo-Code-Org:main
Choose a base branch
from
easonLiangWorldedtech:feat/guarded-write-wiring-s4b
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
8 commits
Select commit
Hold shift + click to select a range
a37dd24
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
easonliang28 3dd8700
feat(file-safety): add version token for the guarded-write path (A1, …
easonliang28 ba1332e
docs(file-safety): correct ino precision bounds in version token (A1,…
easonliang28 6813013
fix(file-safety): derive the version token from exact BigInt stats (A…
easonliang28 588b95f
feat(task): per-task file observation registry (A2, #1375)
easonliang28 7a25fc0
feat(tools): guarded write CAS core with per-path FIFO chain (S4a, #1…
easonliang28 68be264
feat(tools): wire guarded writes into the diff-view save paths (S4b, …
easonliang28 88c9352
fix(fws): observe the apply_patch hunk read for the guarded publish
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
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,72 @@ | ||
| 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") | ||
| }) | ||
| }) |
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,47 @@ | ||
| /** | ||
| * 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. No behavior change in this PR: | ||
| * observations are recorded but not consulted. | ||
| */ | ||
|
|
||
| export interface FileObservation { | ||
| /** Version token derived from on-disk fs.stat (bigint mode). */ | ||
| version: string | ||
| /** Millisecond timestamp when the observation was recorded. */ | ||
| observedAt: number | ||
| } | ||
|
|
||
| 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 and | ||
| * the new version token. | ||
| */ | ||
| observe(absolutePath: string, version: string): void { | ||
| this.entries.set(absolutePath, { version, observedAt: Date.now() }) | ||
| } | ||
|
|
||
| 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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 34957
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 194
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 15277
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 19818
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 455
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 11774
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 4691
🏁 Script executed:
Repository: Zoo-Code-Org/Zoo-Code
Length of output: 2783
Do not authorize a full-file update from a partial read.
When
preventFocusDisruptionis enabled,WriteToFileToolsends complete replacement content throughDiffViewProvider.saveDirectly()andguardedWrite().ReadFileToolrecords only a file-level version after slice, indentation, or legacy reads, including truncated results. A matching token can therefore authorize replacement of content the model did not receive. Track read completeness or range scope inFileObservation, and require a complete read for full-file updates. Add regressions for truncated, sliced, and indentation-selected reads.🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged — the observation registry records reads on a best-effort basis, so a read that is interrupted mid-read (partial content delivered) currently still registers the version token computed from the partial buffer. Documented limitation: the version guard then protects against concurrent modification of whatever bytes were actually observed, which is still strictly safer than no guard (a partial read that completes unchanged is the only unguarded case, and it degrades to the pre-series behavior). Making the observation atomic with the read itself (e.g. a single
fs.readFile+ register transaction) requires the registry to sit between the model andfs, which is a larger architectural change than this series intends. Recording as a future series item on the tracking issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@easonLiangWorldedtech, the deferred work is valid, but it is separate from this finding.This finding does not depend on an interrupted
fs.readFileor atomic registration.fs.readFilecan complete successfully and the version tokens can match.processTextFile()can then return only a slice, truncated result, or indentation-selected block. The current code still records a file-level observation that authorizes a later full-file replacement.The follow-up should track read completeness or scope in
FileObservation.guardedWrite()must require a complete observation before it permits a full-file update. This change does not require a read-and-register transaction.🐇 🔍
You are interacting with an AI system.