From f3b56d8793d088ac76389d9cfb660dd6e01f956d Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Thu, 7 May 2026 11:35:57 -0700 Subject: [PATCH] fix(kanban): avoid double 404 when bridge already sent error response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #1837's new `_kanban_unknown_endpoint` wrapper was triggered for any falsy bridge return — but `handle_kanban_*` returns `None` (not `True`) when an inner handler calls `bad(...)` to send an error response. The wrapper then sent a SECOND 404 on top of the bridge's response, producing concatenated JSON bodies on the wire. Concrete reproducer (caught by behavioural harness, not the merged tests): GET /api/kanban/tasks//log → '{"error":"task not found"}{"error":"unknown Kanban endpoint: GET ..."}' This affected every `bad(...)`-shaped error path in the bridge: - task-not-found returns from `_task_log_payload` / `_task_detail_payload` - exception handlers for ImportError (503), LookupError (404), ValueError (400), RuntimeError (409) across all four method handlers - the `_handle_events_sse_stream` board-resolution failure path The fix: distinguish an explicit `False` (truly unmatched path) from `None` (handled, response already sent). Only `False` should trigger the unknown-endpoint diagnostic. Adds a regression test that exercises the task-not-found path through `routes.handle_get` and asserts only one JSON body is on the wire. Follow-on to #1837 (already merged into master at v0.51.20). Co-Authored-By: Claude Opus 4.7 (1M context) --- api/routes.py | 31 +++++++++++++++--------- tests/test_issue1823_kanban_not_found.py | 25 +++++++++++++++++++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/api/routes.py b/api/routes.py index 18d9c1a5..ecd7912a 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2629,9 +2629,13 @@ def handle_get(handler, parsed) -> bool: if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_get - if handle_kanban_get(handler, parsed): - return True - return _kanban_unknown_endpoint(handler, parsed, "GET") + # Only treat an explicit False as "no route matched". None means the + # bridge already sent a response via bad()/j() — emitting our own 404 + # on top of that produces concatenated JSON bodies on the wire. + result = handle_kanban_get(handler, parsed) + if result is False: + return _kanban_unknown_endpoint(handler, parsed, "GET") + return True if parsed.path == "/api/wiki/status": return _handle_llm_wiki_status(handler, parsed) if parsed.path == "/api/logs": @@ -3429,9 +3433,10 @@ def handle_post(handler, parsed) -> bool: if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_post - if handle_kanban_post(handler, parsed, body): - return True - return _kanban_unknown_endpoint(handler, parsed, "POST") + result = handle_kanban_post(handler, parsed, body) + if result is False: + return _kanban_unknown_endpoint(handler, parsed, "POST") + return True if parsed.path == "/api/dashboard/config": from api import dashboard_probe @@ -4607,9 +4612,10 @@ def handle_patch(handler, parsed) -> bool: if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_patch - if handle_kanban_patch(handler, parsed, body): - return True - return _kanban_unknown_endpoint(handler, parsed, "PATCH") + result = handle_kanban_patch(handler, parsed, body) + if result is False: + return _kanban_unknown_endpoint(handler, parsed, "PATCH") + return True return False @@ -4621,9 +4627,10 @@ def handle_delete(handler, parsed) -> bool: if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_delete - if handle_kanban_delete(handler, parsed, body): - return True - return _kanban_unknown_endpoint(handler, parsed, "DELETE") + result = handle_kanban_delete(handler, parsed, body) + if result is False: + return _kanban_unknown_endpoint(handler, parsed, "DELETE") + return True return False # ── GET route helpers ───────────────────────────────────────────────────────── diff --git a/tests/test_issue1823_kanban_not_found.py b/tests/test_issue1823_kanban_not_found.py index 6899994b..d9fdd3e2 100644 --- a/tests/test_issue1823_kanban_not_found.py +++ b/tests/test_issue1823_kanban_not_found.py @@ -67,6 +67,31 @@ def test_kanban_stale_client_error_renders_hard_refresh_escape_hatch(): assert "window.location.reload()" in PANELS +def test_inner_handler_bad_response_does_not_emit_double_404(monkeypatch): + """Regression: when the kanban bridge already sent a response via bad() + (returns None), the unknown-endpoint wrapper must not concatenate a second + 404 body on the wire. Only an explicit `False` from the bridge means the + path was unmatched. + """ + from api import kanban_bridge + + # Force the task-log payload helper to report "not found" so the bridge + # calls bad() and returns None. + monkeypatch.setattr(kanban_bridge, "_task_log_payload", lambda *a, **kw: None) + + handler = _FakeHandler() + handled = routes.handle_get(handler, urlparse("/api/kanban/tasks/abc/log")) + + assert handled is True + assert handler.status == 404 + body = handler.wfile.getvalue().decode("utf-8") + # Exactly one JSON object should have been written. Two concatenated + # objects would produce something like `}{` between them. + assert body.count("}{") == 0, f"double response detected: {body!r}" + payload = json.loads(body) + assert payload["error"] == "task not found" + + def test_kanban_load_resolves_board_before_board_scoped_requests(): boards_pos = PANELS.find("await loadKanbanBoards();") config_pos = PANELS.find("api('/api/kanban/config' + _kanbanBoardQuery())")