# NousResearch/hermes-agent · Background review fork tool whitelist and read-before-write fixes

## Open now

- 4 issues, 1 PR open on GitHub.
- #58475: \[Bug\]: Background review agent repeatedly gets stuck — prompt ambiguity, vague tool whitelist, and incomprehensible error message
- #59244: Bug: background memory review ignores configured memory provider and only writes legacy memory
- #63964: blocker-report: background curator loops on skill_manage patch errors
- #64131: Background curator skill_manage loop reports repeated guard/schema failures
- PR #75020: fix: require skill_view before background skill writes

These issues and PRs center on the background/cron review fork's tool whitelist being too restrictive (blocking read_file and other read-only tools) and the resulting curator loops caused by a read-before-write guard not being satisfied. Additional adjacent concerns include narrowing the tool schema exposed to local/weak models and configuring which memory providers the background review can write to.

## TL;DR

- 37 member(s): 4 open issue(s), 9 closed issue(s), 1 open PR(s), 6 merged PR(s), 17 PR(s) closed without merging.
- A fix was merged to main in PR #55906. Verify it addresses your case.
- **Recommended:** read PR #55906 (and any competing PRs below) before filing. Confirm the merged change covers your symptom.

## Maintainer resolution verdict

- Internal-tracker verdict over this cluster's issue members: `partially_resolved` · deciding PR(s): `pr-55906,pr-94530,pr-98254,pr-98278`. This is distinct from the fix status above. Weigh it yourself.

## How this cluster was grouped

- Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text.
- Members outside that structural core may belong to a different underlying problem.
- Pairwise link density `0.05`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked.

## Open related issues

- #58475: \[Bug\]: Background review agent repeatedly gets stuck — prompt ambiguity, vague tool whitelist, and incomprehensible error message
- #59244: Bug: background memory review ignores configured memory provider and only writes legacy memory
- #63964: blocker-report: background curator loops on skill_manage patch errors
- #64131: Background curator skill_manage loop reports repeated guard/schema failures

## Closed / duplicate issues

- #39996: \[Bug\]: background-review fork advertises the full tool schema to LOCAL endpoints, making weak local models thrash the deny-wall
- #40003: \[Bug\]: background-review fork can't read an external file — model calls skill_manage(action="read_file") and gets "Unknown action"
- #41465: \[Bug\]: Background review skill creation fails — tool whitelist blocks read_file
- #44672: Background self-review: make the tool whitelist configurable (allow MCP/memory-provider writes)
- #45877: Cron background review blocks read-only tools (read_file, search_files)
- #55647: background-review: skill_manage patches hallucinate existing skill content; review fork writes without read-before-write invariant
- #61521: background_review fork advertises the parent's full toolset but only permits skills+memory → per-turn tool-denial storm starves the self-improvement loop
- #62397: Background review fork can't patch skills: prompt never calls skill_view, guard refuses the write (read-before-write mismatch)
- #63388: \[Bug\] Curator/skill_manage fails in background: "SKILL.md content has not been loaded in this review turn"

## Prior & competing PRs

