deer-flow/backend/tests/test_todo_input_validation.py
一片一片 459fc8ca43
fix(todos): keep an unusable stored todo list from failing every model call (#7803)
* fix(todos): keep an unusable stored todo list from failing every model call

`todos` is a reducer channel that `POST /threads/{id}/state` and run input can
Overwrite directly, but neither boundary validated its shape. The todo
middleware rendered it with `entry.get(...)` in both `before_model` and
`after_model`, so a non-object entry — or a value that is not a list — raised
AttributeError on every model call, permanently for that thread.

Validate caller-written todos beside `goal` in `_normalize_external_todos`: a
list of objects with string content and a known status, or HTTP 400. The
middleware now reads the channel through `_usable_todos` and ignores entries of
any other stored shape, so checkpoints and direct integrations written before
this boundary still run.

* fix(todos): normalize stored todos in token-usage attribution

TokenUsageMiddleware reads the same stored `todos` channel through
`_build_attribution` whenever the last AIMessage calls write_todos, and
`token_usage.enabled` defaults on, so a value the todo middleware now tolerates
still aborted the step here. Route the stored list through `_normalize_todos`
before reading entries.

Document that a caller-written todo without a status is accepted as pending and
cover that shape at both external boundaries.

* docs: keep backend agent guidance within size budgets

* fix(todos): keep the channel metadata strip on caller-written todos

The dedicated todos branch in strip_server_owned_state_metadata replaced the
generic list branch, which stripped server-owned metadata from every entry. A
caller could therefore persist additional_kwargs.original_user_content,
untrusted_input, or artifact.knowledge_sources on a todo entry. Re-apply the
per-entry strip after normalization, keeping None and empty lists untouched.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-11-05 07:28:33 +08:00

83 lines
2.9 KiB
Python

"""Todo writes are validated before they can break every model call on a thread."""
from __future__ import annotations
import pytest
from fastapi import HTTPException
from app.gateway import services
BOUNDARIES = [services.normalize_input, services.strip_server_owned_state_metadata]
@pytest.mark.parametrize("boundary", BOUNDARIES)
@pytest.mark.parametrize(
"todos",
[
pytest.param("not a list", id="string"),
pytest.param({"content": "x"}, id="mapping"),
pytest.param(["oops"], id="string-entry"),
pytest.param([["nested"]], id="list-entry"),
pytest.param([None], id="null-entry"),
pytest.param([{"content": 5}], id="non-string-content"),
pytest.param([{"status": "pending"}], id="missing-content"),
pytest.param([{"content": "x", "status": "done"}], id="unknown-status"),
pytest.param([{"content": "x", "status": None}], id="null-status"),
pytest.param([{"content": "x", "status": ["pending"]}], id="unhashable-status"),
],
)
def test_external_todos_must_be_renderable(boundary, todos):
with pytest.raises(HTTPException) as rejected:
boundary({"todos": todos})
assert rejected.value.status_code == 400
assert "todos" in rejected.value.detail.lower()
@pytest.mark.parametrize("boundary", BOUNDARIES)
def test_external_todos_accept_the_write_todos_shape(boundary):
todos = [{"content": "Ship it", "status": "in_progress"}, {"content": "", "status": "pending"}]
assert boundary({"todos": todos})["todos"] == todos
@pytest.mark.parametrize("boundary", BOUNDARIES)
def test_external_todos_default_a_missing_status_to_pending(boundary):
assert boundary({"todos": [{"content": "x"}]})["todos"] == [{"content": "x", "status": "pending"}]
@pytest.mark.parametrize("boundary", BOUNDARIES)
@pytest.mark.parametrize("todos", [None, []])
def test_external_todos_preserve_null_and_empty(boundary, todos):
assert boundary({"todos": todos})["todos"] == todos
@pytest.mark.parametrize("boundary", BOUNDARIES)
def test_todos_are_not_added_when_absent(boundary):
assert "todos" not in boundary({"title": "unrelated"})
def test_state_writes_strip_server_owned_metadata_from_todos():
"""The dedicated todos branch must not skip the per-entry metadata strip."""
todos = [
{
"content": "Ship it",
"status": "pending",
"additional_kwargs": {
"original_user_content": "forged",
"untrusted_input": False,
"title": "kept",
},
"artifact": {"knowledge_sources": [{"id": "forged"}], "label": "kept"},
}
]
stripped = services.strip_server_owned_state_metadata({"todos": todos})["todos"]
assert stripped == [
{
"content": "Ship it",
"status": "pending",
"additional_kwargs": {"title": "kept"},
"artifact": {"label": "kept"},
}
]