Don't resend read_file when the file hasn't changed.

[Dhirenderchoudhary]

Description

Phase 2 of #793. Repeat read_file on the same path and line range now returns a one-line stub if mtime/size have not changed, instead of dumping the file again.

Compact and /compact (including --restore) bump a generation so the next read is real. /clear wipes the stub map with the edit guard. Writes drop that path. Subagents get their own stub scope so a parent read cannot stub a file they never saw.

markFileSeen is still just the edit guard. Not a 5-stage compiler. Phase 1 (auto-compact on --plain / ACP / subagents) is already on main.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully)
• Tests cover both success and error scenarios

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md)

[github-actions]

nc-review: comments — 1 important, 4 nits

@Dhirenderchoudhary — a few things worth a look, none blocking.

Adds a stub-on-repeat path for read_file when (path, range, mtime, size) match a prior read, plus per-subagent stub scopes via AsyncLocalStorage, and wires compact/clear/writes to invalidate stubs while leaving the edit guard untouched. The core stub logic is sound and the new read-tracker tests are tight; the changes are coherent with issue #793 and ready to merge once the small wording and coverage issues below are addressed.

🟠 important · tests · source/subagents/subagent-executor.ts:268

The behavioural change that subagents now run inside runWithReadContentScope(agentId, ...) is only covered by the unit test on runWithReadContentScope itself. There is no integration test that exercises SubagentExecutor and verifies that a stub established in the parent does not leak into a subagent (or vice versa), and that forgetReadContent inside a subagent does not affect the parent's stubs. The unit test demonstrates the primitive; an executor-level test would catch a regression where someone drops the wrapper at the call site.

⚪ nit · design · source/tools/read-file.tsx:32

The stub message and the new tool description both tell the model it can pass start_line/end_line or metadata_only to force a real read, but metadata_only=true short-circuits to the metadata-only branch and never returns file content. From the model's perspective this is not a 'real read' of the body. The hint still works (start_line/end_line is the real escape hatch), but the metadata_only clause will lead a model to think it is requesting content and instead receive metadata. Worth either removing the metadata_only clause from the hint or rewording to clarify that metadata_only returns metadata, not the file body.

⚪ nit · correctness · source/utils/read-tracker.ts:105

In forgetReadContent the loop compares key === resolved alongside key.startsWith(prefix). readContentKey always emits the form ${resolved}\0${startLine ?? ''}\0${endLine ?? ''}, so the bare equality branch is dead and only startsWith(prefix) does any work. Harmless, but it suggests the author thought keys could be stored unkeyed. Worth removing the dead check or adding a comment.

⚪ nit · correctness · source/utils/read-tracker.ts:49

readContentScopes is a Map that only ever shrinks via clearReadTracker() (on /clear). Each unique agentId passed to runWithReadContentScope permanently adds an entry. cleanupSubagentSession(agentId) removes the parallel session-store entry but leaves the read-content scope in place. For a long session with many distinct subagent IDs this is a slow leak. Either pass the agentId through to a cleanup hook, or scope-bump on completion, or document the leak as accepted.

⚪ nit · tests · source/tools/file-ops/string-replace.spec.tsx:602

The new string_replace validator: still allows edit after a stubbed re-read test is declared with test(...) rather than test.serial(...) like its sibling tests in the same file. The project runs AVA serially, so this is unlikely to flake today, but the validator relies on global read-tracker state populated by readFileTool.tool.execute! two lines above, which is exactly the pattern other tests in this file mark test.serial for.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}