fix(#20): heid-bug-hunt fixups — CLI open-world container-type hardening (slice-5)
Panel (Gróa + Hulda + Regin, source-verified by Heid): adapter/route-map/
ConnectFailed-at-call-sites sound against the declared invariants; 4 real
robustness findings, all in the CLI open-world presenter/probe paths — the
container-type layer BELOW the null/element holes the code-review already fixed.
Fixed (findings 1-3):
- `_format_whoami` (`cli.py`): a non-iterable `scopes`/`allowed_roles` scalar
(`{"scopes": 123}`) made `x or []` yield `123` → `for s in 123` TypeError. New
`_display_seq` helper degrades any non-list (scalar / bare string / null / absent)
to empty; applied to both `scopes` and `allowed_roles`.
- `_characters_probe` (`cli.py`): same class on the model catalog `items` (`{"items":
123}`) — now guards `models` is a Mapping and `items` is a list before iterating.
- `_characters_probe`: the top-level open-world reads `created` / `state` are now
`isinstance(_, Mapping)`-guarded before any `.get` — a non-mapping SDK passthrough
(`created=[...]`) aborts cleanly (exit 20) / renders `pad=None` instead of an
AttributeError.
Accepted (finding 4, documented in contract § slice-5 notes): the `--characters`
probe leaks its transient character on a mid-lifecycle failure. PRE-EXISTING (the
retired probe had the identical linear no-`finally` structure — cutover did not
worsen it), TTL-bounded, one-shot diagnostic; a `try/finally` would swallow a
happy-path delete-failure (delete is both teardown and a tested step). Gróa + Heid
concur accept is defensible.
Dismissed (finding 5): Hulda flagged `sessions.py` dropping `get_me`/etc. as a
caller-contract break — it is the intended DEC-3 no-backwards-compat migration (all
in-repo callers rewired same-diff); Heid labels it intended-surface-change.
Added CLI tests for the three hardened paths (scalar scopes/roles; scalar items +
non-mapping state; non-mapping create abort). Suite 488 green; ruff clean; live
smoke re-run clean (identical happy-path output). Patch bump 0.21.17 → 0.21.18.
This commit is contained in:
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user