Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# FU-P7-T1-1 — Normalize KeyboardInterrupt handling when broker-console reuses an existing host

## Objective Summary

`P7-T1` introduced `--broker-console` as the recommended one-command entrypoint
for the dedicated broker host plus attached TUI. The spawned-host path already
wraps `run_tui(runtime)` in a `try/except KeyboardInterrupt` block and returns
exit code `0`, matching standalone `--tui` behavior. The reuse-existing-host
path does not: when `_probe_broker_console_backend(runtime)` reports a healthy
broker-backed dashboard, `_run_broker_console()` returns `run_tui(runtime)`
directly and lets `Ctrl-C` bubble out differently.

This follow-up should remove that inconsistency without widening scope. The
fix should make `--broker-console` behave the same whether it reuses an already
healthy dashboard or spawns a new host, and it should pin that contract with a
dedicated regression test.

## Deliverables

- Update `src/mcpbridge_wrapper/__main__.py` so `_run_broker_console()` handles
`KeyboardInterrupt` consistently across both the ready-backend reuse path and
the spawn-then-attach path.
- Extend `tests/unit/test_main.py` (or the most targeted CLI/TUI test module)
with regression coverage for `run_tui()` raising `KeyboardInterrupt` while
`--broker-console` is reusing an existing healthy backend.
- Produce `SPECS/INPROGRESS/FU-P7-T1-1_Validation_Report.md` with required
quality-gate evidence and acceptance checks.

## Success Criteria

- `--broker-console` returns exit code `0` when users press `Ctrl-C` after the
command attaches to an already healthy broker-backed dashboard.
- The behavior matches the existing spawned-host `--broker-console` path and
standalone `--tui` mode.
- Regression tests fail before the fix and pass after it.

## Test-First Plan

1. Add a unit test that sets `_probe_broker_console_backend()` to ready,
patches `run_tui()` to raise `KeyboardInterrupt`, and asserts
`_run_broker_console()` returns `0`.
2. Confirm the existing spawned-host interrupt test still covers the deferred
path where `_wait_for_broker_console_backend()` succeeds after spawn.
3. Implement the smallest code change needed in `_run_broker_console()`.
4. Run the required quality gates: `pytest`, `ruff check src/`, `mypy src/`,
and `pytest --cov`.

## Execution Plan

### Phase 1: Pin the reuse-path contract

Inputs:
- `src/mcpbridge_wrapper/__main__.py`
- `tests/unit/test_main.py`

Outputs:
- one focused regression test for the ready-backend reuse path
- verified understanding of current interrupt handling across `--tui` and
`--broker-console`

Verification:
- the new test reproduces the inconsistency on the current implementation

### Phase 2: Normalize the implementation

Inputs:
- `_run_broker_console()` reuse and spawn branches
- existing `KeyboardInterrupt` handling in `main()` and `_run_broker_console()`

Outputs:
- a single interrupt-handling path for both attach modes
- no behavior changes to error/reporting branches unrelated to `Ctrl-C`

Verification:
- both reuse and spawn paths return `0` on `KeyboardInterrupt`

### Phase 3: Validate and document completion

Inputs:
- updated implementation and unit tests
- repo quality gates

Outputs:
- `FU-P7-T1-1_Validation_Report.md`
- pass/fail evidence for task acceptance criteria

Verification:
- targeted tests and full project quality gates remain green

## Acceptance Tests

- `pytest tests/unit/test_main.py`
- `pytest`
- `ruff check src/`
- `mypy src/`
- `pytest --cov`

## Decision Points

- Prefer the narrowest fix in `_run_broker_console()` instead of introducing a
broader TUI wrapper abstraction unless the code clearly benefits from it.
- Keep exit-code normalization local to command entrypoints; the TUI itself
should not need to know whether it was launched in reuse or spawn mode.

## Notes

- No documentation changes are expected for this follow-up because the user
facing command shape stays the same.
- Review subject name for this task: `broker_console_keyboardinterrupt_reuse`.

---
**Archived:** 2026-03-07
**Verdict:** PASS
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# FU-P7-T1-1 Validation Report

