From 1059166cb19aa4264142e62b748b7e62d2407481 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Sun, 28 Jun 2026 14:15:04 +0800 Subject: [PATCH] fix: address codex review round 2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - edit tool: add the read-before-edit requirement to the model-facing prompt (with the just-created/edited-this-session exception), matching write's guidance so the model doesn't only learn it via a failed FS_NOT_OBSERVED call. - fsspec-style-fs-seam RFC: correct the acceptance criteria that still claimed a ctx.fileContext service and a fileContext inject — the landed design is the fs/* event gate with the tool injecting fs. - filesystem-tool-schemas RFC: replace the stale "prior full file state" edit requirement with version-freshness wording (any windowed read authorizes a fresh edit; no partial-view flag). --- .../implemented/feature/2026-06-17-filesystem-tool-schemas.md | 2 +- .../simplification/2026-06-26-fsspec-style-fs-seam.md | 4 ++-- packages/fs/tool-fs/src/edit.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/rfc/implemented/feature/2026-06-17-filesystem-tool-schemas.md b/docs/rfc/implemented/feature/2026-06-17-filesystem-tool-schemas.md index 45928e9871..fa572d4a0b 100644 --- a/docs/rfc/implemented/feature/2026-06-17-filesystem-tool-schemas.md +++ b/docs/rfc/implemented/feature/2026-06-17-filesystem-tool-schemas.md @@ -62,7 +62,7 @@ Arguments: - `new_string: string` — required. Literal replacement text; an empty string deletes the match. - `replace_all?: boolean` — optional. Defaults to false. When false, `old_string` must identify exactly one match. -`edit` requires prior full file state derived from a previous read in the same execution context. `ctx.fs` derives the file-state owner and uses the recorded version as the stale guard. +`edit` requires a prior observation of the file in the same execution context (any windowed read counts — authorization is version freshness, not a full-view requirement), or a prior write/edit by that context. The `dsh-file-context` policy plugin derives the owner and supplies the recorded version as the stale guard; the provider's mutation lock enforces it. The first pass rejects Codex-style patch grammars and multi-mode edit APIs. It uses one strict literal replacement mode so the model-facing contract stays simple and the backend can own exact-match, duplicate-match, line-ending, and stale-version semantics. diff --git a/docs/rfc/implemented/simplification/2026-06-26-fsspec-style-fs-seam.md b/docs/rfc/implemented/simplification/2026-06-26-fsspec-style-fs-seam.md index 7f2e5d7091..26bbb65056 100644 --- a/docs/rfc/implemented/simplification/2026-06-26-fsspec-style-fs-seam.md +++ b/docs/rfc/implemented/simplification/2026-06-26-fsspec-style-fs-seam.md @@ -106,8 +106,8 @@ It keeps the interface/implementation/consumer discipline, consumer-never-import ## Acceptance Criteria - `dsh-fs` exposes exactly `resolve`/`stat`/`readText`/`streamText`/`writeText`/`editText`; `stat` returns `FsInfo | undefined`; `writeText` uses `FsWriteExpectation` (`createIfAbsent` or `replaceIfVersion`); removed types/primitives are gone, and the old `applyEdit` API is replaced by `editText`. -- `dsh-file-context` registers `ctx.fileContext`, owns observed-state plus `read`/`write`/`edit` policy, injects `fs`, and has HMR/disposal coverage. -- `dsh-tool-fs` injects `fileContext`; model-facing schemas stay byte-for-byte unchanged; the no-bypass contract and escape-hatch contract are documented and tested. +- `dsh-file-context` adds the observed-state + `read`/`write`/`edit` freshness policy and has HMR/disposal coverage. (It does so as a gate PLUGIN on the `fs/*` events with no `ctx.fileContext` service, per [the event-gate RFC](../architecture/2026-06-26-file-context-as-event-gate.md) — the original service form this RFC proposed was reworked.) +- `dsh-tool-fs` reaches the policy decisions and model-facing schemas stay byte-for-byte unchanged; the observation contract (a read records observed-state; a direct `ctx.fs` read does not) is documented and tested. (The tool injects `fs` and dispatches the `fs/*` events rather than injecting a `fileContext` service, per the event-gate RFC.) - Windowed read authorizing edit is shown to fail on the pre-refit code and pass after the refit. Existing version-CAS behavior is preserved with a regression test; it is not claimed as a pre-refit failure. An edit based on a stale read must report `FS_STALE_VERSION` before attempting literal matching. - `dsh-fs-local` carries no line, view, or `formatReadBody` logic; it does carry provider-level `editText` logic. - Docs and generated artifacts are updated: `docs/architecture.md`, `packages/README.md`, fs package READMEs, `docs/core-data-structures/filesystem.md`, affected `type-equiv` blocks and `scripts/type-equiv.manifest.json`, Cordis catalog, module graph, and doc references. diff --git a/packages/fs/tool-fs/src/edit.ts b/packages/fs/tool-fs/src/edit.ts index 5d70310502..d3725db1af 100644 --- a/packages/fs/tool-fs/src/edit.ts +++ b/packages/fs/tool-fs/src/edit.ts @@ -54,7 +54,7 @@ export function apply(ctx: Context): void { ctx.systemPrompt.section({ name: 'tool:edit', order: 102, - text: 'Use the edit tool for targeted changes to existing UTF-8 text files. It replaces literal old_string with new_string; by default old_string must appear exactly once. If old_string appears multiple times, provide a more specific old_string or set replace_all to true.', + text: 'Use the edit tool for targeted changes to existing UTF-8 text files. It replaces literal old_string with new_string; by default old_string must appear exactly once. If old_string appears multiple times, provide a more specific old_string or set replace_all to true. Read the file first (the default file-context policy requires it), unless you just created or edited it in this session.', }) ctx.tools.register(defineTool({