Skip to content

Commit 3c070f9

Browse files
authored
fix(curator): only mark agent-created for background-review sediment (#19621)
Tighten the provenance semantics added in #19618: skills a user asks a foreground agent to write via skill_manage(create) now stay invisible to the curator. Only skills the background self-improvement review fork sediments through skill_manage get the created_by=agent marker. - tools/skill_provenance.py — new ContextVar module mirroring the _approval_session_key pattern: set_current_write_origin / reset / get / is_background_review. Default origin is 'foreground'; the review fork sets 'background_review'. - run_agent.py — run_conversation() binds the ContextVar from self._memory_write_origin at the top of each call. The review fork runs on its own thread (fresh context), so foreground and review contexts never cross-contaminate. - tools/skill_manager_tool.py — skill_manage(action='create') now only calls mark_agent_created() when is_background_review(). All other cases (foreground create, patch, edit, write_file, delete) continue as before. - tests: test_skill_provenance.py (6 tests covering the ContextVar surface), split test_full_create_via_dispatcher into foreground vs. review-fork variants, curator status tests now mark-first. Why: the agent routinely edits existing user skills on the user's behalf; those writes must never flip provenance. And when a user explicitly asks the foreground agent to create a skill, that skill belongs to the user. The curator should only be cleaning up after its own autonomous sediment from the review nudge loop.
1 parent bff484a commit 3c070f9

6 files changed

Lines changed: 234 additions & 4 deletions

File tree

run_agent.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10421,6 +10421,15 @@ def run_conversation(
1042110421
from hermes_logging import set_session_context
1042210422
set_session_context(self.session_id)
1042310423

10424+
# Bind the skill write-origin ContextVar for this thread so tool
10425+
# handlers (e.g. skill_manage create) can tell whether they are
10426+
# running inside the background self-improvement review fork vs.
10427+
# a foreground user-directed turn. Set at the top of each call;
10428+
# the review fork runs on its own thread with a fresh context,
10429+
# so the foreground value here does not leak into it.
10430+
from tools.skill_provenance import set_current_write_origin
10431+
set_current_write_origin(getattr(self, "_memory_write_origin", "assistant_tool"))
10432+
1042410433
# If the previous turn activated fallback, restore the primary
1042510434
# runtime so this turn gets a fresh attempt with the preferred model.
1042610435
# No-op when _fallback_activated is False (gateway, first turn, etc.).

tests/hermes_cli/test_curator_status.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,12 @@ def test_status_shows_most_and_least_used_sections(curator_status_env):
114114
env["make_skill"]("top-dog")
115115
env["make_skill"]("middling")
116116
env["make_skill"]("never-used")
117+
# Mark all three as agent-created so they enter the curator's catalog.
118+
# Under the provenance-marker semantics, skills must be explicitly opted
119+
# into curator management (normally via the background-review fork when
120+
# it creates a skill through skill_manage).
121+
for n in ("top-dog", "middling", "never-used"):
122+
env["skill_usage"].mark_agent_created(n)
117123

118124
# Bump use_count differentially. All three counters (use/view/patch) feed
119125
# into activity_count, so bumping use alone is enough to make activity
@@ -150,7 +156,9 @@ def test_status_hides_most_active_when_all_zero(curator_status_env):
150156
env = curator_status_env
151157
env["make_skill"]("a")
152158
env["make_skill"]("b")
153-
# No bumps.
159+
# Mark both as agent-created so the catalog lists them. No bumps.
160+
env["skill_usage"].mark_agent_created("a")
161+
env["skill_usage"].mark_agent_created("b")
154162

155163
out = _capture_status(env["curator_cli"])
156164

tests/tools/test_skill_manager_tool.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,41 @@ def test_patch_without_old_string(self, tmp_path):
531531
assert result["success"] is False
532532

533533
def test_full_create_via_dispatcher(self, tmp_path):
534+
"""Foreground create does NOT mark the skill as agent-created.
535+
536+
Skills created by user-directed foreground turns belong to the user;
537+
only the background self-improvement review fork should mark its
538+
own sediment as agent-created (so the curator can later consolidate
539+
or prune it).
540+
"""
534541
with _skill_dir(tmp_path):
535542
raw = skill_manage(action="create", name="test-skill", content=VALID_SKILL_CONTENT)
536543
from tools.skill_usage import load_usage
537544
usage = load_usage()
538545
result = json.loads(raw)
539546
assert result["success"] is True
540-
assert usage["test-skill"]["created_by"] == "agent"
547+
# No provenance marker on a foreground create — record either missing
548+
# entirely (telemetry best-effort) or present with created_by unset.
549+
rec = usage.get("test-skill") or {}
550+
assert rec.get("created_by") in (None, "", False)
551+
552+
def test_create_from_background_review_marks_agent_created(self, tmp_path):
553+
"""Background-review fork creates ARE marked as agent-created."""
554+
from tools.skill_provenance import set_current_write_origin, BACKGROUND_REVIEW
555+
token = set_current_write_origin(BACKGROUND_REVIEW)
556+
try:
557+
with _skill_dir(tmp_path):
558+
raw = skill_manage(
559+
action="create", name="review-sediment", content=VALID_SKILL_CONTENT
560+
)
561+
from tools.skill_usage import load_usage
562+
usage = load_usage()
563+
finally:
564+
from tools.skill_provenance import reset_current_write_origin
565+
reset_current_write_origin(token)
566+
result = json.loads(raw)
567+
assert result["success"] is True
568+
assert usage["review-sediment"]["created_by"] == "agent"
541569

542570
def test_delete_via_dispatcher_threads_absorbed_into(self, tmp_path):
543571
# Dispatcher must plumb absorbed_into through to _delete_skill so the
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
"""Tests for tools/skill_provenance.py — write-origin ContextVar."""
2+
3+
import contextvars
4+
5+
import pytest
6+
7+
8+
def test_default_origin_is_foreground():
9+
from tools.skill_provenance import get_current_write_origin
10+
# In a fresh ContextVar context, default kicks in.
11+
ctx = contextvars.copy_context()
12+
origin = ctx.run(get_current_write_origin)
13+
assert origin == "foreground"
14+
15+
16+
def test_set_and_get_origin():
17+
from tools.skill_provenance import (
18+
set_current_write_origin,
19+
reset_current_write_origin,
20+
get_current_write_origin,
21+
)
22+
token = set_current_write_origin("background_review")
23+
try:
24+
assert get_current_write_origin() == "background_review"
25+
finally:
26+
reset_current_write_origin(token)
27+
28+
29+
def test_reset_restores_prior_origin():
30+
from tools.skill_provenance import (
31+
set_current_write_origin,
32+
reset_current_write_origin,
33+
get_current_write_origin,
34+
)
35+
outer = set_current_write_origin("assistant_tool")
36+
try:
37+
inner = set_current_write_origin("background_review")
38+
try:
39+
assert get_current_write_origin() == "background_review"
40+
finally:
41+
reset_current_write_origin(inner)
42+
assert get_current_write_origin() == "assistant_tool"
43+
finally:
44+
reset_current_write_origin(outer)
45+
46+
47+
def test_is_background_review_truthy_only_for_review():
48+
from tools.skill_provenance import (
49+
set_current_write_origin,
50+
reset_current_write_origin,
51+
is_background_review,
52+
BACKGROUND_REVIEW,
53+
)
54+
for origin, expected in (
55+
("foreground", False),
56+
("assistant_tool", False),
57+
("random_other_value", False),
58+
(BACKGROUND_REVIEW, True),
59+
):
60+
token = set_current_write_origin(origin)
61+
try:
62+
assert is_background_review() is expected, (
63+
f"is_background_review() wrong for origin={origin!r}"
64+
)
65+
finally:
66+
reset_current_write_origin(token)
67+
68+
69+
def test_empty_origin_falls_back_to_foreground():
70+
from tools.skill_provenance import (
71+
set_current_write_origin,
72+
reset_current_write_origin,
73+
get_current_write_origin,
74+
)
75+
token = set_current_write_origin("")
76+
try:
77+
# Empty is coerced to "foreground" at the set() boundary.
78+
assert get_current_write_origin() == "foreground"
79+
finally:
80+
reset_current_write_origin(token)
81+
82+
83+
def test_context_isolation_between_copies():
84+
"""ContextVar scoping: modifications in one copy do not leak out."""
85+
from tools.skill_provenance import (
86+
set_current_write_origin,
87+
get_current_write_origin,
88+
BACKGROUND_REVIEW,
89+
)
90+
91+
# Start at the module default.
92+
original = get_current_write_origin()
93+
94+
def _run_in_copy():
95+
set_current_write_origin(BACKGROUND_REVIEW)
96+
return get_current_write_origin()
97+
98+
ctx = contextvars.copy_context()
99+
inside = ctx.run(_run_in_copy)
100+
assert inside == BACKGROUND_REVIEW
101+
# Parent context unaffected.
102+
assert get_current_write_origin() == original

tools/skill_manager_tool.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -784,11 +784,16 @@ def skill_manage(
784784
pass
785785
# Curator telemetry: bump patch_count on edit/patch/write_file (the actions
786786
# that mutate an existing skill's guidance), drop the record on delete.
787-
# Best-effort; telemetry failures never break the tool.
787+
# Only mark a skill as agent-created when the background self-improvement
788+
# review fork creates it — foreground `skill_manage(create)` calls are
789+
# user-directed, and those skills belong to the user (the curator must
790+
# not touch them). Best-effort; telemetry failures never break the tool.
788791
try:
789792
from tools.skill_usage import bump_patch, forget, mark_agent_created
793+
from tools.skill_provenance import is_background_review
790794
if action == "create":
791-
mark_agent_created(name)
795+
if is_background_review():
796+
mark_agent_created(name)
792797
elif action in ("patch", "edit", "write_file", "remove_file"):
793798
bump_patch(name)
794799
elif action == "delete":

tools/skill_provenance.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
"""Skill write-origin provenance — ContextVar for distinguishing agent-sediment skill writes from foreground user-directed writes.
2+
3+
The curator only consolidates/prunes skills it autonomously created via the
4+
background self-improvement review fork. Skills a user asks a foreground
5+
agent to write belong to the user and must never be auto-curated.
6+
7+
This module exposes a ContextVar that run_agent.py sets before each tool
8+
loop so tool handlers (e.g. skill_manage create) can check whether they
9+
are executing inside the background-review fork.
10+
11+
The signal piggybacks on AIAgent._memory_write_origin, which is already
12+
set to "background_review" for review-fork instances (see
13+
_spawn_background_review in run_agent.py) and defaults to "assistant_tool"
14+
for normal (foreground) agents.
15+
16+
Usage:
17+
from tools.skill_provenance import (
18+
set_current_write_origin,
19+
reset_current_write_origin,
20+
get_current_write_origin,
21+
)
22+
23+
token = set_current_write_origin("background_review")
24+
try:
25+
... # tool runs here
26+
finally:
27+
reset_current_write_origin(token)
28+
29+
# inside a tool:
30+
if get_current_write_origin() == "background_review":
31+
mark_agent_created(skill_name)
32+
"""
33+
34+
import contextvars
35+
36+
37+
_write_origin: contextvars.ContextVar[str] = contextvars.ContextVar(
38+
"skill_write_origin",
39+
default="foreground",
40+
)
41+
42+
# The sentinel value the background review fork uses; mirrors
43+
# run_agent.py's AIAgent._memory_write_origin override in
44+
# _spawn_background_review().
45+
BACKGROUND_REVIEW = "background_review"
46+
47+
48+
def set_current_write_origin(origin: str) -> contextvars.Token[str]:
49+
"""Bind the active write origin to the current context.
50+
51+
Returns a Token the caller must pass to reset_current_write_origin
52+
in a finally block.
53+
"""
54+
return _write_origin.set(origin or "foreground")
55+
56+
57+
def reset_current_write_origin(token: contextvars.Token[str]) -> None:
58+
"""Restore the prior write origin context."""
59+
_write_origin.reset(token)
60+
61+
62+
def get_current_write_origin() -> str:
63+
"""Return the active write origin.
64+
65+
Default: "foreground" — any tool call made by a regular (non-review)
66+
agent, from the CLI, the gateway, cron, or a subagent.
67+
68+
"background_review" — the self-improvement review fork; only skills
69+
created under this origin should be marked agent-created for curator
70+
management.
71+
"""
72+
return _write_origin.get()
73+
74+
75+
def is_background_review() -> bool:
76+
"""Convenience: True iff the current write origin is the background
77+
review fork."""
78+
return get_current_write_origin() == BACKGROUND_REVIEW

0 commit comments

Comments
 (0)