the_og/better-claude-config
1
0
Fork 0
Code Issues 8 Pull Requests 3 Actions Packages Projects Releases 4 Wiki Activity

feat: stdio server spawn-test (#2) #11

Merged
the_og merged 2 commits from feat/2-stdio-spawn-test into main 2026-07-02 01:11:49 -04:00
Conversation 5 Commits 2 Files Changed 3
+280 -5
the_og commented 2026-07-02 00:58:09 -04:00
Owner
Copy Link
Copy Source

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

## 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
the_og added 1 commit 2026-07-02 00:58:09 -04:00
feat: stdio server spawn-test (issue #2)
CI / Lint (ruff) (pull_request) Successful in 8s
Details
CI / Tests (py3.10) (pull_request) Successful in 8s
Details
CI / Tests (py3.12) (pull_request) Successful in 7s
Details
e98cb7abd7 
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>
the_og commented 2026-07-02 01:03:24 -04:00
Author
Owner
Copy Link
Copy Source

[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
the_og commented 2026-07-02 01:03:36 -04:00
Author
Owner
Copy Link
Copy Source

[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 — 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".
the_og commented 2026-07-02 01:03:36 -04:00
Author
Owner
Copy Link
Copy Source

[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.
the_og commented 2026-07-02 01:03:36 -04:00
Author
Owner
Copy Link
Copy Source

[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.
the_og added 1 commit 2026-07-02 01:05:39 -04:00
fix: spawn-test stderr persistence and drain-cap deadlock (PR #11 review)
CI / Lint (ruff) (pull_request) Successful in 7s
Details
CI / Tests (py3.10) (pull_request) Successful in 8s
Details
CI / Tests (py3.12) (pull_request) Successful in 10s
Details
231403c1d5 
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>
the_og commented 2026-07-02 01:06:23 -04:00
Author
Owner
Copy Link
Copy Source

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 main 2026-07-02 01:11:49 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
P0
P1
P2
blocked
Waiting on an external thing before it can move.
 decision
A decision record (D-007 style) — settled call, referenceable by number from any repo.
 needs-decision
Waiting on AJ's call.
 next
Current focus / next actionable item.
 status
The single pinned STATUS issue for this repo — live state, next action, open items, decisions needed.
No Label P0
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
the_og
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: the_og/better-claude-config#11
Delete Branch "feat/2-stdio-spawn-test"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?