- **#41469: fix: grant background review agent read_file access for skill patching**. closed without merge · fix tier `suggested`
  - maintainer direction: category `uncategorized`; implication `needs_llm`; refs `20871c1d941a697723aae24cd8e88afe701793ae`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#55906: fix(skills): require background-review forks to read before writing skills (#55647)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `agent/background_review.py`, `scripts/release.py`, `tests/tools/test_skill_manager_tool.py`, `tools/skill_manager_tool.py`, `tools/skills_tool.py`
  - fix hunks:
  - `agent/background_review.py`

    ```diff
    @@ -739,6 +739,13 @@ def _bg_review_auto_deny(command, description, **kwargs):
                         "{tool_name}. Only memory/skill tools are allowed."
                     ),
                 )
    +            try:
    +                from tools.skill_manager_tool import _reset_background_review_read_marks
    +
    +                _reset_background_review_read_marks()
    +            except Exception:
    +                pass
    +
                 try:
                     # Routed to a different model -> replay a digest (cache is cold
                     # on that model anyway, so minimise cold-written tokens). Same
    ```
  - `scripts/release.py`

    ```diff
    @@ -271,6 +271,7 @@
         "[redacted:email]": "zhaoleibd",
         "[redacted:email]": "jeffrobodie-glitch",
         "[redacted:email]": "kyssta-exe",
    +    "[redacted:email]": "kyssta-exe",  # PR #55657 salvage (read-before-write invariant for background-review skill patches; #55647)
         "[redacted:email]": "ITheEqualizer",
         "[redacted:email]": "stremtec",
         "[redacted:email]": "solaitken",
    ```
  - `tests/tools/test_skill_manager_tool.py`

    ```diff
    @@ -957,6 +957,9 @@ def test_background_review_unpinned_skill_not_blocked_by_pin_guard(self, tmp_pat
                 _create_skill("my-skill", VALID_SKILL_CONTENT)
                 token = [redacted:secret-assignment]
                 try:
    +                from tools.skill_manager_tool import mark_background_review_skill_read
    +
    +                mark_background_review_skill_read(tmp_path / "my-skill" / "SKILL.md")
                     with patch(
                         "tools.skill_usage.get_record",
                         side_effect=lambda n: {"pinned": False},
    ```

    ```diff
    @@ -1173,6 +1176,7 @@ def _curator_pass(tmp_path, *, monkeypatch):
         skills_root.mkdir(parents=True, exist_ok=True)
         monkeypatch.setenv("HERMES_HOME", str(hermes_home))
         with patch("tools.skill_manager_tool.SKILLS_DIR", skills_root), \
    +         patch("tools.skills_tool.SKILLS_DIR", skills_root), \
              patch("agent.skill_utils.get_all_skills_dirs", return_value=[skills_root]), \
              patch("tools.skill_provenance.is_background_review", return_value=True):
             yield skills_root
    ```

    ```diff
    @@ -1281,3 +1285,66 @@ def test_dispatcher_preserves_usage_record_on_curator_archive(self, tmp_path, mo
                 rec = skill_usage.get_record("narrow")
             # Record kept (not forgotten) and marked archived.
             assert rec.get("state") == skill_usage.STATE_ARCHIVED
    +
    +    def test_background_review_patch_requires_skill_view_first(self, tmp_path, monkeypatch):
    +        from tools.skills_tool import skill_view
    +        from tools.skill_manager_tool import _reset_background_review_read_marks
    +
    +        _reset_background_review_read_marks()
    +        with _curator_pass(tmp_path, monkeypatch=monkeypatch):
    +            _create_skill("reviewed", _skill_content("reviewed"))
    +
    +            blocked = json.loads(skill_manage(
    +                action="patch",
    +                name="reviewed",
    +                old_string="Step 1: Do the thing.",
    +                new_string="Step 1: Do the thing safely.",
    +            ))
    +            assert blocked["success"] is False
    +            assert blocked.get("_read_before_write_required") is True
    +
    +            viewed = json.loads(skill_view("reviewed"))
    +            assert viewed["success"] is True
    +
    +            allowed = json.loads(skill_manage(
    +                action="patch",
    +                name="reviewed",
    +                old_string="Step 1: Do the thing.",
    +                new_string="Step 1: Do the thing safely.",
    +            ))
    +            assert allowed["success"] is True, allowed
    +
    +        _reset_background_review_read_marks()
    +
    +    def test_background_review_support_file_overwrite_requires_that_file_read(self, tmp_path, monkeypatch):
    +        from tools.skills_tool import skill_view
    +        from tools.skill_manager_tool import _reset_background_review_read_marks
    +
    +        _reset_background_review_read_marks()
    +        with _curator_pass(tmp_path, monkeypatch=monkeypatch):
    +            _create_skill("reviewed", _skill_content("reviewed"))
    +            ref = tmp_path / ".hermes" / "skills" / "reviewed" / "references"
    +            ref.mkdir()
    +            (ref / "workflow.md").write_text("old workflow\n", encoding="utf-8")
    +
    +            # Reading SKILL.md does not authorize overwriting a linked file.
    +            assert json.loads(skill_view("reviewed"))["success"] is True
    +            blocked = json.loads(skill_manage(
    +                action="write_file",
    +                name="reviewed",
    +                file_path="references/workflow.md",
    +                file_content="new workflow\n",
    +            ))
    +            assert blocked["success"] is False
    +            assert blocked.get("_read_before_write_required") is True
    +
    +            assert json.loads(skill_view("reviewed", "references/workflow.md"))["success"] is True
    +            allowed = json.loads(skill_manage(
    +                action="write_file",
    +                name="reviewed",
    +                file_path="references/workflow.md",
    +                file_content="new workflow\n",
    +            ))
    +            assert allowed["success"] is True, allowed
    +
    +        _reset_background_review_read_marks()
    ```
  - `tools/skill_manager_tool.py`

    ```diff
    @@ -38,15 +38,58 @@
     import re
     import shutil
     import tempfile
    +import contextvars as _ctxvars
     from pathlib import Path
    -from hermes_constants import get_hermes_home, display_hermes_home
    -from typing import Dict, Any, List, Optional, Tuple
    +from typing import Any, Dict, List, Optional, Tuple
     
    +from hermes_constants import get_hermes_home, display_hermes_home
     from utils import atomic_replace, is_truthy_value
     from hermes_cli.config import cfg_get
     
     logger = logging.getLogger(__name__)
     
    +_background_review_read_paths: "_ctxvars.ContextVar[frozenset[str]]" = _ctxvars.ContextVar(
    +    "background_review_read_paths", default=frozenset()
    +)
    +
    +
    +def mark_background_review_skill_read(path: Path) -> None:
    +    """Record that the active background-review fork has read a skill file.
    +
    +    The autonomous review fork is allowed to evolve skills, but it must not
    +    patch or rewrite content it has only inferred from the transcript.  The
    +    skill_view tool calls this after returning file content to the model; write
    +    paths below require the corresponding target path to be present when the
    +    current origin is ``background_review``.
    +    """
    +    try:
    +        from tools.skill_provenance import is_background_review
    +        if not is_background_review():
    +            return
    +    except Exception:
    +        return
    +
    +    try:
    +        resolved = str(path.resolve())
    +    except Exception:
    +        resolved = str(path)
    +    current = set(_background_review_read_paths.get())
    +    current.add(resolved)
    +    _background_review_read_paths.set(frozenset(current))
    +
    +
    +def _background_review_has_read(path: Path) -> bool:
    +    try:
    +        resolved = str(path.resolve())
    +    except Exception:
    +        resolved = str(path)
    +    return resolved in _background_review_read_paths.get()
    +
    +
    +def _reset_background_review_read_marks() -> None:
    +    """Test helper: clear read-before-write marks for the current context."""
    +    _background_review_read_paths.set(frozenset())
    +
     # Import security scanner — external hub installs always get scanned;
     # agent-created skills only get scanned when skills.guard_agent_created is on.
     try:
    ```

    ```diff
    @@ -320,6 +363,36 @@ def _background_review_write_guard(
         return None
     
     
    +def _background_review_read_before_write_guard(
    +    name: str,
    +    target: Path,
    +    action: str,
    +    file_label: str,
    +) -> Optional[Dict[str, Any]]:
    +    """Require review forks to load the exact target before mutating it."""
    +    try:
    +        from tools.skill_provenance import is_background_review
    +        if not is_background_review():
    +            return None
    +    except Exception:
    +        return None
    +
    +    if _background_review_has_read(target):
    +        return None
    +
    +    return {
    +        "success": False,
    +        "error": (
    +            f"Refusing background curator {action} for skill '{name}': "
    +            f"the current {file_label} content has not been loaded in this "
    +            "review turn. Call skill_view(name) for SKILL.md, or "
    +            "skill_view(name, file_path=...) for a supporting file, then "
    +            "retry the write using the content just returned."
    +        ),
    +        "_read_before_write_required": True,
    +    }
    +
    +
     def _background_review_preflight(action: str, name: str) -> Optional[Dict[str, Any]]:
         if action not in {"edit", "patch", "delete", "write_file", "remove_file"}:
             return None
    ```

    ```diff
    @@ -786,6 +859,12 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]:
             return guard
     
         skill_md = existing["path"] / "SKILL.md"
    +    read_guard = _background_review_read_before_write_guard(
    +        name, skill_md, "edit", "SKILL.md"
    +    )
    +    if read_guard:
    +        return read_guard
    +
         # Back up original content for rollback
         original_content = skill_md.read_text(encoding="utf-8") if skill_md.exists() else None
         _atomic_write_text(skill_md, content)
    ```

    ```diff
    @@ -849,6 +928,7 @@ def _patch_skill(
             target, err = _resolve_skill_target(skill_dir, file_path)
             if err:
                 return {"success": False, "error": err}
    +        assert target is not None
         else:
             # Patching SKILL.md
             target = skill_dir / "SKILL.md"
    ```

    ```diff
    @@ -856,6 +936,15 @@ def _patch_skill(
         if not target.exists():
             return {"success": False, "error": f"File not found: {target.relative_to(skill_dir)}"}
     
    +    read_guard = _background_review_read_before_write_guard(
    +        name,
    +        target,
    +        "patch",
    +        "SKILL.md" if not file_path else file_path,
    +    )
    +    if read_guard:
    +        return read_guard
    +
         content = target.read_text(encoding="utf-8")
     
         # Use the same fuzzy matching engine as the file patch tool.
    ```

    ```diff
    @@ -1057,6 +1146,13 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]:
         target, err = _resolve_skill_target(existing["path"], file_path)
         if err:
             return {"success": False, "error": err}
    +    assert target is not None
    +    if target.exists():
    +        read_guard = _background_review_read_before_write_guard(
    +            name, target, "write_file", file_path
    +        )
    +        if read_guard:
    +            return read_guard
         target.parent.mkdir(parents=True, exist_ok=True)
         # Back up for rollback
         original_content = target.read_text(encoding="utf-8") if target.exists() else None
    ```

    ```diff
    @@ -1096,6 +1192,7 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
         target, err = _resolve_skill_target(skill_dir, file_path)
         if err:
             return {"success": False, "error": err}
    +    assert target is not None
         if not target.exists():
             # List what's actually there for the model to see
             available = []
    ```

    ```diff
    @@ -1111,6 +1208,12 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
                 "available_files": available if available else None,
             }
     
    +    read_guard = _background_review_read_before_write_guard(
    +        name, target, "remove_file", file_path
    +    )
    +    if read_guard:
    +        return read_guard
    +
         target.unlink()
     
         # Clean up empty subdirectories
    ```
  - `tools/skills_tool.py`

    ```diff
    @@ -1281,6 +1281,17 @@ def _record(sd: Optional[Path], smd: Path) -> None:
                         ensure_ascii=False,
                     )
     
    +            try:
    +                from tools.skill_manager_tool import mark_background_review_skill_read
    +
    +                mark_background_review_skill_read(target_file)
    +            except Exception:
    +                logger.debug(
    +                    "Could not record background-review skill read for %s",
    +                    target_file,
    +                    exc_info=True,
    +                )
    +
                 return json.dumps(
                     {
                         "success": True,
    ```

    ```diff
    @@ -1484,6 +1495,17 @@ def _record(sd: Optional[Path], smd: Path) -> None:
             if capture_result["gateway_setup_hint"]:
                 result["gateway_setup_hint"] = capture_result["gateway_setup_hint"]
     
    +        try:
    +            from tools.skill_manager_tool import mark_background_review_skill_read
    +
    +            mark_background_review_skill_read(skill_md)
    +        except Exception:
    +            logger.debug(
    +                "Could not record background-review skill read for %s",
    +                skill_md,
    +                exc_info=True,
    +            )
    +
             if setup_needed:
                 missing_items = [
                     f"env ${env_name}" for env_name in remaining_missing_required_envs
    ```
