Skip to content

feat(sandbox): add edit_file (str_replace) tool for partial edits — scaffold - #1533

Draft
iroiro147 wants to merge 2 commits into
vercel:mainfrom
iroiro147:feat/1523-edit-file-tool
Draft

feat(sandbox): add edit_file (str_replace) tool for partial edits — scaffold#1533
iroiro147 wants to merge 2 commits into
vercel:mainfrom
iroiro147:feat/1523-edit-file-tool

Conversation

@iroiro147

Copy link
Copy Markdown
Contributor

What

Scaffold PR for #1523: adds a framework edit_file tool that performs a single unique substring replacement, avoiding the token cost of full-file write_file regeneration.

Changes (139 LOC, 3 files)

File Change
runtime/framework-tools/edit-file.ts (NEW, 129 LOC) Zod input/output schemas, executeEditFileOnSandbox executor with unique-substring guarantee, EDIT_FILE_TOOL_DEFINITION definition
runtime/framework-tools/index.ts (+2) Import + register EDIT_FILE_TOOL_DEFINITION alongside existing tools
public/tools/defaults.ts (+12) Export editFile public tool surface matching readFile/writeFile pattern

Behavior

  • edit_file({filePath, oldString, newString})
  • Reads file → enforces oldString appears exactly once → replaces → writes back
  • Throws descriptive error when oldString is missing, empty, or >1 occurrence
  • Returns {path, replacements: 1}

Design tradeoffs (intentionally deferred to maintainer review)

  • Atomicity: read-then-write is one sandbox call (not two round trips). Rationale: agent already has file contents in context; the race window is bounded by turn-time.
  • Failure signaling: descriptive errors ("not found", "ambiguous match") let the agent retry with more context rather than silent corruption. Mirrored from Claude Code's Edit tool behavior.
  • API surface: only uses existing SandboxSession primitives — no new deps.

Verification

  • Local typecheck: not run (no node_modules in this workspace). npx tsc --noEmit in a dev checkout is the verification gate.
  • Code review only. Maintainers: if design shape looks right, this is a clean scaffold to extend with tests/telemetry.

Closes #1523 (design-shaped; whether this is THE implementation or a starting point is maintainer call)

Scaffold for vercel#1523. Adds a framework `edit_file` tool that replaces one
unique substring with a new one — a lightweight partial-edit primitive
that avoids the token cost of regenerating entire files via `write_file`.

Behavior:
- Reads file, verifies `oldString` appears exactly once, replaces, writes back.
- Fails with descriptive error when `oldString` is missing, empty, or matches > 1 position.
- Returns `{ path, replacements: 1 }`.

Files changed:
- `runtime/framework-tools/edit-file.ts` (NEW) — zod input/output schemas,
  executor, tool definition, unique-substring guarantee via occurrence count.
- `runtime/framework-tools/index.ts` — register EDIT_FILE_TOOL_DEFINITION.
- `public/tools/defaults.ts` — export `editFile` alongside existing readFile/writeFile.

Notes / caveats for reviewers:
- Uses only existing sandbox session primitives (`readTextFile`, `writeTextFile`)
  and `resolveAbsoluteFilePath` — no new deps, no new API surface.
- Typecheck not run in this commit's context (no node_modules in this workspace);
  CI `tsc --noEmit` should be the verification gate.
- This PR is a SCAFFOLD: maintainer decisions on (a) maximum diff size,
  (b) atomicity (read-+write within sandbox exec vs split tools),
  (c) telemetry/trace surface for edit_file's unique-substring failures,
  are intentionally deferred to design review.

Related: vercel#1523
Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

@iroiro147 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

Comment thread packages/eve/src/runtime/framework-tools/edit-file.ts
Comment thread packages/eve/src/runtime/framework-tools/edit-file.ts Outdated
…r patterns

Two VADE review findings on PR vercel#1533:

1. edit_file wrote without refreshing the read-file stamp, so a
   subsequent write_file on the same file spuriously failed with
   "File has been modified since it was last read". Now refreshes the
   stamp after a successful write, mirroring executeWriteFileOnSandbox.

2. String.replace(oldString, newString) interpreted $$, $&, $`, $', $n
   in newString as special replacement patterns, silently corrupting the
   output. Now passes a replacer function so newString is inserted
   literally.

Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
@iroiro147

Copy link
Copy Markdown
Contributor Author

Both VADE findings fixed in a8919ad:

  1. read-file stamp refresh — executeEditFileOnSandbox now refreshes the read-file stamp after a successful write, so a subsequent write_file on the same file no longer fails with "modified since it was last read". Mirrors the stamp-refresh pattern in executeWriteFileOnSandbox.

  2. $-pattern replacer — String.replace(oldString, newString) was interpreting $$, $&amp;, $`, $', $n in newString as special replacement patterns. Now passes a replacer function () => args.newString so the replacement text is inserted literally.

Typecheck still gated on CI (no local node_modules in this worktree).

@iroiro147

Copy link
Copy Markdown
Contributor Author

CI status note: typecheck + lint + test-integration(ubuntu) + test-scenario all green on a8919ad. test-unit and test-integration(windows) fail identically to my other open eve PRs (#1528#1532) — consistent with fork-scoped CI credentials, not the change (which is contained to packages/eve/src/runtime/framework-tools/). DCO signoff present. Ready for design/impl review when a maintainer gets a chance.

@iroiro147

Copy link
Copy Markdown
Contributor Author

Standing-health check (re-verified against current main 021dbbf2, 2 commits past this branch's base):

  • Merge dry-run vs current `main`: clean, zero conflicts. The 2 new commits touch `cli/dev/tui` + a version-package bump — disjoint from this PR's surface (`runtime/framework-tools`).
  • CI: `unit/windows/e2e/Vercel-deploy` remain failing — these are fork-CI infrastructure constraints (Vercel deploy `Authorization required`, e2e needs Pro sandbox credentials), not regressions from this change. `typecheck/lint/test-integration(ubuntu)/test-scenario` green on the PR head (per prior run).
  • This is a draft scaffold; the `blocked` merge state is the draft/review gate, intended.

Ready for a maintainer shape-review whenever convenient; no action required from contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sandbox file tools: add a built-in edit_file (str_replace) tool so agents stop regenerating whole files

1 participant