Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions SPECS/ARCHIVE/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

| Task ID | Folder | Archived | Verdict |
|---------|--------|----------|---------|
| P7-T4 | [P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/](P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/) | 2026-03-07 | PASS |
| FU-P7-T3-2 | [FU-P7-T3-2_Exclude_broker-owned_dashboard_listeners_from_foreign_port-conflict_guidance/](FU-P7-T3-2_Exclude_broker-owned_dashboard_listeners_from_foreign_port-conflict_guidance/) | 2026-03-07 | PASS |
| FU-P7-T3-1 | [FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/](FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/) | 2026-03-07 | PASS |
| 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 |
Expand Down Expand Up @@ -202,6 +203,7 @@

| File | Description |
|------|-------------|
| [REVIEW_tui_local_status_fallback.md](P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/REVIEW_tui_local_status_fallback.md) | Review report for P7-T4 |
| [REVIEW_broker_owned_listener_guidance.md](_Historical/REVIEW_broker_owned_listener_guidance.md) | Review report for FU-P7-T3-2 |
| [REVIEW_mixed_dashboard_conflict_guidance.md](_Historical/REVIEW_mixed_dashboard_conflict_guidance.md) | Review report for FU-P7-T3-1 |
| [REVIEW_broker_console_keyboardinterrupt_reuse.md](_Historical/REVIEW_broker_console_keyboardinterrupt_reuse.md) | Review report for FU-P7-T1-1 |
Expand Down Expand Up @@ -347,6 +349,8 @@

| Date | Task ID | Action |
|------|---------|--------|
| 2026-03-07 | P7-T4 | Archived REVIEW_tui_local_status_fallback report |
| 2026-03-07 | P7-T4 | Archived with verdict PASS |
| 2026-03-07 | FU-P7-T3-2 | Archived REVIEW_broker_owned_listener_guidance report |
| 2026-03-07 | FU-P7-T3-2 | Archived Exclude_broker-owned_dashboard_listeners_from_foreign_port-conflict_guidance (PASS) |
| 2026-03-07 | FU-P7-T3-1 | Archived REVIEW_mixed_dashboard_conflict_guidance report |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
# P7-T4 — Add direct local-status fallback for TUI when dashboard API is unavailable

## Objective Summary

`P6-T2` introduced a terminal frontend for the broker-hosted Web UI, but the
current TUI is still all-or-nothing: if `/api/control` or `/api/broker/status`
cannot be reached, the screen drops to a generic “Broker runtime is
unavailable” message plus raw local file metadata. That leaves users without a
clear answer to the actual question they care about: is the broker still
running locally, are its state files stale, is the dashboard port occupied, or
do they just need to reconnect later after an approval/restart event?

This task should make the TUI useful even when live dashboard-backed data is
down. The fallback does not need to reimplement the full Web UI API. It should
derive the best available local diagnosis from broker PID/socket/version files,
dashboard-port ownership, and any directly accessible local broker state, then
render that diagnosis explicitly as local fallback data. Live dashboard status
and stop control should still be used whenever the API is reachable.

## Deliverables

- Update `src/mcpbridge_wrapper/tui.py` so `BrokerTUIClient.fetch_snapshot()`
can return a structured local-fallback snapshot when dashboard requests fail.
- Add local fallback diagnostics that distinguish at least:
running broker without dashboard, foreign listener on the dashboard port,
stale local runtime files, and no local broker runtime.
- Update the TUI renderer so the screen clearly marks whether broker runtime
data comes from the live dashboard API or from local fallback state, and
whether stop control is unavailable in fallback mode.
- Add or extend tests in `tests/unit/test_tui.py` and `tests/unit/test_main_tui.py`
for fallback diagnosis, rendering, and existing `--tui` CLI behavior.
- Produce `SPECS/INPROGRESS/P7-T4_Validation_Report.md` with targeted and full
quality-gate evidence.

## Success Criteria

- TUI remains useful when the dashboard API is unavailable and still presents a
best-effort local broker diagnosis instead of only a generic unreachable
error.
- The screen explicitly distinguishes live dashboard-backed runtime data from
local fallback data.
- Users can infer from TUI alone whether they likely need to restart the
broker, free the dashboard port, clean stale files, or simply attach once the
dashboard comes back.

## Test-First Plan