- **#62428: fix(agent): instruct background review fork to call skill_view before patching skills (#62397)**. closed without merge · fix tier `suggested`
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#94530: fix(background-review): curator no longer refused for skipping the pre-read it was never told about (#62397, direction from #60331)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `agent/background_review.py`, `tests/run_agent/test_review_prompt_class_first.py`
- **#97451: fix(skill_manage): batch failures keep the failing op's file_preview — models no longer recover blind**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `tools/skill_manager_tool.py`
- **#97650: fix(skill_manage): missing-old_string errors teach recovery instead of dead-ending into write_file clobbers (#33064, salvage #63879)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `tests/tools/test_skill_manager_tool.py`, `tools/skill_manager_tool.py`
- **#98254: Background review lands skill patches again — read tools whitelisted, cache parity intact (#61521, #39996)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `agent/background_review.py`, `tests/run_agent/test_background_review_toolset_restriction.py`, `tools/file_tools.py`
- **#98278: Background review accepts profile-configured extra tools via opt-in whitelist (closes #44672, salvage #82146)**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`; implication `superseded`
  - files: `agent/background_review.py`, `contributors/emails/[redacted:email]`, `tests/run_agent/test_background_review_toolset_restriction.py`, `website/docs/user-guide/features/memory.md`
- **#39997: fix(agent): narrow background-review tool schema for local providers**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#39996`, `#39997`, `#39997`, `#39996`, `#39997`, `#39997`
- **#40007: fix(agent): allow background review to read files**. closed without merge
  - maintainer direction: category `uncategorized`; implication `needs_llm`
