fix(#20): heid-bug-hunt fixups — open-world presenter degrade-not-crash (slice-4)
Panel (Gróa+Hulda+Regin, 5/5/5, no false positives) confirmed two 3/3 crash
sites where open-world dict reads violate the declared "degrade, never crash the
presenter" invariant — the wt adapter tests + the live smoke used full server
dicts, so partial/drifted wire responses were never exercised:
- FIX (tier3.py _run_define/_run_patch): the CLI hard-indexed the open-world
define/patch dicts (`info["agent_id"]` / `["role"]` / `["agent_name"]`), so a
partial 2xx → KeyError escaping main()'s exit matrix as a raw traceback (exit 1);
and `make_description(info.get("system_prompt", ""))` fed None to .splitlines()
on a present-but-null field → AttributeError. Now reads via `_str_field` (absent/
null/non-str → default), degrades role to '?', indexes only a well-formed identity,
and maps a no-usable-agent_id 2xx to [api_failed] exit 20 (controlled, not a crash).
- FIX (web/server.py _agents_endpoint): the upstream dedup hard-indexed each item
(`{a["agent_id"] for a in upstream}` + `_as_dict`), so a malformed item (`[{}]`,
`["str"]`, `{"name":…}`, non-str agent_id) or a non-list envelope → 500 before the
local fallback merged. Now filters to well-formed mappings first; a non-list
upstream degrades to the local-only list.
- FIX (wt.py _error_field_from_body): type-check the parsed `field` is a str (the
exception surface is `field: str | None`, the CLI prints it) — restores the retired
hand-rolled `_extract_error_field` isinstance guard.
Held (triaged, no change): the 429→Tier3QuotaExceeded / bare-404→Tier3AgentNotFound
maps are ungated-by-error_code BY CONTRACT DESIGN (§ Error map route+status rows; the
SDK's ApiError floor drops Retry-After, so retry_after=0 is canonical) — the arms
flagged them spec-free; Heid's source-check confirmed intended. Dual-keying define's
429 for full row consistency is an available tightening (contract amendment), surfaced
not applied. The persona-endpoint SessionApiFailed gap the arms also caught was
already closed in the prior code-review fixup (aed9429).
Suite 475 green (+5).
This commit is contained in:
+1
-1
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
|
||||
|
||||
[project]
|
||||
name = "ratatoskr"
|
||||
version = "0.21.14"
|
||||
version = "0.21.15"
|
||||
description = "Worldtree Conversation API debug console (web + headless CLI) — multi-pane observability"
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.12"
|
||||
|
||||
+49
-21
@@ -17,6 +17,9 @@ mappings. ``wt`` imports this module's exceptions at module level; this module i
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import sys
|
||||
from collections.abc import Mapping
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
|
||||
@@ -31,6 +34,15 @@ from ratatoskr.sessions import (
|
||||
Tier3UserIdUnsupported,
|
||||
)
|
||||
|
||||
|
||||
def _str_field(info: Mapping[str, Any], key: str, *, default: str = "") -> str:
|
||||
"""A string field off an open-world response dict, or `default` when the key is
|
||||
absent / null / non-string — so a partial or drifted 2xx define/patch response
|
||||
degrades rather than KeyError/AttributeError-crashing the CLI presenter (the
|
||||
"open-world reads degrade, never crash the presenter" invariant; heid-bug-hunt)."""
|
||||
value = info.get(key)
|
||||
return value if isinstance(value, str) else default
|
||||
|
||||
# ---- CLI (`python -m ratatoskr.tier3 <subcommand>`) ------------------------
|
||||
#
|
||||
# Auth + server URL resolution mirrors ratatoskr.cli verbatim. Exit codes
|
||||
@@ -122,18 +134,27 @@ async def _run_define(ns: argparse.Namespace) -> int:
|
||||
system_prompt=ns.system_prompt,
|
||||
role=ns.role,
|
||||
)
|
||||
# v0.8.0: persist to local index so the picker can show it. The SDK returns
|
||||
# the open-world define dict — read `role` (echoed post-b128), not `model`.
|
||||
add_local_agent(
|
||||
LocalAgentEntry(
|
||||
agent_id=info["agent_id"],
|
||||
agent_name=info["agent_name"],
|
||||
role=info["role"],
|
||||
description=make_description(info.get("system_prompt", "")),
|
||||
defined_at=info.get("created_at", ""),
|
||||
# Open-world define dict — read defensively (echoes `role` post-b128, not
|
||||
# `model`). A partial/drifted 2xx must not crash the presenter.
|
||||
agent_id = _str_field(info, "agent_id")
|
||||
if not agent_id: # a 2xx with no usable agent_id is a malformed success
|
||||
sys.stderr.write(f"[api_failed] define returned no usable agent_id: {info!r}\n")
|
||||
return 20
|
||||
role = _str_field(info, "role", default="?")
|
||||
agent_name = _str_field(info, "agent_name")
|
||||
# v0.8.0: persist to local index so the picker can show it — only for a
|
||||
# well-formed identity (agent_id + agent_name); else skip the write, still print.
|
||||
if agent_name:
|
||||
add_local_agent(
|
||||
LocalAgentEntry(
|
||||
agent_id=agent_id,
|
||||
agent_name=agent_name,
|
||||
role=role,
|
||||
description=make_description(_str_field(info, "system_prompt")),
|
||||
defined_at=_str_field(info, "created_at"),
|
||||
)
|
||||
)
|
||||
)
|
||||
print(f"defined {info['agent_id']} ({info['role']})")
|
||||
print(f"defined {agent_id} ({role})")
|
||||
return 0
|
||||
|
||||
|
||||
@@ -158,17 +179,24 @@ async def _run_patch(ns: argparse.Namespace) -> int:
|
||||
system_prompt=ns.system_prompt,
|
||||
role=ns.role,
|
||||
)
|
||||
# v0.8.0: refresh local index with the post-patch state.
|
||||
update_local_agent(
|
||||
LocalAgentEntry(
|
||||
agent_id=info["agent_id"],
|
||||
agent_name=info["agent_name"],
|
||||
role=info["role"],
|
||||
description=make_description(info.get("system_prompt", "")),
|
||||
defined_at=info.get("updated_at", ""),
|
||||
# Open-world patch dict — read defensively (same degrade-not-crash posture).
|
||||
agent_id = _str_field(info, "agent_id")
|
||||
if not agent_id: # a 2xx with no usable agent_id is a malformed success
|
||||
sys.stderr.write(f"[api_failed] patch returned no usable agent_id: {info!r}\n")
|
||||
return 20
|
||||
agent_name = _str_field(info, "agent_name")
|
||||
# v0.8.0: refresh local index with the post-patch state (well-formed identity only).
|
||||
if agent_name:
|
||||
update_local_agent(
|
||||
LocalAgentEntry(
|
||||
agent_id=agent_id,
|
||||
agent_name=agent_name,
|
||||
role=_str_field(info, "role", default="?"),
|
||||
description=make_description(_str_field(info, "system_prompt")),
|
||||
defined_at=_str_field(info, "updated_at"),
|
||||
)
|
||||
)
|
||||
)
|
||||
print(f"patched {info['agent_id']}")
|
||||
print(f"patched {agent_id}")
|
||||
return 0
|
||||
|
||||
|
||||
|
||||
@@ -141,11 +141,18 @@ async def _agents_endpoint(request: Request) -> JSONResponse:
|
||||
{"error_code": "network_error", "message": str(exc)},
|
||||
status_code=502,
|
||||
)
|
||||
# Open-world upstream dicts (parity: no AgentInfo normalization); read `agent_id`
|
||||
# as a mapping key. Local tier3 entries dedup against the upstream ids (remote-wins).
|
||||
upstream_ids = {a["agent_id"] for a in upstream}
|
||||
# Open-world upstream (parity: no AgentInfo normalization). Degrade, never crash:
|
||||
# a non-list envelope OR a malformed item (missing/non-str agent_id, non-mapping)
|
||||
# is dropped rather than KeyError/TypeError'ing the endpoint into a 500 before the
|
||||
# local fallback merges (heid-bug-hunt: the "open-world reads degrade" invariant).
|
||||
upstream_items = upstream if isinstance(upstream, list) else []
|
||||
well_formed = [
|
||||
a for a in upstream_items
|
||||
if isinstance(a, Mapping) and isinstance(a.get("agent_id"), str)
|
||||
]
|
||||
upstream_ids = {a["agent_id"] for a in well_formed}
|
||||
local = _local_agents.load_local_agents()
|
||||
merged = [_as_dict(a) for a in upstream] + [
|
||||
merged = [_as_dict(a) for a in well_formed] + [
|
||||
_as_dict(le) for le in local if le.agent_id not in upstream_ids
|
||||
]
|
||||
return JSONResponse(merged, status_code=200)
|
||||
|
||||
+4
-1
@@ -464,7 +464,10 @@ def _error_field_from_body(body: str | None) -> str | None:
|
||||
field = err.get("field")
|
||||
if field is None and isinstance(err.get("detail"), dict):
|
||||
field = err["detail"].get("field")
|
||||
return field
|
||||
# The exception surface declares `field: str | None` (and the CLI prints it), so a
|
||||
# non-string envelope value (`{"field": {…}}` / `{"field": 1}`) collapses to None —
|
||||
# same guard the retired hand-rolled `_extract_error_field` applied (heid-bug-hunt).
|
||||
return field if isinstance(field, str) else None
|
||||
|
||||
|
||||
async def list_agents(client: WorldtreeClient) -> Sequence[Mapping[str, Any]]:
|
||||
|
||||
@@ -235,3 +235,52 @@ class TestCli:
|
||||
err = capsys.readouterr().err
|
||||
assert rc == 10
|
||||
assert "[usage_error]" in err
|
||||
|
||||
@respx.mock
|
||||
def test_cli_define_partial_response_degrades(
|
||||
self,
|
||||
capsys: pytest.CaptureFixture[str],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
_isolated_local_agents: "Path",
|
||||
) -> None:
|
||||
"""cli_define_partial [error]: a 201 missing `role` + a NULL system_prompt
|
||||
degrades (role '?', description-safe) and exits 0 — never a KeyError/
|
||||
AttributeError traceback (heid-bug-hunt open-world invariant)."""
|
||||
from ratatoskr.local_agents import load_local_agents
|
||||
|
||||
monkeypatch.setenv("WORLDTREE_API_URL", "https://w.example")
|
||||
monkeypatch.setenv("WORLDTREE_API_KEY", "k")
|
||||
respx.post("https://w.example/agents/define").mock(
|
||||
return_value=httpx.Response(201, json={
|
||||
"agent_id": "ratatoskr:wizard", "agent_name": "wizard",
|
||||
"system_prompt": None, # present-but-null: .get(...,'') would NOT default
|
||||
})
|
||||
)
|
||||
rc = main([
|
||||
"define", "--name", "wizard", "--system-prompt", "x", "--role", "m",
|
||||
])
|
||||
out = capsys.readouterr()
|
||||
assert rc == 0
|
||||
assert out.out.strip() == "defined ratatoskr:wizard (?)"
|
||||
# Well-formed identity → still indexed (role degraded to '?').
|
||||
entries = load_local_agents()
|
||||
assert len(entries) == 1
|
||||
assert entries[0].role == "?"
|
||||
|
||||
@respx.mock
|
||||
def test_cli_define_no_agent_id_is_api_failure(
|
||||
self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""cli_define_no_agent_id [error]: a 2xx with no usable agent_id → [api_failed]
|
||||
+ exit 20 (controlled), not an uncaught traceback."""
|
||||
monkeypatch.setenv("WORLDTREE_API_URL", "https://w.example")
|
||||
monkeypatch.setenv("WORLDTREE_API_KEY", "k")
|
||||
respx.post("https://w.example/agents/define").mock(
|
||||
return_value=httpx.Response(201, json={"role": "m"}) # no agent_id
|
||||
)
|
||||
rc = main([
|
||||
"define", "--name", "wizard", "--system-prompt", "x", "--role", "m",
|
||||
])
|
||||
err = capsys.readouterr().err
|
||||
assert rc == 20
|
||||
assert "[api_failed]" in err
|
||||
|
||||
@@ -132,6 +132,57 @@ class TestAgentsEndpoint:
|
||||
# Upstream entry wins (it's first in the merge); local is deduped
|
||||
assert body[0]["name"] == "Sindra-from-server"
|
||||
|
||||
@respx.mock
|
||||
def test_malformed_upstream_items_degrade_not_500(self, monkeypatch, tmp_path) -> None:
|
||||
"""malformed_upstream [error]: a non-mapping / agent_id-less upstream item is
|
||||
dropped, not crashed on — the endpoint degrades to the well-formed + local
|
||||
merge (heid-bug-hunt: open-world reads degrade, never crash)."""
|
||||
monkeypatch.setenv("RATATOSKR_LOCAL_AGENTS", str(tmp_path / "local_agents.json"))
|
||||
respx.get("https://w.example/agents").mock(
|
||||
return_value=httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"agent_id": "mimir", "name": "Mimir", "description": "k"},
|
||||
{}, # no agent_id — dropped
|
||||
{"name": "Ghost"}, # no agent_id — dropped
|
||||
"not-a-mapping", # non-mapping — dropped
|
||||
{"agent_id": 123}, # non-str agent_id — dropped
|
||||
],
|
||||
)
|
||||
)
|
||||
from ratatoskr.local_agents import LocalAgentEntry, add_local_agent
|
||||
add_local_agent(LocalAgentEntry(
|
||||
agent_id="ratatoskr:local", agent_name="local", role="m",
|
||||
description="d", defined_at="t",
|
||||
))
|
||||
from ratatoskr.web.server import create_app
|
||||
app = create_app(_mock_client_factory())
|
||||
resp = TestClient(app).get("/api/agents")
|
||||
assert resp.status_code == 200
|
||||
ids = {a["agent_id"] for a in resp.json()}
|
||||
# Only the one well-formed upstream item + the local entry survive.
|
||||
assert ids == {"mimir", "ratatoskr:local"}
|
||||
|
||||
@respx.mock
|
||||
def test_non_list_upstream_falls_back_to_local(self, monkeypatch, tmp_path) -> None:
|
||||
"""non_list_upstream [error]: an envelope (non-list) upstream body degrades to
|
||||
the local-only list rather than iterating dict keys into a crash."""
|
||||
monkeypatch.setenv("RATATOSKR_LOCAL_AGENTS", str(tmp_path / "local_agents.json"))
|
||||
respx.get("https://w.example/agents").mock(
|
||||
return_value=httpx.Response(200, json={"items": [{"agent_id": "mimir"}]})
|
||||
)
|
||||
from ratatoskr.local_agents import LocalAgentEntry, add_local_agent
|
||||
add_local_agent(LocalAgentEntry(
|
||||
agent_id="ratatoskr:local", agent_name="local", role="m",
|
||||
description="d", defined_at="t",
|
||||
))
|
||||
from ratatoskr.web.server import create_app
|
||||
app = create_app(_mock_client_factory())
|
||||
resp = TestClient(app).get("/api/agents")
|
||||
assert resp.status_code == 200
|
||||
ids = {a["agent_id"] for a in resp.json()}
|
||||
assert ids == {"ratatoskr:local"}
|
||||
|
||||
|
||||
_CREATE_OK = {
|
||||
"session_id": "s-1",
|
||||
|
||||
@@ -705,6 +705,16 @@ class TestDefineAgent:
|
||||
await define_agent(_wta(fake), agent_name="wizard", system_prompt="x", role="m")
|
||||
assert ei.value.field == "valence"
|
||||
|
||||
async def test_layer_deferred_non_string_field_is_none(self) -> None:
|
||||
# A non-string `field` value collapses to None — the exception surface is
|
||||
# `field: str | None` and the CLI prints it (heid-bug-hunt hardening).
|
||||
fake = _FakeAgents(error=ApiError(
|
||||
"layer_deferred", "no", status=422, body='{"detail": {"field": {"x": 1}}}',
|
||||
))
|
||||
with pytest.raises(Tier3LayerDeferred) as ei:
|
||||
await define_agent(_wta(fake), agent_name="wizard", system_prompt="x", role="m")
|
||||
assert ei.value.field is None
|
||||
|
||||
async def test_403_wrong_code_maps_to_default(self) -> None:
|
||||
# Dual-key negative: a 403 whose code is NOT tier3_user_id_unsupported →
|
||||
# generic default, not a spurious Tier3UserIdUnsupported (INV-CUT-2).
|
||||
|
||||
Reference in New Issue
Block a user