fix: spawn_test never raises — coerce env/command to str, wrap unexpected errors (#34)

Closes #34

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

ROADMAP-POST-1.2-COWORK.md

@@ -0,0 +1,112 @@
+# BCC Roadmap — v1.2.0 readiness + what's next (Cowork planning, 2026-07-07)
+
+Written by the Cowork supervisor session. Captures (1) what's ready for the next
+release, (2) the remaining tracked backlog, and (3) genuinely-missing features
+worth planning. Priorities are suggestions for AJ, not commitments.
+
+---
+
+## 1. v1.2.0 — ready to ship
+
+Five feature branches landed as green, isolated, reviewed PRs (all approved by the
+supervisor session; each touches only `bcc.py` / `bcc_core.py` / `tests/test_core.py`
+and passes CI):
+
+| PR | Issue | Feature | CI |
+|----|-------|---------|----|
+| #25 | #18 + #19 | About dialog (Help menu, repo/issues/license links) + notify-only update checker (`__version__`, numeric version-compare, fail-quiet Gitea releases fetch, throttled once/day startup check) | run 160 ✓ |
+| #24 | #9 | Restart Claude Desktop button (post-save, Desktop-profile-only, platform-abstracted `restart_claude_desktop()`) | run 159 ✓ |
+| #23 | #6 | In-app MCP server log viewer (`server_log_path()`, read-only auto-tailing panel per server) | run 158 ✓ |
+| #22 | #17 | Stale-file detection hardened with size (`ConfigStat` mtime+size fingerprint) | run 157 ✓ |
+| #21 | #8 | Duplicate-name collision fix — closes a real silent-overwrite on drag-and-drop import | run 164 ✓ |
+
+Combined they take the suite from 92 tests toward ~130+ (each PR adds 3–20 tests).
+
+### Merge / release checklist (needs AJ — this is the open decision)
+
+The five branches all edit overlapping regions of `bcc.py`/`bcc_core.py`, so they
+will **not** merge cleanly in sequence without conflict resolution:
+
+- **Shared conflict:** #22/#23/#24/#25 each rewrote the one `_normalize_unicode`
+  non-breaking-space line (all behavior-identical). Merge one, then take that line
+  from any branch for the rest.
+- **Save-flow region:** #21 (import prompt), #22 (stale fingerprint), #24 (restart
+  button) all touch the save path in `bcc.py` — review the merged result once.
+- **Version bump:** at cut, set both `__version__` (bcc_core.py) and
+  `pyproject.toml` to `1.2.0` and update `test_dunder_version_matches_pyproject`.
+- **Release is tag-triggered:** push a `v1.2.0` tag → `.github/workflows/release.yml`
+  builds macOS/Windows/Linux and publishes. Don't hand-create the release.
+
+Suggested merge order (least- to most-conflict-prone): **#21 → #22 → #23 → #24 → #25**.
+Recommend AJ does the merges + conflict resolution locally (cleaner than the Gitea
+API for overlapping diffs), or greenlights the supervisor to merge sequentially and
+patch conflicts on each branch via the API.
+
+---
+
+## 2. Remaining tracked backlog (open issues)
+
+**Windows-specific — deferred (can't build/test in the Linux/mac sandbox):**
+
+- **#20** Windows release shows no app icon (P?) — build-config fix in `bcc.spec` /
+  `release.yml`; needs a Windows build to verify. Low risk, cosmetic.
+- **#7** Windows MSIX virtualized-path detection (P1) — config edits silently ignored
+  on MSIX installs. Core path-detection is unit-testable; the runtime behavior needs
+  a real MSIX Windows box. Genuinely valuable (silent failure class).
+- **#13** Windows process-tree kill is best-effort (P1) — follow-up refinement to #9;
+  hard to verify without Windows.
+
+**Deferred by design:**
+
+- **#5** Cross-client support (Cursor `.cursor/mcp.json`, Windsurf
+  `~/.codeium/windsurf/mcp_config.json`, VS Code `servers` key) — P1 but a behemoth;
+  its own branch, revisit after single-client UX is solid. Not a v1.2 item.
+- **#10** Server catalog / one-click add — P2, design-first. See §3 overlap with search.
+
+---
+
+## 3. Genuinely-missing features (not yet tracked) — planning candidates
+
+Grounded against what already exists (enable/disable toggle, duplicate-server,
+`_disabledMcpServers` round-trip, backup/restore, spawn-test, secret masking, JSON
+repair pipeline — all shipped, so not repeated here).
+
+**P1 candidates**
+
+1. **Server search / filter box.** Once a user has many servers (and especially once
+   #10's catalog lands), there's no way to filter the list. Small, high-utility:
+   a `QLineEdit` above the table filtering by name/command. Pure-ish; testable filter
+   fn in core.
+2. **"Test all" / health column.** Extends spawn-test (#2) + log viewer (#6): a status
+   dot per server (untested / ok / crashed) and a one-click "test all". Turns three
+   separate features into an at-a-glance dashboard — the single biggest "is my config
+   actually working?" answer.
+3. **Project-level `.mcp.json` discovery (Claude Code).** Claude Code reads project
+   `.mcp.json` files; BCC only sees the global config. Detect and offer to edit the
+   project file when relevant. Previously listed "not in backlog" — worth reconsidering
+   now that Claude Code support is solid.
+
+**P2 candidates**
+
+4. **`${VAR}` / `.env` environment-variable expansion.** Previously declined (AJ's
+   call). Revisit only if secret-handling demand grows — pairs with keychain below.
+5. **OS keychain integration for secrets.** Previously declined. Bigger lift; would
+   let env secrets live outside the JSON entirely. Design-first if pursued.
+6. **Config schema/lint for well-known servers.** Warn on obvious mistakes (missing
+   `command`, `args` not a list, unknown top-level keys) beyond the JSON-repair layer.
+7. **Theming / dark-mode follow-through.** Style constants (`MUTED`/`ACCENT`/`GOOD`)
+   already exist; a proper light/dark toggle is a polish item.
+
+**Explicitly out (recorded decisions):** renaming BCC (deferred until cross-client
+actually ships — do not raise before then).
+
+---
+
+## 4. Suggested next sprint (after v1.2.0 ships)
+
+1. Merge + release v1.2.0 (decision above).
+2. **#7 MSIX detection** — highest-value untracked-runtime P1; land the testable core
+   path-detection now, gate the runtime behavior behind a Windows verification.
+3. **Server search/filter (§3.1)** + **Test-all health column (§3.2)** — small, high
+   daily-utility, and they compound with the log viewer/spawn-test already in 1.2.
+4. Design pass on **#10 catalog** and **#5 cross-client** (both need design before code).

bcc_core.py

@@ -1591,12 +1591,29 @@ def spawn_test(data: dict, timeout: float = 3.0) -> dict:
                 "crashed"        — exited with a non-zero code before timeout
                 "not_found"      — command could not be resolved to an executable
                 "not_applicable" — remote server or no command; nothing to spawn
+                "error"          — unexpected internal failure while spawning/observing
       returncode: int | None
       stderr:    str  (first ~4 KB)
       detail:    str
 
+    Never raises: the GUI threads (Test launch / Test all) re-enable their
+    buttons only when a result arrives, so an escaping exception would leave
+    the UI stuck. Anything unexpected comes back as outcome "error".
+
     Run this off the UI thread — it blocks for up to `timeout` seconds.
     """
+    try:
+        return _spawn_test_impl(data, timeout)
+    except Exception as e:
+        return {
+            "outcome": "error",
+            "returncode": None,
+            "stderr": "",
+            "detail": f"unexpected error: {e!r}",
+        }
+
+
+def _spawn_test_impl(data: dict, timeout: float) -> dict:
     if "url" in data and "command" not in data:
         return {
             "outcome": "not_applicable",
@@ -1605,7 +1622,9 @@ def spawn_test(data: dict, timeout: float = 3.0) -> dict:
             "detail": "remote server",
         }
 
-    cmd = (data.get("command") or "").strip()
+    # str() first: pasted JSON can legally carry a non-string here and the
+    # value never round-trips through the editor before a Test all run.
+    cmd = str(data.get("command") or "").strip()
     if not cmd:
         return {
             "outcome": "not_applicable",
@@ -1627,7 +1646,8 @@ def spawn_test(data: dict, timeout: float = 3.0) -> dict:
     args_list = [resolved_cmd] + [str(a) for a in (data.get("args") or [])]
 
     merged_env = {**os.environ, "PATH": augmented_path()}
-    merged_env.update(data.get("env") or {})
+    # Popen rejects non-string env values; pasted JSON may carry numbers.
+    merged_env.update({str(k): str(v) for k, v in (data.get("env") or {}).items()})
 
     stderr_chunks: list[bytes] = []
 

tests/test_core.py

@@ -326,6 +326,42 @@ def test_spawn_test_large_stderr_does_not_deadlock():
     assert len(r["stderr"]) <= c._STDERR_CAP
 
 
+def test_spawn_test_non_string_env_value():
+    # Pasted JSON can carry numeric env values ("env": {"PORT": 8080}) that never
+    # round-trip through the editor. Popen rejects non-str env; spawn_test must
+    # coerce instead of letting TypeError escape (which would leave the GUI's
+    # Test launch / Test all buttons stuck disabled).
+    r = c.spawn_test(
+        {
+            "command": sys.executable,
+            "args": ["-c", "import os; raise SystemExit(0 if os.environ['PORT'] == '8080' else 1)"],
+            "env": {"PORT": 8080},
+        },
+        timeout=2.0,
+    )
+    assert r["outcome"] == "exited"
+    assert r["returncode"] == 0
+
+
+def test_spawn_test_non_string_command():
+    # A non-string command must classify, not raise (str(123) resolves to nothing).
+    r = c.spawn_test({"command": 123}, timeout=0.3)
+    assert r["outcome"] == "not_found"
+
+
+def test_spawn_test_internal_error_yields_result(monkeypatch):
+    # Any unexpected exception inside the spawn path must come back as a result
+    # dict (outcome "error"), never escape — the UI only re-enables its buttons
+    # when a result arrives.
+    def boom(*a, **k):
+        raise RuntimeError("simulated internal failure")
+
+    monkeypatch.setattr(c.shutil, "which", boom)
+    r = c.spawn_test({"command": "python3"}, timeout=0.3)
+    assert r["outcome"] == "error"
+    assert "simulated internal failure" in r["detail"]
+
+
 # --------------------------------------------------------------------------- #
 # 8. Backup restore
 # --------------------------------------------------------------------------- #