mirror of
https://github.com/usestrix/strix.git
synced 2026-10-29 06:32:26 +00:00
fix(todo): stop json.loads corrupting bare numeric-looking todo ids
`_normalize_todo_ids` runs a bare id string through `json.loads` and, when the result isn't a list, returns `str(parsed)`. Todo ids are `str(uuid.uuid4())[:6]` hex slugs, and ones like "1e5230" or "2363e0" are valid JSON numbers — so json.loads turns them into `inf` / `2363.0`. `mark_todo_done`, `mark_todo_pending`, and `delete_todo` then operate on a non-existent id and report "Todo with ID 'inf' not found", silently failing whenever such an id is passed as a bare string — a form the comma-split fallback explicitly supports. Use json only to detect the JSON *array* form; treat any non-array input as a literal id (optionally comma-separated). Adds tests/test_todo.py covering the normalization paths plus an end-to-end `_mark` resolution.
This commit is contained in:
parent
302efedca6
commit
946ae52411
2 changed files with 93 additions and 5 deletions
|
|
@ -131,13 +131,20 @@ def _normalize_todo_ids(raw_ids: Any) -> list[str]:
|
|||
stripped = raw_ids.strip()
|
||||
if not stripped:
|
||||
return []
|
||||
# json.loads is used only to unpack the JSON *array* form, e.g.
|
||||
# '["a", "b"]'. A bare string is a literal id (optionally
|
||||
# comma-separated) and must NOT be routed through the parsed
|
||||
# scalar value: ids are 6-char uuid slugs, and ones like "1e5230"
|
||||
# or "2363e0" are valid JSON numbers that json.loads would mangle
|
||||
# (-> inf / "2363.0"), silently targeting a non-existent todo.
|
||||
try:
|
||||
data = json.loads(stripped)
|
||||
parsed = json.loads(stripped)
|
||||
except json.JSONDecodeError:
|
||||
data = stripped.split(",") if "," in stripped else [stripped]
|
||||
if isinstance(data, list):
|
||||
return [str(item).strip() for item in data if str(item).strip()]
|
||||
return [str(data).strip()]
|
||||
parsed = None
|
||||
if isinstance(parsed, list):
|
||||
return [str(item).strip() for item in parsed if str(item).strip()]
|
||||
parts = stripped.split(",") if "," in stripped else [stripped]
|
||||
return [part.strip() for part in parts if part.strip()]
|
||||
if isinstance(raw_ids, list):
|
||||
return [str(item).strip() for item in raw_ids if str(item).strip()]
|
||||
return [str(raw_ids).strip()]
|
||||
|
|
|
|||
81
tests/test_todo.py
Normal file
81
tests/test_todo.py
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
"""Tests for per-agent todo id normalization and resolution."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
import strix.tools.todo.tools as todo_tools
|
||||
from strix.tools.todo.tools import _normalize_todo_ids
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterator
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_todos_storage(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]:
|
||||
monkeypatch.setattr(todo_tools, "_todos_path", None)
|
||||
with todo_tools._todos_io_lock:
|
||||
todo_tools._todos_storage.clear()
|
||||
yield
|
||||
with todo_tools._todos_io_lock:
|
||||
todo_tools._todos_storage.clear()
|
||||
|
||||
|
||||
def test_bare_numeric_looking_id_is_preserved() -> None:
|
||||
# ids are `str(uuid.uuid4())[:6]` hex slugs; slugs like these are valid
|
||||
# JSON numbers, so json.loads would turn "1e5230" into inf and "2363e0"
|
||||
# into 2363.0. They must be kept verbatim as literal ids.
|
||||
assert _normalize_todo_ids("1e5230") == ["1e5230"]
|
||||
assert _normalize_todo_ids("2363e0") == ["2363e0"]
|
||||
assert _normalize_todo_ids("0e4440") == ["0e4440"]
|
||||
|
||||
|
||||
def test_plain_hex_and_digit_ids_are_preserved() -> None:
|
||||
assert _normalize_todo_ids("a3f9c2") == ["a3f9c2"]
|
||||
assert _normalize_todo_ids("123456") == ["123456"]
|
||||
|
||||
|
||||
def test_json_array_of_ids_is_unpacked() -> None:
|
||||
assert _normalize_todo_ids('["1e5230", "a3f9c2"]') == ["1e5230", "a3f9c2"]
|
||||
|
||||
|
||||
def test_comma_separated_ids_are_split() -> None:
|
||||
assert _normalize_todo_ids("1e5230, a3f9c2") == ["1e5230", "a3f9c2"]
|
||||
|
||||
|
||||
def test_list_input_is_stringified_and_stripped() -> None:
|
||||
assert _normalize_todo_ids([" 1e5230 ", "a3f9c2"]) == ["1e5230", "a3f9c2"]
|
||||
|
||||
|
||||
def test_empty_and_none_inputs_yield_no_ids() -> None:
|
||||
assert _normalize_todo_ids("") == []
|
||||
assert _normalize_todo_ids(" ") == []
|
||||
assert _normalize_todo_ids(None) == []
|
||||
|
||||
|
||||
def test_mark_resolves_a_bare_numeric_looking_id() -> None:
|
||||
# End-to-end: marking a bare id whose slug is a valid JSON number must
|
||||
# find and update the real todo, not fail with "Todo with ID 'inf' not
|
||||
# found".
|
||||
agent_id = "agent-1"
|
||||
todos = todo_tools._get_agent_todos(agent_id)
|
||||
todos["1e5230"] = {
|
||||
"title": "probe /admin",
|
||||
"description": None,
|
||||
"priority": "normal",
|
||||
"status": "pending",
|
||||
"created_at": "2026-07-03T00:00:00+00:00",
|
||||
"updated_at": "2026-07-03T00:00:00+00:00",
|
||||
"completed_at": None,
|
||||
}
|
||||
|
||||
result = json.loads(todo_tools._mark(agent_id=agent_id, todo_ids="1e5230", new_status="done"))
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["marked"] == ["1e5230"]
|
||||
assert "errors" not in result
|
||||
assert todos["1e5230"]["status"] == "done"
|
||||
Loading…
Add table
Reference in a new issue