fix(curator): tell the background reviewer to read before it writes
`_background_review_read_before_write_guard` refuses a background-review `skill_manage` write when the target file was not loaded via `skill_view` in the same review turn (patch, edit, write_file over an existing file, remove_file). `CURATOR_REVIEW_PROMPT` never says so. It lists `skill_view` only under "read the current landscape", so the reviewer goes straight to the write and every mutation is refused. The failure is silent from the outside: the curator run completes, writes nothing, and reads like a pass that simply found nothing to consolidate. On our deployment that was 32 of 32 attempted writes rejected over 48h before anyone read the logs. This adds the missing instruction to the toolset block, plus a test that fails if a future guarded action is added to `skill_manager_tool` without being named in the prompt — the guard and the prompt have to drift together or not at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Teknium
parent
a70d2ffce5
commit
610e2e02fb
@@ -539,6 +539,13 @@ CURATOR_REVIEW_PROMPT = (
|
||||
"merges.\n\n"
|
||||
"Your toolset:\n"
|
||||
" - skills_list, skill_view — read the current landscape\n"
|
||||
" READ BEFORE WRITE — enforced, not advisory. Before skill_manage "
|
||||
"action=patch, action=edit, action=write_file on a file that already "
|
||||
"exists, or action=remove_file, call skill_view on that SAME target in "
|
||||
"this review turn — skill_view(name) for SKILL.md, "
|
||||
"skill_view(name, file_path=...) for a supporting file — and build the "
|
||||
"write from the content it just returned. A write without that read is "
|
||||
"REFUSED and nothing is saved.\n"
|
||||
" - skill_manage action=patch — add sections to the umbrella\n"
|
||||
" - skill_manage action=create — create a new umbrella SKILL.md\n"
|
||||
" - skill_manage action=write_file — add a references/, templates/, "
|
||||
|
||||
@@ -525,6 +525,42 @@ def test_curator_does_not_instruct_model_to_pin():
|
||||
|
||||
|
||||
|
||||
def test_curator_prompt_covers_every_read_before_write_guarded_action():
|
||||
"""The review prompt must name every action the read-before-write guard
|
||||
protects.
|
||||
|
||||
``_background_review_read_before_write_guard`` refuses a write when the
|
||||
target was not loaded via ``skill_view`` in the same review turn. The
|
||||
curator's forked reviewer only knows to do that if the prompt says so —
|
||||
a guard the prompt never mentions is a silently jammed write channel,
|
||||
not a safety net. This test fails if a new guarded action is added to
|
||||
``skill_manager_tool`` without teaching the prompt about it.
|
||||
"""
|
||||
import inspect
|
||||
import re
|
||||
|
||||
from agent.curator import CURATOR_REVIEW_PROMPT
|
||||
from tools import skill_manager_tool
|
||||
|
||||
known_actions = {
|
||||
"create", "edit", "patch", "write_file", "remove_file", "delete",
|
||||
}
|
||||
source = inspect.getsource(skill_manager_tool)
|
||||
guarded = set()
|
||||
for call in source.split("_background_review_read_before_write_guard(")[1:]:
|
||||
guarded |= known_actions.intersection(re.findall(r'"([^"]+)"', call[:200]))
|
||||
|
||||
assert guarded, "no guarded actions found — did the guard get renamed?"
|
||||
|
||||
prompt = CURATOR_REVIEW_PROMPT
|
||||
assert "skill_view" in prompt
|
||||
missing = [a for a in sorted(guarded) if f"action={a}" not in prompt]
|
||||
assert not missing, (
|
||||
"read-before-write guards these actions but the curator prompt never "
|
||||
f"tells the reviewer to read first for: {missing}"
|
||||
)
|
||||
|
||||
|
||||
def test_cli_pin_refuses_bundled_skill(curator_env, capsys):
|
||||
from hermes_cli import curator as cli
|
||||
skills_dir = curator_env["home"] / "skills"
|
||||
|
||||
Reference in New Issue
Block a user