Skip to content

Commit fbf13f3

Browse files
SoundBlasterSoundBlaster
authored
Merge pull request #160 from SoundBlaster/codex/fu-p7-t3-1-foreign-port-owner-guidance
FU-P7-T3-1: Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
2 parents 43e016f + d2e7e0c commit fbf13f3

10 files changed

Lines changed: 486 additions & 33 deletions

File tree

‎SPECS/ARCHIVE/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.md‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
2+
3+
## Objective Summary
4+
5+
`P7-T3` removed the silent “broker alive, dashboard absent” partial state, but
6+
its review found one remaining mixed-state failure mode: when a broker PID is
7+
still live and the requested dashboard port is simultaneously occupied by a
8+
foreign listener, startup guidance prioritizes the broker reset path and can
9+
hide the actual port owner. That can send users into a reset loop while the
10+
real blocker, the unrelated listener on the port, remains untouched.
11+
12+
This follow-up should make mixed-state diagnostics explicit across both startup
13+
and `--doctor`. The goal is not to invent a new recovery workflow. Instead,
14+
the code should surface the foreign port owner first or name both blockers in
15+
one clear remediation path, while preserving the existing single-listener and
16+
broker-without-dashboard behavior for non-mixed states.
17+
18+
## Deliverables
19+
20+
- Update `src/mcpbridge_wrapper/__main__.py` so mixed-state startup failures
21+
can report both a live broker PID and a foreign dashboard-port listener
22+
instead of hiding the listener behind broker-reset guidance.
23+
- Update `src/mcpbridge_wrapper/doctor.py` so mixed broker/listener conflicts
24+
classify as a port-ownership issue rather than the generic
25+
`broker-without-dashboard` diagnosis.
26+
- Add regression coverage in `tests/unit/test_main.py` and
27+
`tests/unit/test_doctor.py` for the mixed-state conflict path.
28+
- Produce `SPECS/INPROGRESS/FU-P7-T3-1_Validation_Report.md` with required
29+
quality-gate evidence.
30+
31+
## Success Criteria
32+
33+
- `--broker-console` and `--broker-daemon --web-ui` mention the foreign
34+
dashboard-port listener or both blockers when a live broker PID and foreign
35+
listener coexist.
36+
- `--doctor` no longer hides the foreign listener behind a generic
37+
broker-without-dashboard diagnosis in the same mixed state.
38+
- Regression tests pin the mixed-state ordering so future changes cannot revert
39+
to broker-only guidance.
40+
41+
## Test-First Plan
42+
43+
1. Add startup tests that model the mixed state for both `_run_broker_console()`
44+
and `main()`’s `--broker-daemon --web-ui` path, asserting the foreign port
45+
owner is surfaced alongside the live broker PID.
46+
2. Add a doctor-classification test for a live local broker plus foreign
47+
listener on the configured dashboard port, asserting the report stays in the
48+
port-occupied family instead of `broker-without-dashboard`.
49+
3. Implement the smallest production changes needed to collect both blockers
50+
before choosing the user-facing guidance path.
51+
4. Run the required quality gates: `pytest`, `ruff check src/`, `mypy src/`,
52+
and `pytest --cov`.
53+
54+
## Execution Plan
55+
56+
### Phase 1: Pin the mixed-state startup contract
57+
58+
Inputs:
59+
- `src/mcpbridge_wrapper/__main__.py`
60+
- existing `P7-T3` startup tests in `tests/unit/test_main.py`
61+
62+
Outputs:
63+
- regression tests for mixed-state broker PID + foreign listener conflicts
64+
- a precise expected stderr contract for broker-console and broker-daemon flows
65+
66+
Verification:
67+
- the new startup tests fail on the current ordering that hides the foreign
68+
listener
69+
70+
### Phase 2: Align startup guidance
71+
72+
Inputs:
73+
- `_run_broker_console()`
74+
- `_report_requested_dashboard_unavailable()`
75+
- broker-daemon web-ui startup branch in `main()`
76+
77+
Outputs:
78+
- startup logic that can see both `running_broker_pid` and `listener_pids`
79+
- one explicit remediation path that surfaces the foreign listener without
80+
dropping broker-state context
81+
82+
Verification:
83+
- startup messaging for single-blocker states remains unchanged
84+
- mixed-state messaging mentions the foreign listener or both blockers
85+
86+
### Phase 3: Align doctor classification and validate
87+
88+
Inputs:
89+
- `src/mcpbridge_wrapper/doctor.py`
90+
- doctor classification tests
91+
- full repo quality gates
92+
93+
Outputs:
94+
- mixed-state doctor report that stays actionable and consistent with startup
95+
- validation report with targeted and full quality-gate evidence
96+
97+
Verification:
98+
- `--doctor` and startup both direct the user toward resolving the foreign port
99+
conflict before or alongside broker reset
100+
- coverage remains at or above the repository threshold
101+
102+
## Acceptance Tests
103+
104+
- `pytest tests/unit/test_main.py`
105+
- `pytest tests/unit/test_doctor.py`
106+
- `pytest`
107+
- `ruff check src/`
108+
- `mypy src/`
109+
- `pytest --cov`
110+
111+
## Decision Points
112+
113+
- Prefer a combined-message approach when both blockers are real; users should
114+
not lose visibility into the live broker PID just because the foreign
115+
listener is now prioritized.
116+
- Keep the public recovery commands aligned with `P7-T3` and `P7-T2` rather
117+
than inventing a new remediation surface for this follow-up.
118+
119+
## Notes
120+
121+
- No documentation changes are expected unless the implementation forces
122+
observable command/help text changes beyond stderr diagnostics.
123+
- Review subject name for this task: `mixed_dashboard_conflict_guidance`.
124+
125+
---
126+
**Archived:** 2026-03-07
127+
**Verdict:** PASS

