Skip to content

Commit b4e383e

Browse files
authored
Merge pull request #158 from SoundBlaster/codex/p7-t3-dashboard-port-conflicts
P7-T3: Auto-recover or guide on dashboard port ownership conflicts
2 parents a745228 + 8e84c28 commit b4e383e

12 files changed

Lines changed: 644 additions & 60 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ A Python wrapper (`xcodemcpwrapper`) that intercepts responses from `xcrun mcpbr
4040

4141
### Metrics
4242

43-
- **Test Coverage:** 92.19%
43+
- **Test Coverage:** 91.62%
4444
- **Performance:** <0.01ms overhead per transformation (0.0023ms avg)
4545
- **Memory:** <10MB footprint
4646
- **Lines of Code:** ~400 Python + 2000+ lines documentation

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<!-- version-badge:end -->
88
[![Python 3.9+](https://img.shields.io/badge/python-3.9+-blue.svg)](https://www.python.org/downloads/)
99
[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT)
10-
[![Coverage](https://img.shields.io/badge/coverage-90.91%25-brightgreen.svg)](./SPECS/ARCHIVE/P5-T14_Code_Coverage/)
10+
[![Coverage](https://img.shields.io/badge/coverage-91.62%25-brightgreen.svg)](./SPECS/ARCHIVE/P5-T14_Code_Coverage/)
1111
<!-- coverage-sync: keep README and DocC coverage metrics aligned -->
1212
[![MCP Registry](https://img.shields.io/badge/MCP%20Registry-io.github.SoundBlaster%2Fxcode--mcpbridge--wrapper-blue)](https://registry.modelcontextprotocol.io)
1313

@@ -664,7 +664,7 @@ make test && make lint && make typecheck
664664

665665
- **Overhead:** <0.01ms per transformation
666666
- **Memory:** <10MB footprint
667-
- **Coverage:** 90.91% test coverage
667+
- **Coverage:** 91.62% test coverage
668668

669669
## License
670670

SPECS/ARCHIVE/INDEX.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
# mcpbridge-wrapper Tasks Archive
22

3-
**Last Updated:** 2026-03-07 (P7-T2 review archived)
3+
**Last Updated:** 2026-03-07 (P7-T3 review archived)
44

55
## Archived Tasks
66

77
| Task ID | Folder | Archived | Verdict |
88
|---------|--------|----------|---------|
9+
| 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 |
910
| 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 |
1011
| 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 |
1112
| P6-T3 | [P6-T3_Document_the_explicit_dedicated-host_frontend_workflow/](P6-T3_Document_the_explicit_dedicated-host_frontend_workflow/) | 2026-03-07 | PASS |
@@ -198,6 +199,7 @@
198199

199200
| File | Description |
200201
|------|-------------|
202+
| [REVIEW_dashboard_port_ownership_conflicts.md](_Historical/REVIEW_dashboard_port_ownership_conflicts.md) | Review report for P7-T3 |
201203
| [REVIEW_broker_doctor_diagnostics.md](_Historical/REVIEW_broker_doctor_diagnostics.md) | Review report for P7-T2 |
202204
| [REVIEW_broker_console_startup.md](_Historical/REVIEW_broker_console_startup.md) | Review report for P7-T1 |
203205
| [REVIEW_dedicated_host_frontend_docs.md](_Historical/REVIEW_dedicated_host_frontend_docs.md) | Review report for P6-T3 |
@@ -609,3 +611,5 @@
609611
| 2026-03-07 | P6-T2 | Archived REVIEW_broker_terminal_frontend report |
610612
| 2026-03-07 | P7-T2 | Archived Implement_a_broker_doctor_command_for_cross-black-box_diagnostics (PASS) |
611613
| 2026-03-07 | P7-T2 | Archived REVIEW_broker_doctor_diagnostics report |
614+
| 2026-03-07 | P7-T3 | Archived Auto-recover_or_guide_on_dashboard_port_ownership_conflicts (PASS) |
615+
| 2026-03-07 | P7-T3 | Archived REVIEW_dashboard_port_ownership_conflicts report |
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
# P7-T3 — Auto-recover or guide on dashboard port ownership conflicts
2+
3+
## Objective Summary
4+
5+
`P7-T1` introduced the explicit dedicated-host workflow and `P7-T2` gave users
6+
one-command diagnostics, but the most confusing failure mode is still in the
7+
startup path itself: `--broker-daemon --web-ui` can leave a running broker
8+
behind while silently skipping the dashboard whenever the desired port is
9+
occupied. That produces a partial state where the recommended frontend is down,
10+
TUI cannot attach, and the user is forced to reason about multiple black boxes
11+
at once.
12+
13+
`P7-T3` should remove that ambiguity. When users request the broker-hosted
14+
dashboard, startup must either recover deterministically into a usable state or
15+
stop with one explicit remediation path. The implementation should align
16+
broker-daemon startup, broker-console orchestration, doctor messaging, and TUI
17+
feedback around the same model so the user is never told that a degraded
18+
dashboard-less host is “good enough”.
19+
20+
## Deliverables
21+
22+
- Tighten the broker-daemon startup path in
23+
`src/mcpbridge_wrapper/__main__.py` so an explicit `--web-ui` request does not
24+
silently degrade into “broker without dashboard”.
25+
- Add shared logic that classifies dashboard port conflicts into actionable
26+
buckets:
27+
- healthy broker-backed dashboard already serving the port
28+
- foreign or stale listener on the desired port
29+
- local broker already running without dashboard on the desired port
30+
- restart-eligible wrapper-owned listener when `--web-ui-restart` is used
31+
- Update user-facing messaging so the failing command prints one clear next
32+
action instead of continuing in a partial state.
33+
- Extend doctor/TUI-facing guidance where needed so conflict explanations and
34+
remediation text stay consistent with the startup behavior.
35+
- Add regression tests for broker-daemon, broker-console, and diagnostics
36+
scenarios that previously stranded users.
37+
38+
## Success Criteria
39+
40+
- `mcpbridge-wrapper --broker-daemon --web-ui` no longer leaves a healthy-seeming
41+
broker running without the requested dashboard when the port cannot be used.
42+
- Port conflicts resolve into one of two outcomes only:
43+
- the requested broker-backed dashboard becomes available, or
44+
- the command exits non-zero with an explicit remediation path
45+
- `--broker-console` and `--doctor` surface the same conflict class and
46+
recommended next action for the same runtime state.
47+
- Tests pin both safe recovery and fail-fast behavior so future changes do not
48+
reintroduce the hidden partial state.
49+
50+
## Test-First Plan
51+
52+
1. Add broker-daemon CLI tests that lock in the new fail-fast behavior when
53+
`--web-ui` is requested but the target port is occupied by a foreign
54+
listener, stale listener, or unusable existing runtime.
55+
2. Add orchestration tests for `--broker-console` that verify it reuses a
56+
healthy broker-backed dashboard, restarts only when explicitly permitted, and
57+
reports a single remediation path for foreign ownership conflicts.
58+
3. Add doctor classification/rendering tests for the updated conflict buckets so
59+
diagnostics remain aligned with the startup behavior.
60+
4. Only after the new expectations are pinned, implement the shared conflict
61+
classifier and wire it into the daemon startup/orchestration flow.
62+
5. Run required quality gates: `pytest`, `ruff check src/`, `mypy src/`, and
63+
`pytest --cov`.
64+
65+
## Execution Plan
66+
67+
### Phase 1: Define the startup contract
68+
69+
Inputs:
70+
- `src/mcpbridge_wrapper/__main__.py`
71+
- existing broker-console/dashboard probe helpers
72+
- `src/mcpbridge_wrapper/doctor.py`
73+
74+
Outputs:
75+
- a clear contract for what counts as “dashboard ready”
76+
- shared conflict categories for healthy reuse, recoverable ownership, and
77+
explicit remediation
78+
- deterministic stderr wording for broker-daemon and broker-console entrypoints
79+
80+
Verification:
81+
- every dashboard startup branch ends in either usable dashboard availability or
82+
non-zero failure
83+
- no code path continues with `--web-ui` requested after a port conflict unless
84+
the dashboard is actually reachable
85+
86+
### Phase 2: Implement shared conflict handling
87+
88+
Inputs:
89+
- dashboard port probes and listener ownership helpers
90+
- broker PID/socket/version state
91+
- existing `/api/control` and `/api/broker/status` probes
92+
93+
Outputs:
94+
- reusable conflict-resolution helper(s) for broker startup/orchestration
95+
- explicit reuse path for already-healthy broker-backed dashboards
96+
- fail-fast or restart-assisted path for foreign/stale ownership conflicts
97+
98+
Verification:
99+
- running broker + healthy dashboard remains a no-op/reuse case
100+
- foreign listeners and broker-without-dashboard states do not continue as
101+
“success”
102+
103+
### Phase 3: Align diagnostics and finalize validation
104+
105+
Inputs:
106+
- shared conflict results from startup/orchestration
107+
- doctor and TUI user guidance
108+
- unit and integration tests covering startup/diagnostic flows
109+
110+
Outputs:
111+
- aligned doctor guidance for port ownership conflicts
112+
- regression tests across broker-daemon, broker-console, and doctor
113+
- validation report with required quality-gate output
114+
115+
Verification:
116+
- the same runtime state yields the same diagnosis and remediation whether users
117+
encounter it during startup, in TUI, or via `--doctor`
118+
- no existing broker/web-ui/TUI entrypoints regress
119+
120+
## Acceptance Tests
121+
122+
- `pytest tests/unit/test_main.py`
123+
- `pytest tests/unit/test_main_tui.py`
124+
- `pytest tests/unit/test_doctor.py`
125+
- `pytest tests/unit/test_tui.py`
126+
- `pytest`
127+
- `ruff check src/`
128+
- `mypy src/`
129+
- `pytest --cov`
130+
131+
## Decision Points
132+
133+
- Prefer fail-fast over silent degradation for explicit `--web-ui` requests; a
134+
running broker without the requested dashboard is a broken UX state, not a
135+
successful startup.
136+
- Reuse the existing dashboard probe surfaces instead of inventing a second
137+
ownership model just for startup recovery.
138+
- Keep automatic recovery narrowly scoped to deterministic wrapper-owned cases;
139+
otherwise print one clear remediation path and exit.
140+
141+
## Notes
142+
143+
- If implementation exposes common conflict helpers that both `doctor.py` and
144+
`__main__.py` should consume, prefer that refactor now over duplicating
145+
ownership logic again.
146+
- The dedicated-host workflow introduced in `P7-T1` remains the product path;
147+
messaging should continue to steer users toward `--broker-console` or
148+
`--broker-daemon --web-ui`, not ad-hoc manual recovery.
149+
- Review subject name for this task: `dashboard_port_ownership_conflicts`.
150+
151+
---
152+
**Archived:** 2026-03-07
153+
**Verdict:** PASS
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# P7-T3 Validation Report
2+
3+
**Task:** P7-T3 — Auto-recover or guide on dashboard port ownership conflicts
4+
**Date:** 2026-03-07
5+
**Verdict:** PASS
6+
7+
## Summary
8+
9+
Implemented the broker-hosted dashboard conflict fix by:
10+
11+
- removing the silent broker-daemon partial state where `--broker-daemon --web-ui`
12+
could continue without the requested dashboard
13+
- adding shared user-facing remediation helpers in
14+
`src/mcpbridge_wrapper/__main__.py` so startup now resolves occupied-port
15+
states into explicit attach, reset, or restart-assisted guidance
16+
- aligning `--broker-console` conflict messaging with the dedicated-host
17+
workflow already surfaced by `--doctor`
18+
- preserving the safe recovery path via
19+
`mcpbridge-wrapper --broker-console --web-ui-restart`
20+
- extending unit coverage for foreign listeners, running brokers without a
21+
dashboard, already-healthy broker-backed endpoints, and restart-assisted
22+
recovery
23+
- refreshing published coverage references in `README.md`,
24+
`Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md`, and
25+
`AGENTS.md` to the validated current total
26+
27+
## Files Validated
28+
29+
- `src/mcpbridge_wrapper/__main__.py`
30+
- `tests/unit/test_main.py`
31+
- `tests/unit/test_doctor.py`
32+
- `README.md`
33+
- `Sources/XcodeMCPWrapper/Documentation.docc/XcodeMCPWrapper.md`
34+
- `AGENTS.md`
35+
36+
## Targeted Verification
37+
38+
```bash
39+
pytest tests/unit/test_main.py tests/unit/test_main_tui.py tests/unit/test_doctor.py tests/unit/test_tui.py
40+
```
41+
42+
- Result: `163 passed`
43+
44+
```bash
45+
pytest tests/unit/test_main.py -k 'broker_console or broker_daemon_webui_port_occupied or broker_lacks_dashboard'
46+
```
47+
48+
- Result: `18 passed`
49+
50+
```bash
51+
python scripts/check_doc_sync.py --all --require-same-commit
52+
```
53+
54+
- Result: `PASS`
55+
- Observed outcome: README and DocC coverage references remained in sync after
56+
updating the validated coverage metric
57+
58+
## Required Quality Gates
59+
60+
```bash
61+
pytest
62+
```
63+
64+
- Result: `887 passed, 5 skipped in 7.98s`
65+
66+
```bash
67+
python -m ruff check src/ tests/
68+
```
69+
70+
- Result: `All checks passed!`
71+
72+
```bash
73+
mypy src/
74+
```
75+
76+
- Result: `Success: no issues found in 20 source files`
77+
78+
```bash
79+
make format-check
80+
```
81+
82+
- Result: `55 files already formatted`
83+
84+
```bash
85+
pytest --cov=src --cov-report=term
86+
```
87+
88+
- Result: `887 passed, 5 skipped in 8.97s`
89+
- Coverage: `91.62%`
90+
91+
## Notes
92+
93+
- The new startup contract is intentionally fail-fast for explicit `--web-ui`
94+
requests. A running broker without the requested frontend is now treated as a
95+
broken state, not a successful launch.
96+
- Doctor messaging already matched the intended dedicated-host recovery path, so
97+
this task focused on aligning startup/orchestration behavior to that model
98+
instead of inventing a separate recovery surface.
99+
- Remaining warnings are the pre-existing `websockets` / `uvicorn`
100+
deprecations already visible in the repository test suite.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
## REVIEW REPORT — dashboard_port_ownership_conflicts
2+
3+
**Scope:** origin/main..HEAD
4+
**Files:** 11
5+
6+
### Summary Verdict
7+
- [ ] Approve
8+
- [x] Approve with comments
9+
- [ ] Request changes
10+
- [ ] Block
11+
12+
### Critical Issues
13+
14+
None.
15+
16+
### Secondary Issues
17+
18+
- [Medium] `src/mcpbridge_wrapper/__main__.py:604-640`,
19+
`src/mcpbridge_wrapper/__main__.py:1072-1080`:
20+
`_report_requested_dashboard_unavailable()` prioritizes
21+
`running_broker_pid` over `listener_pids`. In the mixed state where a broker
22+
PID is still live but the requested dashboard port is actually occupied by an
23+
unrelated listener, both `--broker-console` and `--broker-daemon --web-ui`
24+
will tell users to reset the broker first and omit the foreign port owner.
25+
That remediation can loop back into the same conflict because the real
26+
blocker, the non-broker listener on the port, was never surfaced. The
27+
conflict classifier should prefer observable foreign port ownership or name
28+
both blockers in one explicit recovery path.
29+
30+
### Architectural Notes
31+
32+
- The task correctly removes the most confusing partial state: explicit
33+
dashboard startup no longer silently degrades into “broker alive, dashboard
34+
absent”.
35+
- Keeping `--broker-console --web-ui-restart` as the safe recovery path is the
36+
right product choice; the remaining issue is about mixed-state prioritization,
37+
not the core startup contract.
38+
- Coverage-reference updates were kept in lockstep across README and the DocC
39+
mirror, which avoids a repeat of the earlier documentation drift problem.
40+
41+
### Tests
42+
43+
- `pytest tests/unit/test_main.py tests/unit/test_main_tui.py tests/unit/test_doctor.py tests/unit/test_tui.py` passed (`163 passed`)
44+
- `pytest` passed (`887 passed, 5 skipped`)
45+
- `python -m ruff check src/ tests/` passed
46+
- `mypy src/` passed
47+
- `make format-check` passed
48+
- `python scripts/check_doc_sync.py --all --require-same-commit` passed
49+
- `pytest --cov=src --cov-report=term` passed with `91.62%` coverage
50+
51+
### Next Steps
52+
53+
- Add a focused follow-up task to improve mixed-state conflict prioritization
54+
when a live broker PID and a foreign dashboard-port listener are both
55+
observable.

0 commit comments

Comments
 (0)