fix(#20): heid-code-review fixups — CLI presenter degrade-not-crash (slice-5)
Panel: Gróa + Regin returned zero (adapter/route-map/error-map faithful);
Hulda flagged two source-confirmed open-world-presenter crash holes — the same
class the slice-4 bug-hunt found in the agents presenters. Both fixed:
- `_format_whoami` scopes (`cli.py`): `', '.join(me.get('scopes', []))` crashes on
a present-null `scopes` (`.get(k, [])` returns None, not the default) or a
non-string element. Now `', '.join(str(s) for s in (me.get('scopes') or []))` —
matching the `allowed_roles` hardening on the same function. The contract names
`_format_whoami` as the degrade-not-crash exemplar (contract:144-146); the cited
exemplar had an un-hardened line.
- `_characters_probe` model items (`cli.py`): the slice-5 `or []` guarded the
list-level null but not each entry — `[None]` / `["x"]` / `[{"name":123}]` would
raise. Now guards each item is a dict and str-coerces `name` (element-level
completion of the list-level guard).
Hulda #3 (live-smoke not in the reviewed file set) → accept: the smoke WAS run and
is recorded in deab762 + coverage-map (artifact-only review couldn't see it).
Added CLI tests for both hardened paths (present-null/non-string scopes; malformed
model items). Suite 485 green; ruff clean; live smoke re-run clean (identical
happy-path output). Patch bump 0.21.16 → 0.21.17.
This commit is contained in:
@@ -1879,6 +1879,25 @@ class TestWhoami:
|
||||
assert rc == 20
|
||||
assert "[session_api_failed]" in capsys.readouterr().err
|
||||
|
||||
@respx.mock
|
||||
def test_whoami_tolerates_null_and_nonstring_scopes(
|
||||
self, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
"""scopes present-null / non-string → renders '(none)' or str-coerced, never a
|
||||
`join(None)` TypeError (heid-code-review slice-5: `_format_whoami` is the
|
||||
contract's degrade-not-crash exemplar; `allowed_roles` was hardened, `scopes`
|
||||
was not)."""
|
||||
# scopes: null (present, not absent) → `.get('scopes', [])` would return None.
|
||||
respx.get("https://w.example/me").mock(
|
||||
return_value=httpx.Response(200, json={"user_id": "u", "scopes": None, "tier": "user"})
|
||||
)
|
||||
respx.get("https://w.example/capabilities").mock(
|
||||
return_value=httpx.Response(200, json={"ephemeral_templates": {}})
|
||||
)
|
||||
rc = main(["--whoami", "--api-key", "k", "--server", "https://w.example"])
|
||||
assert rc == 0
|
||||
assert "scopes: (none)" in capsys.readouterr().out
|
||||
|
||||
|
||||
class TestTier2Probes:
|
||||
"""--characters + --set-persona-pad one-shot probes (Tier-2: #161 + persona_state-write)."""
|
||||
@@ -1923,6 +1942,32 @@ class TestTier2Probes:
|
||||
assert "deleted: char_z" in out
|
||||
assert del_route.call_count == 1 # lifecycle cleaned up
|
||||
|
||||
@respx.mock
|
||||
def test_characters_probe_tolerates_malformed_models(
|
||||
self, capsys: pytest.CaptureFixture[str]
|
||||
) -> None:
|
||||
"""models catalog with non-mapping / non-string-name items → degrades (no
|
||||
AttributeError/TypeError), lifecycle still proceeds (heid-code-review slice-5:
|
||||
element-level completion of the list-level `or []` guard)."""
|
||||
respx.get("https://w.example/models/available-for-characters").mock(
|
||||
return_value=httpx.Response(
|
||||
200, json={"items": [None, "x", {"name": 123}, {"name": "ok"}]}
|
||||
)
|
||||
)
|
||||
respx.post("https://w.example/characters").mock(
|
||||
return_value=httpx.Response(201, json={"character_id": "c1", "ttl_expires_at": "t"})
|
||||
)
|
||||
respx.get("https://w.example/characters/c1/state").mock(
|
||||
return_value=httpx.Response(200, json={"pad": [0.0, 0.0, 0.0]})
|
||||
)
|
||||
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
|
||||
# non-mappings dropped; {"name":123}→"123", {"name":"ok"}→"ok" — no crash.
|
||||
assert "character models: 123, ok" in out
|
||||
assert "created: c1" in out
|
||||
|
||||
@respx.mock
|
||||
def test_characters_probe_create_missing_id_aborts(
|
||||
self, capsys: pytest.CaptureFixture[str]
|
||||
|
||||
Reference in New Issue
Block a user