diff --git a/docs/contracts/worldtree_sdk_cutover.contract.md b/docs/contracts/worldtree_sdk_cutover.contract.md index 315f66a..d0ab81c 100644 --- a/docs/contracts/worldtree_sdk_cutover.contract.md +++ b/docs/contracts/worldtree_sdk_cutover.contract.md @@ -274,6 +274,23 @@ re-anchor its coverage-map rows. is already hardened (slice-4 heid bug-hunt). The rewired `_characters_probe` extracts the created id defensively (`created.get("character_id")` + type-guard → clean abort, never a hard-index KeyError) since the create ACK is now an open-world SDK read. +- **Container-type hardening (heid code-review + bug-hunt slice-5).** The degrade-not- + crash floor is guarded at THREE levels for the CLI presenters, not just one: (a) the + list-typed fields `scopes` / `allowed_roles` / model `items` degrade a non-list scalar + (`123`) or a bare string to empty via `_display_seq` / an `isinstance(_, list)` guard — + the older `or []` idiom only caught null/absent and would `for x in 123` `TypeError`; + (b) each element is type-guarded (`isinstance(m, dict)`); (c) the top-level open-world + reads `created` / `models` / `state` are `isinstance(_, Mapping)`-guarded before any + `.get` (a non-mapping passthrough would otherwise `AttributeError`). All three feed + `--whoami` / `--characters` only. +- **Accepted (not fixed): the `--characters` probe leaks its transient character on a + mid-lifecycle failure.** create → get-state → delete runs linearly with no `finally`, + so a state/delete failure after a successful create orphans the probe character until + its TTL. This is PRE-EXISTING (the retired hand-rolled probe had the identical + structure — the cutover did not worsen it), TTL-bounded, and `--characters` is a + one-shot diagnostic smoke; a `try/finally` cleanup would also swallow a happy-path + delete-failure (delete is both the teardown AND a tested lifecycle step). Accepted as + known-risk per the heid bug-hunt (Gróa + Heid concur accept is defensible). - **CLI-only rewire.** `me` / `capabilities` / `characters` / `models` have NO web-server caller — only the `--whoami` and `--characters` CLI one-shot probes. The web surface is untouched this slice. diff --git a/pyproject.toml b/pyproject.toml index 940034c..5397696 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "ratatoskr" -version = "0.21.17" +version = "0.21.18" description = "Worldtree Conversation API debug console (web + headless CLI) — multi-pane observability" readme = "README.md" requires-python = ">=3.12" diff --git a/src/ratatoskr/cli.py b/src/ratatoskr/cli.py index 0c38fb3..b99b865 100644 --- a/src/ratatoskr/cli.py +++ b/src/ratatoskr/cli.py @@ -745,16 +745,29 @@ async def _amain(args: ParsedArgs) -> int: loop.remove_signal_handler(signal.SIGINT) +def _display_seq(value: Any) -> list[str]: + """Coerce an open-world wire value to a list of display strings — the degrade-not- + crash floor for a list-typed field (`scopes`, `allowed_roles`, model `items`, ...). + + A non-list scalar (absent, null, `123`, or a bare string) → empty rather than a + crash: the older `or []` idiom handles absent/null but NOT a truthy non-iterable + (`123 or [] == 123` → `for x in 123` `TypeError`) and would char-iterate a bare + string. Only a genuine list/tuple is str-mapped (heid bug-hunt slice-5, findings 1-2). + """ + if not isinstance(value, (list, tuple)): + return [] + return [str(x) for x in value] + + def _format_whoami(me: Mapping[str, Any], caps: Mapping[str, Any]) -> str: """Render the --whoami report: identity (GET /me) + server capabilities.""" lines = ["identity:"] lines.append(f" user_id: {me.get('user_id', '?')}") lines.append(f" tier: {me.get('tier', '?')}") - # Open-world read: `scopes` may be absent, present-null, or carry non-strings — - # `or []` + str() degrades all three (a present-null `.get('scopes', [])` returns - # None, not the default), matching the `allowed_roles` hardening below (heid-code- - # review slice-5: the contract names this function the degrade-not-crash exemplar). - lines.append(f" scopes: {', '.join(str(s) for s in (me.get('scopes') or [])) or '(none)'}") + # Open-world read: `scopes` may be absent, null, a scalar, or carry non-strings — + # `_display_seq` degrades every non-list to empty (the contract names this function + # the degrade-not-crash exemplar; heid code-review + bug-hunt slice-5). + lines.append(f" scopes: {', '.join(_display_seq(me.get('scopes'))) or '(none)'}") for k in ("display_name", "key_id", "key_label"): if k in me: lines.append(f" {k}: {me[k]}") @@ -763,16 +776,17 @@ def _format_whoami(me: Mapping[str, Any], caps: Mapping[str, Any]) -> str: if isinstance(templates, dict) and templates: for name, spec in templates.items(): # A diagnostic renderer must tolerate a malformed / partially-cutover - # server (heid bug-hunt Gróa#1/#2): a non-mapping template value, or an - # explicit-null `allowed_roles` (`.get(k, [])` returns None on null, not - # the default), must degrade — not abort the whole --whoami report. + # server: a non-mapping template value, or an `allowed_roles` that is null + # / a scalar / carries non-strings, must degrade — not abort the whole + # --whoami report (heid bug-hunt slice-5: `_display_seq` guards the + # container type, not just null/element as the prior `or []` did). if not isinstance(spec, dict): lines.append(f" ephemeral_template {name}: (malformed)") continue # Canonical post-cutover shape (worldtree-dev althing 2026-07-18, # ADR-0012): roles, not models. `config.role` selects; `config.model` # is now rejected server-side. - roles = ", ".join(str(r) for r in (spec.get("allowed_roles") or [])) + roles = ", ".join(_display_seq(spec.get("allowed_roles"))) lines.append( f" ephemeral_template {name}: default={spec.get('default_role', '?')} " f"max_bytes={spec.get('system_prompt_max_bytes', '?')} roles=[{roles}]" @@ -832,15 +846,14 @@ async def _characters_probe(args: ParsedArgs) -> int: client = wt.build_client(args.server_url, api_key=args.api_key, transport=transport) try: models = await wt.list_character_models(client) - # Open-world read: `items` may be absent/null (`or []`) AND each entry may be - # a non-mapping (`[None]` / `["x"]`) or carry a non-string `name` — guard the - # item is a dict and coerce `name` to str so a malformed catalog degrades - # rather than crashing (heid-code-review slice-5; element-level completion of - # the list-level `or []` guard). + # Open-world reads degrade, never crash (heid code-review + bug-hunt slice-5): + # guard the top-level `models` is a mapping AND `items` is a list before + # iterating (a scalar `items: 123` makes `... or []` yield `123` → `for m in + # 123` TypeError), then guard each entry is a dict with a str-coerced `name`. + raw_items = models.get("items") if isinstance(models, Mapping) else None + items = raw_items if isinstance(raw_items, (list, tuple)) else [] names = ", ".join( - str(m.get("name", "?")) - for m in (models.get("items") or []) - if isinstance(m, dict) + str(m.get("name", "?")) for m in items if isinstance(m, dict) ) sys.stdout.write(f"character models: {names or '(none)'}\n") created = await wt.create_character( @@ -858,9 +871,11 @@ async def _characters_probe(args: ParsedArgs) -> int: }, ) # Open-world create ACK: degrade, don't hard-index (cumulative cutover - # foot-gun). A malformed/absent character_id aborts the probe cleanly rather - # than raising a KeyError — the lifecycle needs the id for state + delete. - cid = created.get("character_id") + # foot-gun). A non-mapping ACK or an absent/blank character_id aborts the + # probe cleanly (exit 20) rather than raising AttributeError/KeyError — the + # lifecycle needs the id for state + delete (heid bug-hunt slice-5). Past the + # guard, `created`/`state` are known mappings. + cid = created.get("character_id") if isinstance(created, Mapping) else None if not (isinstance(cid, str) and cid): sys.stderr.write( f"[session_api_failed] create returned no character_id: {created!r}\n" @@ -868,7 +883,8 @@ async def _characters_probe(args: ParsedArgs) -> int: return 20 sys.stdout.write(f"created: {cid} (ttl {created.get('ttl_expires_at')})\n") state = await wt.get_character_state(client, cid) - sys.stdout.write(f"state: pad={state.get('pad')}\n") + pad = state.get("pad") if isinstance(state, Mapping) else None + sys.stdout.write(f"state: pad={pad}\n") await wt.delete_character(client, cid) sys.stdout.write(f"deleted: {cid}\n") except wt.SessionApiFailed as exc: diff --git a/tests/test_cli.py b/tests/test_cli.py index 075a06b..4bd3e4e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1898,6 +1898,30 @@ class TestWhoami: assert rc == 0 assert "scopes: (none)" in capsys.readouterr().out + @respx.mock + def test_whoami_tolerates_scalar_scopes_and_roles( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + """Non-iterable (scalar) `scopes` / `allowed_roles` → degrade to empty, never a + `for x in 123` TypeError (heid bug-hunt slice-5: `_display_seq` guards the + container TYPE, the next layer past the code-review null/element fix).""" + respx.get("https://w.example/me").mock( + return_value=httpx.Response(200, json={"user_id": "u", "scopes": 123, "tier": "user"}) + ) + respx.get("https://w.example/capabilities").mock( + return_value=httpx.Response( + 200, + json={ + "ephemeral_templates": {"echo": {"allowed_roles": 7, "default_role": "echo"}} + }, + ) + ) + rc = main(["--whoami", "--api-key", "k", "--server", "https://w.example"]) + assert rc == 0 + out = capsys.readouterr().out + assert "scopes: (none)" in out + assert "roles=[]" in out + class TestTier2Probes: """--characters + --set-persona-pad one-shot probes (Tier-2: #161 + persona_state-write).""" @@ -1968,6 +1992,34 @@ class TestTier2Probes: assert "character models: 123, ok" in out assert "created: c1" in out + @respx.mock + def test_characters_probe_tolerates_scalar_items_and_nonmapping_state( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + """Scalar `items` (`123`) → '(none)' not a `for m in 123` TypeError; a non-mapping + `state` → 'pad=None' not an AttributeError. Lifecycle still completes (heid + bug-hunt slice-5: container-type + top-level-mapping guards).""" + respx.get("https://w.example/models/available-for-characters").mock( + return_value=httpx.Response(200, json={"items": 123}) + ) + respx.post("https://w.example/characters").mock( + return_value=httpx.Response(201, json={"character_id": "c1", "ttl_expires_at": "t"}) + ) + # non-mapping state body (open-world passthrough of a JSON array). + respx.get("https://w.example/characters/c1/state").mock( + return_value=httpx.Response(200, json=["not", "a", "mapping"]) + ) + del_route = respx.delete("https://w.example/characters/c1").mock( + return_value=httpx.Response(204) + ) + rc = main(["--characters", "--api-key", "k", "--server", "https://w.example"]) + assert rc == 0 + out = capsys.readouterr().out + assert "character models: (none)" in out + assert "state: pad=None" in out + assert "deleted: c1" in out + assert del_route.call_count == 1 + @respx.mock def test_characters_probe_create_missing_id_aborts( self, capsys: pytest.CaptureFixture[str] @@ -1989,6 +2041,27 @@ class TestTier2Probes: assert "no character_id" in capsys.readouterr().err assert del_route.call_count == 0 # aborted before state/delete — nothing to clean + @respx.mock + def test_characters_probe_non_mapping_create_aborts( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + """A non-mapping create ACK (open-world passthrough of a JSON array/scalar) → + clean exit-20 abort, never an AttributeError on `created.get(...)` (heid + bug-hunt slice-5, finding #3).""" + respx.get("https://w.example/models/available-for-characters").mock( + return_value=httpx.Response(200, json={"items": []}) + ) + respx.post("https://w.example/characters").mock( + return_value=httpx.Response(201, json=["not", "a", "mapping"]) + ) + del_route = respx.delete(url__regex=r"https://w\.example/characters/.+").mock( + return_value=httpx.Response(204) + ) + rc = main(["--characters", "--api-key", "k", "--server", "https://w.example"]) + assert rc == 20 + assert "no character_id" in capsys.readouterr().err + assert del_route.call_count == 0 + @respx.mock def test_set_persona_probe(self, capsys: pytest.CaptureFixture[str]) -> None: """set_persona_probe [happy,tracer]: POST pad to /sessions/{id}/persona_state; 204.""" diff --git a/uv.lock b/uv.lock index 68ac1f4..c993a30 100644 --- a/uv.lock +++ b/uv.lock @@ -472,7 +472,7 @@ wheels = [ [[package]] name = "ratatoskr" -version = "0.21.17" +version = "0.21.18" source = { editable = "." } dependencies = [ { name = "httpx" },