…d tools resolve to the subagent (#3873)

* fix(core): rebuild tool registry on subagent Config overrides so bound tools resolve to the subagent

PR-B (#3774) added per-Config FileReadCache isolation via Object.create
overrides at two subagent spawn sites — agent.ts:createApprovalModeOverride
and subagent-manager.ts:maybeOverrideContentGenerator. The override
shielded code that read FileReadCache directly through the Config
instance, but missed the bound-tool path: Config.createToolRegistry runs
once at parent initialise time, so the parent's EditTool / WriteFileTool
/ ReadFileTool instances are bound with `this.config = parent`. The
subagent's Object.create wrapper inherited getToolRegistry via the
prototype chain, reaching the parent registry whose bound tools then
read FileReadCache and approval mode from the parent.

This change closes that gap by rebuilding the tool registry on the
override at both sites — the same pattern InProcessBackend.createPerAgentConfig
already uses:

  - override.createToolRegistry(undefined, { skipDiscovery: true })
  - registry.copyDiscoveredToolsFrom(base.getToolRegistry())
  - override.getToolRegistry = () => registry

createApprovalModeOverride becomes async; its single call site already
ran inside an async block. maybeOverrideContentGenerator skips the
rebuild when the upstream Config already has its own getToolRegistry
(real-world case: agent.ts wrapper passed through createAgentHeadless),
avoiding wasted work, listener accumulation on shared SubagentManager /
SkillManager, and a cache split where the bound tools' registry layer
diverges from the runtime context's lazy-init cache.

Includes regression tests in agent-override.test.ts and
subagent-manager-override.test.ts that exercise the bound-tool path:
they instantiate the lazy factories on the override registry and
assert that EditTool / WriteFileTool / ReadFileTool resolve
this.config to the override Config (and thus to the override's
FileReadCache / approval mode), not the parent.

* fix(core): close bound-tool gap on resumed background agents too

Follow-up audit on PR #3873 surfaced a duplicate, pre-rebuild copy of
`createApprovalModeOverride` living in `background-agent-resume.ts`
(L142-150). Resumed fork agents go through `createResumedForkSubagent`
which bypasses `SubagentManager.maybeOverrideContentGenerator` (where
the registry rebuild now lives), so the resumed fork's
`EditTool` / `WriteFileTool` / `ReadFileTool` were still resolving
`this.config` to the parent and reading the parent's `FileReadCache`.

The non-fork resume path went through `subagent-manager` and worked
correctly only because `maybeOverrideContentGenerator` saw no upstream
own-registry on `bgConfig` and rebuilt one — but with that fallback
the fork path could never benefit.

This change deletes the local copy and switches `background-agent-resume.ts`
to import the now-async exported `createApprovalModeOverride` from
`agent.ts`. Drops the previous `?: this.config` short-circuit so the
resumed agent ALWAYS gets a wrapper Config — the same behaviour
`agent.ts` already enforces; reusing the parent directly defeats the
per-Config FileReadCache isolation.

Updates `background-agent-resume.test.ts` mock config with the
`createToolRegistry` / `getToolRegistry` stubs the rebuild path now
exercises.

* fix(core): address bound-tool isolation review feedback

Three independent fixes from PR #3873 review feedback:

1. Switch the upstream-rebuild guard from
   `hasOwnProperty(base, 'getToolRegistry')` to a Symbol-keyed marker
   `TOOL_REGISTRY_REBUILT`. The own-property check missed the case
   where the override is reached via an Object.create wrapper above
   the rebuilt Config (e.g. `bgConfig = Object.create(agentConfig)` in
   the agent.ts background path) — it would falsely report "no
   upstream rebuild" and cause a redundant third rebuild that wastes
   work and doubles the listener-leak surface. Symbol property reads
   walk the prototype chain via normal lookup, so a marker stored on
   any ancestor is correctly observed.

   Extracts the shared rebuild logic into
   `rebuildToolRegistryOnOverride(override, base)` so the three spawn
   sites (agent.ts:createApprovalModeOverride, the inherits branch,
   the non-inherit branch) cannot drift apart.

2. Stop the per-subagent ToolRegistry in the lifecycle finally blocks:
   - agent.ts foreground finally (after the inner try wrapping
     `runFramed`)
   - agent.ts background bgBody finally (after `bgSubagent.execute`
     resolves)
   - background-agent-resume.ts resume body finally (same shape)

   Without this, every AgentTool / SkillTool the model instantiates
   from the per-subagent registry registers a change-listener on
   shared SubagentManager / SkillManager, and repeated subagent runs
   accumulate listeners for the rest of the session. Stop is
   fire-and-forget, matching `InProcessBackend.cleanup` and
   `stopAgent`.

3. Add bound-tool isolation tests for the non-inherit branch
   (explicit-model selector). The original PR only covered the
   inherits branch directly; the non-inherit branch now goes through
   the same helper, but a dedicated test pins
   `tool.config === override` and the FileReadCache binding so a
   regression cannot leave explicit-model subagents reading the
   parent's cache while existing model-override tests still pass.

Tests now exercise:
- Symbol marker propagation via Object.create chain (3 cases)
- Non-inherit rebuild + bound-tool isolation
- Non-inherit skip-rebuild when upstream wrapper has the marker
- Pre-existing inherits / chained-override / approval-mode propagation
- Mock configs in agent.test.ts / subagent-manager.test.ts /
  background-agent-resume.test.ts gain `stop` and `tools: Map` stubs
  to model the registry contract the override path now exercises.

`npx vitest run packages/core/src` — 268 files / 6943 passed.

… for Edit

The previous commit dropped the `lastReadWasFull` clause from
`checkPriorRead` for both Edit and WriteFile. That over-relaxes
WriteFile: unlike Edit, the overwrite path replaces the entire
file and has no `old_string` matching as a content-derived guard.
A model that has only seen a slice via `read_file(offset, limit)`
followed by a WriteFile would necessarily hallucinate the rest of
the bytes — which is exactly the issue #2499 data-loss scenario
PR #3774 was opened to address.

Split the policy by operation:
  - EditTool: partial read counts (`old_string` matching is the
    floor — a fabricated string that misses the actual bytes is
    caught by `0 occurrences`).
  - WriteFileTool overwrite: full read required (no equivalent
    floor; partial-read-then-overwrite would silently destroy
    unread bytes).

Mechanism: add `requireFullRead?: boolean` to `CheckPriorReadOptions`.
WriteFileTool's 5 enforcement call sites pass `true`; EditTool's 3
pass nothing (default `false`). New-file creation paths still hit
the ENOENT → `ok: true` branch before the flag is consulted, so
brand-new file creation remains exempt regardless.

The `fresh + cacheable + partial + requireFullRead` case gets a
dedicated rejection branch so the model sees a clear "you have a
partial read; do a full read" message rather than the generic
"has not been read" wording. The `unknown` branch's message also
varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

Tests: write-file.test.ts's "rejects when previous read was
ranged" test (which the previous commit had inverted) is restored
and asserts on the new "partially read / replaces the entire file"
message. Edit's partial-OK test is unchanged. 198 / 198 in the
prior-read-related suites; tsc clean.

…quired for WriteFile

PR #3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue #2499 data-loss scenario PR #3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue #2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue #2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.

…quired for WriteFile (#3932)

PR #3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue #2499 data-loss scenario PR #3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue #2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue #2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.

…#3774)

* feat(core): enforce prior read before Edit / WriteFile mutates a file

Introduces a session-scoped invariant: the model cannot mutate an
existing file without having actually Read it (or its post-write
state) earlier in this conversation. Builds on the FileReadCache
landed in #3717.

Two new ToolErrorType codes:
- EDIT_REQUIRES_PRIOR_READ — file has no entry in the session cache.
  The model is told to use read_file first.
- FILE_CHANGED_SINCE_READ — file has an entry but its mtime or size
  drifted since the recorded fingerprint. The model is told to
  re-read before retrying.

EditTool blocks the existing-file path on cache.check; new-file
creation (old_string === '' on a non-existent target) is exempt.
WriteFileTool blocks the overwrite path; new-file creation
(fileExists === false) is exempt.

Both tools route through the existing fileReadCacheDisabled escape
hatch on Config — flipping it bypasses enforcement byte-for-byte,
matching pre-cache behaviour. Operators can use this as a kill switch
if a session falls into a state where the cache cannot be trusted.

ReadFile fix on the auto-memory path: PR #3717 had auto-memory reads
skip the cache entirely (both lookup and record), but with the new
enforcement that means a model that just Read AGENTS.md cannot then
Edit it. Decoupled the two: auto-memory reads still skip the
file_unchanged fast-path (the per-read freshness <system-reminder>
must always reach the model) but DO record into the cache so the
follow-up Edit sees `fresh`. New regression test asserts this.

Test plan
- vitest run (all of @qwen-code/qwen-code-core): 6308 passed, 2 skipped
- 9 new enforcement tests across edit.test.ts and write-file.test.ts:
  unknown rejects, stale rejects, new-file exempt, edit chain stays
  authorised, escape hatch bypasses, plus the auto-memory record
  regression in read-file.test.ts.
- tsc --noEmit clean. eslint clean. core build succeeds.

* test(core): clear shared fileReadCache between write-file.test.ts cases

CI surfaced one Linux-only failure: the prior-read enforcement test
'rejects a write that would overwrite an unread existing file'
returned FILE_CHANGED_SINCE_READ instead of EDIT_REQUIRES_PRIOR_READ.

Root cause: the FileReadCache instance is declared at module scope
(line 41) and shared across every test in write-file.test.ts. State
from earlier tests — most recently the 'records a write' integration
test that records the same path — leaks forward. On Linux the test
ordering puts a record-bearing test before the enforcement test, so
the cache reports `stale` (mtime drifted) instead of `unknown`.
macOS / Windows happen to order them differently and never hit it.

Adding a fileReadCache.clear() to beforeEach gives every test a
known-empty cache, matching how edit.test.ts already isolates its
per-test cache by re-instantiating it.

* fix(core): close prior-read enforcement gaps flagged in 3rd review

Three concrete loopholes / regressions that the original PR-B
introduction left open. All three are addressed in the same commit
because the underlying refactor (move enforcement earlier and tighten
the fresh predicate) is shared across them.

1. fresh != "model has seen the bytes". Pre-fix, requirePriorRead()
   accepted any cache.check === 'fresh'. ReadFileTool records every
   successful read into the cache, including ranged reads
   (offset/limit), truncated full reads, and non-cacheable
   binary/image/audio/video/PDF/notebook reads (lastReadCacheable
   = false). This let the model peek at a slice or a structured
   payload of a file and then mutate the whole thing. Tightened the
   accept predicate to fresh && lastReadAt && lastReadWasFull &&
   lastReadCacheable.

2. Read-less content oracle through calculateEdit error codes. Pre-fix,
   execute() ran calculateEdit (which reads file bytes and counts
   matches) before the enforcement check. A model could probe an
   unread file by attempting Edits with candidate old_strings and
   observing NO_OCCURRENCE_FOUND vs EXPECTED_OCCURRENCE_MISMATCH vs
   EDIT_NO_CHANGE — reverse-engineering content without ever calling
   read_file. Moved enforcement to the top of calculateEdit, before
   any content read; only a stat is performed up to the rejection
   point.

3. Confirmation flow regression. Pre-fix, getConfirmationDetails()
   read the existing file to render a diff for the user, then
   approval flowed to execute() which would freshly check the cache
   and reject. The user could approve a diff computed from current
   bytes the model never saw, and the call would still fail. Moved
   enforcement before the confirmation read in both EditTool (via the
   shared calculateEdit path) and WriteFileTool (explicit check at
   the top of getConfirmationDetails). The user now never sees a
   confirmation diff for an unread file — the call rejects up front.

Public API surface change: requirePriorRead() -> checkPriorRead() that
returns a structured decision, so the same predicate can route into
a CalculatedEdit.error (calculateEdit), a thrown error
(getConfirmationDetails), or a ToolResult (execute) without
duplicating the boolean / message / type plumbing in three shapes.

Reported by pomelo-nwu (3 inline comments on PR #3774).

* refactor(core): close 4 prior-read enforcement gaps from 4th review

1. recordWrite now seeds read metadata on brand-new entries
   (lastReadAt / lastReadWasFull / lastReadCacheable). The strict
   accept predicate added in the previous round (#3 review) requires
   all three, but recordWrite only set lastWriteAt — so a model
   creating a file with Edit (old_string="") or WriteFile and then
   editing it again was rejected on the second edit. The model
   authored the bytes it just wrote; for the purposes of prior-read
   enforcement that counts as having seen them. New regression test
   in edit.test.ts: "allows a create-then-edit-then-edit chain
   without an intervening read".

2. Extracted checkPriorRead into src/tools/priorReadEnforcement.ts.
   The two copies in edit.ts and write-file.ts had already drifted
   (one used ${ReadFileTool.Name}, the other hardcoded 'read_file');
   the boolean guard is security-sensitive and a one-sided fix
   would silently weaken the boundary. The shared utility takes a
   verb ('editing' | 'overwriting') so the user-facing prose can
   differ between callers without duplicating the decision logic.

3. WriteFileTool.execute now runs prior-read enforcement BEFORE
   readTextFile. Pre-fix, an unread overwrite still slurped the
   entire file into memory (encoding / BOM / line-ending detection)
   and only then rejected it: wasted I/O, and momentary in-memory
   custody of bytes the model never legitimately read. Now matches
   the order in getConfirmationDetails().

4. The "rejects a write that would overwrite an unread existing
   file" test now spies on FileSystemService.readTextFile and
   asserts not.toHaveBeenCalled() — without that, the test gave
   false confidence: it passed both pre-fix (read happened, then
   reject) and post-fix (reject before read), so the ordering
   regression in (3) was invisible to the assertion.

Reported by glm-5.1 via /review on PR #3774.

* refactor(core): close 4 prior-read enforcement gaps from 4th review (Copilot)

Five concrete gaps that the previous round of enforcement work left
open. Reported by Copilot via /review on PR #3774.

1. Confirmation-time rejections lost their ToolErrorType code.
   getConfirmationDetails() in both EditTool and WriteFileTool threw
   a plain Error on prior-read failure, which coreToolScheduler
   collapsed into UNHANDLED_EXCEPTION — silently breaking the
   EDIT_REQUIRES_PRIOR_READ / FILE_CHANGED_SINCE_READ contract for
   any approval-required flow.

   Fix: introduce PriorReadEnforcementError that carries
   `errorType: ToolErrorType`. Both confirmation paths now throw it,
   and coreToolScheduler reads `error.errorType` (falling back to
   UNHANDLED_EXCEPTION when absent). New regression tests assert
   the thrown error's `errorType` field for both tools.

2. checkPriorRead's "re-read with read_file" advice was wrong for
   binary / image / audio / video / PDF / notebook files. Their
   ReadFile result always sets lastReadCacheable=false, so the
   message would loop the agent forever on the same rejection.

   Fix: detect the fresh-but-non-cacheable case explicitly and
   return a dedicated message that explains the dead end ("Edit /
   WriteFile cannot mutate that payload safely") instead of asking
   for another read. Updated the existing non-cacheable regression
   test to assert the new message and the absence of "use the
   read_file tool first".

3. checkPriorRead swallowed every stat() failure and returned
   ok:true. EACCES, EBUSY, NFS hiccups, etc. would silently
   re-open the blind-write path the helper exists to block.

   Fix: only ENOENT continues to return ok:true (disappearance
   race). Any other code is fail-closed: returns
   EDIT_REQUIRES_PRIOR_READ with a message that names the errno.
   New regression test in write-file.test.ts spies on fs.promises
   .stat to inject EACCES and asserts the rejection.

4. The auto-memory record regression test only asserted `state ===
   'fresh'`. A future change that recorded auto-memory reads as
   partial / non-cacheable would still satisfy that assertion but
   would actually fail enforcement on every follow-up Edit.

   Fix: also assert lastReadAt is defined, lastReadWasFull is true,
   and lastReadCacheable is true. The full "what enforcement
   requires" predicate is now explicit in the test.

(The 5th item, the WriteFile mirror of (1), is covered by the same
PriorReadEnforcementError change.)

* refactor(core): tighten StructuredToolError naming + add scheduler test

Four follow-ups raised by deepseek-v4-pro on PR #3774. None of them
change the enforcement boundary; they are all about making the
contract clearer and harder to break in future changes.

1. PriorReadEnforcementError -> StructuredToolError. The class now
   wraps any content-derived ToolErrorType from calculateEdit
   (EDIT_NO_OCCURRENCE_FOUND, EDIT_EXPECTED_OCCURRENCE_MISMATCH,
   EDIT_NO_CHANGE, ATTEMPT_TO_CREATE_EXISTING_FILE) on top of the
   prior-read codes. The old name suggested the class was prior-
   read-specific, which would mislead any oncall engineer seeing
   it paired with one of the calculateEdit error codes.

2. EDIT_REQUIRES_PRIOR_READ kept its name (the prefix mentions
   "edit" but the enum is shared with WriteFileTool) — chose
   documentation over rename to avoid the churn of a value rename
   across logs/dashboards already keyed on it. JSDoc now spells
   out the cross-tool usage explicitly.

3. Stat failures other than ENOENT now map to a new
   PRIOR_READ_VERIFICATION_FAILED code instead of being conflated
   with EDIT_REQUIRES_PRIOR_READ. The failure mode is "we cannot
   verify" rather than "definitely not read" — operators routing
   on error codes can distinguish the two populations.

4. Added a coreToolScheduler test (`surfaces error.errorType from
   a confirmation throw instead of UNHANDLED_EXCEPTION`) that
   constructs a stub tool whose getConfirmationDetails throws
   StructuredToolError and asserts the surfaced ToolCall response
   carries the correct ToolErrorType. Without this test the
   scheduler's explicitErrorType branch would have no coverage at
   all.

Tool tests updated for the new StructuredToolError class name and
the PRIOR_READ_VERIFICATION_FAILED code on the EACCES path.

* fix(core): close TOCTOU + grammar + directory regressions in PR-B

Six concrete issues that the previous round of enforcement work
left open. Reported by Copilot via /review on PR #3774.

1. TOCTOU window between pre-read checkPriorRead and readTextFile.
   The pre-read stat could pass enforcement, then an external writer
   could land between that stat and the actual read, leaving
   currentContent reflecting bytes the model never saw — exactly the
   stale-write path the PR is supposed to block. Closed by re-running
   checkPriorRead immediately after every readTextFile that fed
   currentContent / originalContent: EditTool.calculateEdit and the
   two WriteFileTool paths (execute + getConfirmationDetails). A
   `stale` outcome now fails the operation with
   FILE_CHANGED_SINCE_READ at the correct moment.

2. Directory targets sent the model into an enforcement loop.
   `fileExists` is a plain access check, so directories also entered
   the enforcement branch — the model would be told to call
   `read_file`, but `read_file` rejects directories with
   TARGET_IS_DIRECTORY, so the loop never terminated. Fixed in
   checkPriorRead: if `fs.stat` reports the path is not a regular
   file, return `ok: true` so the downstream readTextFile / write
   path can surface its own EISDIR / similar error.

3. Confirmation-time error messages used the short `display` form
   instead of the full `raw` form. Approval-required Edit calls
   therefore lost the remediation detail (file path, stale-vs-unread
   distinction, "without offset / limit / pages" hint) that the
   execute path already surfaced and that the WriteFile confirmation
   path already preserved. EditTool.getConfirmationDetails now
   throws StructuredToolError with `editData.error.raw`.

4. Non-text payload displayMessage was grammatically broken: built
   from the gerund `editing` / `overwriting`, it rendered as
   "cannot editing via this tool" / "cannot overwriting via this
   tool". Fixed by deriving a bare-verb form (`edit` / `overwrite`)
   alongside the gerund and using it in displayMessage.

(Items 1, 5 and 6 from Copilot's batch are the same TOCTOU class —
EditTool calculateEdit + WriteFile execute + WriteFile confirmation —
addressed together in (1) above.)

The "bypasses enforcement entirely" test now uses mockReturnValue
instead of mockReturnValueOnce because calculateEdit calls
getFileReadCacheDisabled twice — once for the pre-read check and
once for the post-read TOCTOU re-check. Both must see disabled=true
to actually bypass.

* fix(core): close fileExists TOCTOU on WriteFile prior-read enforcement

WriteFile gated prior-read enforcement on `fileExists` from
`isFilefileExists()`, but a file that sprang into existence between
that check and the write would still be overwritten without
enforcement — `fileExists === false` skipped the check entirely.

Made the gate unconditional on `fileExists`. checkPriorRead's own
`fs.stat` decides what to do:
- ENOENT → ok:true, fall through to the new-file path as before
- file exists right now (whether or not isFilefileExists saw it) →
  unknown / stale check runs, the race-created file is rejected.

Applied to both getConfirmationDetails and execute. The path that
actually creates new files is unchanged because checkPriorRead's
ENOENT branch is the disappearance-race exit, which is the correct
exit for "the file truly does not exist yet".

Reported by glm-5.1 via /review on PR #3774.

* fix(core): close 4 enforcement gaps + 1 critical bug from 5th Copilot review

Six issues raised by deepseek-v4-pro / glm-5.1 / qwen3.6-plus on
PR #3774. Listed by reviewer-assigned severity.

[Critical] (qwen3.6-plus) recordWrite previously only seeded the
read metadata for brand-new entries. The reproduction was real:
ReadFile(limit=10) → WriteFile(full content) → Edit. The partial
read's lastReadWasFull=false would persist through the write, and
the Edit would be rejected with EDIT_REQUIRES_PRIOR_READ even
though the model just authored every byte. recordWrite now
unconditionally refreshes lastReadAt, lastReadWasFull=true, and
lastReadCacheable=true. The fileReadCache.test.ts case that
previously asserted "preserves lastReadAt" is rewritten to assert
the new "refreshes lastReadAt to match the write" contract, and a
new "upgrades lastReadWasFull / lastReadCacheable after a full
write" regression test pins the reproduction reviewer described.

[Suggestion] (deepseek-v4-pro) Narrowed the non-regular-file
bypass in priorReadEnforcement from `!stats.isFile()` to
`stats.isDirectory()`. The earlier broad form covered FIFOs,
sockets, and devices that the model has no legitimate "read first"
recourse for and that can block readTextFile (FIFO) or
over-allocate (/dev/urandom). Those now flow through to
cache.check() and reject with the unread-file path before any I/O.

[Suggestion] (glm-5.1) Removed the `fileExists && ...` gate from
EditTool.calculateEdit, mirroring the f4ef756 fix on WriteFile.
A file that springs into existence between isFilefileExists() and
the enforcement check is now caught here as well; ENOENT inside
checkPriorRead remains the disappearance-race exit and new-file
creation flow is unchanged.

[Suggestion] (deepseek-v4-pro) Added debugLogger.warn() at every
post-read TOCTOU rejection site (Edit calculateEdit, WriteFile
getConfirmationDetails, WriteFile execute). These rejections are
rare and self-healing — without a debug record, an operator
investigating "why did this Edit fail once?" had nothing to grep.
debugLogger uses dedicated 'EDIT_PRIOR_READ' / 'WRITE_FILE' tags.

[Suggestion] (qwen3.6-plus) Added a final pre-write checkPriorRead
in EditTool.execute() and WriteFileTool.execute(). The earlier
post-read check ran inside calculateEdit (Edit) or before mkdirSync
(WriteFile), but the actual writeTextFile call could be arbitrarily
later — user approval, modify-and-confirm, etc. The window from
"post-read check → writeTextFile" is now bounded to "pre-write
stat → writeTextFile" (two adjacent syscalls).

* fix(core): close new-file race + special-file enforcement loop

Three issues from the latest Copilot review on PR #3774.

1. New-file race in pre-write enforcement (write-file.ts:348,
   edit.ts:487). The earlier pre-write checkPriorRead was gated on
   `fileExists` (WriteFile) and `!editData.isNewFile` (Edit). If the
   path was absent at planning time and another process created it
   while approval was pending, the gated form would skip enforcement
   and silently overwrite a pre-existing file the model never read.
   Run unconditionally in both tools — checkPriorRead's own ENOENT
   branch is the disappearance-race exit, so genuine new-file
   creation is unaffected, but a race-created file now hits the
   `unknown` branch and is rejected as unread.

2. FIFO / socket / device sent the model into an enforcement loop
   (priorReadEnforcement.ts:220). After narrowing the
   non-regular-file bypass to directories only, FIFOs etc. fell
   through to cache.check, returned `unknown`, and produced a
   "use read_file first" message — but read_file rejects those same
   targets as "not a regular file", so the model would loop on
   read_file forever. Added a dedicated `!stats.isFile()` branch
   (after the directory exemption) that returns a "special file;
   cannot edit/overwrite via this tool — use shell instead" message,
   matching the shape of the existing non-text-payload guidance.

(Tool-error.ts and the non-cacheable policy notes are addressed in
the PR description update — not in code.)

* fix(core): close 4 enforcement gaps from 6th Copilot review

(Plus a doc-only update for the 5th — the mtime+size limitation
warning in the Risk section now mentions the silent-overwrite
escalation that this PR's mutation paths bring along.)

1. ENOENT after the model has already read the file is no longer
   silently treated as `ok: true`. Added an `expectExisting` option
   to `checkPriorRead`; post-read and pre-write callers pass `true`.
   ENOENT under that flag now rejects with `FILE_CHANGED_SINCE_READ`
   ("file disappeared after the model read it") rather than falling
   through to the new-file path with stale bytes. Pre-read callers
   keep the old default (ENOENT → ok:true → fall through to genuine
   new-file creation). EditTool's pre-write check derives the flag
   from `editData.isNewFile`; WriteFile's pre-write check derives it
   from the post-read `fileExists` value.

2. Directory targets now reject with `TARGET_IS_DIRECTORY` and a
   structured message instead of returning `ok: true`. The previous
   form fell through to readTextFile(), which on the WriteFile
   confirmation path threw a plain Error and was surfaced by the
   scheduler as `UNHANDLED_EXCEPTION`. Both Edit and WriteFile now
   emit a structured rejection at enforcement time. (WriteFile's
   build-time validateToolParamValues already rejects directories,
   so the change matters most for EditTool.)

3. Non-cacheable rejection's `rawMessage` no longer hard-codes
   "overwrite" — it now uses the same `verbBare` derivation as the
   `displayMessage`, so EditTool's path correctly says "if you need
   to edit it" and WriteFile's path stays "if you need to overwrite
   it". The previous form was confusing for in-place edits.

4. WriteFile.getConfirmationDetails now mirrors execute()'s
   ENOENT-to-new-file fallback: a file that disappears between
   isFilefileExists() and the readTextFile-for-diff call no longer
   throws a plain Error (which would surface as
   UNHANDLED_EXCEPTION) — it falls back to the brand-new-file diff
   so the user sees a clean confirmation rather than an unstructured
   crash.

Tests:
- New: `rejects an edit on a directory with TARGET_IS_DIRECTORY`
- New: `confirmation falls back to a new-file diff when the file
  disappears mid-flight` (WriteFile)
- Updated: non-cacheable rejection asserts `verbBare` is "edit" on
  the EditTool path and "overwrite" on the WriteFile path.

Reported by Copilot via /review on PR #3774.

* docs(core): clarify stat→write race + EDIT_REQUIRES_PRIOR_READ scope

Three doc-only follow-ups from Copilot's latest review pass on PR
#3774. None change behaviour; the pre-fix code state was already
the actual contract — the docs just lagged it.

1. EDIT_REQUIRES_PRIOR_READ enum comment now lists the three cases
   the code actually returns it for (never-read, partial / ranged /
   non-cacheable read, structural dead end — non-text payload or
   special file). The previous one-liner described only the first
   case and would mislead future maintainers.

2. The Final pre-write freshness check blocks in EditTool.execute
   and WriteFileTool.execute now spell out that they DO NOT
   eliminate the stat → writeTextFile race. The window narrows
   from the previously-unbounded post-read-to-write gap down to
   two adjacent syscalls, but a concurrent writer landing in
   that pair can still be clobbered. Closing the residual would
   require an atomic write (write-to-temp + rename) or a
   content-hash post-write recheck — both deferred. Operators who
   need strict protection set `fileReadCacheDisabled: true` and
   rely on application-level locking.

3. PR description Risk section gains a "Known unmitigated: stat →
   write race window" subsection (English + Chinese mirror)
   matching the code comments.

* chore(core): minor follow-ups from review #4229917446

Three of the five MINOR items raised in the independent code review
on 2026-05-05 — the cheap, isolated ones. The other two (race-
simulating integration test, moving StructuredToolError out of
priorReadEnforcement.ts) are deferred as the reviewer suggested.

1. EditTool now has a symmetric `PRIOR_READ_VERIFICATION_FAILED`
   regression test (mocks fs.promises.stat to reject with EACCES,
   asserts the EditTool path produces the same fail-closed result
   that the existing WriteFile EACCES test pins). Five-line fix to
   close the asymmetry that, while harmless today (the helper is
   shared), would let a future Edit-side change to checkPriorRead
   slip through without test coverage.

2. ensureParentDirectoriesExist / mkdirSync now run AFTER the
   pre-write checkPriorRead in both EditTool.execute() and
   WriteFileTool.execute(). Doing it before would leak intermediate
   directories on the rejection path — a real (if minor) FS litter
   the previous order created on every rejected new-file write.

3. EDIT_REQUIRES_PRIOR_READ enum docstring gains a one-line note
   for operators routing alerts on this code: a single
   `edit_requires_prior_read` signal can mean any of the three
   cases (no read / partial read / structural dead-end), and if
   per-cause monitoring becomes important the enum can be split
   in a follow-up. The originating tool name and the message text
   already disambiguate at runtime.

* fix(core): close 2 correctness gaps from maintainer review #4232751470

Both tracked back to the cache's "track most recent read shape"
model diverging from prior-read enforcement's "model has seen
these bytes" model.

1. SVG (and similar string-content fallbacks) recorded as
   non-cacheable, blocking subsequent Edit / WriteFile.

   `read-file.ts` derives `cacheable` from
   `originalLineCount !== undefined && !isTruncated`. The SVG
   branch in `fileUtils.ts` returned content without
   `originalLineCount`, so `cacheable` collapsed to false and a
   follow-up Edit hit the dead-end "non-text payload — use shell"
   rejection — telling the model to use shell to mutate a file it
   had just successfully read as text. This was a real regression
   vs pre-PR behaviour where SVG-as-text editing worked.

   Fix: SVG-as-text branch now sets `originalLineCount` (split
   on '\n') and `isTruncated: false`, so ReadFile records it as
   a full cacheable read. The binary-fallback string and
   over-1MB SVG branches are deliberately left non-cacheable —
   they return placeholder strings ("Cannot display content of
   ...") rather than file content, so blocking edits there is
   correct. New regression test in `read-file.test.ts`:
   `records SVG-as-text reads with cacheable=true so a follow-up
   Edit passes enforcement`.

2. recordRead unconditionally overwriting lastReadWasFull /
   lastReadCacheable, revoking prior write-author or full-read
   rights.

   The `WriteFile(create) → ReadFile(offset/limit) → Edit`
   sequence rejected the Edit because the partial read clobbered
   the `lastReadWasFull = true` that `recordWrite` had stamped
   at create time. Same shape applies to a full text read
   followed by a partial one of the same inode.

   Fix: `recordRead` is now sticky-on-true for the read flags —
   `if (opts.full) entry.lastReadWasFull = true;` and the
   matching guard for `cacheable`. Prior `true` survives a later
   partial / non-cacheable read. The fast-path `file_unchanged`
   check still gates on the incoming request's own `isFullRead`
   in `read-file.ts`, so a partial read still does not get a
   placeholder it shouldn't. Updated the existing
   "overwrites earlier lastReadWasFull" test to assert the new
   sticky semantics, and added a `lastReadCacheable` symmetric
   test plus a `Write → partial-Read → Edit` end-to-end test in
   `edit.test.ts`.

Reported by tanzhenxin via independent maintainer review on
2026-05-06.

* fix(core): close 3 correctness gaps from re-review #4233904930

All three are tightenings of the prior `de8ddf530` round.

1. **Sticky-on-true narrowed to "no fingerprint drift"**.
   `fileReadCache.recordRead` previously kept `lastReadWasFull` /
   `lastReadCacheable` true across drifted recordings, which
   re-opened a `Read full @x → external write @y → Read partial
   @y → Edit` hole: the partial recordRead silently advanced the
   entry's mtime+size to Y while preserving the sticky `full=true`
   from X, so a follow-up Edit ran against bytes the model only
   saw the first 10 lines of. Now the sticky branch only fires
   when `(mtimeMs, sizeBytes)` matches the existing entry; on
   drift, both flags reset to exactly what this read produced.
   New regression test in `fileReadCache.test.ts` reproduces the
   reviewer's reported sequence.

2. **Subagent FileReadCache isolation now covers the
   inherits-model + same-approval-mode common case**. The
   own-property machinery from #3717 only triggers when an
   `Object.create(parent)` actually fires; both
   `agent.ts:990-993` (same-approval-mode) and
   `subagent-manager.ts:699-701` (inherits-model) had paths that
   returned the parent Config directly, so the subagent's
   `getFileReadCache()` resolved to the parent's instance — a
   parent Read could satisfy the subagent's Edit on a path the
   subagent's transcript never contained. Both sites now build
   a thin `Object.create(base)` override unconditionally; no
   method changes for the inherits / same-mode cases, but a
   distinct instance triggers the lazy-init in
   `Config.getFileReadCache()` so the subagent gets an isolated
   cache.

3. **Cache records the read pipeline's internal stat instead of
   a post-read re-stat**. `processSingleFileContent` now
   surfaces its internal stat via `result.stats`, and read-file
   uses that for `recordRead` instead of running its own stat
   after the read returns. Pre-fix, an external write between
   the pipeline call and the post-read stat let the cache record
   fingerprint Y for content the model received at X — a
   subsequent Edit would pass enforcement against bytes the
   model never legitimately saw. The internal-stat-to-read
   window is still a few microseconds wide; that residue is the
   same content-hash territory acknowledged in the Risk section.

Reported by tanzhenxin via re-review on PR #3774.

* docs(core): clarify partial subagent isolation per review #4234090906

tanzhenxin's third review correctly observed that the
`Object.create(parent)` wrappers in `agent.ts:createApprovalModeOverride`
and `subagent-manager.ts:maybeOverrideContentGenerator` only isolate
the FileReadCache for code that consults `Config.getFileReadCache()`
directly. Bound `EditTool` / `WriteFileTool` instances were registered
against the parent's tool registry at initialise time, so tool
invocations still resolve `this.config` to the parent and reach the
parent's cache. `InProcessBackend.createPerAgentConfig` already does
the right thing (`override.createToolRegistry()` +
`copyDiscoveredToolsFrom(base.getToolRegistry())`); bringing that to
these two spawn sites is the real fix.

Reviewer's verdict was COMMENT, not REQUEST_CHANGES — the gap
pre-dates this PR (it's a property of #3717's per-Config own-property
machinery) and pre-PR there was no enforcement on subagent mutations
at all, so the PR is strictly an improvement on every spawn path.
Documented the partial guarantee explicitly:

- Inline comments on both spawn sites note the bound-tool caveat
  and point at `InProcessBackend.createPerAgentConfig` as the model
  for the follow-up.
- PR description's subagent paragraph (English + Chinese mirror) now
  splits into "fully isolated" (`InProcessBackend.createPerAgentConfig`)
  and "partial isolation" (the two sites in this PR) so readers don't
  walk away with the wrong contract.

Filing the registry-rebuild work as a follow-up; not in this PR.

…d tools resolve to the subagent (#3873)

* fix(core): rebuild tool registry on subagent Config overrides so bound tools resolve to the subagent

PR-B (#3774) added per-Config FileReadCache isolation via Object.create
overrides at two subagent spawn sites — agent.ts:createApprovalModeOverride
and subagent-manager.ts:maybeOverrideContentGenerator. The override
shielded code that read FileReadCache directly through the Config
instance, but missed the bound-tool path: Config.createToolRegistry runs
once at parent initialise time, so the parent's EditTool / WriteFileTool
/ ReadFileTool instances are bound with `this.config = parent`. The
subagent's Object.create wrapper inherited getToolRegistry via the
prototype chain, reaching the parent registry whose bound tools then
read FileReadCache and approval mode from the parent.

This change closes that gap by rebuilding the tool registry on the
override at both sites — the same pattern InProcessBackend.createPerAgentConfig
already uses:

  - override.createToolRegistry(undefined, { skipDiscovery: true })
  - registry.copyDiscoveredToolsFrom(base.getToolRegistry())
  - override.getToolRegistry = () => registry

createApprovalModeOverride becomes async; its single call site already
ran inside an async block. maybeOverrideContentGenerator skips the
rebuild when the upstream Config already has its own getToolRegistry
(real-world case: agent.ts wrapper passed through createAgentHeadless),
avoiding wasted work, listener accumulation on shared SubagentManager /
SkillManager, and a cache split where the bound tools' registry layer
diverges from the runtime context's lazy-init cache.

Includes regression tests in agent-override.test.ts and
subagent-manager-override.test.ts that exercise the bound-tool path:
they instantiate the lazy factories on the override registry and
assert that EditTool / WriteFileTool / ReadFileTool resolve
this.config to the override Config (and thus to the override's
FileReadCache / approval mode), not the parent.

* fix(core): close bound-tool gap on resumed background agents too

Follow-up audit on PR #3873 surfaced a duplicate, pre-rebuild copy of
`createApprovalModeOverride` living in `background-agent-resume.ts`
(L142-150). Resumed fork agents go through `createResumedForkSubagent`
which bypasses `SubagentManager.maybeOverrideContentGenerator` (where
the registry rebuild now lives), so the resumed fork's
`EditTool` / `WriteFileTool` / `ReadFileTool` were still resolving
`this.config` to the parent and reading the parent's `FileReadCache`.

The non-fork resume path went through `subagent-manager` and worked
correctly only because `maybeOverrideContentGenerator` saw no upstream
own-registry on `bgConfig` and rebuilt one — but with that fallback
the fork path could never benefit.

This change deletes the local copy and switches `background-agent-resume.ts`
to import the now-async exported `createApprovalModeOverride` from
`agent.ts`. Drops the previous `?: this.config` short-circuit so the
resumed agent ALWAYS gets a wrapper Config — the same behaviour
`agent.ts` already enforces; reusing the parent directly defeats the
per-Config FileReadCache isolation.

Updates `background-agent-resume.test.ts` mock config with the
`createToolRegistry` / `getToolRegistry` stubs the rebuild path now
exercises.

* fix(core): address bound-tool isolation review feedback

Three independent fixes from PR #3873 review feedback:

1. Switch the upstream-rebuild guard from
   `hasOwnProperty(base, 'getToolRegistry')` to a Symbol-keyed marker
   `TOOL_REGISTRY_REBUILT`. The own-property check missed the case
   where the override is reached via an Object.create wrapper above
   the rebuilt Config (e.g. `bgConfig = Object.create(agentConfig)` in
   the agent.ts background path) — it would falsely report "no
   upstream rebuild" and cause a redundant third rebuild that wastes
   work and doubles the listener-leak surface. Symbol property reads
   walk the prototype chain via normal lookup, so a marker stored on
   any ancestor is correctly observed.

   Extracts the shared rebuild logic into
   `rebuildToolRegistryOnOverride(override, base)` so the three spawn
   sites (agent.ts:createApprovalModeOverride, the inherits branch,
   the non-inherit branch) cannot drift apart.

2. Stop the per-subagent ToolRegistry in the lifecycle finally blocks:
   - agent.ts foreground finally (after the inner try wrapping
     `runFramed`)
   - agent.ts background bgBody finally (after `bgSubagent.execute`
     resolves)
   - background-agent-resume.ts resume body finally (same shape)

   Without this, every AgentTool / SkillTool the model instantiates
   from the per-subagent registry registers a change-listener on
   shared SubagentManager / SkillManager, and repeated subagent runs
   accumulate listeners for the rest of the session. Stop is
   fire-and-forget, matching `InProcessBackend.cleanup` and
   `stopAgent`.

3. Add bound-tool isolation tests for the non-inherit branch
   (explicit-model selector). The original PR only covered the
   inherits branch directly; the non-inherit branch now goes through
   the same helper, but a dedicated test pins
   `tool.config === override` and the FileReadCache binding so a
   regression cannot leave explicit-model subagents reading the
   parent's cache while existing model-override tests still pass.

Tests now exercise:
- Symbol marker propagation via Object.create chain (3 cases)
- Non-inherit rebuild + bound-tool isolation
- Non-inherit skip-rebuild when upstream wrapper has the marker
- Pre-existing inherits / chained-override / approval-mode propagation
- Mock configs in agent.test.ts / subagent-manager.test.ts /
  background-agent-resume.test.ts gain `stop` and `tools: Map` stubs
  to model the registry contract the override path now exercises.

`npx vitest run packages/core/src` — 268 files / 6943 passed.

…quired for WriteFile (#3932)

PR #3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue #2499 data-loss scenario PR #3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue #2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue #2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.

…quired for WriteFile (QwenLM#3932)

PR QwenLM#3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue QwenLM#2499 data-loss scenario PR QwenLM#3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue QwenLM#2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue QwenLM#2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.

…quired for WriteFile (QwenLM#3932)

PR QwenLM#3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue QwenLM#2499 data-loss scenario PR QwenLM#3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue QwenLM#2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue QwenLM#2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.

 + #3945 (#4002)

* fix(core): decouple cacheable flag from truncation in FileReadCache

PR #3774 introduced prior-read enforcement that consults
`lastReadCacheable` to discriminate text from binary / image / PDF /
notebook payloads. ReadFileToolInvocation derived `cacheable` as
`string && originalLineCount && !isTruncated`, conflating two
unrelated concerns: "is the content text" and "did we see all the
bytes". A partial read (offset/limit) or a truncated full read of a
regular `.kt` / `.cpp` / `.py` source file therefore set
`cacheable: false`, and priorReadEnforcement.ts mistook that for a
non-text payload and rejected the next Edit with the misleading
"binary / image / audio / video / PDF / notebook payload" error.

PR #3932 split prior-read enforcement so Edit accepts partial reads
(`lastReadWasFull`-relaxed for Edit, kept for WriteFile), but the
`lastReadCacheable` conflation persisted, so partial / truncated text
reads still hit the binary-payload rejection on Edit. Issue #3964 is
the resulting field reports: .kt / .cpp / .py / .ts files on both
Linux and Windows misclassified as binary across 0.15.7-0.15.9.

Decouple the two concerns:
  - `cacheable` is now purely about content type. A partial or
    truncated text read records `cacheable: true` because the bytes
    the model saw were text.
  - Truncation gating moves to `full`. A request-level full read
    (no offset/limit/pages) only counts as full at the cache level
    when the produced content was not truncated; otherwise the model
    only saw the head of the file.

The fast-path `file_unchanged` placeholder still requires both
`lastReadWasFull && lastReadCacheable`, so its semantics are unchanged
— a truncated full read now fails the AND on the moved flag instead
of the original. WriteFile's `requireFullRead` still rejects partial
or truncated text reads; it now reports the accurate "partial read"
error instead of the wrong "binary payload" message.

Also fixes issue #3945 (edit tool unusable for large files) as a
side effect: the truncated-full case there hit the same misclassified
path before the rejection wording could even surface the truncation
question.

Tests: 2 regression tests added in read-file.test.ts (partial .kt
read and truncated full .cpp read both record `lastReadCacheable:
true`). Existing 7386 / 7391 (5 skipped) core tests pass; tsc
--noEmit clean.

Issue #3964 also reports a separate scenario on Windows
encrypted/DRM-protected file systems where .cpp source files are
misclassified by `isBinaryFile`'s 4KB content sampling. That path is
content-detection-side, not cache-side, and is left to a follow-up
(extension- or mime-based override of the content sample for known
text types).

* fix(core): trust extension/mime over isBinaryFile sampling for known text

Issue #3964's first report (Frank-Shaw-FS) describes `.cpp` / `.c` /
`.h` source files on a Windows encrypted / DRM-protected file system
being misclassified as binary. The OS surfaces encrypted bytes to
`fs.open()` random-access reads, so `isBinaryFile`'s 4 KB sample
sees nulls / non-printable characters and concludes binary — even
though the higher-level `readFile` returns the decrypted text and
the extension declares the file as text.

Layer-2 fix on top of the cache-side decoupling: change
`detectFileType` to trust the registry / curated extension list
*before* running the content sample, so a known text extension is
not subject to false positives from raw-bytes sampling.

  - Trust mime types declared as text: `text/*`, `application/*`
    text-likes (`application/javascript`, `application/json`,
    `application/toml`, ...), and any mime ending in `+xml` / `+json`.
  - Trust a curated set of source-code / config / markup
    extensions whose `mime/lite` registry coverage is patchy (`.py`,
    `.kt`, `.go`, `.rb`, `.swift`, `.scala`, `.rs`, `.proto`,
    `.graphql`, `.toml`, `.hcl`, `.tf`, ...). The list is restricted
    to extensions we have observed to be misclassified by
    `isBinaryFile` in the field; obscure extensions still go through
    the content sampler.

Order in `detectFileType`:
  1. Hardcoded `.ts` / `.svg` / `.ipynb`
  2. Mime check (image / audio / video / pdf / declared-text)
  3. `BINARY_EXTENSIONS` pre-empt (so `.png` with text-looking
     content stays binary)
  4. Curated text extension override (for mime-less source code)
  5. `isBinaryFile` content sampler (final fallback for
     unrecognised extensions)
  6. Default text

Tests: 5 new cases in `fileUtils.test.ts` and 1 end-to-end in
`read-file.test.ts` covering: text mime override on binary-looking
content, application/* text mimes, `+xml` / `+json` suffix match,
curated extension override on `.py` / `.kt` / `.go` / `.rb` /
`.swift`, and the `BINARY_EXTENSIONS` pre-empt still winning over
the new override (a `.png` whose first bytes happen to be ASCII
text stays binary). Full core suite passes (7392 / 7397, 5 pre-
existing skips); `tsc --noEmit` clean.

Together with the earlier commit, this PR closes both arms of issue
#3964: the cache-side `cacheable` conflation that affected partial /
truncated reads, and the content-detection-side false positive on
encrypted file systems.

* fix(core): tighten detectFileType after self-review on #4002

Three follow-ups flagged by `/review` on #4002:

1. `KNOWN_TEXT_APPLICATION_MIMES` had 10 dead entries — names like
   `application/x-sh`, `application/x-perl`, `application/x-yaml`,
   `application/x-tex`, `application/x-sql`, `application/graphql`
   are real mimes seen in HTTP `Content-Type` contexts but are not
   in `mime/lite`'s registry, so `mime.getType()` never returns
   them and the entries are unreachable. Strip the set to the 6
   values the registry actually emits (`javascript`, `ecmascript`,
   `node`, `json`, `xml`, `toml`); the shells / tex / sql / graphql
   extensions reach the text fallback through `KNOWN_TEXT_EXTENSIONS`
   instead. Add a scope rule in the docstring so future additions
   stay aligned with what mime/lite actually emits.

2. The early-return at the top of `detectFileType` listed
   `.ts / .mts / .cts / .tsx` in its comment but the array only
   contained `.ts / .mts / .cts`. `.tsx` was reaching the text
   verdict via `KNOWN_TEXT_EXTENSIONS`, which works today but
   would break if a future `mime/lite` update mapped `.tsx` to
   `video/mp2t` (mirroring `.ts`): the `startsWith('video/')`
   guard would fire before the text fallback. Move `.tsx` up to
   the early-return array so the comment matches the code and the
   defence is consistent across the TypeScript family. Drop the
   duplicate listing in `KNOWN_TEXT_EXTENSIONS`.

3. `isTextMime()` short-circuits `isBinaryFile` for any `text/*`
   mime, which is the necessary tradeoff for the encrypted-FS fix
   but removes the safety net for *corrupted* text files (a binary
   blob saved with a `.txt` / `.md` extension via redirection).
   Document the tradeoff explicitly with a concrete counter-example
   and call out that Edit's `0 occurrences` failure mode is the
   fallback for the corrupted-text population.

Tests: 261 / 262 (1 skipped, pre-existing) on
`fileUtils.test.ts` + `read-file.test.ts` + `edit.test.ts` +
`write-file.test.ts`. `tsc --noEmit` clean.

* fix(core): drop full-read requirement on WriteFile, align with Claude Code

PR #3932 deliberately diverged from Claude Code's `readFileState` by
keeping `requireFullRead: true` on WriteFile's prior-read
enforcement, citing issue #2499 (LLM hallucinates content of an
unread file and clobbers user changes) as evidence that the
asymmetric stance was justified. In practice that stance leaves a
hard deadlock: when a file is larger than `truncate-tool-output-
lines`, `read_file` without offset/limit still records
`lastReadWasFull: false` (the model only saw the head), and the
"only been partially read … re-read without offset / limit /
pages" rejection sends the model back to the same truncated read
with no escape — the exact deadlock issue #3945 reported.

Drop the `requireFullRead` option from `checkPriorRead` and remove
all 5 `requireFullRead: true` call sites in WriteFileTool. After
this change the contract is identical to Claude Code's: any prior
read of an existing file clears enforcement; the mtime/size drift
check is the only gate that distinguishes "the model has seen
current bytes" from "the model has seen older bytes", and it fires
identically for Edit and WriteFile.

The residual #2499 risk is acknowledged in the docstring: a model
that reads only a slice and then overwrites would necessarily
hallucinate the rest of the bytes. Mitigations:
  - `fileReadCacheDisabled: true` for users who want stricter
    behaviour (existing escape hatch, unchanged).
  - The mtime/size drift check still rejects Writes against bytes
    the model saw at fingerprint X if disk has moved to Y.

Cleanup: drops the dedicated "fresh + cacheable + partial +
requireFullRead" rejection branch and the `requireFullRead`-aware
wording variant in the `unknown` branch — both unreachable now.

Tests:
  - `write-file.test.ts:932` inverted from "rejects a write when
    the previous read was ranged" to "allows a write after a
    ranged read", matching the equivalent `edit.test.ts:1077`.
  - New `write-file.test.ts:961` regression for the issue #3945
    deadlock: a `recordRead({ full: false, cacheable: true })`
    entry (what a truncated full read produces) clears WriteFile
    enforcement.
  - 7393 / 7398 (5 skipped, all pre-existing) on the full core
    suite. `tsc --noEmit` clean.

* docs(core): add anti-regression notes locking in the WriteFile relax

Three sites a future contributor might naturally try to "tighten up"
back into the deadlock-prone shape, now carrying explicit guard
comments that name the prior PR (#3932), the issue it broke (#3945),
and the residual risk this stance accepts (#2499):

  - `priorReadEnforcement.ts:CheckPriorReadOptions` — interface-level
    note: do not re-introduce `requireFullRead` (or any "stricter for
    WriteFile than Edit") option here. References the function
    docstring for the full rationale.

  - `fileReadCache.ts:lastReadWasFull` — field-level note: sole
    consumer is the Read fast-path; `priorReadEnforcement` does not
    consult this and must not start.

  - `write-file.ts` first checkPriorRead call site — anchor comment
    that explains why no extra option is passed and applies to all
    5 call sites in the file.

No code changes; test suite unchanged at 7393 / 7398 (5 pre-existing
skips); `tsc --noEmit` clean.

* fix(core): #4002 review wave — basename allowlist + correct stale comments

3 #4002 review threads addressed:

- fileUtils.ts: added KNOWN_TEXT_BASENAMES allowlist for extensionless
  build / config / lockfiles (Dockerfile, Containerfile, Makefile,
  GNUmakefile, Jenkinsfile, Vagrantfile, Rakefile, Gemfile, Procfile,
  BUILD, WORKSPACE, CMakeLists.txt, go.mod, go.sum, go.work,
  Cargo.lock, Pipfile, Pipfile.lock, poetry.lock, package-lock.json,
  yarn.lock, pnpm-lock.yaml, requirements.txt, .gitignore,
  .gitattributes, .dockerignore, .npmignore, .editorconfig, .env,
  .bashrc, .zshrc, .profile, LICENSE, COPYING, AUTHORS, CHANGELOG,
  README, NOTICE). `path.extname('Dockerfile')` returns `''`, so the
  KNOWN_TEXT_EXTENSIONS check above misses these — an
  encrypted-volume read whose 4 KB sample looks binary would
  misclassify them as binary. Regression test pinned with
  fake-encrypted bytes for Dockerfile / Makefile / Jenkinsfile /
  go.mod / package-lock.json / .gitignore / LICENSE.

- priorReadEnforcement.ts: rewrote two misleading comments that
  pointed users to `fileReadCacheDisabled: true` for "stricter
  behaviour". That setting actually DISABLES enforcement entirely
  (skips checkPriorRead). Updated to make the opt-out semantics
  explicit and clarify that there is no built-in stricter mode —
  users who want stricter built-in enforcement than the residual
  #2499 risk accepts have no flag here today and should file a
  feature request.

- read-file.ts: updated the `lastReadWasFull` comment to reflect that
  PR #4002 removed WriteFile's `requireFullRead`. The flag now gates
  ONLY the `file_unchanged` fast-path; the stale "and WriteFile's
  full-read requirement" wording would have confused future readers
  into thinking WriteFile still consults `lastReadWasFull`.

Tests: 89/89 fileUtils.test.ts pass; tsc + ESLint clean.

* fix(core): split priorReadEnforcement guidance — partial OK for edit, full for overwrite

#4002 review: shared "never read" error said `(a partial read with
offset / limit is fine — you only need to have seen the bytes you
intend to edit/overwrite)` for BOTH Edit and WriteFile. For Edit
this is correct — the model only needs to have seen the
`old_string`-bearing bytes; the rest passes through untouched.
For WriteFile this is misleading: overwriting replaces EVERY byte,
so a partial read leaves any unseen bytes as collateral damage.
The mtime/size drift check still catches the worst-case #2499
hallucinated-bytes risk, but recommending a partial read in the
WriteFile guidance would actively encourage the footgun.

Fix: branch the partial-read guidance on `verb`. Edit keeps the
current "partial OK" text. WriteFile gets `(read the full file —
overwriting replaces every byte, so any unseen bytes would be
discarded)`.

120/120 edit + write-file tests pass; tsc + ESLint clean.

* docs(core): finish #4002 review wave — drop two stale "fileReadCacheDisabled is escape hatch" mentions

cc30278 + c6e2bde addressed 4 of the 6 #4002 review threads but
left two prior occurrences of the misleading "fileReadCacheDisabled:
true is the escape hatch for users who want stricter behaviour"
wording untouched. The flag actually goes the OPPOSITE way (skips
checkPriorRead entirely so application-level locking can take over),
so describing it as a "stricter" escape hatch is exactly the
guidance the c6e2bde review thread asked us to stop giving.

Files updated:

  - fileReadCache.ts:lastReadWasFull docstring — replaces the
    "stricter behaviour" sentence with the same opt-out / opt-in
    distinction c6e2bde used in priorReadEnforcement.ts.

  - write-file.ts anchor comment for all 5 checkPriorRead call
    sites — replaces the "fileReadCacheDisabled: true is the
    escape hatch" sentence with an explicit note on the opt-out
    direction matching the docstring.

Plus a coverage-split comment on the issue #3945 deadlock-free
regression test in write-file.test.ts (review thread #6 from
glm-5.1: pointed out the test seeds the cache directly rather
than driving a full ReadFile→WriteFile pipeline). A real
integration test would need ReadFile-side mockConfig plumbing
(`getFileService`, `getTruncateToolOutputLines`, etc.) ported
into write-file.test.ts; the comment captures the link to
read-file.test.ts's matching cache-population assertion so a
future cache-entry schema change has to update both halves to
keep the end-to-end guarantee.

Tests: 295 / 296 (1 pre-existing skip) on the affected files;
tsc --noEmit clean.

* chore(core): add debug logs to detectFileType text fast-paths

#4002 review (DeepSeek): the new text-classification branches
returned `'text'` without logging which path fired, leaving future
#3964-class troubleshooting unable to tell mime-trust from
extension-override from basename-override from the content-sample
fallback without re-deriving by code reading.

Add `debugLogger.debug` calls on the three new fast-path branches:
mime trust (`isTextMime` match), extension override
(`KNOWN_TEXT_EXTENSIONS`), and basename override
(`KNOWN_TEXT_BASENAMES`). Each log includes the path, the chosen
classification, and the looked-up mime when relevant — enough to
disambiguate the four classification paths from a single line.

Off by default (`debug` level on the `FILE_UTILS` logger). Older
branches (image / audio / video / pdf / hardcoded TS / SVG / ipynb /
BINARY_EXTENSIONS / isBinaryFile / default text) keep their existing
silent behaviour: they predate the issue this is paving for and
adding logs there would be scope creep.

Tests: 89 / 89 fileUtils.test.ts pass; tsc --noEmit clean.

…QwenLM#3774)

* feat(core): enforce prior read before Edit / WriteFile mutates a file

Introduces a session-scoped invariant: the model cannot mutate an
existing file without having actually Read it (or its post-write
state) earlier in this conversation. Builds on the FileReadCache
landed in QwenLM#3717.

Two new ToolErrorType codes:
- EDIT_REQUIRES_PRIOR_READ — file has no entry in the session cache.
  The model is told to use read_file first.
- FILE_CHANGED_SINCE_READ — file has an entry but its mtime or size
  drifted since the recorded fingerprint. The model is told to
  re-read before retrying.

EditTool blocks the existing-file path on cache.check; new-file
creation (old_string === '' on a non-existent target) is exempt.
WriteFileTool blocks the overwrite path; new-file creation
(fileExists === false) is exempt.

Both tools route through the existing fileReadCacheDisabled escape
hatch on Config — flipping it bypasses enforcement byte-for-byte,
matching pre-cache behaviour. Operators can use this as a kill switch
if a session falls into a state where the cache cannot be trusted.

ReadFile fix on the auto-memory path: PR QwenLM#3717 had auto-memory reads
skip the cache entirely (both lookup and record), but with the new
enforcement that means a model that just Read AGENTS.md cannot then
Edit it. Decoupled the two: auto-memory reads still skip the
file_unchanged fast-path (the per-read freshness <system-reminder>
must always reach the model) but DO record into the cache so the
follow-up Edit sees `fresh`. New regression test asserts this.

Test plan
- vitest run (all of @qwen-code/qwen-code-core): 6308 passed, 2 skipped
- 9 new enforcement tests across edit.test.ts and write-file.test.ts:
  unknown rejects, stale rejects, new-file exempt, edit chain stays
  authorised, escape hatch bypasses, plus the auto-memory record
  regression in read-file.test.ts.
- tsc --noEmit clean. eslint clean. core build succeeds.

* test(core): clear shared fileReadCache between write-file.test.ts cases

CI surfaced one Linux-only failure: the prior-read enforcement test
'rejects a write that would overwrite an unread existing file'
returned FILE_CHANGED_SINCE_READ instead of EDIT_REQUIRES_PRIOR_READ.

Root cause: the FileReadCache instance is declared at module scope
(line 41) and shared across every test in write-file.test.ts. State
from earlier tests — most recently the 'records a write' integration
test that records the same path — leaks forward. On Linux the test
ordering puts a record-bearing test before the enforcement test, so
the cache reports `stale` (mtime drifted) instead of `unknown`.
macOS / Windows happen to order them differently and never hit it.

Adding a fileReadCache.clear() to beforeEach gives every test a
known-empty cache, matching how edit.test.ts already isolates its
per-test cache by re-instantiating it.

* fix(core): close prior-read enforcement gaps flagged in 3rd review

Three concrete loopholes / regressions that the original PR-B
introduction left open. All three are addressed in the same commit
because the underlying refactor (move enforcement earlier and tighten
the fresh predicate) is shared across them.

1. fresh != "model has seen the bytes". Pre-fix, requirePriorRead()
   accepted any cache.check === 'fresh'. ReadFileTool records every
   successful read into the cache, including ranged reads
   (offset/limit), truncated full reads, and non-cacheable
   binary/image/audio/video/PDF/notebook reads (lastReadCacheable
   = false). This let the model peek at a slice or a structured
   payload of a file and then mutate the whole thing. Tightened the
   accept predicate to fresh && lastReadAt && lastReadWasFull &&
   lastReadCacheable.

2. Read-less content oracle through calculateEdit error codes. Pre-fix,
   execute() ran calculateEdit (which reads file bytes and counts
   matches) before the enforcement check. A model could probe an
   unread file by attempting Edits with candidate old_strings and
   observing NO_OCCURRENCE_FOUND vs EXPECTED_OCCURRENCE_MISMATCH vs
   EDIT_NO_CHANGE — reverse-engineering content without ever calling
   read_file. Moved enforcement to the top of calculateEdit, before
   any content read; only a stat is performed up to the rejection
   point.

3. Confirmation flow regression. Pre-fix, getConfirmationDetails()
   read the existing file to render a diff for the user, then
   approval flowed to execute() which would freshly check the cache
   and reject. The user could approve a diff computed from current
   bytes the model never saw, and the call would still fail. Moved
   enforcement before the confirmation read in both EditTool (via the
   shared calculateEdit path) and WriteFileTool (explicit check at
   the top of getConfirmationDetails). The user now never sees a
   confirmation diff for an unread file — the call rejects up front.

Public API surface change: requirePriorRead() -> checkPriorRead() that
returns a structured decision, so the same predicate can route into
a CalculatedEdit.error (calculateEdit), a thrown error
(getConfirmationDetails), or a ToolResult (execute) without
duplicating the boolean / message / type plumbing in three shapes.

Reported by pomelo-nwu (3 inline comments on PR QwenLM#3774).

* refactor(core): close 4 prior-read enforcement gaps from 4th review

1. recordWrite now seeds read metadata on brand-new entries
   (lastReadAt / lastReadWasFull / lastReadCacheable). The strict
   accept predicate added in the previous round (QwenLM#3 review) requires
   all three, but recordWrite only set lastWriteAt — so a model
   creating a file with Edit (old_string="") or WriteFile and then
   editing it again was rejected on the second edit. The model
   authored the bytes it just wrote; for the purposes of prior-read
   enforcement that counts as having seen them. New regression test
   in edit.test.ts: "allows a create-then-edit-then-edit chain
   without an intervening read".

2. Extracted checkPriorRead into src/tools/priorReadEnforcement.ts.
   The two copies in edit.ts and write-file.ts had already drifted
   (one used ${ReadFileTool.Name}, the other hardcoded 'read_file');
   the boolean guard is security-sensitive and a one-sided fix
   would silently weaken the boundary. The shared utility takes a
   verb ('editing' | 'overwriting') so the user-facing prose can
   differ between callers without duplicating the decision logic.

3. WriteFileTool.execute now runs prior-read enforcement BEFORE
   readTextFile. Pre-fix, an unread overwrite still slurped the
   entire file into memory (encoding / BOM / line-ending detection)
   and only then rejected it: wasted I/O, and momentary in-memory
   custody of bytes the model never legitimately read. Now matches
   the order in getConfirmationDetails().

4. The "rejects a write that would overwrite an unread existing
   file" test now spies on FileSystemService.readTextFile and
   asserts not.toHaveBeenCalled() — without that, the test gave
   false confidence: it passed both pre-fix (read happened, then
   reject) and post-fix (reject before read), so the ordering
   regression in (3) was invisible to the assertion.

Reported by glm-5.1 via /review on PR QwenLM#3774.

* refactor(core): close 4 prior-read enforcement gaps from 4th review (Copilot)

Five concrete gaps that the previous round of enforcement work left
open. Reported by Copilot via /review on PR QwenLM#3774.

1. Confirmation-time rejections lost their ToolErrorType code.
   getConfirmationDetails() in both EditTool and WriteFileTool threw
   a plain Error on prior-read failure, which coreToolScheduler
   collapsed into UNHANDLED_EXCEPTION — silently breaking the
   EDIT_REQUIRES_PRIOR_READ / FILE_CHANGED_SINCE_READ contract for
   any approval-required flow.

   Fix: introduce PriorReadEnforcementError that carries
   `errorType: ToolErrorType`. Both confirmation paths now throw it,
   and coreToolScheduler reads `error.errorType` (falling back to
   UNHANDLED_EXCEPTION when absent). New regression tests assert
   the thrown error's `errorType` field for both tools.

2. checkPriorRead's "re-read with read_file" advice was wrong for
   binary / image / audio / video / PDF / notebook files. Their
   ReadFile result always sets lastReadCacheable=false, so the
   message would loop the agent forever on the same rejection.

   Fix: detect the fresh-but-non-cacheable case explicitly and
   return a dedicated message that explains the dead end ("Edit /
   WriteFile cannot mutate that payload safely") instead of asking
   for another read. Updated the existing non-cacheable regression
   test to assert the new message and the absence of "use the
   read_file tool first".

3. checkPriorRead swallowed every stat() failure and returned
   ok:true. EACCES, EBUSY, NFS hiccups, etc. would silently
   re-open the blind-write path the helper exists to block.

   Fix: only ENOENT continues to return ok:true (disappearance
   race). Any other code is fail-closed: returns
   EDIT_REQUIRES_PRIOR_READ with a message that names the errno.
   New regression test in write-file.test.ts spies on fs.promises
   .stat to inject EACCES and asserts the rejection.

4. The auto-memory record regression test only asserted `state ===
   'fresh'`. A future change that recorded auto-memory reads as
   partial / non-cacheable would still satisfy that assertion but
   would actually fail enforcement on every follow-up Edit.

   Fix: also assert lastReadAt is defined, lastReadWasFull is true,
   and lastReadCacheable is true. The full "what enforcement
   requires" predicate is now explicit in the test.

(The 5th item, the WriteFile mirror of (1), is covered by the same
PriorReadEnforcementError change.)

* refactor(core): tighten StructuredToolError naming + add scheduler test

Four follow-ups raised by deepseek-v4-pro on PR QwenLM#3774. None of them
change the enforcement boundary; they are all about making the
contract clearer and harder to break in future changes.

1. PriorReadEnforcementError -> StructuredToolError. The class now
   wraps any content-derived ToolErrorType from calculateEdit
   (EDIT_NO_OCCURRENCE_FOUND, EDIT_EXPECTED_OCCURRENCE_MISMATCH,
   EDIT_NO_CHANGE, ATTEMPT_TO_CREATE_EXISTING_FILE) on top of the
   prior-read codes. The old name suggested the class was prior-
   read-specific, which would mislead any oncall engineer seeing
   it paired with one of the calculateEdit error codes.

2. EDIT_REQUIRES_PRIOR_READ kept its name (the prefix mentions
   "edit" but the enum is shared with WriteFileTool) — chose
   documentation over rename to avoid the churn of a value rename
   across logs/dashboards already keyed on it. JSDoc now spells
   out the cross-tool usage explicitly.

3. Stat failures other than ENOENT now map to a new
   PRIOR_READ_VERIFICATION_FAILED code instead of being conflated
   with EDIT_REQUIRES_PRIOR_READ. The failure mode is "we cannot
   verify" rather than "definitely not read" — operators routing
   on error codes can distinguish the two populations.

4. Added a coreToolScheduler test (`surfaces error.errorType from
   a confirmation throw instead of UNHANDLED_EXCEPTION`) that
   constructs a stub tool whose getConfirmationDetails throws
   StructuredToolError and asserts the surfaced ToolCall response
   carries the correct ToolErrorType. Without this test the
   scheduler's explicitErrorType branch would have no coverage at
   all.

Tool tests updated for the new StructuredToolError class name and
the PRIOR_READ_VERIFICATION_FAILED code on the EACCES path.

* fix(core): close TOCTOU + grammar + directory regressions in PR-B

Six concrete issues that the previous round of enforcement work
left open. Reported by Copilot via /review on PR QwenLM#3774.

1. TOCTOU window between pre-read checkPriorRead and readTextFile.
   The pre-read stat could pass enforcement, then an external writer
   could land between that stat and the actual read, leaving
   currentContent reflecting bytes the model never saw — exactly the
   stale-write path the PR is supposed to block. Closed by re-running
   checkPriorRead immediately after every readTextFile that fed
   currentContent / originalContent: EditTool.calculateEdit and the
   two WriteFileTool paths (execute + getConfirmationDetails). A
   `stale` outcome now fails the operation with
   FILE_CHANGED_SINCE_READ at the correct moment.

2. Directory targets sent the model into an enforcement loop.
   `fileExists` is a plain access check, so directories also entered
   the enforcement branch — the model would be told to call
   `read_file`, but `read_file` rejects directories with
   TARGET_IS_DIRECTORY, so the loop never terminated. Fixed in
   checkPriorRead: if `fs.stat` reports the path is not a regular
   file, return `ok: true` so the downstream readTextFile / write
   path can surface its own EISDIR / similar error.

3. Confirmation-time error messages used the short `display` form
   instead of the full `raw` form. Approval-required Edit calls
   therefore lost the remediation detail (file path, stale-vs-unread
   distinction, "without offset / limit / pages" hint) that the
   execute path already surfaced and that the WriteFile confirmation
   path already preserved. EditTool.getConfirmationDetails now
   throws StructuredToolError with `editData.error.raw`.

4. Non-text payload displayMessage was grammatically broken: built
   from the gerund `editing` / `overwriting`, it rendered as
   "cannot editing via this tool" / "cannot overwriting via this
   tool". Fixed by deriving a bare-verb form (`edit` / `overwrite`)
   alongside the gerund and using it in displayMessage.

(Items 1, 5 and 6 from Copilot's batch are the same TOCTOU class —
EditTool calculateEdit + WriteFile execute + WriteFile confirmation —
addressed together in (1) above.)

The "bypasses enforcement entirely" test now uses mockReturnValue
instead of mockReturnValueOnce because calculateEdit calls
getFileReadCacheDisabled twice — once for the pre-read check and
once for the post-read TOCTOU re-check. Both must see disabled=true
to actually bypass.

* fix(core): close fileExists TOCTOU on WriteFile prior-read enforcement

WriteFile gated prior-read enforcement on `fileExists` from
`isFilefileExists()`, but a file that sprang into existence between
that check and the write would still be overwritten without
enforcement — `fileExists === false` skipped the check entirely.

Made the gate unconditional on `fileExists`. checkPriorRead's own
`fs.stat` decides what to do:
- ENOENT → ok:true, fall through to the new-file path as before
- file exists right now (whether or not isFilefileExists saw it) →
  unknown / stale check runs, the race-created file is rejected.

Applied to both getConfirmationDetails and execute. The path that
actually creates new files is unchanged because checkPriorRead's
ENOENT branch is the disappearance-race exit, which is the correct
exit for "the file truly does not exist yet".

Reported by glm-5.1 via /review on PR QwenLM#3774.

* fix(core): close 4 enforcement gaps + 1 critical bug from 5th Copilot review

Six issues raised by deepseek-v4-pro / glm-5.1 / qwen3.6-plus on
PR QwenLM#3774. Listed by reviewer-assigned severity.

[Critical] (qwen3.6-plus) recordWrite previously only seeded the
read metadata for brand-new entries. The reproduction was real:
ReadFile(limit=10) → WriteFile(full content) → Edit. The partial
read's lastReadWasFull=false would persist through the write, and
the Edit would be rejected with EDIT_REQUIRES_PRIOR_READ even
though the model just authored every byte. recordWrite now
unconditionally refreshes lastReadAt, lastReadWasFull=true, and
lastReadCacheable=true. The fileReadCache.test.ts case that
previously asserted "preserves lastReadAt" is rewritten to assert
the new "refreshes lastReadAt to match the write" contract, and a
new "upgrades lastReadWasFull / lastReadCacheable after a full
write" regression test pins the reproduction reviewer described.

[Suggestion] (deepseek-v4-pro) Narrowed the non-regular-file
bypass in priorReadEnforcement from `!stats.isFile()` to
`stats.isDirectory()`. The earlier broad form covered FIFOs,
sockets, and devices that the model has no legitimate "read first"
recourse for and that can block readTextFile (FIFO) or
over-allocate (/dev/urandom). Those now flow through to
cache.check() and reject with the unread-file path before any I/O.

[Suggestion] (glm-5.1) Removed the `fileExists && ...` gate from
EditTool.calculateEdit, mirroring the 16a3a5761 fix on WriteFile.
A file that springs into existence between isFilefileExists() and
the enforcement check is now caught here as well; ENOENT inside
checkPriorRead remains the disappearance-race exit and new-file
creation flow is unchanged.

[Suggestion] (deepseek-v4-pro) Added debugLogger.warn() at every
post-read TOCTOU rejection site (Edit calculateEdit, WriteFile
getConfirmationDetails, WriteFile execute). These rejections are
rare and self-healing — without a debug record, an operator
investigating "why did this Edit fail once?" had nothing to grep.
debugLogger uses dedicated 'EDIT_PRIOR_READ' / 'WRITE_FILE' tags.

[Suggestion] (qwen3.6-plus) Added a final pre-write checkPriorRead
in EditTool.execute() and WriteFileTool.execute(). The earlier
post-read check ran inside calculateEdit (Edit) or before mkdirSync
(WriteFile), but the actual writeTextFile call could be arbitrarily
later — user approval, modify-and-confirm, etc. The window from
"post-read check → writeTextFile" is now bounded to "pre-write
stat → writeTextFile" (two adjacent syscalls).

* fix(core): close new-file race + special-file enforcement loop

Three issues from the latest Copilot review on PR QwenLM#3774.

1. New-file race in pre-write enforcement (write-file.ts:348,
   edit.ts:487). The earlier pre-write checkPriorRead was gated on
   `fileExists` (WriteFile) and `!editData.isNewFile` (Edit). If the
   path was absent at planning time and another process created it
   while approval was pending, the gated form would skip enforcement
   and silently overwrite a pre-existing file the model never read.
   Run unconditionally in both tools — checkPriorRead's own ENOENT
   branch is the disappearance-race exit, so genuine new-file
   creation is unaffected, but a race-created file now hits the
   `unknown` branch and is rejected as unread.

2. FIFO / socket / device sent the model into an enforcement loop
   (priorReadEnforcement.ts:220). After narrowing the
   non-regular-file bypass to directories only, FIFOs etc. fell
   through to cache.check, returned `unknown`, and produced a
   "use read_file first" message — but read_file rejects those same
   targets as "not a regular file", so the model would loop on
   read_file forever. Added a dedicated `!stats.isFile()` branch
   (after the directory exemption) that returns a "special file;
   cannot edit/overwrite via this tool — use shell instead" message,
   matching the shape of the existing non-text-payload guidance.

(Tool-error.ts and the non-cacheable policy notes are addressed in
the PR description update — not in code.)

* fix(core): close 4 enforcement gaps from 6th Copilot review

(Plus a doc-only update for the 5th — the mtime+size limitation
warning in the Risk section now mentions the silent-overwrite
escalation that this PR's mutation paths bring along.)

1. ENOENT after the model has already read the file is no longer
   silently treated as `ok: true`. Added an `expectExisting` option
   to `checkPriorRead`; post-read and pre-write callers pass `true`.
   ENOENT under that flag now rejects with `FILE_CHANGED_SINCE_READ`
   ("file disappeared after the model read it") rather than falling
   through to the new-file path with stale bytes. Pre-read callers
   keep the old default (ENOENT → ok:true → fall through to genuine
   new-file creation). EditTool's pre-write check derives the flag
   from `editData.isNewFile`; WriteFile's pre-write check derives it
   from the post-read `fileExists` value.

2. Directory targets now reject with `TARGET_IS_DIRECTORY` and a
   structured message instead of returning `ok: true`. The previous
   form fell through to readTextFile(), which on the WriteFile
   confirmation path threw a plain Error and was surfaced by the
   scheduler as `UNHANDLED_EXCEPTION`. Both Edit and WriteFile now
   emit a structured rejection at enforcement time. (WriteFile's
   build-time validateToolParamValues already rejects directories,
   so the change matters most for EditTool.)

3. Non-cacheable rejection's `rawMessage` no longer hard-codes
   "overwrite" — it now uses the same `verbBare` derivation as the
   `displayMessage`, so EditTool's path correctly says "if you need
   to edit it" and WriteFile's path stays "if you need to overwrite
   it". The previous form was confusing for in-place edits.

4. WriteFile.getConfirmationDetails now mirrors execute()'s
   ENOENT-to-new-file fallback: a file that disappears between
   isFilefileExists() and the readTextFile-for-diff call no longer
   throws a plain Error (which would surface as
   UNHANDLED_EXCEPTION) — it falls back to the brand-new-file diff
   so the user sees a clean confirmation rather than an unstructured
   crash.

Tests:
- New: `rejects an edit on a directory with TARGET_IS_DIRECTORY`
- New: `confirmation falls back to a new-file diff when the file
  disappears mid-flight` (WriteFile)
- Updated: non-cacheable rejection asserts `verbBare` is "edit" on
  the EditTool path and "overwrite" on the WriteFile path.

Reported by Copilot via /review on PR QwenLM#3774.

* docs(core): clarify stat→write race + EDIT_REQUIRES_PRIOR_READ scope

Three doc-only follow-ups from Copilot's latest review pass on PR
QwenLM#3774. None change behaviour; the pre-fix code state was already
the actual contract — the docs just lagged it.

1. EDIT_REQUIRES_PRIOR_READ enum comment now lists the three cases
   the code actually returns it for (never-read, partial / ranged /
   non-cacheable read, structural dead end — non-text payload or
   special file). The previous one-liner described only the first
   case and would mislead future maintainers.

2. The Final pre-write freshness check blocks in EditTool.execute
   and WriteFileTool.execute now spell out that they DO NOT
   eliminate the stat → writeTextFile race. The window narrows
   from the previously-unbounded post-read-to-write gap down to
   two adjacent syscalls, but a concurrent writer landing in
   that pair can still be clobbered. Closing the residual would
   require an atomic write (write-to-temp + rename) or a
   content-hash post-write recheck — both deferred. Operators who
   need strict protection set `fileReadCacheDisabled: true` and
   rely on application-level locking.

3. PR description Risk section gains a "Known unmitigated: stat →
   write race window" subsection (English + Chinese mirror)
   matching the code comments.

* chore(core): minor follow-ups from review #4229917446

Three of the five MINOR items raised in the independent code review
on 2026-05-05 — the cheap, isolated ones. The other two (race-
simulating integration test, moving StructuredToolError out of
priorReadEnforcement.ts) are deferred as the reviewer suggested.

1. EditTool now has a symmetric `PRIOR_READ_VERIFICATION_FAILED`
   regression test (mocks fs.promises.stat to reject with EACCES,
   asserts the EditTool path produces the same fail-closed result
   that the existing WriteFile EACCES test pins). Five-line fix to
   close the asymmetry that, while harmless today (the helper is
   shared), would let a future Edit-side change to checkPriorRead
   slip through without test coverage.

2. ensureParentDirectoriesExist / mkdirSync now run AFTER the
   pre-write checkPriorRead in both EditTool.execute() and
   WriteFileTool.execute(). Doing it before would leak intermediate
   directories on the rejection path — a real (if minor) FS litter
   the previous order created on every rejected new-file write.

3. EDIT_REQUIRES_PRIOR_READ enum docstring gains a one-line note
   for operators routing alerts on this code: a single
   `edit_requires_prior_read` signal can mean any of the three
   cases (no read / partial read / structural dead-end), and if
   per-cause monitoring becomes important the enum can be split
   in a follow-up. The originating tool name and the message text
   already disambiguate at runtime.

* fix(core): close 2 correctness gaps from maintainer review #4232751470

Both tracked back to the cache's "track most recent read shape"
model diverging from prior-read enforcement's "model has seen
these bytes" model.

1. SVG (and similar string-content fallbacks) recorded as
   non-cacheable, blocking subsequent Edit / WriteFile.

   `read-file.ts` derives `cacheable` from
   `originalLineCount !== undefined && !isTruncated`. The SVG
   branch in `fileUtils.ts` returned content without
   `originalLineCount`, so `cacheable` collapsed to false and a
   follow-up Edit hit the dead-end "non-text payload — use shell"
   rejection — telling the model to use shell to mutate a file it
   had just successfully read as text. This was a real regression
   vs pre-PR behaviour where SVG-as-text editing worked.

   Fix: SVG-as-text branch now sets `originalLineCount` (split
   on '\n') and `isTruncated: false`, so ReadFile records it as
   a full cacheable read. The binary-fallback string and
   over-1MB SVG branches are deliberately left non-cacheable —
   they return placeholder strings ("Cannot display content of
   ...") rather than file content, so blocking edits there is
   correct. New regression test in `read-file.test.ts`:
   `records SVG-as-text reads with cacheable=true so a follow-up
   Edit passes enforcement`.

2. recordRead unconditionally overwriting lastReadWasFull /
   lastReadCacheable, revoking prior write-author or full-read
   rights.

   The `WriteFile(create) → ReadFile(offset/limit) → Edit`
   sequence rejected the Edit because the partial read clobbered
   the `lastReadWasFull = true` that `recordWrite` had stamped
   at create time. Same shape applies to a full text read
   followed by a partial one of the same inode.

   Fix: `recordRead` is now sticky-on-true for the read flags —
   `if (opts.full) entry.lastReadWasFull = true;` and the
   matching guard for `cacheable`. Prior `true` survives a later
   partial / non-cacheable read. The fast-path `file_unchanged`
   check still gates on the incoming request's own `isFullRead`
   in `read-file.ts`, so a partial read still does not get a
   placeholder it shouldn't. Updated the existing
   "overwrites earlier lastReadWasFull" test to assert the new
   sticky semantics, and added a `lastReadCacheable` symmetric
   test plus a `Write → partial-Read → Edit` end-to-end test in
   `edit.test.ts`.

Reported by tanzhenxin via independent maintainer review on
2026-05-06.

* fix(core): close 3 correctness gaps from re-review #4233904930

All three are tightenings of the prior `92218152c` round.

1. **Sticky-on-true narrowed to "no fingerprint drift"**.
   `fileReadCache.recordRead` previously kept `lastReadWasFull` /
   `lastReadCacheable` true across drifted recordings, which
   re-opened a `Read full @x → external write @y → Read partial
   @y → Edit` hole: the partial recordRead silently advanced the
   entry's mtime+size to Y while preserving the sticky `full=true`
   from X, so a follow-up Edit ran against bytes the model only
   saw the first 10 lines of. Now the sticky branch only fires
   when `(mtimeMs, sizeBytes)` matches the existing entry; on
   drift, both flags reset to exactly what this read produced.
   New regression test in `fileReadCache.test.ts` reproduces the
   reviewer's reported sequence.

2. **Subagent FileReadCache isolation now covers the
   inherits-model + same-approval-mode common case**. The
   own-property machinery from QwenLM#3717 only triggers when an
   `Object.create(parent)` actually fires; both
   `agent.ts:990-993` (same-approval-mode) and
   `subagent-manager.ts:699-701` (inherits-model) had paths that
   returned the parent Config directly, so the subagent's
   `getFileReadCache()` resolved to the parent's instance — a
   parent Read could satisfy the subagent's Edit on a path the
   subagent's transcript never contained. Both sites now build
   a thin `Object.create(base)` override unconditionally; no
   method changes for the inherits / same-mode cases, but a
   distinct instance triggers the lazy-init in
   `Config.getFileReadCache()` so the subagent gets an isolated
   cache.

3. **Cache records the read pipeline's internal stat instead of
   a post-read re-stat**. `processSingleFileContent` now
   surfaces its internal stat via `result.stats`, and read-file
   uses that for `recordRead` instead of running its own stat
   after the read returns. Pre-fix, an external write between
   the pipeline call and the post-read stat let the cache record
   fingerprint Y for content the model received at X — a
   subsequent Edit would pass enforcement against bytes the
   model never legitimately saw. The internal-stat-to-read
   window is still a few microseconds wide; that residue is the
   same content-hash territory acknowledged in the Risk section.

Reported by tanzhenxin via re-review on PR QwenLM#3774.

* docs(core): clarify partial subagent isolation per review #4234090906

tanzhenxin's third review correctly observed that the
`Object.create(parent)` wrappers in `agent.ts:createApprovalModeOverride`
and `subagent-manager.ts:maybeOverrideContentGenerator` only isolate
the FileReadCache for code that consults `Config.getFileReadCache()`
directly. Bound `EditTool` / `WriteFileTool` instances were registered
against the parent's tool registry at initialise time, so tool
invocations still resolve `this.config` to the parent and reach the
parent's cache. `InProcessBackend.createPerAgentConfig` already does
the right thing (`override.createToolRegistry()` +
`copyDiscoveredToolsFrom(base.getToolRegistry())`); bringing that to
these two spawn sites is the real fix.

Reviewer's verdict was COMMENT, not REQUEST_CHANGES — the gap
pre-dates this PR (it's a property of QwenLM#3717's per-Config own-property
machinery) and pre-PR there was no enforcement on subagent mutations
at all, so the PR is strictly an improvement on every spawn path.
Documented the partial guarantee explicitly:

- Inline comments on both spawn sites note the bound-tool caveat
  and point at `InProcessBackend.createPerAgentConfig` as the model
  for the follow-up.
- PR description's subagent paragraph (English + Chinese mirror) now
  splits into "fully isolated" (`InProcessBackend.createPerAgentConfig`)
  and "partial isolation" (the two sites in this PR) so readers don't
  walk away with the wrong contract.

Filing the registry-rebuild work as a follow-up; not in this PR.

…d tools resolve to the subagent (QwenLM#3873)

* fix(core): rebuild tool registry on subagent Config overrides so bound tools resolve to the subagent

PR-B (QwenLM#3774) added per-Config FileReadCache isolation via Object.create
overrides at two subagent spawn sites — agent.ts:createApprovalModeOverride
and subagent-manager.ts:maybeOverrideContentGenerator. The override
shielded code that read FileReadCache directly through the Config
instance, but missed the bound-tool path: Config.createToolRegistry runs
once at parent initialise time, so the parent's EditTool / WriteFileTool
/ ReadFileTool instances are bound with `this.config = parent`. The
subagent's Object.create wrapper inherited getToolRegistry via the
prototype chain, reaching the parent registry whose bound tools then
read FileReadCache and approval mode from the parent.

This change closes that gap by rebuilding the tool registry on the
override at both sites — the same pattern InProcessBackend.createPerAgentConfig
already uses:

  - override.createToolRegistry(undefined, { skipDiscovery: true })
  - registry.copyDiscoveredToolsFrom(base.getToolRegistry())
  - override.getToolRegistry = () => registry

createApprovalModeOverride becomes async; its single call site already
ran inside an async block. maybeOverrideContentGenerator skips the
rebuild when the upstream Config already has its own getToolRegistry
(real-world case: agent.ts wrapper passed through createAgentHeadless),
avoiding wasted work, listener accumulation on shared SubagentManager /
SkillManager, and a cache split where the bound tools' registry layer
diverges from the runtime context's lazy-init cache.

Includes regression tests in agent-override.test.ts and
subagent-manager-override.test.ts that exercise the bound-tool path:
they instantiate the lazy factories on the override registry and
assert that EditTool / WriteFileTool / ReadFileTool resolve
this.config to the override Config (and thus to the override's
FileReadCache / approval mode), not the parent.

* fix(core): close bound-tool gap on resumed background agents too

Follow-up audit on PR QwenLM#3873 surfaced a duplicate, pre-rebuild copy of
`createApprovalModeOverride` living in `background-agent-resume.ts`
(L142-150). Resumed fork agents go through `createResumedForkSubagent`
which bypasses `SubagentManager.maybeOverrideContentGenerator` (where
the registry rebuild now lives), so the resumed fork's
`EditTool` / `WriteFileTool` / `ReadFileTool` were still resolving
`this.config` to the parent and reading the parent's `FileReadCache`.

The non-fork resume path went through `subagent-manager` and worked
correctly only because `maybeOverrideContentGenerator` saw no upstream
own-registry on `bgConfig` and rebuilt one — but with that fallback
the fork path could never benefit.

This change deletes the local copy and switches `background-agent-resume.ts`
to import the now-async exported `createApprovalModeOverride` from
`agent.ts`. Drops the previous `?: this.config` short-circuit so the
resumed agent ALWAYS gets a wrapper Config — the same behaviour
`agent.ts` already enforces; reusing the parent directly defeats the
per-Config FileReadCache isolation.

Updates `background-agent-resume.test.ts` mock config with the
`createToolRegistry` / `getToolRegistry` stubs the rebuild path now
exercises.

* fix(core): address bound-tool isolation review feedback

Three independent fixes from PR QwenLM#3873 review feedback:

1. Switch the upstream-rebuild guard from
   `hasOwnProperty(base, 'getToolRegistry')` to a Symbol-keyed marker
   `TOOL_REGISTRY_REBUILT`. The own-property check missed the case
   where the override is reached via an Object.create wrapper above
   the rebuilt Config (e.g. `bgConfig = Object.create(agentConfig)` in
   the agent.ts background path) — it would falsely report "no
   upstream rebuild" and cause a redundant third rebuild that wastes
   work and doubles the listener-leak surface. Symbol property reads
   walk the prototype chain via normal lookup, so a marker stored on
   any ancestor is correctly observed.

   Extracts the shared rebuild logic into
   `rebuildToolRegistryOnOverride(override, base)` so the three spawn
   sites (agent.ts:createApprovalModeOverride, the inherits branch,
   the non-inherit branch) cannot drift apart.

2. Stop the per-subagent ToolRegistry in the lifecycle finally blocks:
   - agent.ts foreground finally (after the inner try wrapping
     `runFramed`)
   - agent.ts background bgBody finally (after `bgSubagent.execute`
     resolves)
   - background-agent-resume.ts resume body finally (same shape)

   Without this, every AgentTool / SkillTool the model instantiates
   from the per-subagent registry registers a change-listener on
   shared SubagentManager / SkillManager, and repeated subagent runs
   accumulate listeners for the rest of the session. Stop is
   fire-and-forget, matching `InProcessBackend.cleanup` and
   `stopAgent`.

3. Add bound-tool isolation tests for the non-inherit branch
   (explicit-model selector). The original PR only covered the
   inherits branch directly; the non-inherit branch now goes through
   the same helper, but a dedicated test pins
   `tool.config === override` and the FileReadCache binding so a
   regression cannot leave explicit-model subagents reading the
   parent's cache while existing model-override tests still pass.

Tests now exercise:
- Symbol marker propagation via Object.create chain (3 cases)
- Non-inherit rebuild + bound-tool isolation
- Non-inherit skip-rebuild when upstream wrapper has the marker
- Pre-existing inherits / chained-override / approval-mode propagation
- Mock configs in agent.test.ts / subagent-manager.test.ts /
  background-agent-resume.test.ts gain `stop` and `tools: Map` stubs
  to model the registry contract the override path now exercises.

`npx vitest run packages/core/src` — 268 files / 6943 passed.

…quired for WriteFile (QwenLM#3932)

PR QwenLM#3774 introduced a `lastReadWasFull` requirement to `checkPriorRead`
that forced models to re-read multi-thousand-line files just to make
a single-line edit. The `0 occurrences` failure mode in `calculateEdit`
already catches a fabricated `old_string` that misses the actual bytes,
so requiring a full read on top of that is over-defence at a real
context cost — but only for Edit, not for WriteFile.

WriteFile is asymmetric: it replaces the entire file and has no
content-derived guard equivalent to `old_string` matching. A model
that has only seen a slice via `read_file(offset, limit)` followed
by a `WriteFile` would necessarily hallucinate the rest of the bytes
— the issue QwenLM#2499 data-loss scenario PR QwenLM#3774 was opened to address.

Split the policy along that line. `checkPriorRead` gains a
`requireFullRead?: boolean` option. WriteFileTool's 5 enforcement
call sites pass `true`; EditTool's 3 leave it unset (default `false`):

  - EditTool: partial read counts (old_string is the floor)
  - WriteFileTool overwrite: full read required
  - Either: new-file creation exempt (ENOENT → ok:true before
    requireFullRead is consulted); `fileReadCacheDisabled` escape
    hatch unchanged

A dedicated `fresh + cacheable + partial + requireFullRead` rejection
branch surfaces a clear "only been partially read … overwriting
replaces the entire file" message instead of falling through to the
generic "has not been read" wording. The `unknown` branch's wording
also varies by `requireFullRead` so the read instruction matches the
operation's actual requirement.

For comparison, Claude Code's `readFileState` enforcement treats
partial and full reads identically for both Edit and WriteFile. This
PR is stricter on WriteFile (full read required) and identical on
Edit (partial OK). Issue QwenLM#2499 is empirical evidence that the
partial-read-then-overwrite case is real on at least some model
populations, so the additional WriteFile constraint is justified.

Single-commit shape (versus the earlier afc1b91 / 503fc0b split)
to avoid an intermediate state in which Edit's relaxation has landed
but WriteFile is still on the relaxed path: cherry-picks or bisect
walks crossing such a boundary would re-introduce the issue QwenLM#2499
data-loss case.

Tests: edit.test.ts ranged-read test inverted to "allows after ranged
read"; write-file.test.ts ranged-read test asserts the new partial /
full-required message. Three error-message regex matchers updated
from /fully read/ to /read/. 198 / 198 prior-read-related tests pass;
tsc --noEmit clean.