1. Add `fetch_snapshot()` tests for unavailable dashboard requests where local
state indicates:
a running broker without dashboard,
a foreign listener on the dashboard port,
stale broker files,
and no live broker.
2. Add `render_screen()` tests that assert the UI labels local fallback mode
distinctly from live dashboard mode and hides stop control as unavailable
when running on fallback data.
3. Keep existing live-dashboard tests intact so fallback logic does not regress
healthy TUI behavior or stop control handling.
4. Implement the smallest production change needed to populate structured local
fallback fields and render them clearly.
5. Run required quality gates: `pytest`, `ruff check src/`, `mypy src/`, and
`pytest --cov`.

## Execution Plan

### Phase 1: Define fallback snapshot and screen contract

Inputs:
- `src/mcpbridge_wrapper/tui.py`
- existing `BrokerTUISnapshot` rendering path
- current TUI tests in `tests/unit/test_tui.py`

Outputs:
- failing tests that pin fallback-mode labels and diagnosis text
- a clear screen contract for live data vs local fallback data

Verification:
- fallback tests fail against the current “runtime unavailable” behavior
- existing healthy-runtime rendering expectations remain unchanged

### Phase 2: Add local diagnosis collection

Inputs:
- local broker files (`broker.pid`, `broker.sock`, `broker.version`)
- configured dashboard port derived from `runtime.base_url`
- any reusable local listener/process helpers already in the repo

Outputs:
- local fallback classification that can tell apart:
broker running without dashboard,
foreign listener conflict,
stale runtime files,
and broker-not-running states
- fallback snapshot fields suitable for pure rendering tests

Verification:
- unavailable dashboard path returns structured fallback data instead of only
an opaque error string
- foreign dashboard-port listeners remain visible in fallback mode

### Phase 3: Render and validate fallback mode

Inputs:
- fallback snapshot data
- `render_screen()` and `BrokerTUI` loop behavior
- full repository quality gates

Outputs:
- TUI screen that clearly identifies fallback mode, shows the local diagnosis,
and marks live dashboard controls as unavailable when appropriate
- validation report with targeted tests and full gate results

Verification:
- the screen communicates both the diagnosis and the reduced capability surface
- coverage remains at or above the repository threshold

## Acceptance Tests

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

## Decision Points

- Prefer a lightweight local diagnosis helper inside the TUI path or a shared
helper reused from existing diagnostics code, but avoid duplicating large
chunks of doctor rendering logic in the curses UI.
- Fallback mode should remain read-only for broker runtime data; if the
dashboard API is unavailable, stop control should be shown as unavailable even
if local files suggest the broker is running.
- The screen should preserve the current live dashboard view when HTTP succeeds,
and only switch to local fallback when the dashboard API is unavailable.

## Notes

- No documentation changes are expected unless the visible TUI command/help
text changes.
- Review subject name for this task: `tui_local_status_fallback`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# P7-T4 Validation Report — Add direct local-status fallback for TUI when dashboard API is unavailable

**Date:** 2026-03-07
**Branch:** `codex/p7-t4-local-status-fallback`
**Verdict:** PASS

---

## Summary of Changes

### `src/mcpbridge_wrapper/tui.py`

- Added `_build_local_fallback_broker()` helper: constructs a bounded local-only broker view from
PID/socket/version files when the dashboard API is unavailable. Distinguishes between:
- `running (local fallback)` — PID alive and visible to the process table
- `stale local state` — PID or version files exist but the process is not running
- `None` — no local state at all
- Updated `BrokerTUIClient.fetch_snapshot()` to call `_build_local_fallback_broker()` before
probing the dashboard. When `probe_backend()` raises, the snapshot is built from local state
with `runtime_source` set to `"local-fallback"` (broker data available) or
`"dashboard-unavailable"` (no local broker data).
- Added `_read_local_pid()` and `_read_local_version()` helpers for PID liveness check and
version file reading.
- Updated `BrokerTUISnapshot` with new fields: `local_pid`, `local_daemon_running`,
`local_socket_present`, `local_daemon_version`, `local_pid_file`, `local_socket_path`,
`local_version_file`, `runtime_source`, `error_message`, `status_message`.
- Updated `render_screen()` to:
- Always show a "Local Broker Files" section with PID/socket/version from local state.
- Label runtime source as `"local broker files only"`, `"no reachable dashboard data"`, or
`"live dashboard API"`.
- Show banner messages when in local fallback mode indicating dashboard unavailability and
that live control is not available.
- Updated `BrokerTUI._run_loop()` to emit a clear message when 's' is pressed in local-fallback
mode instead of silently doing nothing.

### `tests/unit/test_tui.py`

Added targeted tests for all new fallback paths:

