fix(background-review): curator no longer refused for skipping the pre-read it was never told about (#62397, direction from #60331) - #94530
Merged
Conversation
…-write handshake 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.
Contributor
૮ >ﻌ< ა ci reviewran on 15b2723 — fix(background-review): teach review prompts the enforced re
|
alt-glitch
added
type/bug
This was referenced Aug 25, 2026
fix(curator): consolidation pass no longer writes into a silent refusal wall (salvage #76754)
#94541
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The background-review fork no longer walks blind into the read-before-write guard — both review prompts (
_SKILL_REVIEW_PROMPTand_COMBINED_REVIEW_PROMPT) now teach the enforced handshake, so the model callsskill_viewon the exact target before patching instead of issuing the write, getting refused, and burning review iterations. Fixes #62397.Root cause: #55906 shipped the enforcing guard in
skill_manage, but neither review prompt ever mentioned it — the model had no way to know the pre-read was required.Direction follows #60331 by @kkwills13, with the scope corrections requested in that PR's review: existing-target-only wording, no claim that delete-skill is guarded (it isn't), bounded one-view-one-retry recovery, and contract tests for both prompt variants.
Changes
agent/background_review.py: Read-before-write section added to both prompts, scoped to the guard's actual contract —skill_view(name)before patch/edit of an existing SKILL.md,skill_view(name, file_path=...)before overwrite/removal of an existing support file, transcript quotes don't count, new skills/new support files exempt, refusal recovery bounded to one view + one retry. Preference-order item 1 in each prompt points at the rule.tests/run_agent/test_review_prompt_class_first.py: behavior-contract tests (not snapshots) asserting both prompts carry the rule, the exact call forms, the new-target exemption, and the no-loop bound.Validation
test_review_prompt_class_first.py(all 7)_background_review_read_before_write_guardcall sites: edit/patch on SKILL.md, write_file only when target exists, remove_file;_delete_skillunguarded, so prompt makes no delete claimCluster
Infographic