‎SPECS/ARCHIVE/FU-P7-T3-1_Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts/FU-P7-T3-1_Validation_Report.md‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# FU-P7-T3-1 Validation Report
2+
3+
**Task:** FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
4+
**Date:** 2026-03-07
5+
**Verdict:** PASS
6+
7+
## Summary
8+
9+
Implemented the `FU-P7-T3-1` follow-up by:
10+
11+
- updating startup guidance in `src/mcpbridge_wrapper/__main__.py` so mixed
12+
broker/dashboard conflicts can surface both a live broker PID and a foreign
13+
listener on the requested dashboard port
14+
- fixing `_run_broker_console()` so it inspects the configured dashboard port
15+
before returning early on a running broker PID, allowing mixed-state
16+
reporting instead of broker-only guidance
17+
- aligning `src/mcpbridge_wrapper/doctor.py` so mixed broker-plus-listener
18+
states classify as a port-ownership issue rather than being hidden behind the
19+
generic `broker-without-dashboard` diagnosis
20+
- adding regression coverage for broker-console, broker-daemon `--web-ui`, and
21+
doctor mixed-state flows
22+
23+
## Files Validated
24+
25+
- `src/mcpbridge_wrapper/__main__.py`
26+
- `src/mcpbridge_wrapper/doctor.py`
27+
- `tests/unit/test_main.py`
28+
- `tests/unit/test_doctor.py`
29+
30+
## Targeted Verification
31+
32+
```bash
33+
pytest tests/unit/test_main.py tests/unit/test_doctor.py -k 'mixed_state_mentions_foreign_listener or mixed_broker_and_foreign_listener_prefers_port_conflict'
34+
```
35+
36+
- Result: `3 passed`
37+
- Observed outcome: startup and doctor now keep the foreign port owner visible
38+
in mixed broker/listener conflicts
39+
40+
```bash
41+
pytest tests/unit/test_main.py tests/unit/test_doctor.py
42+
```
43+
44+
- Result: `119 passed`
45+
46+
## Required Quality Gates
47+
48+
```bash
49+
pytest
50+
```
51+
52+
- Result: `891 passed, 5 skipped in 8.14s`
53+
54+
```bash
55+
ruff check src/
56+
```
57+
58+
- Result: `All checks passed!`
59+
60+
```bash
61+
mypy src/
62+
```
63+
64+
- Result: `Success: no issues found in 20 source files`
65+
66+
```bash
67+
pytest --cov
68+
```
69+
70+
- Result: `891 passed, 5 skipped in 9.16s`
71+
- Coverage: `91.64%`
72+
73+
## Acceptance Criteria Evidence
74+
75+
- [x] `--broker-console` and `--broker-daemon --web-ui` surface the foreign
76+
dashboard-port owner or both blockers when a live broker PID and foreign
77+
listener coexist.
78+
- Evidence: `tests/unit/test_main.py::TestBrokerConsoleHelpers::test_run_broker_console_mixed_state_mentions_foreign_listener`
79+
and
80+
`tests/unit/test_main.py::TestMainBrokerWebUIFlowCoverage::test_main_broker_daemon_webui_mixed_state_mentions_foreign_listener`
81+
both passed.
82+
- [x] `--doctor` does not hide the foreign listener behind a generic
83+
broker-without-dashboard diagnosis in the same mixed state.
84+
- Evidence:
85+
`tests/unit/test_doctor.py::TestClassifyDoctorReport::test_classify_mixed_broker_and_foreign_listener_prefers_port_conflict`
86+
passed with `report.code == "port-occupied"`.
87+
- [x] Regression tests cover the mixed-state conflict and prevent reordering
88+
back to broker-only guidance.
89+
- Evidence: new mixed-state tests were added in `tests/unit/test_main.py` and
90+
`tests/unit/test_doctor.py`, and the targeted plus full affected-module
91+
suites passed.
92+
93+
## Notes
94+
95+
- The mixed-state fix keeps existing single-blocker behavior unchanged: plain
96+
broker-without-dashboard and plain foreign-listener paths still use their
97+
original guidance.
98+
- Existing `websockets` / `uvicorn` deprecation warnings remain in the suite
99+
and are unrelated to this task.

