[5662] test(sdk): fix two builtin-tool tests that don't exercise what they claim

[Yuvakunaal]

Closes #5662

Summary

Two unit tests in the SDK tests from the built-ins directory pass but don't actually test what they are supposed to be testing.

test_a_bare_tool_name_string_is_ignored_too (test_resolver.py) invoked
ToolResolver().resolve(coerce_tool_configs(["read"])). coerce_tool_configs
returns a ToolConfigParseResult pydantic model rather than a list of
configs, thus iterating over it inside resolve() will yield pairs
(field, value) rather than the actual BuiltinToolConfig objects.
resolve() will quietly accept this wrong input and return
tool_specs == [] which is indeed correct, but this doesn't mean that the
coercion path was actually taken. It now first asserts
coerce_tool_configs(["read"]).tool_configs == [BuiltinToolConfig(name="read")],
and only then resolves that list.

test_no_json_example_writes_a_builtin_tool_entry
(test_agenta_builtins_reference_files.py) was checking that
'"type": "builtin"' not in block as an unprocessed string. However, any
variant with whitespace, like "type":"builtin", would pass silently.
It now json.loadss each block and recursively walks dicts and lists
for any object with type == "builtin", and fails loudly if a block
isn't even valid JSON.

Testing

Verified locally

• Targeted files (2): 48 tests passed.
• Full SDK suite (run-tests.py): 2382 passed, 4 skipped, 10 xfailed.
  The single unit test failure (test_cli_stream_terminal_only_on_empty_request)
  and the 97 acceptance test errors are pre-existing and totally
  unrelated – confirmed by stashing this change and re-running against
  an unmodified main, yielding the same results. The acceptance errors
  require a live AGENTA_API_URL.
• Sanity checked that both rewrites can indeed fail as requested in the
  issue: corrupted the expected value in the resolver assertion and
  verified pytest fails with a proper diff, then reverted it. For the
  JSON example check, verified that the new algorithm correctly catches
  the no-space "type":"builtin" case where the old substring check
  would silently miss it.
• ruff format and ruff check: clean.

Added or updated tests

N/A — this PR only strengthens two existing tests, no new tests added.

QA follow-up

N/A — test-only change, no user-visible behavior.

Demo

N/A — not a UI change.

AI assistance disclosure

Written with Claude Sonnet 5 via Claude Code (CLI), standard interactive
reasoning (no extended/deep-thinking budget configured for this session).
Session: read the actual source (ToolConfigParseResult, BuiltinToolConfig,
the reference-file regex extraction) to verify both bugs independently rather
than trust the issue text alone, found and reused an existing idiomatic
assertion pattern already in the codebase (test_parsing.py) for the first
fix, implemented both rewrites, ran the targeted files and the full SDK suite,
then did the sanity checks described above before handing off for commit/push.

Checklist

• I have included a video or screen recording for UI changes, or marked Demo as N/A
• Relevant tests pass locally
• Relevant linting and formatting pass locally
• I have signed the CLA, or I will sign it when the bot prompts me

Contributor Resources

• Contributing guide
• Creating your first PR
• Development mode
• Testing
• Formatting and linting
• Slack support

[vercel]

@Yuvakunaal is attempting to deploy a commit to the agenta projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Tests
  • Improved validation of JSON examples, including nested structures and invalid JSON.
  • Added coverage to verify that builtin tool configurations are correctly parsed and resolved.

Walkthrough

The changes strengthen two SDK unit tests. JSON examples are parsed and recursively checked for builtin tool entries. The resolver test separately validates bare tool-name coercion before resolution.

Changes

Built-in tool test validation

Layer / File(s)	Summary
Recursive JSON reference validation sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py	The test parses each fenced JSON block, reports invalid JSON, and rejects nested objects with "type": "builtin".
Resolver coercion validation sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py	The test verifies that "read" becomes a BuiltinToolConfig before passing the parsed configuration to ToolResolver.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue #5662 by correcting coercion usage and replacing substring checks with recursive JSON validation.
Out of Scope Changes check	✅ Passed	The changes are limited to the two SDK unit tests covered by issue #5662.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly identifies the SDK test fixes and explains that the affected built-in-tool tests did not exercise their intended behavior.
Description check	✅ Passed	The description directly explains both test defects, their fixes, and the reported validation results.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

Strengthens two existing SDK unit tests in the built-ins area so they actually validate the intended behavior (tool config coercion and JSON example validation), preventing vacuous passes and whitespace-sensitive false negatives.

Changes:

• Fixes the resolver test to assert coerce_tool_configs(["read"]) produces the expected BuiltinToolConfig, and then resolves the extracted tool_configs list.
• Replaces a brittle substring check in the reference-file test with json.loads plus a recursive scan for any object with type == "builtin".

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py	Ensures the coercion path is exercised by asserting parsed tool configs before resolving.
sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py	Makes the “no builtin tool entries in JSON examples” check robust by parsing JSON and scanning structurally.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b0e898ca-84cc-4ec7-9260-210d5cf12df3

📥 Commits

Reviewing files that changed from the base of the PR and between 7e9206e and 379733f.

📒 Files selected for processing (2)

• sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py
• sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.py:195

• The failure output here can be hard to act on because config-schema.md contains multiple JSON fenced blocks (see _CONFIG_SCHEMA_REFERENCE in agenta_builtins.py). If one block is invalid JSON or still contains a builtin tool entry, the assertion doesn’t identify which block failed, making debugging slower.

Consider enumerating the blocks and including the index (and total) in both the JSONDecodeError assertion and the builtin-entry assertion message.

                f"config-schema.md has a json fence that is not valid JSON: {exc}"
            ) from exc
        assert not _contains_builtin_tool_entry(parsed), (
            "a JSON example in config-schema.md still writes a builtin tool entry"
        )