fix(tools): defensive type coercion for todos in todo_tool#14187
Open
JayGwod wants to merge 1 commit into
Open
fix(tools): defensive type coercion for todos in todo_tool#14187JayGwod wants to merge 1 commit into
JayGwod wants to merge 1 commit into
Conversation
19 tasks
1c05ee1 to
32a0d11
Compare
Author
|
Bumping — open for 10 days with no review activity. Status check on current upstream/main (
Happy to address any feedback or close as superseded if there's a preferred path. Thanks! |
ffe25d2 to
a51128c
Compare
a51128c to
cedf341
Compare
This was referenced May 10, 2026
374d230 to
61a8886
Compare
7bc55fd to
bb75231
Compare
Symptoms:
- todo_tool crashes with "'str' object has no attribute 'get'"
when the LLM emits `todos` as a JSON-encoded string instead of an array.
- Also crashes if individual items are non-dict (schema violation).
Root cause:
- todo_tool() and TodoStore._validate/_dedupe_by_id assumed inputs
strictly follow the declared schema; no defensive parsing.
- After a prior tool_call rejection, the model sometimes self-corrects
by wrapping the list in json.dumps, triggering the crash.
Fix (3 guards, pure additive, ~30 lines):
1. todo_tool() entry: parse `todos` if str, reject non-list with clear error.
2. TodoStore._validate: coerce non-dict item to placeholder instead of .get() crash.
3. TodoStore._dedupe_by_id: skip .get() on non-dict, keep slot via synthetic key.
Rollback:
- Backup at tools/todo_tool.py.bak.pre-str-coerce-20260422-170109
- Or: git revert <this-sha>
See also skill: devops/hermes-todo-str-coerce-patch (reapply procedure after `hermes update`).
bb75231 to
52575a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14185.
What
Three additive defensive guards in
tools/todo_tool.pyagainst schema-violatingtodosinputs.Why
See #14185 for full context. TL;DR: Claude-family models occasionally emit
todosas ajson.dumps(...)-ed string (especially after a prior tool_call rejection), which crashestodo_toolwithAttributeError: 'str' object has no attribute 'get'.Changes
todo_tool()entry —json.loads()stringtodos; reject non-list/None with a clear error.TodoStore._validate()— coerce non-dict items to an error placeholder instead of crashing on.get().TodoStore._dedupe_by_id()—isinstance(dict)guard before key access.All guards fail-closed: invalid input → structured error response, never a raised exception.
Non-goals
Testing
Todo_toolwith a stringifiedtodosnow routes to the handler and returns a normal result.test_todo*.py(passed onvenv/bin/pytest tests/test_todo_*.py).Risk
Minimal. ~30 LOC, pure additive, only reachable on schema violations that currently crash.
Related
Companion PR for the sibling dispatch-layer defense: will be linked once it lands.