‎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 (FU-P7-T1-1 review archived)
3+
**Last Updated:** 2026-03-07
44

55
## Archived Tasks
66

77
| Task ID | Folder | Archived | Verdict |
88
|---------|--------|----------|---------|
9+
| 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 |
910
| 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 |
1011
| 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 |
1112
| 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 |
@@ -200,6 +201,7 @@
200201

201202
| File | Description |
202203
|------|-------------|
204+
| [REVIEW_mixed_dashboard_conflict_guidance.md](_Historical/REVIEW_mixed_dashboard_conflict_guidance.md) | Review report for FU-P7-T3-1 |
203205
| [REVIEW_broker_console_keyboardinterrupt_reuse.md](_Historical/REVIEW_broker_console_keyboardinterrupt_reuse.md) | Review report for FU-P7-T1-1 |
204206
| [REVIEW_dashboard_port_ownership_conflicts.md](_Historical/REVIEW_dashboard_port_ownership_conflicts.md) | Review report for P7-T3 |
205207
| [REVIEW_broker_doctor_diagnostics.md](_Historical/REVIEW_broker_doctor_diagnostics.md) | Review report for P7-T2 |
@@ -343,6 +345,8 @@
343345

344346
| Date | Task ID | Action |
345347
|------|---------|--------|
348+
| 2026-03-07 | FU-P7-T3-1 | Archived REVIEW_mixed_dashboard_conflict_guidance report |
349+
| 2026-03-07 | FU-P7-T3-1 | Archived Prioritize_foreign_port-owner_guidance_in_mixed_broker-dashboard_conflicts (PASS) |
346350
| 2026-03-07 | FU-P7-T1-1 | Archived REVIEW_broker_console_keyboardinterrupt_reuse report |
347351
| 2026-03-07 | FU-P7-T1-1 | Archived Normalize_KeyboardInterrupt_handling_when_broker-console_reuses_an_existing_host (PASS) |
348352
| 2026-03-07 | P7-T1 | Archived REVIEW_broker_console_startup report |

