Skip to content

fix(tools): guard against catastrophic content loss in background review patches - #55665

Closed
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/background-review-patch-content-guard
Closed

fix(tools): guard against catastrophic content loss in background review patches#55665
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/background-review-patch-content-guard

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Summary

Adds a content-loss guard in _patch_skill to prevent the background review agent from silently corrupting skills via hallucinated patches.

Problem (P1 #55647)

The background review agent can call skill_manage(patch) on a skill it has not read via skill_view. When the model hallucinates old_string from the conversation transcript alone, fuzzy matching may find a partial match and the replacement silently deletes most of the file. Observed: 807 lines → 186 lines in a real session.

Fix

When _patch_skill is called from a background review context (is_background_review()), reject patches that would shrink the target file by more than 50%. The error message instructs the agent to read the skill first with skill_view before making targeted patches.

Changes

  • tools/skill_manager_tool.py: Add content-loss guard after size validation in _patch_skill (26 lines added)

Fixes #55647

…iew patches

The background review agent can call skill_manage(patch) on a skill it
has not read via skill_view. When the model hallucinates old_string from
the conversation transcript alone, fuzzy matching may find a partial
match and the replacement silently deletes most of the file (observed:
807 lines -> 186 lines, issue NousResearch#55647).

Add a content-loss guard in _patch_skill: when called from a background
review context, reject patches that would shrink the file by more than
50%. The error message instructs the agent to read the skill first with
skill_view before making targeted patches.

Fixes NousResearch#55647
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) P1 High — major feature broken, no workaround labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing with #55657 for the same bug (#55647). This PR adds a >50%-shrink content-loss guard in _patch_skill for background-review context; #55657 enforces a read-before-write invariant (refuse autonomous patch unless the file was loaded via skill_view this turn). #55657 addresses the root cause (no read-before-write gate) while this PR is a defensive size heuristic. Both open against the same issue — flagging for a maintainer to pick the canonical fix.

@teknium1

Copy link
Copy Markdown
Contributor

Closing in favor of #55906 (merged), which fixes the root cause of #55647 with a read-before-write invariant: the background-review fork must skill_view a skill before skill_manage can patch it. That prevents the corruption structurally rather than via a size-delta heuristic.

Your >50%-shrink guard was a reasonable backstop, but it can't catch sub-50% corruption and the arbitrary ceiling would also reject legitimate large condensations. The invariant approach supersedes it. Thanks for jumping on this fast.

#55906

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

background-review: skill_manage patches hallucinate existing skill content; review fork writes without read-before-write invariant

3 participants