Skip to content

get_memories errors on new memories#1827

Merged
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:fix/newhandler-empty-arguments
Feb 23, 2026
Merged

get_memories errors on new memories#1827
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:fix/newhandler-empty-arguments

Conversation

@derekmisler
Copy link
Contributor

@derekmisler derekmisler commented Feb 23, 2026

I noticed this a bunch when testing the PR reviewer locally:
Screenshot 2026-02-23 at 11 01 40 AM

The agent seems to work just fine, but never seems to be able to save memories if the initial get memories fails.

  • Fix NewHandler panic when a model calls a tool with no parameters (e.g. get_memories) and the streaming API delivers an empty string for Function.Arguments
  • Default empty arguments to "{}" before passing to json.Unmarshal, matching the existing guard in script_shell tools (If a shell script tool has no arguments, cagent cannot call it #585)
  • Add unit tests covering normal arguments, empty arguments, empty object arguments, and invalid JSON arguments

Details

When a model invokes a tool that takes no parameters, the streaming API may deliver an empty string for Function.Arguments. The generic NewHandler[T] function passed this directly to json.Unmarshal, which fails with "unexpected end of JSON input".

This was previously fixed for script_shell tools in #585 by guarding with if Arguments != "", but NewHandler (used by memory, think, and any typed handler) was missed.

Test plan

  • Added TestNewHandler_EmptyArguments — verifies empty string arguments no longer cause an error
  • Added TestNewHandler_WithArguments — verifies normal JSON arguments still parse correctly
  • Added TestNewHandler_EmptyObjectArguments — verifies "{}" arguments work
  • Added TestNewHandler_InvalidArguments — verifies malformed JSON still returns an error

When a model calls a tool with no parameters (e.g. get_memories),
the streaming API may deliver an empty string for Function.Arguments.
NewHandler passed this directly to json.Unmarshal, which fails with
"unexpected end of JSON input".

This was previously fixed for script_shell tools in docker#585 by guarding
with `if Arguments != ""`, but NewHandler (used by memory, think, and
any typed handler) was missed.

Default empty arguments to "{}" before unmarshaling.

Fixes the same root cause as docker#585.
@derekmisler derekmisler self-assigned this Feb 23, 2026
@derekmisler derekmisler marked this pull request as ready for review February 23, 2026 16:00
@derekmisler derekmisler requested a review from a team as a code owner February 23, 2026 16:00
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Review Summary

APPROVED — This PR correctly fixes the panic when NewHandler receives empty string arguments from the streaming API.

The fix follows the established pattern from #585 (script_shell tools) and properly handles the case where tools with no parameters receive empty strings. The test coverage is comprehensive, covering normal arguments, empty arguments, empty object arguments, and invalid JSON.

All potential concerns were verified and dismissed:

  • Whitespace handling: json.Unmarshal correctly handles whitespace per JSON RFC standards
  • Struct validation: Required field validation should happen in tool handlers, not in the generic wrapper
  • Protocol masking: The empty string → "{}" conversion is expected behavior for the streaming API, not a bug

Great fix with solid test coverage! 🚀

@derekmisler derekmisler merged commit 59413d3 into docker:main Feb 23, 2026
11 checks passed
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.

2 participants