- `test_fetch_snapshot_builds_local_fallback_when_broker_is_running` — running PID → `local-fallback`
- `test_fetch_snapshot_builds_stale_local_fallback_when_files_remain` — stale files → `local-fallback`
- `test_fetch_snapshot_surfaces_runtime_errors` — no local state → `dashboard-unavailable`
- `test_render_screen_shows_local_fallback_source_and_control_state` — screen labels
- `test_run_loop_does_not_call_stop_without_live_control` — 's' key in fallback mode

---

## Targeted Tests

```
tests/unit/test_tui.py::TestBrokerTUIClient::test_fetch_snapshot_builds_local_fallback_when_broker_is_running PASSED
tests/unit/test_tui.py::TestBrokerTUIClient::test_fetch_snapshot_builds_stale_local_fallback_when_files_remain PASSED
tests/unit/test_tui.py::TestBrokerTUIClient::test_fetch_snapshot_surfaces_runtime_errors PASSED
tests/unit/test_tui.py::TestRenderScreen::test_render_screen_shows_local_fallback_source_and_control_state PASSED
tests/unit/test_tui.py::TestBrokerTUI::test_run_loop_does_not_call_stop_without_live_control PASSED
```

All 38 TUI tests pass.

---

## Full Quality Gate Results

### `pytest` — all tests
```
898 passed, 5 skipped, 2 warnings in 8.10s
```

### `ruff check src/`
```
All checks passed!
```

### `mypy src/`
```
Success: no issues found in 20 source files
```

### `pytest --cov` — coverage
```
TOTAL: 91.75% (threshold: 90%) — PASS
tui.py: 96.1%
```

---

## Acceptance Criteria

| Criterion | Status |
|-----------|--------|
| TUI useful when dashboard API unavailable | PASS — local fallback snapshot shown |
| Screen distinguishes live dashboard from local fallback | PASS — `Runtime Source` label + banners |
| Users can infer broker state from TUI alone | PASS — state field shows `running (local fallback)` or `stale local state` |
| Stop control clearly unavailable in fallback mode | PASS — key 's' shows informational message |
| All tests pass | PASS — 898/898 |
| Ruff clean | PASS |
| Mypy clean | PASS |
| Coverage ≥ 90% | PASS — 91.75% |
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
## REVIEW REPORT — tui_local_status_fallback

**Scope:** origin/main..HEAD (P7-T4 commits)
**Files:** 2 (`src/mcpbridge_wrapper/tui.py`, `tests/unit/test_tui.py`)

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

---

### Critical Issues

None.

---

### Secondary Issues

**[Low] `_build_local_fallback_broker` does not expose `upstream_pid`, `connected_clients`, or other rich fields**
The fallback broker dict only exposes `state`, `pid`, `socket_path`, and `version`. The render path renders several fields (`upstream_pid`, `connected_clients`, `upstream_alive`, etc.) that will show as `n/a` in fallback mode. This is by design per the PRD ("bounded local-only broker view"), but an inline comment or docstring note would clarify this intentional limitation to future readers.
_Suggestion:_ Add a brief comment to `_build_local_fallback_broker` noting that fields not derivable from local files are intentionally omitted.

**[Low] `service_name` uses raw string `"local-fallback"` as a status value**
`BrokerTUISnapshot.service_name` is re-used as both a display label and a status sentinel (`"local-fallback"`). This dual purpose is subtle; the render path uses it only for display and doesn't branch on it, so there is no practical bug, but it couples unrelated concerns.
_Suggestion:_ Acceptable as-is for now; could be separated in a future refactor.

**[Nit] `test_fetch_snapshot_surfaces_runtime_errors` previously only asserted `available is False`**
The test was strengthened as part of this task (now also asserts `broker is None` and `runtime_source == "dashboard-unavailable"`), which is good. The original assertion was undershooting — this is a positive improvement.

---

### Architectural Notes

- The fallback is correctly read-only: `can_stop=False` is always set when the dashboard is unavailable, and `request_stop()` is never called from the TUI loop in fallback mode. This preserves the invariant that broker control only flows through the authenticated Web UI API.
- `_read_local_pid` + `_read_local_version` are pure helpers with no side effects, making them easy to test and reuse if a future doctor integration needs them.
- The three-tier `runtime_source` taxonomy (`"dashboard-api"`, `"local-fallback"`, `"dashboard-unavailable"`) is clean and stable. The render label mapping in `_runtime_source_label` ensures the user never sees the raw internal token.
- `render_screen` condition changed from `if snapshot.available and broker` to `if broker` — this is the correct gate for fallback mode, since `available` is always `False` when the dashboard is down but local data exists. The old condition was silently suppressing useful fallback output.

