Skip to content

fix(anthropic): salvage text-embedded tool calls not promoted to tool_use - #52847

Closed
teknium1 wants to merge 3 commits into
mainfrom
fix/anthropic-text-tool-salvage-47472
Closed

teknium1 wants to merge 3 commits into
mainfrom
fix/anthropic-text-tool-salvage-47472

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

Summary

The skill curator no longer touches skills.external_dirs skills — the user's
shared, often version-controlled skill library is now read-only to autonomous
curation, while foreground user-directed edits still work.

Salvages #51875 (@yu-xin-c) and closes the gap that made its headline guard
inert, plus credits @x7peeps (#47752) who reported the same fix first.

Root cause

External-dir skills were visible to the curator's LLM review fork, which could
archive/skill_manage/raw-terminal them in place. The automatic sweep was
already safe (rooted at the local skills dir), but the LLM agent operated
through general-purpose tools with no external-ownership boundary.

#51875 added an is_external_skill_path boundary and a background-review write
guard in skill_managebut that guard only fires when
is_background_review() is true, and the curator fork ran under the default
assistant_tool origin
, so the guard never triggered during the curation
pass it was built to stop. This PR fixes that keystone.

Changes

  • agent/skill_utils.py: is_external_skill_path() — central ownership boundary.
  • tools/skill_usage.py: external skills are ineligible for curation; excluded
    from list_agent_created_skill_names / agent_created_report (so the LLM is
    never shown them); archive_skill refuses them.
  • tools/skill_manager_tool.py: _background_review_write_guard refuses
    autonomous patch/edit/delete/write_file/remove_file on external (and
    bundled / hub-installed) skills; foreground user edits still pass.
  • agent/curator.py: set _memory_write_origin="background_review" on the
    review fork so turn_context binds the origin and the guard actually fires;
    prompt rule marks external dirs read-only.
  • Tests: external-path predicate, eligibility/archive refusal, candidate-list
    exclusion, background-review skill_manage refusal, and a regression test
    asserting the fork runs under the background_review origin.

Validation

Scenario Result
Curator (background_review) patch/delete of external skill refused, file untouched
Foreground (assistant_tool) patch of same external skill succeeds (design nuance preserved)
External skill in curator candidate list (even with stale created_by: agent) excluded
is_curation_eligible(external) / archive_skill(external) False / refused

scripts/run_tests.sh tests/agent/test_skill_utils.py tests/tools/test_skill_usage.py tests/tools/test_skill_manager_tool.py tests/agent/test_curator.py tests/agent/test_turn_context.py → 236 passed, 0 failed. Plus an isolated E2E run against a temp HERMES_HOME confirming the table above.

Known residual

The raw-terminal mv/rm path is deterred by the prompt rule (the LLM is
never handed an external path in the candidate list), not hard-blocked — shell
commands can't be intercepted per-path. The enforceable layers (candidate-list
exclusion, eligibility, archive_skill, skill_manage guard) are all hard.

Closes #47688. Supersedes #51875 and #47752.

Infographic

anthropic-tool-call-salvage

Willhong and others added 3 commits June 25, 2026 21:23
…_use

Cherry-pick of upstream PR #47473 (commit 85babcb, author yf.jin) for
issue #47472. Recovers bare <invoke> tool-call markup that opus-4-8 leaves
in assistant text when the antml: namespace prefix is dropped and the
serving layer cannot promote it to a structured tool_use block.

Carried locally — upstream PR still OPEN/BLOCKED as of cherry-pick.
Tests: tests/agent/transports/test_anthropic_invoke_salvage.py (19 passed).
Extend the carried text-embedded tool-call salvage for the observed opus-4-8 edge case where the outer invoke tag loses only its opening '<' and arrives as 'antml:invoke ...>' while the closing '</invoke>' remains complete.

Keeps plain 'invoke name=...' prose unsalvaged by requiring either '<invoke'/'<antml:invoke' or 'antml:invoke'.

Follow-up to carried upstream PR #47473 / issue #47472.
Tests: python -m pytest tests/agent/transports/test_anthropic_invoke_salvage.py -o 'addopts=' -q (22 passed)
Follow-up on the cherry-picked salvage: some gateways render the
Anthropic-ML namespace separator as an underscore (antml_invoke /
antml_parameter) rather than a colon. Widen _SALVAGE_INVOKE_RE /
_SALVAGE_PARAM_RE from (?:antml:) to (?:antml[:_]) so those leak
shapes are recovered too, add a regression test, and map the
contributor in scripts/release.py AUTHOR_MAP.
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/anthropic-text-tool-salvage-47472 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11371 on HEAD, 11368 on base (🆕 +3)

🆕 New issues (3):

Rule Count
unresolved-attribute 2
unresolved-import 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
tests/agent/transports/test_anthropic_invoke_salvage.py:25: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
run_agent.py:2989: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`

✅ Fixed issues (1):

Rule Count
invalid-assignment 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5993 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@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 provider/anthropic Anthropic native Messages API P1 High — major feature broken, no workaround labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Collaborator Author

Closing without merge — the bug doesn't reproduce on native Anthropic. See issue #47472 for the full writeup. CI was green; this is a taste/premise call, not a quality one.

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 provider/anthropic Anthropic native Messages API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Curator's LLM consolidation agent mutates/deletes skills in external_dirs; per-profile pins can't protect shared skills

3 participants