Skip to content

fix(definition): accept the legacy 'wildcard = {}' tool scope table - #441

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:tool-scope-legacy-wildcard
Oct 13, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
senamakel:tool-scope-legacy-wildcard

Conversation

@senamakel

Copy link
Copy Markdown
Member

Existing OpenHuman agent TOML spells an unrestricted scope as [tools] wildcard = {}. After #438 the untagged wire enum rejected it, so such a definition was silently skipped by the loader. ToolScope now parses with an explicit visitor accepting "wildcard", wildcard = {}, { named = [..] } and a bare list. Regression test: every_tool_scope_spelling_in_existing_toml_still_parses. Host follow-up: openhuman#7773.

Existing OpenHuman agent TOML spells an unrestricted scope as [tools] wildcard = {}.
The untagged wire enum rejected it; parse the scope with an explicit visitor.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 24 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f0cc06df-4f69-47bd-a26c-76b9da1ae5b0
📥 Commits

Reviewing files that changed from the base of the PR and between f1e64cc and 254c7aa.

📒 Files selected for processing: 2
  • crates/tinyagents-definition/src/execution_spec.rs
  • crates/tinyagents-definition/src/execution_spec_tests.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T23:09:49.724367Z 254c7aa PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 254c7aa8d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

.ok_or_else(|| A::Error::custom("empty tool scope table"))?;
let scope = match key.as_str() {
"wildcard" => {
map.next_value::<IgnoredAny>()?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject arbitrary wildcard payloads

When an authored TOML/JSON scope uses the wildcard key with anything other than the legacy empty table (for example, wildcard = false or {"wildcard":["read"]}), IgnoredAny discards that payload and returns ToolScope::Wildcard. The previous enum deserializer rejected these malformed shapes, while Wildcard grants every parent tool and passes the loader's semantic diagnostics, so a typo or attempted restriction now fails open; deserialize and validate an empty map specifically instead of ignoring every value.

Useful? React with 👍 / 👎.

@senamakel
senamakel merged commit 2e706e8 into tinyhumansai:main Oct 13, 2026
11 checks passed
@tinysweeper

tinysweeper Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper completed its review; deterministic results follow.

State: Ready for maintainer review
Priority: none
Reviewed head: 254c7aa8d0e0
Updated: 2026-10-14T02:38:56Z

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 0
Tests 2 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

`ToolScope` loses its derived untagged `Wire`/`Tagged` enum and gains a hand-written `Deserialize` visitor in `crates/tinyagents-definition/src/execution_spec.rs` that accepts the `"wildcard"` string, a sequence of tool names (bare list), a map with a single `named` key, and the legacy map form `wildcard = {}`. Unknown variants, an empty table, and a table with more than one key are rejected with errors. A supporting test was added to `crates/tinyagents-definition/src/execution_spec_tests.rs`.

Features

  • Modified — ToolScope deserialization accepts the legacy `wildcard = {}` table form: The hand-written visitor over `deserialize_any` now parses the legacy TOML table spelling `[tools] wildcard = {}` in addition to the previously accepted `"wildcard"` string, bare name list, and `{ named = [..] }` forms, restoring compatibility with existing on-disk definitions; unknown variants, an empty table, and a multi-key table still error out. (crates/tinyagents-definition/src/execution_spec.rs)
  • Added — Unit test pinning every accepted tool scope spelling: A new test parses each supported spelling through a `Holder` wrapper and asserts concrete expected `ToolScope` values, including `[tools] wildcard = {}`, `tools = "wildcard"`, `[tools] named = ["a", "b"]`, the bare list `tools = ["a"]`, and the empty `named = []` list, so a regression in any accepted spelling would fail a test. (crates/tinyagents-definition/src/execution_spec_tests.rs)

Tests

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

Findings

No active actionable findings.

Before merge

None.

How this fits together

flowchart LR
  n0["ModelSpec<br/>changed"]:::changed
  n1["pin"]:::impacted
  n2["...reaches_the_resolver_without_an_exact_pin"]:::impacted
  n3["Hint"]:::impacted
  n4["deserialize"]:::impacted
  n1 -->|uses| n3
  n2 -->|uses| n0
  n2 -->|calls| n3
  n2 -->|tests| n3
  n4 -->|calls| n3
  n4 -->|uses| n3
  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: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The rewrite of the `ToolScope` `Deserialize` impl replaces the derived untagged enum with a hand-written visitor over `deserialize_any`. The canonical forms in the new test (`"wildcard"`, bare list, `[tools] named = [..]`, `[tools] wildcard = {}`) are handled, but the previous implementation's `Tagged` enum was an externally-tagged enum accepting the *newtype variant* spellings `wildcard = {}` and `named = [...]`, so the set of accepted spellings appears preserved. Before concluding, I want to check two things I cannot see: how `ToolScope` is serialized for TOML (the `Serialize` derive with `rename_all = _The code index is behind this pull request (indexed at `d523fe4a8a8c`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Every branch of the visitor either produces `Wildcard`/`Named` or returns an error for an unknown variant, an empty table, or a multi-key table, so no malformed input can silently widen a tool scope.
  • Lane summary: The hand-written `Deserialize` for `ToolScope` replaces the previous untagged derive and accepts the same wire forms plus the legacy TOML table form (`[tools] wildcard = {}`). Every branch either produces `Wildcard`/`Named`, or returns an error for unknown variants, an empty table, or a multi-key table, so no malformed input can silently widen a tool scope. The new tests exercise each spelling. No security or correctness defect found in this diff. _The code index is behind this pull request (indexed at `d523fe4a8a8c`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The added test pins every accepted spelling, including the legacy `wildcard = {}` table and the empty named list, against concrete expected values, so a regression in any of them fails a test.
  • Positive: The previously accepted forms are unchanged, and the new rejection branches reject exactly the inputs the old untagged enum also rejected, so no behaviour regresses silently.
  • Lane summary: The change replaces ToolScope's untagged deserialize with a visitor that also accepts the legacy TOML table form `wildcard = {}`, and adds `every_tool_scope_spelling_in_existing_toml_still_parses`, which pins every accepted spelling (`wildcard = {}`, the `"wildcard"` string, `named = [..]`, and the bare list, including the empty list) against concrete expected values — so a regression in any of them fails a test. The previously accepted forms are unchanged, and the new rejection branches (empty table, two keys) reject exactly the inputs the old untagged enum also rejected, so I found no behaviour that regresses silently and no defect to anchor. _The code index is behind this pull request (indexed at `d523fe4a8a8c`), so retrieved context may be out of date._

commits

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

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The title and body match the code exactly: the previously accepted forms still parse and no repository rule is reported as broken.
  • Lane summary: The diff replaces ToolScope's untagged wire enum with a hand-written visitor that accepts `"wildcard"`, the legacy table `wildcard = {}`, `{ named = [..] }` and a bare name list, with a new unit test covering each spelling; the title and body match the code exactly, the previously accepted forms still parse, and no repository rule is broken. Nothing here looks unsafe to merge. _The code index is behind this pull request (indexed at `d523fe4a8a8c`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.004330
  • Tokens: 193580 input · 60165 output · 17024 cached · 180 embedding
Head State Pass summary
254c7aa8d0e0 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-14T02:38:56Z)

tinysweeper 0.1.2

@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.0043 · 193,580 in / 60,165 out · 17,024 cached (9%) · ladder/vectors-oai3, deepseek-v4.1-flash · 180 embedded
critique:    $0.0001 · 58,587 in  / 706 out    · 6,144 cached (10%) · deepseek-v4.1-flash
security:    $0.0009 · 57,450 in  / 12,410 out · 3,456 cached (6%)  · deepseek-v4.1-flash
tests:       $0.0018 · 48,546 in  / 25,021 out · 2,816 cached (6%)  · deepseek-v4.1-flash
description: $0.0014 · 26,070 in  / 19,990 out · 4,608 cached (18%) · deepseek-v4.1-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant