fix(curator): make external-skill write guard actually fire during curation

The salvaged #51875 added a background-review write guard in skill_manage
that refuses mutations to skills.external_dirs skills — but it only fires
when is_background_review() is true. The curator's LLM review fork ran with
the default _memory_write_origin='assistant_tool', so the guard never
triggered during the exact curation pass it exists to protect against
(GH-47688).

- Set _memory_write_origin='background_review' on the curator review fork so
  turn_context binds it onto the write-origin ContextVar and the guard fires.
- Add a regression test asserting the fork runs under the background_review
  origin (the invariant linking the fork to the guard).
- AUTHOR_MAP: map yu-xin-c for the salvaged commit.
This commit is contained in:
teknium1 2026-06-25 21:27:11 -07:00 committed by Teknium
parent 96bc524a71
commit 4d04c652f2
3 changed files with 60 additions and 1 deletions

View file

@ -1846,6 +1846,14 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]:
# Disable recursive nudges — the curator must never spawn its own review.
review_agent._memory_nudge_interval = 0
review_agent._skill_nudge_interval = 0
# Tag this fork as autonomous background curation so skill_manage's
# background-review write guard fires. Without this the fork inherits
# the default "assistant_tool" origin, is_background_review() is False,
# and the external/bundled/hub-installed skill_manage guards never
# trigger during the curation pass they exist to protect against.
# turn_context.py binds this onto the write-origin ContextVar at turn
# start (see agent/turn_context.py).
review_agent._memory_write_origin = "background_review"
# Redirect the forked agent's stdout/stderr to /dev/null while it
# runs so its tool-call chatter doesn't pollute the foreground

View file

@ -1645,7 +1645,7 @@ AUTHOR_MAP = {
"mordred@inaugust.com": "emonty",
"rodrigoeq@hotmail.com": "rodrigoeqnit",
"soliva.johnpaul@icloud.com": "jonpol01",
"2182712990@qq.com": "yu-xin-c", # PR #32122 (Docker audio bridge notes)
"2182712990@qq.com": "yu-xin-c", # PR #51875 salvage (protect external_dirs skills from curator)
"baxter@bitreserve.ai": "BaxBit", # PR #30200 (Svix webhook signature validation)
"chris.eth@qq.com": "duyua9", # PR #10949 (render object config values structurally)
"ethie@nous": "ethernet8023", # PR #29342 (TUI clipboard copy on linux/wayland)

View file

@ -1119,3 +1119,54 @@ def test_curator_slot_is_canonical_aux_task():
# 4. web/src/pages/ModelsPage.tsx is checked at build time; the tsx
# array and this tuple share a ``Must match _AUX_TASK_SLOTS`` comment.
def test_review_fork_runs_under_background_review_origin(curator_env, monkeypatch):
"""The curator LLM fork must tag itself as background_review.
This is the keystone that makes skill_manager_tool's
``_background_review_write_guard`` fire during a curation pass. Without
``_memory_write_origin = "background_review"`` on the fork, the agent
inherits the default ``assistant_tool`` origin, ``is_background_review()``
stays False, and the external/bundled/hub-installed skill_manage guards
never trigger leaving the LLM agent free to mutate skills.external_dirs
skills (GH-47688). turn_context.py binds this attribute onto the
write-origin ContextVar at turn start, so asserting it is set is the
enforceable invariant linking the fork to the guard.
"""
curator = curator_env["curator"]
# The curator_env fixture stubs out _run_llm_review wholesale; this test
# exercises the real implementation, so reload the module to restore it.
import importlib
importlib.reload(curator)
monkeypatch.setattr(curator, "_load_config", lambda: {})
captured = {}
class _StubAgent:
def __init__(self, *args, **kwargs):
# AIAgent.__init__ normally sets this default; mirror it so the
# production assignment in _run_llm_review is what flips it.
self._memory_write_origin = "assistant_tool"
self._memory_nudge_interval = 10
self._skill_nudge_interval = 10
self.platform = kwargs.get("platform")
self._session_messages = []
def run_conversation(self, user_message=None, **kwargs):
# Capture the origin AT RUN TIME — i.e. after _run_llm_review has
# finished configuring the fork, which is exactly when it matters.
captured["write_origin"] = self._memory_write_origin
return {"final_response": "no change"}
monkeypatch.setattr("run_agent.AIAgent", _StubAgent)
meta = curator._run_llm_review("review prompt")
assert meta.get("error") is None, meta.get("error")
assert captured.get("write_origin") == "background_review", (
"curator review fork did not set _memory_write_origin to "
"'background_review' — the skill_manage background-review write "
"guard would not fire (GH-47688 regression)"
)