fix: survive malformed provider config at startup

Eight config shapes, all valid TOML, crashed config.load() before any
window existed, so the user got a traceback on a terminal a desktop
launch does not have. Skip malformed providers instead, keeping the
local path working, and say on stderr which one was dropped and why.

The guards are isinstance tests rather than a try/except because the
shapes raise three different exception types, and the local entry is
checked the same way: [[providers.local]] is a truthy list that raised
during the bare base_url merge, before the loop could skip it.

A config that cannot load at all now reports the file and exits rather
than half-starting on defaults the user never configured.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0176tzAW6H1i2Kz8vm2XXVGV

4 files changed, 326 insertions, 5 deletions

diff --git a/docs/superpowers/plans/2026-08-09-external-providers.md b/docs/superpowers/plans/2026-08-09-external-providers.md
index 158306b..4a428a1 100644
--- a/docs/superpowers/plans/2026-08-09-external-providers.md
+++ b/docs/superpowers/plans/2026-08-09-external-providers.md
@@ -890,6 +890,188 @@ Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>"
 
 ---
 
+## Task 4b: Harden the startup path
+
+Not in the original plan. Added after a review of Task 4 found that malformed
+config shapes crash the app during `config.load()`, which runs before any
+window exists, so the user gets a bare traceback on a terminal a desktop
+launch does not have.
+
+Eight shapes, all reachable from valid TOML a hand-editing user can write.
+Three die coercing the table itself (`providers = "oops"`, `providers = ["a"]`,
+`providers = 5`), four die on a single entry that is not a table (`a = 5`,
+`a = "x"`, `a = ["x"]`, and `[[providers.a]]`), and one dies at the local
+merge before the loop is reached (`[[providers.local]]`). The bracket slip is
+the one that matters most: `[[providers.together]]` for `[providers.together]`
+is an easy miscount and reads as a list of tables.
+
+Note the first group raises two different exception types, `ValueError` for a
+string or list and `TypeError` for a number. That is why the guard is an
+isinstance test rather than a try/except.
+
+**Files:**
+- Modify: `llamachat/providers.py`
+- Modify: `llamachat/__main__.py`
+- Modify: `test_llamachat.py`
+
+- [ ] **Step 1: Write the failing tests**
+
+Add `test_provider_malformed_shapes()` after `test_provider_parsing()`,
+asserting that every shape above leaves a working local provider, that a
+`None` entry is skipped, and that each skip names the provider on stderr
+(capture it with `contextlib.redirect_stderr`).
+
+Add `test_unusable_config_exits()`, which runs the real entry point in a
+child process. It must be a subprocess: `CONFIG_PATH` is read from the
+environment at import time and bound into `load()`'s default argument, so
+rebinding the constant after import does nothing. Only a fresh interpreter
+under a redirected `XDG_CONFIG_HOME` moves the file the app actually reads.
+Use a syntax error that carries a line number, such as `socket = = ""`;
+"unclosed array" is reported without one. Assert `returncode == 1` and, the
+load-bearing one, that `"Traceback"` is absent from stderr.
+
+Register both in the `__main__` block:
+
+```python
+    test_provider_parsing()
+    test_provider_malformed_shapes()
+    test_unusable_config_exits()
+```
+
+- [ ] **Step 2: Run the suite to verify it fails**
+
+Run: `./test_llamachat.py`
+Expected: FAIL with `ValueError: dictionary update sequence element #0 has
+length 1; 2 is required`, and, for the entry-point test, an `AssertionError`
+showing a real traceback captured from the child.
+
+- [ ] **Step 3: Fix `providers.py`**
+
+Skip, do not raise. This matches the convention Tasks 1 and 2 set for an
+empty name, a colon in a name, and a missing `base_url`. The reasoning: a
+`providers` key that is not a table is not a partial table, so falling
+through to the bare-`base_url` synthesis means the local provider still
+works and the user loses only what they never had. A malformed single entry
+costs one cloud provider, and the local path is untouched.
+
+Add `import sys`, and a helper above `parse()`:
+
+```python
+def _skipped(name: str, reason: str) -> None:
+    """Say that a provider was dropped, so the loss is not silent.
+
+    A skipped provider is invisible: its models are simply absent from the
+    picker, which looks identical to the provider being down. One stderr line
+    gives a user running from a terminal something to act on.
+
+    ponytail: stderr only, so a launch from a .desktop file still says
+    nothing. Surfacing this in the window is Task 12's job; there is no
+    window to put it in yet.
+    """
+    print(f"llamachat: ignoring provider {name!r}: {reason}", file=sys.stderr)
+```
+
+Replace the table coercion and the local merge at the top of `parse()`:
+
+```python
+    raw = values.get("providers")
+    # `providers = "oops"` is valid TOML, and coercing it raises: ValueError
+    # for a string or list, TypeError for a number, which is why this is an
+    # isinstance test rather than a try/except. A providers key that is not a
+    # table is not a partial table, it is a different shape entirely, so fall
+    # through to the bare base_url path below and keep local working.
+    table = dict(raw) if isinstance(raw, dict) else {}
+```
+
+and, in the merge below it, read the local entry through `isinstance` rather
+than truthiness, because `[[providers.local]]` is a truthy list that would
+raise here before the loop could skip it:
+
+```python
+    bare = values.get("base_url")
+    local = table.get(LOCAL) if isinstance(table.get(LOCAL), dict) else {}
+    if bare and not local.get("base_url"):
+        table[LOCAL] = {**local, "base_url": bare}
+```
+
+In the loop, replace the `entry = entry or {}` guard, which caught falsy
+values but not a truthy non-dict. The isinstance test subsumes the `None`
+case, so nothing is lost by dropping it:
+
+```python
+    for name, entry in table.items():
+        name = str(name)
+        if not isinstance(entry, dict):
+            # Most likely [[providers.x]] written for [providers.x], which
+            # TOML reads as a list of tables. Skipping keeps every other
+            # provider working, including local.
+            _skipped(name, "it is not a [providers.<name>] table")
+            continue
+```
+
+Give the two existing skips the same voice, naming what was wrong:
+`_skipped(name, "a provider name cannot be empty or contain ':'")` and
+`_skipped(name, "it has no base_url")`.
+
+- [ ] **Step 4: Fix `__main__.py`**
+
+Wrap the `config.load()` call in `main()`:
+
+```python
+    try:
+        cfg = config.load()
+    except Exception as exc:
+        # Deliberately broad. The point is that no config problem may kill a
+        # launch silently, and enumerating the coercion errors would be both
+        # longer and out of date the next time a config key is added.
+        #
+        # Failing beats falling back to DEFAULTS: that would quietly point the
+        # app at localhost:8181 and a presets.ini the user may not have, which
+        # is a different app than the one they configured.
+        print(
+            f"llamachat: {config.CONFIG_PATH} is unusable: {exc}",
+            file=sys.stderr,
+        )
+        return 1
+```
+
+`return 1` rather than a `DEFAULTS` fallback is deliberate, for the reason in
+the comment. The broad `except` is deliberate too and should not be narrowed.
+`tomllib.TOMLDecodeError` messages already carry line numbers, so a plain
+syntax error produces a good message through this path with no extra work.
+
+- [ ] **Step 5: Run the suite to verify it passes**
+
+Run: `./test_llamachat.py`
+Expected: PASS, including `ok  malformed provider shapes are skipped, not
+raised` and `ok  unusable config exits without a traceback`.
+
+Then verify against the real entry point, which is what the review actually
+reproduced. For each shape, write it to a temp config and run
+`XDG_CONFIG_HOME=$T python3 -m llamachat --ping`. Every provider shape must
+exit 0 with `local` still pointing at its configured URL; the syntax error
+must exit 1 with the path and line number and no traceback.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add llamachat/providers.py llamachat/__main__.py test_llamachat.py \
+  docs/superpowers/plans/2026-08-09-external-providers.md
+git commit -m "fix: survive malformed provider config at startup
+
+Eight config shapes, all valid TOML, crashed config.load() before any
+window existed, so the user got a traceback on a terminal a desktop
+launch does not have. Skip malformed providers instead, keeping the
+local path working, and say on stderr which one was dropped and why.
+
+A config that cannot load at all now reports the file and exits rather
+than half-starting on defaults the user never configured.
+
+Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>"
+```
+
+---
+
 ## Task 5: models.ini storage
 
 **Files:**
diff --git a/llamachat/__main__.py b/llamachat/__main__.py
index 0faf88a..b8979e4 100644
--- a/llamachat/__main__.py
+++ b/llamachat/__main__.py
@@ -64,7 +64,21 @@ def main(argv: list[str] | None = None) -> int:
         print(f"llamachat {__version__}")
         return 0
 
-    cfg = config.load()
+    try:
+        cfg = config.load()
+    except Exception as exc:
+        # Deliberately broad. The point is that no config problem may kill a
+        # launch silently, and enumerating the coercion errors would be both
+        # longer and out of date the next time a config key is added.
+        #
+        # Failing beats falling back to DEFAULTS: that would quietly point the
+        # app at localhost:8181 and a presets.ini the user may not have, which
+        # is a different app than the one they configured.
+        print(
+            f"llamachat: {config.CONFIG_PATH} is unusable: {exc}",
+            file=sys.stderr,
+        )
+        return 1
 
     for name in ("toggle", "show", "hide", "ping", "quit"):
         if getattr(args, name):
diff --git a/llamachat/providers.py b/llamachat/providers.py
index 56466a3..52a1132 100644
--- a/llamachat/providers.py
+++ b/llamachat/providers.py
@@ -15,6 +15,7 @@
 
 import os
 import subprocess
+import sys
 from dataclasses import dataclass, field
 
 # The local llama.cpp router. Its models are shown and stored without a
@@ -58,13 +59,33 @@ def _number(raw, cast):
         return None
 
 
+def _skipped(name: str, reason: str) -> None:
+    """Say that a provider was dropped, so the loss is not silent.
+
+    A skipped provider is invisible: its models are simply absent from the
+    picker, which looks identical to the provider being down. One stderr line
+    gives a user running from a terminal something to act on.
+
+    ponytail: stderr only, so a launch from a .desktop file still says
+    nothing. Surfacing this in the window is Task 12's job; there is no
+    window to put it in yet.
+    """
+    print(f"llamachat: ignoring provider {name!r}: {reason}", file=sys.stderr)
+
+
 def parse(values: dict) -> dict[str, Provider]:
     """Build the provider table from already-loaded config values.
 
     Takes the raw dict rather than a path so config.py owns file reading and
     this stays testable without touching disk.
     """
-    table = dict(values.get("providers") or {})
+    raw = values.get("providers")
+    # `providers = "oops"` is valid TOML, and coercing it raises: ValueError
+    # for a string or list, TypeError for a number, which is why this is an
+    # isinstance test rather than a try/except. A providers key that is not a
+    # table is not a partial table, it is a different shape entirely, so fall
+    # through to the bare base_url path below and keep local working.
+    table = dict(raw) if isinstance(raw, dict) else {}
 
     # An old config has only a bare base_url. Fill it in as the local
     # provider's URL so nothing needs migrating, but never override an
@@ -72,24 +93,36 @@ def parse(values: dict) -> dict[str, Provider]:
     # [providers.local] that only sets an api_key is adding detail to the
     # provider the user already has, not replacing it, and treating it as a
     # replacement would silently delete local entirely.
+    #
+    # The local entry is read through isinstance too, not just truthiness: a
+    # [[providers.local]] is a truthy list, and merging into it would raise
+    # here, before the loop below ever gets a chance to skip it.
     bare = values.get("base_url")
-    if bare and not (table.get(LOCAL) or {}).get("base_url"):
-        table[LOCAL] = {**(table.get(LOCAL) or {}), "base_url": bare}
+    local = table.get(LOCAL) if isinstance(table.get(LOCAL), dict) else {}
+    if bare and not local.get("base_url"):
+        table[LOCAL] = {**local, "base_url": bare}
 
     out: dict[str, Provider] = {}
     for name, entry in table.items():
-        entry = entry or {}
         name = str(name)
+        if not isinstance(entry, dict):
+            # Most likely [[providers.x]] written for [providers.x], which
+            # TOML reads as a list of tables. Skipping keeps every other
+            # provider working, including local.
+            _skipped(name, "it is not a [providers.<name>] table")
+            continue
         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.
+            _skipped(name, "a provider name cannot be empty or contain ':'")
             continue
         base_url = str(entry.get("base_url") or "").rstrip("/")
         if not base_url:
             # ponytail: a provider with no URL is misconfigured, not a
             # partial one. Skipping beats inventing a default endpoint.
+            _skipped(name, "it has no base_url")
             continue
         # filter = "qwen" is an easy TOML slip for filter = ["qwen"], and
         # iterating the string would turn it into four single-character
diff --git a/test_llamachat.py b/test_llamachat.py
index 0eea9e1..ed50995 100755
--- a/test_llamachat.py
+++ b/test_llamachat.py
@@ -1003,6 +1003,96 @@ def test_provider_parsing():
     print("ok  provider config parsing")
 
 
+def test_provider_malformed_shapes():
+    """Config shapes that are not tables are skipped, never raised on.
+
+    Every case here is valid TOML a hand-editing user can write, and every
+    one of them used to reach the GUI as a traceback before any window
+    existed. The invariant is that the local provider survives all of them.
+    """
+    import io
+    from contextlib import redirect_stderr
+
+    from llamachat import providers
+
+    # `providers` itself is not a table. Three shapes, and note they used to
+    # raise two different exception types, which is why the guard is an
+    # isinstance test and not a try/except.
+    for bad in ("oops", ["a"], 5):
+        fallen_back = providers.parse(
+            {"base_url": "http://localhost:8181", "providers": bad}
+        )
+        assert set(fallen_back) == {"local"}, bad
+        assert fallen_back["local"].base_url == "http://localhost:8181"
+
+    # A single entry that is not a table. The list case is the one that
+    # matters: [[providers.a]] is a plausible slip for [providers.a].
+    for bad in (5, "x", ["x"], [{"base_url": "http://y.example.org"}]):
+        mixed = providers.parse(
+            {"providers": {"local": {"base_url": "http://x.example.org"},
+                           "a": bad}}
+        )
+        assert set(mixed) == {"local"}, bad
+
+    # A None entry, which TOML cannot produce but callers can, is still
+    # skipped rather than crashing.
+    assert providers.parse({"providers": {"a": None}}) == {}
+
+    # Each skip says which provider it dropped and why, so a user running
+    # from a terminal has something to act on.
+    err = io.StringIO()
+    with redirect_stderr(err):
+        providers.parse(
+            {
+                "providers": {
+                    "listy": [{"base_url": "http://x.example.org"}],
+                    "urlless": {"api_key": "env:SOME_VAR"},
+                    "a:b": {"base_url": "http://y.example.org"},
+                }
+            }
+        )
+    messages = err.getvalue()
+    assert "listy" in messages and "urlless" in messages and "a:b" in messages
+    assert "base_url" in messages  # the missing-URL case names what is missing
+    print("ok  malformed provider shapes are skipped, not raised")
+
+
+def test_unusable_config_exits():
+    """A config that cannot load exits 1 with a message, not a traceback.
+
+    Runs the real entry point in a child process rather than calling main()
+    in-process. CONFIG_PATH is read from the environment at import time and
+    baked into load()'s default argument, so rebinding the constant after
+    import does nothing: only a fresh interpreter with a redirected
+    XDG_CONFIG_HOME actually moves the file the app reads.
+    """
+    with tempfile.TemporaryDirectory() as tmp:
+        path = Path(tmp) / "llamachat" / "config.toml"
+        path.parent.mkdir(parents=True)
+        # A plain syntax error, the commonest way a hand-edited file breaks.
+        path.write_text('base_url = "http://localhost:8181"\nsocket = = ""\n')
+
+        env = dict(os.environ, XDG_CONFIG_HOME=tmp)
+        # --ping is the cheapest path that still loads the config, and it
+        # fails before it looks for a socket, so an instance actually running
+        # on this machine cannot turn this into a false pass.
+        proc = subprocess.run(
+            [sys.executable, "-m", "llamachat", "--ping"],
+            capture_output=True, text=True, env=env,
+            cwd=str(Path(__file__).resolve().parent),
+        )
+
+    assert proc.returncode == 1, proc.returncode
+    # A traceback here would mean the app died rather than reported.
+    assert "Traceback" not in proc.stderr, proc.stderr
+    # The path is what makes the message actionable: it says which file to fix.
+    assert str(path) in proc.stderr, proc.stderr
+    assert "unusable" in proc.stderr
+    # tomllib names the line, and that detail survives into the report.
+    assert "line" in proc.stderr, proc.stderr
+    print("ok  unusable config exits without a traceback")
+
+
 def test_model_ids_and_filtering():
     """Ids are provider:model, local stays bare, filters are substrings."""
     from llamachat import providers
@@ -1986,6 +2076,8 @@ if __name__ == "__main__":
     test_venv_discovery()
     test_config_defaults()
     test_provider_parsing()
+    test_provider_malformed_shapes()
+    test_unusable_config_exits()
     test_model_ids_and_filtering()
     test_key_resolution()
     test_config_providers()