- **#40060: fix(background_review): add read_file to review fork tool whitelist**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `1ee30352ca60f79d984bba62ba1ffd2716c36924`, `#98254`, `#40003`
- **#45892: fix(agent): allow read-only file tools in background review whitelist**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `1ee30352ca60f79d984bba62ba1ffd2716c36924`, `#98254`, `#40060`, `#46049`
- **#46049: fix(agent): allow read-only file tools in cron background review**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `1ee30352ca60f79d984bba62ba1ffd2716c36924`, `#98254`, `#40060`
- **#55657: fix(skills): require review forks to read before writing skills**. closed without merge
  - maintainer direction: category `merged_via`; implication `superseded`; refs `20871c1d9`, `#55906`
- **#55665: fix(tools): guard against catastrophic content loss in background review patches**. closed without merge
  - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#55906`, `#55647`
- **#59175: \[codex\] Fix background review skill read-before-write**. closed without merge
  - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#75020`
- **#60331: fix(review): teach skill-review prompts the v2026.7.1 read-before-write guard**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `a70d2ffce59b58b9bdb36a518ddffd16953b67f2`, `#94530`
- **#61529: fix(agent): use restricted toolset for background review fork**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `1ee30352ca60f79d984bba62ba1ffd2716c36924`, `#61521`, `#98254`, `#39997`
- **#62414: fix(review): teach skill-review prompts the v2026.7.1 read-before-write guard**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `a70d2ffce59b58b9bdb36a518ddffd16953b67f2`, `#94530`, `#60331`
- **#62664: fix(agent): document read-before-write handshake in background-review prompt (#62397)**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#60331`, `#55906`, `#62397`, `#62428`, `#62414`, `#62400`, `#60331`
- **#63433: fix(curator): add explicit skill_view-before-skill_manage instruction**. closed without merge
  - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#61024`