**Task:** FU-P7-T1-1 — Normalize KeyboardInterrupt handling when broker-console reuses an existing host
**Date:** 2026-03-07
**Verdict:** PASS

## Summary

Implemented the `FU-P7-T1-1` follow-up by:

- normalizing `_run_broker_console()` so both the ready-backend reuse path and
the spawn-then-attach path return exit code `0` when `run_tui()` is
interrupted with `KeyboardInterrupt`
- adding a regression test that exercises the previously uncovered reuse path
and verifies the same `Ctrl-C` behavior already enforced for the spawned-host
path
- preserving all non-interrupt error handling and dashboard availability checks
unchanged

## Files Validated

- `src/mcpbridge_wrapper/__main__.py`
- `tests/unit/test_main.py`

## Targeted Verification

```bash
pytest tests/unit/test_main.py -k 'reuse_path_returns_0_on_keyboard_interrupt or returns_0_on_keyboard_interrupt or reuses_ready_backend'
```

- Result: `4 passed`
- Observed outcome: both broker-console attach modes now return `0` on
`KeyboardInterrupt`

## Required Quality Gates

```bash
pytest
```

- Result: `888 passed, 5 skipped in 8.07s`

```bash
ruff check src/
```

- Result: `All checks passed!`

```bash
mypy src/
```

- Result: `Success: no issues found in 20 source files`

```bash
pytest --cov
```

- Result: `888 passed, 5 skipped in 9.01s`
- Coverage: `91.63%`

## Acceptance Criteria Evidence

- [x] `--broker-console` returns exit code `0` on `KeyboardInterrupt` whether it
spawns a host or reuses an existing broker-backed dashboard.
- Evidence: `tests/unit/test_main.py::TestBrokerConsoleHelpers::test_run_broker_console_reuse_path_returns_0_on_keyboard_interrupt`
and
`tests/unit/test_main.py::TestBrokerConsoleHelpers::test_run_broker_console_returns_0_on_keyboard_interrupt`
both passed.
- [x] Unit tests cover the reuse-existing-dashboard interrupt path.
- Evidence: `tests/unit/test_main.py` now includes
`test_run_broker_console_reuse_path_returns_0_on_keyboard_interrupt`, which
patches `_probe_broker_console_backend()` to return ready and
`run_tui()` to raise `KeyboardInterrupt`.

## Notes

- The original gap was limited to the fast-path reuse branch in
`_run_broker_console()`; all other observed `KeyboardInterrupt` handling in
`--tui` and spawned broker-console flow already normalized to exit code `0`.
- Existing `websockets` / `uvicorn` deprecation warnings still appear in the
suite and remain unrelated to this task.
6 changes: 5 additions & 1 deletion SPECS/ARCHIVE/INDEX.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# mcpbridge-wrapper Tasks Archive

**Last Updated:** 2026-03-07 (P7-T3 review archived)
**Last Updated:** 2026-03-07 (FU-P7-T1-1 review archived)

## Archived Tasks

| Task ID | Folder | Archived | Verdict |
|---------|--------|----------|---------|
| FU-P7-T1-1 | [FU-P7-T1-1_Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host/](FU-P7-T1-1_Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host/) | 2026-03-07 | PASS |
| P7-T3 | [P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/](P7-T3_Auto-recover_or_guide_on_dashboard_port_ownership_conflicts/) | 2026-03-07 | PASS |
| P7-T2 | [P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/](P7-T2_Implement_a_broker_doctor_command_for_cross-black-box_diagnostics/) | 2026-03-07 | PASS |
| P7-T1 | [P7-T1_Add_one-command_broker_host_startup_with_attached_frontend/](P7-T1_Add_one-command_broker_host_startup_with_attached_frontend/) | 2026-03-07 | PASS |
Expand Down Expand Up @@ -199,6 +200,7 @@