‎SPECS/ARCHIVE/_Historical/REVIEW_mixed_dashboard_conflict_guidance.md‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
## REVIEW REPORT — mixed_dashboard_conflict_guidance
2+
3+
**Scope:** origin/main..HEAD
4+
**Files:** 9
5+
6+
### Summary Verdict
7+
- [ ] Approve
8+
- [x] Approve with comments
9+
- [ ] Request changes
10+
- [ ] Block
11+
12+
### Critical Issues
13+
14+
- [High] `src/mcpbridge_wrapper/__main__.py:613-621`,
15+
`src/mcpbridge_wrapper/__main__.py:761-779`,
16+
`src/mcpbridge_wrapper/doctor.py:437-465`:
17+
the new mixed-state branch treats any listener on the configured dashboard
18+
port as a foreign port conflict whenever a broker PID is also live. That
19+
includes the broker daemon’s own listener if it is still bound to the port
20+
while `/api/control` or `/api/broker/status` is degraded. In that state, the
21+
new guidance can incorrectly tell users to stop the “existing listener” or
22+
use `--web-ui-restart`, even though the listener may be the same broker
23+
process named by the PID file. The fix should distinguish foreign listener
24+
PIDs from the broker’s own PID before taking the mixed-state port-conflict
25+
path.
26+
27+
### Secondary Issues
28+
29+
- [Medium] `tests/unit/test_main.py:1788-1824`,
30+
`tests/unit/test_main.py:2276-2329`,
31+
`tests/unit/test_doctor.py:313-326`:
32+
the new regression tests only cover the foreign-listener case. There is no
33+
test where `listener_pids` equals the broker PID (or contains the local
34+
doctor PID), so the self-listener misclassification above would currently
35+
pass unnoticed. Add same-PID coverage alongside the existing foreign-listener
36+
assertions.
37+
38+
### Architectural Notes
39+
40+
- The task correctly fixed the user-visible blind spot that motivated
41+
`FU-P7-T3-1`: foreign listeners are now surfaced in the mixed-state path
42+
instead of being hidden behind broker-reset guidance.
43+
- The remaining issue is about PID ownership precision, not about whether mixed
44+
state should be surfaced at all. The next iteration should refine the
45+
classifier to separate foreign listeners from broker-owned listeners.
46+
47+
### Tests
48+
49+
- `pytest tests/unit/test_main.py tests/unit/test_doctor.py -k 'mixed_state_mentions_foreign_listener or mixed_broker_and_foreign_listener_prefers_port_conflict'` passed (`3 passed`)
50+
- `pytest tests/unit/test_main.py tests/unit/test_doctor.py` passed (`119 passed`)
51+
- `pytest` passed (`891 passed, 5 skipped`)
52+
- `ruff check src/` passed
53+
- `mypy src/` passed
54+
- `pytest --cov` passed with `91.64%` coverage
55+
56+
### Next Steps
57+
58+
- Add a focused follow-up task to distinguish foreign listener PIDs from the
59+
broker’s own PID in startup and doctor mixed-state guidance.
60+
- Add regression coverage for the broker-owned-listener case so the mixed-state
61+
fix does not regress into self-conflict messaging.

‎SPECS/INPROGRESS/next.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
1-
# Next Task: FU-P7-T3-1 — Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
1+
# Next Task: FU-P7-T3-2 — Exclude broker-owned dashboard listeners from foreign port-conflict guidance
22

33
**Priority:** P1
44
**Phase:** Phase 7: Broker UX and Diagnostics
55
**Effort:** 2-3 hours
6-
**Dependencies:** P7-T3
6+
**Dependencies:** FU-P7-T3-1
77
**Status:** Ready
88

99
## Description
1010

11-
When startup sees both a live broker PID and a non-broker listener on the
12-
requested dashboard port, current remediation prioritizes broker reset guidance
13-
and can hide the actual foreign port owner. Update startup and diagnostics
14-
conflict ordering so users see the real blocker or one combined recovery path
15-
instead of being sent into a reset loop.
11+
Refine the mixed broker/dashboard conflict classifier so it distinguishes the
12+
broker daemon's own dashboard listener from a foreign process on the same port.
13+
When degraded probes occur against a broker-owned listener, startup and
14+
diagnostics should keep users on broker-health guidance instead of reporting a
15+
foreign port owner.
1616

