ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 10 Packages Projects Releases Wiki Activity

fix(background-review): teach review prompts the enforced read-before-write handshake

Browse Source 
The skill_manage guard (added in #55906) refuses any patch/edit of an
existing SKILL.md, or overwrite/removal of an existing support file,
unless the exact target was loaded via skill_view during the review.
Neither _SKILL_REVIEW_PROMPT nor _COMBINED_REVIEW_PROMPT ever mentioned
this, so models routinely issued the write without the pre-read, got
refused, and burned review iterations (#62397).

Both prompts now carry a Read-before-write section scoped to the
guard's actual contract: existing targets only, exact-path pre-read for
support files, transcript quotes don't count, new skills/new support
files exempt, and a bounded one-view-one-retry recovery instead of a
loop. Direction follows #60331 by @kkwills13 with the scope corrections
requested in review (existing-target-only wording, no delete claim,
bounded retry, contract tests for both prompt variants).

Fixes #62397.
This commit is contained in:
Teknium
2026-08-25 00:07:55 -07:00
parent b0cf2597c2
commit a70d2ffce5
2 changed files with 66 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/background_review.py
+28 -3
View File
@@ -480,8 +480,10 @@ _SKILL_REVIEW_PROMPT = (
" 1. UPDATE A CURRENTLY-LOADED SKILL. Look back through the "
"conversation for skills the user loaded via /skill-name or you "
"read via skill_view. If any of them covers the territory of the "
"new learning, PATCH that one first. It is the skill that was in "
"play, so it's the right one to extend — but only if it is "
"new learning, PATCH that one first (re-load it with skill_view "
"during this review — see Read-before-write below). It is the "
"skill that was in play, so it's the right one to extend — but "
"only if it is "
"curator-managed. Bundled, hub, pinned, and user-owned skills are "
"off-limits to you no matter how relevant (see Protected skills "
"below); for those, fall through to the next option.\n"
@@ -514,6 +516,18 @@ _SKILL_REVIEW_PROMPT = (
"codename, library-alone name, or 'fix-X / debug-Y / audit-Z-today' "
"session artifact. If the proposed name only makes sense for "
"today's task, it's wrong — fall back to (1), (2), or (3).\n\n"
"Read-before-write (ENFORCED — skill_manage refuses otherwise): "
"before you patch or edit an existing skill's SKILL.md, call "
"skill_view(name) for that skill during this review. Before you "
"overwrite or remove an EXISTING supporting file, call "
"skill_view(name, file_path=...) for that exact file. Content "
"quoted earlier in the conversation transcript does NOT count — "
"the guard requires a fresh load within this review, and your "
"write must be based on what skill_view just returned. Creating "
"a brand-new skill or adding a NEW supporting file needs no "
"prior read. If a write is refused with a read-before-write "
"error, call skill_view for the named target once and retry the "
"write once; do not loop.\n\n"
"User-preference embedding (important): when the user expressed a "
"style/format/workflow preference, the update belongs in the "
"SKILL.md body, not just in memory. Memory captures 'who the user "
@@ -600,7 +614,9 @@ _COMBINED_REVIEW_PROMPT = (
"Preference order for skills — pick the earliest that fits:\n"
" 1. UPDATE A CURRENTLY-LOADED SKILL. Check what skills were "
"loaded via /skill-name or skill_view in the conversation. If one "
"of them covers the learning, PATCH it first. It was in play; "
"of them covers the learning, PATCH it first (re-load it with "
"skill_view during this review — see Read-before-write below). "
"It was in play; "
"it's the right place — provided it is curator-managed. Protected "
"and user-owned skills are off-limits however relevant; fall "
"through when one of those is the best fit.\n"
@@ -620,6 +636,15 @@ _COMBINED_REVIEW_PROMPT = (
"codename, library-alone name, or 'fix-X / debug-Y' session "
"artifact. If the name only fits today's task, fall back to (1), "
"(2), or (3).\n\n"
"Read-before-write (ENFORCED — skill_manage refuses otherwise): "
"before patching or editing an existing skill's SKILL.md, call "
"skill_view(name) during this review; before overwriting or "
"removing an EXISTING supporting file, call skill_view(name, "
"file_path=...) for that exact file. Content quoted earlier in "
"the transcript does NOT count — base the write on what "
"skill_view just returned. New skills and NEW supporting files "
"need no prior read. On a read-before-write refusal: view the "
"named target once, retry the write once, do not loop.\n\n"
"User-preference embedding: when the user complains about how "
"you handled a task, update the skill that governs that task — "
"memory alone isn't enough. Memory says 'who the user is and "
tests/run_agent/test_review_prompt_class_first.py
+38
View File
@@ -144,6 +144,44 @@ def test_combined_review_prompt_rejects_unresolved_failures():
_assert_unresolved_failure_guidance(AIAgent._COMBINED_REVIEW_PROMPT, "_COMBINED_REVIEW_PROMPT")
def _assert_read_before_write_guidance(prompt: str, label: str) -> None:
"""Both review prompts must teach the enforced read-before-write handshake.
The skill_manage guard refuses patch/edit of an existing SKILL.md (and
overwrite/remove of an existing support file) unless the exact target was
loaded via skill_view during the review. Without prompt guidance the model
walks into the refusal and burns iterations retrying (#62397).
"""
lower = prompt.lower()
assert "read-before-write" in lower, f"{label}: must name the read-before-write rule"
assert "skill_view(name)" in prompt, (
f"{label}: must give the exact SKILL.md pre-read call"
)
assert "file_path=..." in prompt, (
f"{label}: must give the support-file pre-read form"
)
# Scope: only EXISTING targets need a pre-read; new creations are exempt.
assert "new" in lower and "no prior read" in lower, (
f"{label}: must exempt new skills / new support files from the pre-read"
)
# Transcript quotes must not be treated as satisfying the guard.
assert "does not count" in lower or "does NOT count" in prompt or "not satisfy" in lower, (
f"{label}: must say transcript-quoted content doesn't satisfy the guard"
)
# Bounded recovery: one view + one retry, never a loop.
assert "do not loop" in lower, (
f"{label}: must bound refusal recovery to a single retry"
)
def test_skill_review_prompt_teaches_read_before_write():
_assert_read_before_write_guidance(AIAgent._SKILL_REVIEW_PROMPT, "_SKILL_REVIEW_PROMPT")
def test_combined_review_prompt_teaches_read_before_write():
_assert_read_before_write_guidance(AIAgent._COMBINED_REVIEW_PROMPT, "_COMBINED_REVIEW_PROMPT")