| File | Description |
|------|-------------|
| [REVIEW_broker_console_keyboardinterrupt_reuse.md](_Historical/REVIEW_broker_console_keyboardinterrupt_reuse.md) | Review report for FU-P7-T1-1 |
| [REVIEW_dashboard_port_ownership_conflicts.md](_Historical/REVIEW_dashboard_port_ownership_conflicts.md) | Review report for P7-T3 |
| [REVIEW_broker_doctor_diagnostics.md](_Historical/REVIEW_broker_doctor_diagnostics.md) | Review report for P7-T2 |
| [REVIEW_broker_console_startup.md](_Historical/REVIEW_broker_console_startup.md) | Review report for P7-T1 |
Expand Down Expand Up @@ -341,6 +343,8 @@

| Date | Task ID | Action |
|------|---------|--------|
| 2026-03-07 | FU-P7-T1-1 | Archived REVIEW_broker_console_keyboardinterrupt_reuse report |
| 2026-03-07 | FU-P7-T1-1 | Archived Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host (PASS) |
| 2026-03-07 | P7-T1 | Archived REVIEW_broker_console_startup report |
| 2026-03-07 | P7-T1 | Archived task artifacts and validation report |
| 2026-03-07 | P6-T3 | Archived REVIEW_dedicated_host_frontend_docs report |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
## REVIEW REPORT — broker_console_keyboardinterrupt_reuse

**Scope:** origin/main..HEAD
**Files:** 7

### Summary Verdict
- [x] Approve
- [ ] Approve with comments
- [ ] Request changes
- [ ] Block

### Critical Issues

None.

### Secondary Issues

None.

### Architectural Notes

- The production change stays tightly scoped to `_run_broker_console()` in
`src/mcpbridge_wrapper/__main__.py` and does not alter any broker startup,
dashboard readiness, or error-reporting branches unrelated to `Ctrl-C`.
- Extracting the local `_run_console_tui()` helper avoids duplicating
`KeyboardInterrupt` handling while keeping exit-code normalization at the CLI
entrypoint layer where the rest of the repository already handles it.
- The added regression test closes the exact gap reported by the follow-up: the
ready-backend reuse path now has explicit interrupt coverage alongside the
existing spawned-host path.

### Tests

- `pytest tests/unit/test_main.py -k 'reuse_path_returns_0_on_keyboard_interrupt or returns_0_on_keyboard_interrupt or reuses_ready_backend'` passed (`4 passed`)
- `pytest` passed (`888 passed, 5 skipped`)
- `ruff check src/` passed
- `mypy src/` passed
- `pytest --cov` passed with `91.63%` coverage

### Next Steps

- No actionable review findings. FOLLOW-UP is skipped.
- Proceed to archive `REVIEW_broker_console_keyboardinterrupt_reuse.md` and
continue with the next queued task `FU-P7-T3-1`.
21 changes: 11 additions & 10 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
# Next Task: P7-T4Add direct local-status fallback for TUI when dashboard API is unavailable
# Next Task: FU-P7-T3-1Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts

**Priority:** P1
**Phase:** Phase 7: Broker UX and Diagnostics
**Effort:** 4-5 hours
**Dependencies:** P6-T2
**Effort:** 2-3 hours
**Dependencies:** P7-T3
**Status:** Ready

## Description

Reduce TUI dependence on the dashboard HTTP API by letting it fall back to the
best available local broker state when the dashboard endpoint is unavailable.
Users should still be able to tell whether the broker is alive, whether the
frontend/control plane is degraded, and which restart/recovery step to take
next without leaving the TUI.
When startup sees both a live broker PID and a non-broker listener on the
requested dashboard port, current remediation prioritizes broker reset guidance
and can hide the actual foreign port owner. Update startup and diagnostics
conflict ordering so users see the real blocker or one combined recovery path
instead of being sent into a reset loop.

## Recently Archived

- `2026-03-07` — `FU-P7-T1-1` archived with verdict `PASS`
- `2026-03-07` — `P7-T3` archived with verdict `PASS`
- `2026-03-07` — `P7-T2` archived with verdict `PASS`

## Next Step

