Skip to content

fix(contrib): isolate tracked raw tool payloads - #176

Open
ningyixi wants to merge 1 commit into
agentclientprotocol:mainfrom
ningyixi:fix/tracked-tool-raw-payload-isolation
Open

ningyixi wants to merge 1 commit into
agentclientprotocol:mainfrom
ningyixi:fix/tracked-tool-raw-payload-isolation

Conversation

@ningyixi

Copy link
Copy Markdown
Contributor

Summary

ToolCallTracker retains references to nested raw_input and raw_output values. Mutating arguments after start()/progress(), or editing a returned notification/view, silently changes the tracked state. tool_call_model() documents a deep copy but shares those raw values too: a PermissionBroker requester that normalizes its request can alter the tracker's data.

Copy raw payloads on ingress and at each export boundary, matching the existing content/location ownership policy and SessionAccumulator's raw-payload handling. Start/progress notifications, views, and permission models each have detached nested data. Explicit None clearing and scalar/list JSON values retain their behavior. The public tracker docstring documents ownership.

Validation

  • Base 11f8a4c, with the new tests: 13 failed, 15 passed in tracker/permission tests. Every ingress/export path shares mutable raw data before this change, including the public PermissionBroker flow.
  • After: 37 contrib tests passed. Coverage includes both raw fields, start/progress ingress, four export paths, permission-request mutation, scalar/list values, and explicit clearing.
  • make check: passed (lockfile, all pre-commit hooks, Ruff, ty, deptry).
  • make test: 468 passed, 1 skipped (optional Gemini CLI test).
  • git diff --check: passed.

Tests use public tracker/broker APIs and a local requester callback. No model calls, external services, schema generation, API signatures, or dependency changes.

AI assistance disclosure: Codex prepared the patch, tests, and description and ran the checks above. No human code review is claimed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant