fix(tools): read_file's dedup stub skips the review fork too

Sibling of the previous commit: read_file_tool returned the unchanged-stub
before _record_read, and only _record_read calls
mark_background_review_skill_read. A skill file the parent had already read
left the fork with a stub and a refused skill_manage write. Same rule as
skill_view: no dedup stub when is_background_review().

Also: is_background_review hoisted to module level in skills_tool (no cycle —
skill_provenance imports only contextvars), comment reworded to the two real
reasons, test renamed to the shipped design, fixture seeds a fresh read-mark
store so the mark cannot leak between tests.

tests/tools/test_file_read_guards.py

@@ -422,6 +422,26 @@ class TestFileDedup(unittest.TestCase):
         self.assertFalse(r2.get("content_returned"))
         self.assertNotIn("content", r2)
 
+    @patch("tools.file_tools._get_file_ops")
+    def test_background_review_fork_gets_content_and_read_mark_not_stub(self, mock_ops):
+        """The review fork shares the parent's task_id; a dedup stub there would skip the
+        read-mark its read-before-write guard requires (#95976)."""
+        from pathlib import Path
+        from tools.skill_manager_guards import _background_review_has_read, _reset_background_review_read_marks
+        from tools.skill_provenance import reset_current_write_origin, set_current_write_origin
+
+        mock_ops.return_value = _make_fake_ops(content="line one\nline two\n", file_size=20)
+        read_file_tool(self._tmpfile, task_id="dup")  # parent's read arms the dedup
+        _reset_background_review_read_marks()
+        token = set_current_write_origin("background_review")
+        try:
+            fork = json.loads(read_file_tool(self._tmpfile, task_id="dup"))
+        finally:
+            reset_current_write_origin(token)
+        self.assertNotIn("dedup", fork)
+        self.assertIn("content", fork)
+        self.assertTrue(_background_review_has_read(Path(self._tmpfile)))
+
     @patch("tools.file_tools._get_file_ops")
     def test_write_rejects_internal_read_status_text(self, mock_ops):
         """write_file must not persist internal read_file status text."""

tests/tools/test_skill_view_dedup.py

@@ -29,6 +29,8 @@ def skills_home(tmp_path, monkeypatch):
     (refs / "guide.md").write_text("# Guide\n\nDetailed reference content here.\n", encoding="utf-8")
     monkeypatch.setenv("HERMES_HOME", str(home))
     reset_skill_view_dedup()
+    from tools.skill_manager_guards import _reset_background_review_read_marks
+    _reset_background_review_read_marks()
     return home
 
 
@@ -89,7 +91,7 @@ class TestSkillViewDedup:
         r2 = json.loads(_skill_view_with_bump(args, task_id=None))
         assert "Step one" in r2.get("content", "")
 
-    def test_background_review_has_its_own_dedup_namespace(self, skills_home):
+    def test_background_review_skips_dedup_and_marks_read(self, skills_home):
         from tools.skill_provenance import (
             reset_current_write_origin,
             set_current_write_origin,

tools/file_tools.py

@@ -21,6 +21,7 @@ from pathlib import Path
 from agent.file_safety import get_nt_namespace_error, get_read_block_error
 from agent.tool_result_classification import GUARDRAIL_REFUSAL_KEY
 from tools.binary_extensions import has_binary_extension
+from tools.skill_provenance import is_background_review
 from tools.file_operations import (
     ShellFileOperations, normalize_read_pagination, normalize_search_pagination)
 from tools.file_operations_common import DEFAULT_READ_LIMIT
@@ -644,7 +645,9 @@ def read_file_tool(path: str, offset: int = 1, limit: int = DEFAULT_READ_LIMIT,
             # First unchanged read after a compaction boundary serves full content
             # (the summary may have dropped exact bytes); later ones get the stub.
             content_served_in_generation = dedup_key in task_data["dedup_generation_reads"]
-        if cached_mtime is not None:
+        # Same rule as skill_view: the review fork shares the parent's task_id and its
+        # read-before-write guard needs a real read, which the stub path never records (#95976).
+        if cached_mtime is not None and not is_background_review():
             try:
                 if os.path.getmtime(resolved_str) == cached_mtime and content_served_in_generation:
                     return _dedup_stub_or_block(task_data, dedup_key, path)

tools/skills_tool.py

@@ -27,6 +27,7 @@ from tools.skills_tool_plugin import (  # noqa: F401
     _serve_plugin_skill, _serve_skill_file, _truncate_description)
 from tools.skills_tool_dedup import (  # noqa: F401
     _check_skill_view_dedup, _record_skill_view, reset_skill_view_dedup)
+from tools.skill_provenance import is_background_review
 
 logger = logging.getLogger(__name__)
 
@@ -695,10 +696,10 @@ def _skill_view_with_bump(args, **kw):
     session returns a short stub (cache cleared on context compression)."""
     name = args.get("name", "")
     task_id = kw.get("task_id")
-    # The background-review fork shares the parent's task_id (prefix-cache parity), so its views
-    # would hit stubs for content that is in the PARENT's context, not the fork's — and the stub
-    # path never marks the read the fork's write guard requires (#95976). No dedup in the fork.
-    from tools.skill_provenance import is_background_review
+    # The background-review fork shares the parent's task_id (prefix-cache parity). A stub there
+    # (a) skips the read-mark its read-before-write guard requires and (b) lets it patch from a
+    # possibly-pruned transcript copy (#95976). No dedup in the fork; None also keeps its views
+    # out of the parent's bucket.
     dedup_task_id = None if is_background_review() else task_id
     if (stub := _check_skill_view_dedup(dedup_task_id, name, args.get("file_path"))) is not None:
         return stub