- **#63991: fix(tools): prevent curator infinite loop on skill_manage patch errors (#63964)**. closed without merge
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `#97650`, `#63879`, `#97451`
- **#65803: fix(agent): steer background review to permitted tools**. closed without merge
  - maintainer direction: category `partial_fix`; implication `needs_llm`; refs `802d74a910bd`, `#25434`, `#29704`, `#64379`, `#61521`, `#65803`, `#17276`, `#25434`, `#18973`, `#64379`, `#20674`, `#64379`, `#25330`, `#25434`, `#25427`, `#25434`, `#25434`, `#25322`, `#26850`, `#27422`, `#27510`, `#64379`, `#29568`, `#29704`, `#29704`, `#29567`, `#30532`, `#64379`, `#36967`, `#36995`, `#64379`, `#40007`, `#40003`, `#40060`, `#40007`, `#45014`, `#45892`, `#45902`, `#45902`, `#45877`, `#46049`, `#45902`, `#46684`, `#40007`, `#61529`, `#64379`, `#65803`, `#61521`, `#61618`, `#17276`, `#25427`, `#25434`, `#29568`, `#29704`, `#18973`, `#20674`, `#27510`, `#30532`, `#36995`, `#64379`, `#27422`, `#40007`, `#40060`, `#46684`, `#45892`, `#45902`, `#46049`, `#61529`, `#65803`, `#61521`, `#65803`, `#61618`, `#65803`, `#61618`, `#61529`, `#40060`, `#40007`, `#45892`, `#46049`, `#45902`, `#40007`, `#1`, `#1`, `#15803`, `#14532`, `#6`, `#1`, `#9`, `#374151`, `#4338`, `#61521`, `#61529`, `#65803`
- **#75020: fix: require skill_view before background skill writes**. open
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `a70d2ffce59b`, `335c60ecdd76`, `#94530`, `#94511`
  - files: `agent/background_review.py`, `tests/run_agent/test_background_review_toolset_restriction.py`, `tests/run_agent/test_review_prompt_class_first.py`, `tests/tools/test_skill_manager_tool.py`, `tools/skill_manager_tool.py`

## Fix status

- Merged to main in PR #55906 (cited above). No release data is tracked. Verify downstream.
- Fix last verified (merged) `2026-08-30`.
- Subsystem: `agent`