Adds spawn_test(data, timeout) to bcc_core.py: spawns a stdio server for up to 3 s, captures stderr, classifies as ok / exited / crashed / not_found / not_applicable.
Adds SpawnTester(QThread) in bcc.py and a Test launch button in ServerEditor (stdio only, visible when command resolves ok/warn).
7 new unit tests covering all outcomes including the stdin-open regression guard.
Design decisions
stdin=PIPE (never written): MCP servers block on JSON-RPC stdin, so still-running after timeout reliably means healthy. stdin=DEVNULL sends EOF and a well-behaved server exits 0, being misclassified as exited.
Command resolved via shutil.which(augmented_path()) before Popen — no PATH ambiguity.
start_new_session=True + os.killpg on POSIX: kills the whole process group so npx/uvx grandchildren (the actual server process) don'''t get orphaned.
stdout=DEVNULL: a PIPE we don'''t drain deadlocks at ~64 KB.
Local GUI test
Ran python bcc.py locally:
Test launch button appears for stdio servers whose command resolves (ok/warn), hidden for remote and missing-command servers.
Clicking on a sleeping-process server (simulated with python -c "import time; time.sleep(60)") shows green "started · still running after 3s".
Clicking on a crashing server shows red "crashed · process exited with code 1".
## Summary
- Adds `spawn_test(data, timeout)` to `bcc_core.py`: spawns a stdio server for up to 3 s, captures stderr, classifies as `ok` / `exited` / `crashed` / `not_found` / `not_applicable`.
- Adds `SpawnTester(QThread)` in `bcc.py` and a **Test launch** button in `ServerEditor` (stdio only, visible when command resolves ok/warn).
- 7 new unit tests covering all outcomes including the stdin-open regression guard.
## Design decisions
- `stdin=PIPE` (never written): MCP servers block on JSON-RPC stdin, so still-running after timeout reliably means healthy. `stdin=DEVNULL` sends EOF and a well-behaved server exits 0, being misclassified as `exited`.
- Command resolved via `shutil.which(augmented_path())` before `Popen` — no PATH ambiguity.
- `start_new_session=True` + `os.killpg` on POSIX: kills the whole process group so `npx`/`uvx` grandchildren (the actual server process) don'''t get orphaned.
- `stdout=DEVNULL`: a PIPE we don'''t drain deadlocks at ~64 KB.
## Local GUI test
Ran `python bcc.py` locally:
- Test launch button appears for stdio servers whose command resolves (ok/warn), hidden for remote and missing-command servers.
- Clicking on a sleeping-process server (simulated with `python -c "import time; time.sleep(60)"`) shows green "started · still running after 3s".
- Clicking on a crashing server shows red "crashed · process exited with code 1".
## Test plan
- [ ] CI green (lint + py3.10/3.12)
- [ ] `python -m pytest` locally — 65 passed
- [ ] `ruff check . && ruff format --check .` — clean
Closes #2
the_og
added the P0 label 2026-07-02 00:58:09 -04:00
Add spawn_test() to bcc_core — spawns a stdio server for up to 3 s,
captures stderr, and reports ok/exited/crashed/not_found. Key design
decisions driven by real MCP server behaviour:
- stdin=PIPE (never written): servers block on JSON-RPC input and stay
alive, so "still running after timeout" reliably signals a healthy
start. stdin=DEVNULL would send EOF, causing well-behaved servers to
exit 0 and be misclassified as "exited".
- Command resolved via shutil.which(augmented_path()) before Popen so
subprocess PATH resolution is unambiguous across platforms.
- start_new_session=True on POSIX + os.killpg on timeout: kills the
whole process group, not just the launcher (npx, uvx), which would
otherwise orphan the actual node/python grandchild process.
- stdout=DEVNULL: draining a PIPE we don't read would deadlock at ~64 KB.
- stderr drained in a daemon thread, capped at 4 KB.
GUI: SpawnTester(QThread) wraps spawn_test; "Test launch" button in
ServerEditor dep row (stdio only, visible when command resolves ok/warn).
Result colours match the existing dep-status palette (green/amber/red).
Stderr appended to the diagnostics panel if it is open.
7 new unit tests cover all outcomes and the stdin-open regression guard.
Ran python bcc.py locally: button appears for stdio servers whose command
resolves, is hidden for remote servers and missing-command servers.
Closes#2
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
[Supervisor review — fix before merge] 1. stderr silently dropped
_on_spawn_done only appends captured stderr when the Details panel is already open, and refresh_dependency() rewrites diag_text on any field change, wiping the appended text. Result: on a crash with the panel closed, stderr is lost and the AC from issue #2 (stderr
**[Supervisor review — fix before merge] 1. stderr silently dropped**
`_on_spawn_done` only appends captured stderr when the Details panel is already open, and `refresh_dependency()` rewrites `diag_text` on any field change, wiping the appended text. Result: on a crash with the panel closed, stderr is lost and the AC from issue #2 (stderr
_drain stops reading once _STDERR_CAP (4 KB) is reached. A process that writes more than 4 KB of stderr then blocks on the full pipe buffer, never exits, and is misclassified as "ok" (still running) instead of "crashed".
Fix: keep reading until EOF regardless of the cap; discard bytes beyond the cap (buffer the cap, drain the rest to /dev/null). Add a test: process that writes ~64 KB to stderr then exits non-zero → outcome "crashed".
**[Supervisor review — fix before merge] 2. Drain-cap deadlock misclassifies crashes**
`_drain` stops reading once `_STDERR_CAP` (4 KB) is reached. A process that writes more than 4 KB of stderr then blocks on the full pipe buffer, never exits, and is misclassified as "ok" (still running) instead of "crashed".
**Fix:** keep reading until EOF regardless of the cap; discard bytes beyond the cap (buffer the cap, drain the rest to `/dev/null`). Add a test: process that writes ~64 KB to stderr then exits non-zero → outcome "crashed".
[Supervisor review — follow-up ok] 3. Test timing flakiness
The crashed/exited tests use 0.3–0.5 s timeouts; a slow CI runner plus interpreter startup could flip an outcome (a Python subprocess that takes >300 ms to start up would time out and be classified "ok" instead of "crashed" or "exited").
Proposal: File as a follow-up issue and bump those timeouts to 1–2 s in a small patch. The "ok" (timeout) path can stay short since expiry means success.
**[Supervisor review — follow-up ok] 3. Test timing flakiness**
The crashed/exited tests use 0.3–0.5 s timeouts; a slow CI runner plus interpreter startup could flip an outcome (a Python subprocess that takes >300 ms to start up would time out and be classified "ok" instead of "crashed" or "exited").
**Proposal:** File as a follow-up issue and bump those timeouts to 1–2 s in a small patch. The "ok" (timeout) path can stay short since expiry means success.
[Supervisor review — follow-up ok] 4. Windows process tree
proc.kill() on Windows won't reach npx/uvx grandchildren (already noted in the PR as best-effort). When filed as a follow-up, reference issue #9 — same concern family as not blanket-killing claude* processes when restarting Claude Desktop.
Proposal: File as a follow-up issue tied to issue #9 scoping. No code change needed here.
**[Supervisor review — follow-up ok] 4. Windows process tree**
`proc.kill()` on Windows won't reach npx/uvx grandchildren (already noted in the PR as best-effort). When filed as a follow-up, reference issue #9 — same concern family as not blanket-killing `claude*` processes when restarting Claude Desktop.
**Proposal:** File as a follow-up issue tied to issue #9 scoping. No code change needed here.
Two bugs caught in supervisor review:
1. Stderr was silently dropped when the Details panel was closed during a
crash. _on_spawn_done appended to diag_text only when the panel was
already open, and refresh_dependency() clobbered that text on the next
field change anyway.
Fix: stash the result in self._last_spawn; _full_diag_text() appends
the stderr section whenever diag text is generated; _on_spawn_done
auto-opens the panel on non-ok outcomes (same pattern as the existing
auto_open for missing commands).
2. _drain stopped reading once _STDERR_CAP (4 KB) was reached. A process
that writes more than 4 KB then blocked on a full pipe buffer, never
exited, and was misclassified as "ok" instead of "crashed".
Fix: drain to EOF unconditionally; keep only the first _STDERR_CAP bytes.
Regression test: 64 KB stderr + exit(3) → outcome "crashed".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both fixes from items 1 and 2 are addressed in commit 231403c:
Fix 1 — stderr persistence:
Added self._last_spawn: dict | None = None to ServerEditor.__init__.
New _full_diag_text() helper appends the stderr section whenever diag text is generated — so refresh_dependency() and _toggle_diag() both include it without clobbering.
_on_spawn_done stores the result in self._last_spawn and auto-opens the Details panel on non-ok outcomes (same pattern as the existing auto_open for missing commands).
load_entry() clears _last_spawn when switching servers.
Fix 2 — drain-cap deadlock:
_drain now reads until EOF unconditionally; only the first _STDERR_CAP bytes are kept.
New test test_spawn_test_large_stderr_does_not_deadlock: 64 KB stderr + exit(3) → outcome \crashed, returncode 3, len(stderr) <= _STDERR_CAP.
Items 3 and 4 filed as follow-up issues: #12 (test timeout flakiness) and #13 (Windows process-tree kill, cross-referenced with #9).
66 tests pass, ruff clean.
Both fixes from items 1 and 2 are addressed in commit 231403c:
**Fix 1 — stderr persistence:**
- Added `self._last_spawn: dict | None = None` to `ServerEditor.__init__`.
- New `_full_diag_text()` helper appends the stderr section whenever diag text is generated — so `refresh_dependency()` and `_toggle_diag()` both include it without clobbering.
- `_on_spawn_done` stores the result in `self._last_spawn` and auto-opens the Details panel on non-ok outcomes (same pattern as the existing `auto_open` for missing commands).
- `load_entry()` clears `_last_spawn` when switching servers.
**Fix 2 — drain-cap deadlock:**
- `_drain` now reads until EOF unconditionally; only the first `_STDERR_CAP` bytes are kept.
- New test `test_spawn_test_large_stderr_does_not_deadlock`: 64 KB stderr + `exit(3)` → outcome \crashed\, returncode 3, `len(stderr) <= _STDERR_CAP`.
**Items 3 and 4** filed as follow-up issues: #12 (test timeout flakiness) and #13 (Windows process-tree kill, cross-referenced with #9).
66 tests pass, ruff clean.
the_og
merged commit 111fa8e367 into main2026-07-02 01:11:49 -04:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
spawn_test(data, timeout)tobcc_core.py: spawns a stdio server for up to 3 s, captures stderr, classifies asok/exited/crashed/not_found/not_applicable.SpawnTester(QThread)inbcc.pyand a Test launch button inServerEditor(stdio only, visible when command resolves ok/warn).Design decisions
stdin=PIPE(never written): MCP servers block on JSON-RPC stdin, so still-running after timeout reliably means healthy.stdin=DEVNULLsends EOF and a well-behaved server exits 0, being misclassified asexited.shutil.which(augmented_path())beforePopen— no PATH ambiguity.start_new_session=True+os.killpgon POSIX: kills the whole process group sonpx/uvxgrandchildren (the actual server process) don'''t get orphaned.stdout=DEVNULL: a PIPE we don'''t drain deadlocks at ~64 KB.Local GUI test
Ran
python bcc.pylocally:python -c "import time; time.sleep(60)") shows green "started · still running after 3s".Test plan
python -m pytestlocally — 65 passedruff check . && ruff format --check .— cleanCloses #2
[Supervisor review — fix before merge] 1. stderr silently dropped
_on_spawn_doneonly appends captured stderr when the Details panel is already open, andrefresh_dependency()rewritesdiag_texton any field change, wiping the appended text. Result: on a crash with the panel closed, stderr is lost and the AC from issue #2 (stderr[Supervisor review — fix before merge] 2. Drain-cap deadlock misclassifies crashes
_drainstops reading once_STDERR_CAP(4 KB) is reached. A process that writes more than 4 KB of stderr then blocks on the full pipe buffer, never exits, and is misclassified as "ok" (still running) instead of "crashed".Fix: keep reading until EOF regardless of the cap; discard bytes beyond the cap (buffer the cap, drain the rest to
/dev/null). Add a test: process that writes ~64 KB to stderr then exits non-zero → outcome "crashed".[Supervisor review — follow-up ok] 3. Test timing flakiness
The crashed/exited tests use 0.3–0.5 s timeouts; a slow CI runner plus interpreter startup could flip an outcome (a Python subprocess that takes >300 ms to start up would time out and be classified "ok" instead of "crashed" or "exited").
Proposal: File as a follow-up issue and bump those timeouts to 1–2 s in a small patch. The "ok" (timeout) path can stay short since expiry means success.
[Supervisor review — follow-up ok] 4. Windows process tree
proc.kill()on Windows won't reach npx/uvx grandchildren (already noted in the PR as best-effort). When filed as a follow-up, reference issue #9 — same concern family as not blanket-killingclaude*processes when restarting Claude Desktop.Proposal: File as a follow-up issue tied to issue #9 scoping. No code change needed here.
Both fixes from items 1 and 2 are addressed in commit
231403c:Fix 1 — stderr persistence:
self._last_spawn: dict | None = NonetoServerEditor.__init__._full_diag_text()helper appends the stderr section whenever diag text is generated — sorefresh_dependency()and_toggle_diag()both include it without clobbering._on_spawn_donestores the result inself._last_spawnand auto-opens the Details panel on non-ok outcomes (same pattern as the existingauto_openfor missing commands).load_entry()clears_last_spawnwhen switching servers.Fix 2 — drain-cap deadlock:
_drainnow reads until EOF unconditionally; only the first_STDERR_CAPbytes are kept.test_spawn_test_large_stderr_does_not_deadlock: 64 KB stderr +exit(3)→ outcome \crashed, returncode 3,len(stderr) <= _STDERR_CAP.Items 3 and 4 filed as follow-up issues: #12 (test timeout flakiness) and #13 (Windows process-tree kill, cross-referenced with #9).
66 tests pass, ruff clean.
the_og referenced this pull request2026-08-12 02:34:47 -04:00