1717
## Recently Archived
1818

19+
- `2026-03-07` — `FU-P7-T3-1` archived with verdict `PASS`
1920
- `2026-03-07` — `FU-P7-T1-1` archived with verdict `PASS`
2021
- `2026-03-07` — `P7-T3` archived with verdict `PASS`
21-
- `2026-03-07` — `P7-T2` archived with verdict `PASS`
2222

2323
## Next Step
2424

25-
Create the `FU-P7-T3-1` PRD in `SPECS/INPROGRESS/`, then implement and
26-
validate the mixed-state dashboard conflict guidance updates.
25+
Create the `FU-P7-T3-2` PRD in `SPECS/INPROGRESS/`, then implement and validate
26+
the broker-owned-listener exclusion in startup and doctor mixed-state guidance.

‎SPECS/Workplan.md‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,8 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
528528
- [x] Port conflicts result in either automatic safe recovery or one explicit remediation path with exact commands or next steps
529529
- [x] TUI and diagnostics clearly explain the conflict source and whether the current runtime is usable
530530

531-
#### ⬜️ FU-P7-T3-1: Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
531+
#### ✅ FU-P7-T3-1: Prioritize foreign port-owner guidance in mixed broker/dashboard conflicts
532+
- **Status:** ✅ Completed (2026-03-07)
532533
- **Description:** 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.
533534
- **Priority:** P1
534535
- **Dependencies:** P7-T3
@@ -538,9 +539,23 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
538539
- startup and diagnostics tests covering live broker PID plus foreign dashboard-port listener
539540
- any wording changes needed so recovery stays explicit and single-path
540541
- **Acceptance Criteria:**
541-
- [ ] `--broker-console` and `--broker-daemon --web-ui` surface the foreign dashboard-port owner or both blockers when a live broker PID and foreign listener coexist
542-
- [ ] `--doctor` does not hide the foreign listener behind a generic broker-without-dashboard diagnosis in the same mixed state
543-
- [ ] Regression tests cover the mixed-state conflict and prevent reordering back to broker-only guidance
542+
- [x] `--broker-console` and `--broker-daemon --web-ui` surface the foreign dashboard-port owner or both blockers when a live broker PID and foreign listener coexist
543+
- [x] `--doctor` does not hide the foreign listener behind a generic broker-without-dashboard diagnosis in the same mixed state
544+
- [x] Regression tests cover the mixed-state conflict and prevent reordering back to broker-only guidance
545+
546+
#### ⬜️ FU-P7-T3-2: Exclude broker-owned dashboard listeners from foreign port-conflict guidance
547+
- **Description:** Refine the mixed broker/dashboard conflict classifier so it distinguishes the broker daemon's own dashboard listener from a foreign process on the same port. When degraded probes occur against a broker-owned listener, startup and diagnostics should keep users on broker-health guidance instead of reporting a foreign port owner.
548+
- **Priority:** P1
549+
- **Dependencies:** FU-P7-T3-1
550+
- **Parallelizable:** yes
551+
- **Outputs/Artifacts:**
552+
- `src/mcpbridge_wrapper/__main__.py` mixed-state startup guidance that filters broker-owned listener PIDs out of foreign-conflict messaging
553+
- `src/mcpbridge_wrapper/doctor.py` mixed-state diagnostic guidance with the same broker-owned listener exclusion
554+
- `tests/unit/test_main.py` and `tests/unit/test_doctor.py` regression coverage for foreign-listener and broker-owned-listener mixed states
555+
- **Acceptance Criteria:**
556+
- [ ] Mixed-state foreign-port guidance triggers only when the dashboard listener PID differs from the running broker PID
557+
- [ ] Broker-owned listeners with degraded dashboard probes do not tell users to stop an "existing listener" or use restart guidance meant for foreign ownership
558+
- [ ] Regression tests cover both foreign-listener and broker-owned-listener mixed states in startup and doctor paths
544559

545560
#### ⬜️ P7-T4: Add direct local-status fallback for TUI when dashboard API is unavailable
546561
- **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.

0 commit comments

Comments
 (0)