Create the `P7-T4` PRD in `SPECS/INPROGRESS/`, then implement and validate the
local broker-status fallback path for TUI.
Create the `FU-P7-T3-1` PRD in `SPECS/INPROGRESS/`, then implement and
validate the mixed-state dashboard conflict guidance updates.
7 changes: 4 additions & 3 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,8 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
- [x] The command either starts the broker-hosted dashboard successfully or surfaces a precise actionable error before opening the frontend
- [x] The implementation avoids requiring users to manually sequence `--broker-daemon`, `--web-ui`, and `--tui`

#### ⬜️ FU-P7-T1-1: Normalize KeyboardInterrupt handling when broker-console reuses an existing host
#### ✅ FU-P7-T1-1: Normalize KeyboardInterrupt handling when broker-console reuses an existing host
- **Status:** ✅ Completed (2026-03-07)
- **Description:** Align `--broker-console` exit behavior when it attaches to an already healthy broker-backed dashboard. Today the spawn path normalizes `KeyboardInterrupt` to exit code `0`, but the reuse-existing-dashboard path returns `run_tui(runtime)` directly and lets `Ctrl-C` bubble out differently from both `--tui` mode and the spawned console path.
- **Priority:** P1
- **Dependencies:** P7-T1
Expand All @@ -494,8 +495,8 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
- `src/mcpbridge_wrapper/__main__.py` reuse path updated to handle `KeyboardInterrupt` consistently
- regression coverage in `tests/unit/test_main.py` or `tests/unit/test_main_tui.py`
- **Acceptance Criteria:**
- [ ] `--broker-console` returns exit code `0` on `KeyboardInterrupt` whether it spawns a host or reuses an existing broker-backed dashboard
- [ ] Unit tests cover the reuse-existing-dashboard interrupt path
- [x] `--broker-console` returns exit code `0` on `KeyboardInterrupt` whether it spawns a host or reuses an existing broker-backed dashboard
- [x] Unit tests cover the reuse-existing-dashboard interrupt path

#### ✅ P7-T2: Implement a broker doctor command for cross-black-box diagnostics
- **Status:** ✅ Completed (2026-03-07)
Expand Down
13 changes: 8 additions & 5 deletions src/mcpbridge_wrapper/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,14 +733,20 @@ def _run_broker_console(
"""Start or reuse the recommended dedicated broker host and attach the TUI."""
from mcpbridge_wrapper.tui import build_tui_runtime, run_tui

def _run_console_tui() -> int:
try:
return run_tui(runtime)
except KeyboardInterrupt:
return 0

runtime = build_tui_runtime(
web_ui_port=web_ui_port,
web_ui_config=web_ui_config,
)

ready, error = _probe_broker_console_backend(runtime)
if ready:
return run_tui(runtime)
return _run_console_tui()

running_pid = _read_running_broker_pid()
if running_pid is not None:
Expand Down Expand Up @@ -778,10 +784,7 @@ def _run_broker_console(
print(f"Error: {wait_error}", file=sys.stderr)
return 1

try:
return run_tui(runtime)
except KeyboardInterrupt:
return 0
return _run_console_tui()


def main() -> int:
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,29 @@ def test_run_broker_console_reuses_ready_backend(self):
run_tui.assert_called_once_with(runtime)
spawn_host.assert_not_called()

def test_run_broker_console_reuse_path_returns_0_on_keyboard_interrupt(self):
from mcpbridge_wrapper.__main__ import _run_broker_console

runtime = SimpleNamespace(base_url="http://127.0.0.1:8080", log_path="/tmp/broker.log")
with patch(
"mcpbridge_wrapper.tui.build_tui_runtime",
return_value=runtime,
), patch(
"mcpbridge_wrapper.__main__._probe_broker_console_backend",
return_value=(True, None),
), patch(
"mcpbridge_wrapper.tui.run_tui",
side_effect=KeyboardInterrupt(),
), patch("mcpbridge_wrapper.__main__._spawn_broker_console_host") as spawn_host:
result = _run_broker_console(
web_ui_port=None,
web_ui_config=None,
web_ui_restart=False,
)

assert result == 0
spawn_host.assert_not_called()

def test_run_broker_console_spawns_host_then_attaches(self):
from mcpbridge_wrapper.__main__ import _run_broker_console

Expand Down