Skip to content

fix(agent): decode a named invoke wrapped in a tool_call tag - #29

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/tagged-wrapped-invoke
Sep 28, 2026
Merged

senamakel merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/tagged-wrapped-invoke

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Told to call tools "inside <tool_call> tags", DeepSeek V4 writes its native invoke XML inside the tag: <tool_call><invoke name="x"><parameter …>…</invoke></tool_call>. The tagged grammar takes the block because it is the earliest opener. decode_body then finds no P-Format, code, JSON, Kimi or GLM body, so the block decodes to Malformed and the call is dropped silently, even though the same invoke parses when it appears bare.

This change adds invoke_xml::decode_body, which tagged::decode_body tries first on the fence-stripped body. It fires only when the body opens with a named invoke. That anchor stops an invoke quoted inside other body text (a JSON string, say) from executing, which is the same guarantee as recovery_does_not_execute_a_named_invoke_inside_malformed_json.

Scope: this is one of two changes needed for the user turn behind the linked issue. That turn also puts the tag on the fence line itself (```<tool_call>) and never closes the fence, so protected.rs treats the call as an example up to the end of the text. A follow-up PR handles that case. This PR alone does not fix that reproduction.

Related issue

Refs tinyhumansai/openhuman#6722 (partial; see Scope).

API or behavior changes

A <tool_call> (or other tag-family / fenced) block whose body opens with <invoke name="…"> now yields CallSource::InvokeXml calls. Before this change it yielded none. There is no public API change.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check: clean
  • cargo clippy --all-targets --all-features -- -D warnings: ran without --all-features (cargo clippy --all-targets -- -D warnings): clean
  • cargo build --all-targets --all-features: not run with --all-features; the default-feature build via clippy/test succeeded
  • cargo test --all-features: ran cargo test (default features): all pass (113 + 330 + 20 + 1)

Also ran RUSTDOCFLAGS="-D warnings" cargo doc --no-deps: clean.

Tests

Added to src/parse/test/tagged.rs:

  • a_named_invoke_wrapped_in_a_tool_call_tag_is_decoded
  • a_wrapped_invoke_with_string_attributes_is_decoded (string="true"/"false" attrs; 5 decodes as a number)
  • a_todo_block_then_a_closed_bare_fenced_wrapped_invoke_is_decoded
  • an_invoke_after_other_body_text_in_the_tag_is_not_decoded: control for the anchor
  • an_invoke_quoted_in_a_closed_json_tag_body_is_not_executed: a closed JSON body quoting a shell invoke; fails without the anchor (added after review, 32d6eb4)
  • a_function_equals_body_in_a_tool_call_tag_is_decoded: Qwen3-Coder's <tool_call><function=…> form; dropped on main, decoded here

Revert check: with the grammar change removed, the first three fail on assert_eq!(outcome.calls.len(), 1) (left == right failed: []). The control passes both with and without the fix, by design.

Documentation

This is an internal grammar change. The new function's doc comment explains the anchor.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

Review status: tinysweeper/review timed out at the current head (900s, 'No code was reviewed'). It is advisory, not a required check. tinysweeper APPROVED the earlier head fdb223f, and the fleet reviewer reviewed every head.

Told to call tools inside <tool_call> tags, DeepSeek V4 writes its
native invoke XML there. The tagged grammar claimed the block, found no
JSON body, and dropped the call as malformed, though the same invoke
parses bare. When a tag body opens with a named invoke, decode it with
invoke_xml. The anchor keeps an invoke quoted inside other body text
from executing.

Refs tinyhumansai/openhuman#6722
@tinysweeper

tinysweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: critical
Reviewed head: 32d6eb419d47
Updated: 1790599649 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 2 Active findings 2
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • critical · critique · Implement wrapped invoke decoding before adding the test — This assertion requires `<tool_call>` bodies containing `<invoke>` XML to be decoded as `CallSource::InvokeXml`, but the pull request contains no production change to the tagged gr (crates/tinytools\-agent/src/parse/test/tagged\.rs:753)
  • medium · critique · Make the quoted invoke regression test parse valid JSON — The inner `name="shell"` and `name="command"` quotes are unescaped in the runtime JSON, so the tagged body is malformed before the nested `<invoke>` can be tested. The assertion al (crates/tinytools\-agent/src/parse/test/tagged\.rs:808)

Before merge

  • Address Implement wrapped invoke decoding before adding the test (crates/tinytools\-agent/src/parse/test/tagged\.rs).

How this fits together

flowchart LR
  n0["probe_decided"]:::impacted
  n1["ok"]:::impacted
  n2["decode_arguments"]:::impacted
  n3["probe_decided"]:::impacted
  n0 -->|calls| n1
  n3 -->|calls| n2
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 2 files; 2 findings. _The code index is behind this pull request (indexed at `10622c4fd911`), so retrieved context may be out of date._ _5 memory call(s) failed (model: cortex: v1/recall: timed out after 10s), so this review saw part of what the engine holds._
  • Evidence: crates/tinytools\-agent/src/parse/test/tagged\.rs — Implement wrapped invoke decoding before adding the test
  • Evidence: crates/tinytools\-agent/src/parse/test/tagged\.rs — Make the quoted invoke regression test parse valid JSON

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 2 files; 0 findings. _The code index is behind this pull request (indexed at `10622c4fd911`), so retrieved context may be out of date._ _5 memory call(s) failed (model: cortex: v1/recall: timed out after 10s), so this review saw part of what the engine holds._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: _The code index is behind this pull request (indexed at `10622c4fd911`), so retrieved context may be out of date._ _5 memory call(s) failed (model: cortex: v1/recall: timed out after 10s), so this review saw part of what the engine holds._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Adds `invoke_xml::decode_body` to decode named invokes inside `<tool_call>` tags, with an anchor that prevents executing invokes quoted inside other body text. The tests cover the new behavior and the revert check passes. The change is safe to merge.
    No findings: the implementation is correct, the anchor logic matches the described guarantee, and the tests are thorough and pass both with and without the fix as designed.
    (There is no earlier cycle to report resolved findings from.) _The code index is behind this pull request (indexed at `10622c4fd911`), so retrieved context may be out of date._ _5 memory call(s) failed (model: cortex: v1/recall: timed out after 10s), so this review saw part of what the engine holds._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash
  • Spend: $0.021533
  • Tokens: 170296 input · 17293 output · 20700 cached · 545 embedding
Head State Pass summary
fdb223f14251 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 1790594040)
32d6eb419d47 changes requested 2 active finding(s), 0 resolved finding(s) (at 1790599649)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 667d6bf3-5112-4485-a3c9-c9b5848817e9


Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

             $0.0224 · 152,374 in / 15,691 out · 2,118 cached (1%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 470 embedded
critique:    $0.0090 · 62,185 in  / 2,581 out  · 2,118 cached (3%) · gpt-5.6-luna
security:    $0.0088 · 61,453 in  / 2,014 out  · 0 cached (0%)     · gpt-5.6-luna
tests:       $0.0023 · 17,144 in  / 4,136 out  · 0 cached (0%)     · deepseek/deepseek-v4-flash
description: $0.0016 · 8,417 in   / 4,887 out  · 0 cached (0%)     · deepseek/deepseek-v4-flash

@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Review at fdb223f (comment only). The change is correct. I have two test gaps and one wording fix. Everything below was run locally with cargo test -p tinytools-agent: 331 tests pass at this head, including temporary probes I did not commit.

1. The anchor is load-bearing, but the committed tests don't cover the case its doc comment names.
I replaced .is_none_or(|m| m.start() != 0) with false. Only an_invoke_after_other_body_text_in_the_tag_is_not_decoded fails.
The existing recovery_does_not_execute_a_named_invoke_inside_malformed_json passes with or without the anchor. Its invoke uses escaped quotes (name=\"shell\"), which OPEN_RE never matches, and the tag has no closer.
This input is the one the anchor actually protects:

let raw = "<tool_call>{\"name\":\"tool_search\",\"arguments\":{\"query\":\"<invoke name=\"shell\"><parameter name=\"command\">rm -rf /</parameter></invoke>\"}}</tool_call>";
// with the anchor:    no calls
// without the anchor: [("shell", InvokeXml)]

Please add it as a test asserting that no shell call is produced.

2. The anchor only covers the FIRST invoke. probe_decided does a forward search, so once the body opens with a named invoke, any later invoke in the body is decoded too, even after prose:
<tool_call><invoke name="tool_search">…</invoke> see also <invoke name="shell">…</invoke></tool_call> gives ["tool_search", "shell"].
This is not a regression: the same text outside a tag also gives ["tool_search", "shell"]. But the doc comment ("an invoke quoted inside some other body … is never executed") claims more than the code guarantees. Either narrow the wording to "unless the body opens with one", or pin the multi-invoke behaviour with a test so it's a deliberate choice.

3. The reordering also fixes Qwen3-Coder's native format; worth pinning.
OPEN_RE also matches <function=NAME>, and no fixture has a <tool_call><function=…> body.
For Checking.\n<tool_call>\n<function=tool_search>\n<parameter=query>\nrepos\n</parameter>\n</function>\n</tool_call>:

  • at origin/main: calls=[] (silently dropped)
  • at this head: [("tool_search", {"query":"repos"}, InvokeXml)], text "Checking."
    Since that is exactly how Qwen3-Coder is trained to call tools, a fixture would lock the improvement in.

Checked, no action needed

  • Existing fixtures: all 328 pre-existing tests pass with invoke-XML tried first. A body opening with <invoke/<function cannot be valid P-Format, code-call, sentinel, JSON or GLM, so the earlier return cannot take a body from those paths.
  • Name repair and known tools: the calls still go through resolve_names in both batch (parse/mod.rs:96) and stream (stream/mod.rs:106,125). <invoke name="Tool-Search"> with known ["tool_search"] becomes tool_search with a NameRepaired diagnostic. The new tests fail when the tagged.rs early return is removed (all 3 positive tests), so they cover the fix.
  • CI (36413934788): the all-features clippy, build and tests, the default-features tests and the per-file 90% coverage gate all ran and passed. The new test names appear in the log, and the 328-test result is reported three times.

Adds a closed JSON tag body quoting an invoke (fails without the anchor)
and a <tool_call><function=…> body (dropped on main). Narrows the
decode_body doc to what the anchor guarantees: only the first invoke is
anchored, as on the bare path.
@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Addressed in 32d6eb4:

  1. Added an_invoke_quoted_in_a_closed_json_tag_body_is_not_executed with your probe input. I replaced the anchor with false and it fails at tagged.rs:810 (together with the existing prose control). With the anchor restored it passes.
  2. I narrowed the doc comment rather than anchoring every invoke. It now says only the first invoke is anchored, and that later invokes in the body decode exactly as the same text outside a tag does. That keeps it consistent with the bare path.
  3. Added a_function_equals_body_in_a_tool_call_tag_is_decoded (Qwen3-Coder <tool_call><function=…>). With the tagged.rs early return removed, it fails at tagged.rs:818.
    fmt, clippy -D warnings, doc -D warnings and cargo test (330 lib tests) are clean.

@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Re-checked at 32d6eb4: all three points are addressed. an_invoke_quoted_in_a_closed_json_tag_body_is_not_executed is exactly the input that produced [shell] with the anchor removed, so it bites. The doc now states the "every later invoke is decoded, as outside a tag" behaviour, and the Qwen3-Coder form is pinned. CI is green at this head.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 1 lane(s) blocking, worst finding is critical.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0215 · 170,296 in / 17,293 out · 20,700 cached (12%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 545 embedded
critique:    $0.0117 · 83,277 in  / 8,427 out  · 8,672 cached (10%)  · gpt-5.6-luna, deepseek/deepseek-v4-flash
security:    $0.0065 · 56,371 in  / 1,167 out  · 1,788 cached (3%)   · gpt-5.6-luna
tests:       $0.0020 · 17,613 in  / 2,758 out  · 1,280 cached (7%)   · deepseek/deepseek-v4-flash
description: $0.0008 · 9,014 in   / 3,368 out  · 8,960 cached (99%)  · deepseek/deepseek-v4-flash

Comment thread crates/tinytools-agent/src/parse/test/tagged.rs
Comment thread crates/tinytools-agent/src/parse/test/tagged.rs
@senamakel
senamakel merged commit f5cd8fe into tinyhumansai:main Sep 28, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants