fix: skip unusable provider names, name the split ceiling

A provider name containing a colon builds ids that split back to a
different provider, and an empty name builds ':model'; both routed to the
local router under a nonsense name with no error, so parse() now skips
them. split() stays non-injective by design, since bare local ids are the
premise here, so the residual risk is recorded as a ponytail comment with
its upgrade path rather than hidden. Also pin the trailing-colon form Task
9 relies on, and the empty-needle filter, which were correct but untested.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

1 files changed, 65 insertions, 1 deletions

diff --git a/docs/superpowers/plans/2026-08-09-external-providers.md b/docs/superpowers/plans/2026-08-09-external-providers.md
index 0c8fc19..72db023 100644
--- a/docs/superpowers/plans/2026-08-09-external-providers.md
+++ b/docs/superpowers/plans/2026-08-09-external-providers.md
@@ -332,6 +332,8 @@ def test_model_ids_and_filtering():
     assert providers.split("weird:name", table) == ("local", "weird:name")
     # Only the first colon splits.
     assert providers.split("together:a:b", table) == ("together", "a:b")
+    # Task 9 addresses a provider itself with an empty model name.
+    assert providers.split("together:", table) == ("together", "")
 
     # Filtering is case-insensitive substring, any match wins.
     listed = [
@@ -353,11 +355,41 @@ def test_model_ids_and_filtering():
     # A filter matching nothing yields nothing, it does not fall back to all.
     table["together"].filter = ["zzz"]
     assert providers.apply_filter(table["together"], listed) == []
+    # An empty needle is a typo rather than a request to hide everything, so
+    # it means no filter. The opposite of the "zzz" case above, deliberately.
+    table["together"].filter = [""]
+    assert providers.apply_filter(table["together"], listed) == listed
     print("ok  model ids and filtering")
 ```
 
 Register it after `test_provider_parsing()` in the `__main__` block.
 
+Task 2 also closes three malformed-config shapes found reviewing Task 1, by
+adding to `test_provider_parsing()`:
+
+```python
+    # Any other non-list shape is wrapped too, rather than iterated: a number
+    # would raise, and a dict would silently degrade into its keys.
+    odd = providers.parse(
+        {"providers": {"n": {"base_url": "http://x.example.org", "filter": 5},
+                       "d": {"base_url": "http://y.example.org",
+                             "filter": {"a": 1}}}}
+    )
+    assert odd["n"].filter == ["5"]
+    # The invariant is that the dict was wrapped whole, not iterated into its
+    # keys. Asserting that rather than its repr, which is not ours to pin.
+    assert odd["d"].filter != ["a"] and len(odd["d"].filter) == 1
+
+    # A colon in a provider name would make every id built from it ambiguous,
+    # so such a provider is skipped rather than silently routed to local.
+    colonic = providers.parse(
+        {"providers": {"local": {"base_url": "http://x.example.org"},
+                       "a:b": {"base_url": "http://y.example.org"},
+                       "": {"base_url": "http://z.example.org"}}}
+    )
+    assert set(colonic) == {"local"}
+```
+
 - [ ] **Step 2: Run the suite to verify it fails**
 
 Run: `./test_llamachat.py`
@@ -365,7 +397,26 @@ Expected: FAIL with `AttributeError: module 'llamachat.providers' has no attribu
 
 - [ ] **Step 3: Write the implementation**
 
-Append to `llamachat/providers.py`:
+In `parse()`, broaden the filter guard so any non-list shape is wrapped as one
+needle rather than iterated, and skip a provider whose name is unusable:
+
+```python
+        name = str(name)
+        if not name or ":" in name:
+            # The id scheme splits on the first colon, so a name containing
+            # one builds ids that split back to something else entirely, and
+            # an empty name builds ":model". Both route to local under a
+            # nonsense name, silently. Skipping is the only honest option.
+            continue
+```
+
+```python
+        needles = entry.get("filter") or []
+        if not isinstance(needles, (list, tuple)):
+            needles = [needles]
+```
+
+Then append to `llamachat/providers.py`:
 
 ```python
 def qualify(provider: str, model: str) -> str:
@@ -381,6 +432,14 @@ def split(model_id: str, table: dict[str, Provider]) -> tuple[str, str]:
     A prefix that is not a configured provider is treated as part of the
     model name, which keeps a bare local model containing a colon working.
     """
+    # ponytail: this is not injective, and cannot be while local ids stay
+    # bare. A local model actually named "together:x" is indistinguishable
+    # from together's model "x", so it resolves to local only until the user
+    # configures a provider called "together", at which point the stored
+    # session id rebinds to the cloud model and starts billing without a
+    # word. Unlikely, since local names are usually filenames, and the bare
+    # local id is the feature's premise. The upgrade path is to qualify local
+    # as "local:<model>" too and migrate the sessions.model column.
     prefix, sep, rest = model_id.partition(":")
     if sep and prefix in table and prefix != LOCAL:
         return prefix, rest
@@ -393,8 +452,13 @@ def apply_filter(provider: Provider, listed: list[str]) -> list[str]:
     Case-insensitive, because provider ids capitalise inconsistently:
     "qwen" has to match "Qwen/Qwen2.5-72B-Instruct-Turbo".
     """
+    # Every path returns a fresh list on purpose: callers hold onto the
+    # result, and handing back "listed" itself would let them mutate the
+    # caller's own list. Not a redundant copy, do not simplify it away.
     if provider.is_local or not provider.filter:
         return list(listed)
+    # An empty needle is a typo, not a request to hide every model, so a
+    # filter of only empty strings means no filter.
     needles = [f.lower() for f in provider.filter if f]
     if not needles:
         return list(listed)