---

### Tests

- 38 TUI tests pass (5 new tests covering fallback paths).
- New tests are well-isolated using `patch` for `_read_local_pid`, `_read_local_version`, and `tail_log_lines`.
- `test_run_loop_does_not_call_stop_without_live_control` is a particularly valuable regression guard.
- Coverage: `tui.py` 96.1%, total 91.75% — above the 90% threshold.
- Uncovered lines in `tui.py` (470, 509–510) relate to `_display_value` edge path and `_read_local_pid` PermissionError branch — acceptable misses given the mock-based test strategy.

---

### Next Steps

- No blockers. Low/nit findings do not warrant follow-up tasks.
- FOLLOW-UP: **skipped** — no actionable issues.
- Proceed to ARCHIVE-REVIEW → PR → CI-REVIEW.
22 changes: 11 additions & 11 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
# Next Task: P7-T4Add direct local-status fallback for TUI when dashboard API is unavailable
# Next Task: P7-T5Document the simplest supported broker UX and failure recovery flow

**Priority:** P1
**Phase:** Phase 7: Broker UX and Diagnostics
**Effort:** 4-5 hours
**Dependencies:** P6-T2
**Status:** Ready
**Effort:** TBD
**Dependencies:** P7-T1, P7-T2, P7-T3, P7-T4
**Status:** Pending

## Description

Reduce TUI dependence on the Web UI API by letting it fall back to local broker
state when the dashboard endpoint is unavailable. The TUI should still provide
useful diagnostics from PID/socket/version files and any directly accessible
broker status sources, while clearly indicating that live dashboard-backed
controls are unavailable.
After the orchestration and diagnostics improvements land, rewrite the user-facing docs around
the simplest supported broker UX. The docs should present one recommended command path first,
then one short failure-recovery path using the new diagnostic surfaces, instead of forcing users
to piece together behavior from multiple guides.

## Recently Archived

- `2026-03-07` — `P7-T4` archived with verdict `PASS`
- `2026-03-07` — `FU-P7-T3-2` archived with verdict `PASS`
- `2026-03-07` — `FU-P7-T3-1` archived with verdict `PASS`
- `2026-03-07` — `FU-P7-T1-1` 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 `P7-T5` PRD in `SPECS/INPROGRESS/`, then implement and validate the simplified
broker UX documentation.
8 changes: 4 additions & 4 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
- [x] Broker-owned listeners with degraded dashboard probes do not tell users to stop an "existing listener" or use restart guidance meant for foreign ownership
- [x] Regression tests cover both foreign-listener and broker-owned-listener mixed states in startup and doctor paths

#### ⬜️ P7-T4: Add direct local-status fallback for TUI when dashboard API is unavailable
#### P7-T4: Add direct local-status fallback for TUI when dashboard API is unavailable
- **Description:** Reduce TUI dependence on the Web UI API by letting it fall back to local broker state when the dashboard endpoint is unavailable. The TUI should still provide useful diagnostics from PID/socket/version files and any directly accessible broker status sources, while clearly indicating that live dashboard-backed controls are unavailable.
- **Priority:** P1
- **Dependencies:** P6-T2
Expand All @@ -568,9 +568,9 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
- any supporting runtime/status helpers needed to expose broker health without HTTP
- tests covering unavailable dashboard with live broker, dead broker, and degraded-control states
- **Acceptance Criteria:**
- [ ] TUI remains useful when the dashboard API is down and still shows the best available local broker diagnosis
- [ ] The screen clearly distinguishes live dashboard-backed runtime data from local fallback data
- [ ] Users can tell from TUI alone whether they need to restart the broker, free a port, or just attach a client
- [x] TUI remains useful when the dashboard API is down and still shows the best available local broker diagnosis
- [x] The screen clearly distinguishes live dashboard-backed runtime data from local fallback data
- [x] Users can tell from TUI alone whether they need to restart the broker, free a port, or just attach a client

#### ⬜️ P7-T5: Document the simplest supported broker UX and failure recovery flow
- **Description:** After the orchestration and diagnostics improvements land, rewrite the user-facing docs around the simplest supported broker UX. The docs should present one recommended command path first, then one short failure-recovery path using the new diagnostic surfaces, instead of forcing users to piece together behavior from multiple guides.
Expand Down
Loading