fix(review): teach skill-review prompts the v2026.7.1 read-before-write guard

[kkwills13]

Problem

Commit 20871c1 (shipped in v2026.7.1) made skill_manage refuse background-review writes unless the target skill was loaded via skill_view in the same review turn — a good guard. But _SKILL_REVIEW_PROMPT and _COMBINED_REVIEW_PROMPT in agent/background_review.py still tell the review fork to patch skills straight from conversation context, so every review fork's first skill_manage call is refused and retried.

On one of our production profiles this produced ~300 refusal round-trips/day at 60–80k input tokens each — pure waste, and the curator effectively stopped landing skill updates.

Example refusal (repeats forever):

Tool skill_manage returned error: {"success": false, "error": "Refusing background
curator patch for skill '<name>': the current SKILL.md content has not been loaded
in this review turn. Call skill_view(name) for SKILL.md..."}

Fix

Prompt-only change, mirrored in both prompts:

• Add an explicit read-before-write rule (skill_view the exact target in this review turn before any patch/edit/delete or supporting-file write; handle multiple files one at a time).
• Amend preference-order option 1 to re-load the currently-loaded skill via skill_view before patching it.

Deployed on our fleet since 2026-07-07: refusals went from ~300/day to zero (13+ hours clean at time of writing).

Test plan

• tests/test_background_review_list_shapes.py + tests/test_background_review_session_isolation.py: 21 passed, 1 pre-existing failure also present on vanilla main (TestPersistDisabledHardStop::test_flush_is_a_noop_when_persist_disabled), unrelated to this change.

Related observation (not addressed here)

There appears to be a latent race in the concurrent tool executor: ContextVar read-marks set by parallelized skill_view calls can be dropped (tools/thread_context.py ctx.run snapshot), which would intermittently re-trigger refusals even with correct prompt guidance. Happy to file a separate issue with details.

[2001Y]

I prepared a fresh support branch for this existing PR rather than opening a duplicate:

2001Y/hermes-agent@07be37d...support/pr-60331-read-before-write-scope

It preserves @kkwills13's implementation commit and adds the scope corrections raised in the duplicate review cluster:

• require skill_view only before writes to existing SKILL.md / existing support files;
• explicitly exempt a new skill or new support file, which cannot be pre-read;
• do not describe delete as guarded, because the current delete path is not covered;
• retry exactly once after _read_before_write_required;
• cover both background-review prompt variants with relational contract tests;
• include mutation controls that reject new-target pre-read inversions, exemption removal, and repeated/unbounded retry guidance.

Verification:

• scripts/run_tests.sh tests/run_agent/test_review_prompt_class_first.py -q — 26 passed;
• py_compile, Windows-footgun scan, git diff --check — passed;
• the full suite was not clean. Observed failures either reproduced on the base revision or passed focused reruns, so I found no patch-specific regression, but this is not a full-suite PASS;
• the final update after review feedback is test-only; the production prompt patch is unchanged;
• independent review of the exact final diff returned PASS.

Current support-branch head: 7a767e7fd34c9e1c0cc5151aa0e5d1a36f14baaf.

[teknium1]

Thanks for tracing the prompt/guard mismatch. The premise is confirmed on current main: tools/skill_manager_tool.py:382-408 rejects un-read background-review mutations, while agent/background_review.py:208-215 and 309-317 still instruct patching without a current-turn reload.

Problems

• agent/background_review.py:234-243 applies the pre-read to every support-file write. Current write_file only invokes the guard when the target exists (tools/skill_manager_tool.py:1167-1172), so a new support file cannot be pre-read and should be exempt.
• The same rule says delete is guarded, but _delete_skill does not call _background_review_read_before_write_guard (tools/skill_manager_tool.py:1040-1115).
• The PR diff changes only the prompt; add contract coverage for both prompt variants.

Suggested changes

• The linked support branch at 7a767e7fd34c9e1c0cc5151aa0e5d1a36f14baaf has the needed narrower wording: existing-file-only coverage, new-target exemptions, no delete claim, one bounded recovery retry, and prompt-contract tests.

Automated hermes-sweeper review.

[teknium1]

Please scope this to existing-file mutations. write_file only enforces the read guard when the support-file target exists (tools/skill_manager_tool.py:1167-1172), so a new support file cannot be pre-read; _delete_skill also has no read-before-write guard. Exempt new skills/files and omit delete from this enforcement description.