Skip to content

Commit 492a49f

Browse files
authored
Merge pull request #70 from SoundBlaster/codex/feature/FU-P13-T4-2-reconnect-parameter
FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy
2 parents b1fc406 + 2c33255 commit 492a49f

9 files changed

Lines changed: 209 additions & 14 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# PRD: FU-P13-T4-2 — Implement or remove reconnect parameter in BrokerProxy
2+
3+
**Created:** 2026-02-18
4+
**Priority:** P2
5+
**Branch:** `codex/feature/FU-P13-T4-2-reconnect-parameter`
6+
**Status:** PLAN
7+
8+
---
9+
10+
## 1. Problem Statement
11+
12+
`BrokerProxy` accepts a `reconnect` parameter, stores it, and documents reconnect behavior, but `_run_bridge` never reads this setting. This leaves dead configuration in the API and ambiguous expectations for callers.
13+
14+
---
15+
16+
## 2. Scope
17+
18+
### In Scope
19+
- Remove the unused `reconnect` parameter from `BrokerProxy.__init__` and internal state.
20+
- Remove reconnect claims from proxy module docs/docstrings that no longer match behavior.
21+
- Update call sites and tests that pass `reconnect=`.
22+
- Add/adjust tests to ensure no dead reconnect API remains.
23+
24+
### Out of Scope
25+
- Adding reconnect loop behavior to proxy I/O lifecycle.
26+
- Broker daemon reconnect logic (`broker/daemon.py`).
27+
- New CLI flags for reconnect tuning.
28+
29+
---
30+
31+
## 3. Deliverables
32+
33+
1. `src/mcpbridge_wrapper/broker/proxy.py`
34+
- Remove `reconnect` constructor argument and `_reconnect` field.
35+
- Update docstrings to reflect current behavior (no proxy-level reconnect retry).
36+
37+
2. `src/mcpbridge_wrapper/__main__.py`
38+
- Remove obsolete `reconnect=False` argument at proxy construction.
39+
40+
3. `tests/unit/test_broker_proxy.py`
41+
- Keep coverage around proxy forwarding/connect/exit behavior and ensure API remains clean.
42+
43+
4. `SPECS/INPROGRESS/FU-P13-T4-2_Validation_Report.md`
44+
- Record quality-gate results and acceptance evidence.
45+
46+
---
47+
48+
## 4. Acceptance Criteria
49+
50+
- [ ] `BrokerProxy.__init__` no longer exposes an unused `reconnect` parameter.
51+
- [ ] No dead reconnect state remains in proxy implementation.
52+
- [ ] Relevant unit tests pass with updated API.
53+
- [ ] Full quality gates pass:
54+
- `pytest`
55+
- `ruff check src/`
56+
- `mypy src/`
57+
- `pytest --cov` (coverage >= 90%)
58+
59+
---
60+
61+
## 5. Dependencies
62+
63+
- P13-T4 ✅
64+
65+
---
66+
67+
## 6. Risks and Mitigations
68+
69+
- **Risk:** Existing callers may still pass `reconnect=` and break at runtime.
70+
- **Mitigation:** Update known call site (`__main__.py`) and run full test suite to catch hidden usage.
71+
72+
---
73+
74+
## 7. Validation Plan
75+
76+
1. Search repository for `reconnect=` uses in proxy construction and remove obsolete usage.
77+
2. Run `pytest tests/unit/test_broker_proxy.py`.
78+
3. Run required quality gates and record outputs in validation report.
79+
80+
81+
---
82+
**Archived:** 2026-02-18
83+
**Verdict:** PASS
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Validation Report: FU-P13-T4-2
2+
3+
**Task:** FU-P13-T4-2 — Implement or remove reconnect parameter in BrokerProxy
4+
**Date:** 2026-02-18
5+
**Branch:** `codex/feature/FU-P13-T4-2-reconnect-parameter`
6+
7+
## Scope validated
8+
9+
- Removed the unused `reconnect` parameter from `BrokerProxy.__init__` and deleted dead `_reconnect` state.
10+
- Updated proxy construction in `src/mcpbridge_wrapper/__main__.py` to remove obsolete argument.
11+
- Updated proxy tests and added constructor-signature assertion to prevent reconnect API regression.
12+
13+
## Quality gates
14+
15+
### 1) Targeted task test
16+
17+
Command:
18+
19+
```bash
20+
pytest tests/unit/test_broker_proxy.py
21+
```
22+
23+
Result: **PASS** (`16 passed`)
24+
25+
### 2) Full test suite
26+
27+
Command:
28+
29+
```bash
30+
pytest
31+
```
32+
33+
Result: **PASS** (`578 passed, 5 skipped`)
34+
35+
### 3) Lint
36+
37+
Command:
38+
39+
```bash
40+
ruff check src/
41+
```
42+
43+
Result: **PASS** (`All checks passed!`)
44+
45+
### 4) Type checks
46+
47+
Command:
48+
49+
```bash
50+
mypy src/
51+
```
52+
53+
Result: **PASS** (`Success: no issues found in 18 source files`)
54+
55+
### 5) Coverage
56+
57+
Command:
58+
59+
```bash
60+
pytest --cov
61+
```
62+
63+
Result: **PASS** (`Total coverage: 92.31%`, threshold: `>= 90%`)
64+
65+
## Acceptance criteria evidence
66+
67+
- [x] `BrokerProxy.__init__` no longer exposes an unused `reconnect` parameter.
68+
- Evidence: `tests/unit/test_broker_proxy.py::TestBrokerProxyConnectTimeout::test_constructor_has_no_reconnect_parameter` passed.
69+
- [x] No dead reconnect state remains in proxy implementation.
70+
- Evidence: `rg -n "reconnect|_reconnect" src/mcpbridge_wrapper/broker/proxy.py src/mcpbridge_wrapper/__main__.py` returned no reconnect parameter/state references.
71+
- [x] Relevant unit tests pass with updated API.
72+
- Evidence: `pytest tests/unit/test_broker_proxy.py` -> `16 passed`.
73+
- [x] Full quality gates pass.
74+
- Evidence: `pytest`, `ruff check src/`, `mypy src/`, and `pytest --cov` all passed; coverage `92.31%`.
75+
76+
## Notes
77+
78+
- Existing `websockets` deprecation warnings in Web UI tests remain unchanged and unrelated to this task.

SPECS/ARCHIVE/INDEX.md

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

3-
**Last Updated:** 2026-02-18 (FU-P13-T4-1)
3+
**Last Updated:** 2026-02-18 (FU-P13-T4-2)
44

55
## Archived Tasks
66

@@ -111,6 +111,7 @@
111111
| P13-T5 | [P13-T5_Validate_prompt_reduction_and_multi_client_stability/](P13-T5_Validate_prompt_reduction_and_multi_client_stability/) | 2026-02-18 | PARTIAL |
112112
| P13-T6 | [P13-T6_Document_broker_mode_configuration_migration_and_rollback/](P13-T6_Document_broker_mode_configuration_migration_and_rollback/) | 2026-02-18 | PASS |
113113
| FU-P13-T4-1 | [FU-P13-T4-1_Fix_asyncio_get_event_loop_deprecation_in_BrokerProxy/](FU-P13-T4-1_Fix_asyncio_get_event_loop_deprecation_in_BrokerProxy/) | 2026-02-18 | PASS |
114+
| FU-P13-T4-2 | [FU-P13-T4-2_Implement_or_remove_reconnect_parameter_in_BrokerProxy/](FU-P13-T4-2_Implement_or_remove_reconnect_parameter_in_BrokerProxy/) | 2026-02-18 | PASS |
114115

115116
## Historical Artifacts
116117

@@ -185,6 +186,7 @@
185186
| [REVIEW_P13-T5_prompt_reduction_multi_client_stability.md](_Historical/REVIEW_P13-T5_prompt_reduction_multi_client_stability.md) | Review report for P13-T5 |
186187
| [REVIEW_P13-T6_broker_mode_configuration_migration.md](_Historical/REVIEW_P13-T6_broker_mode_configuration_migration.md) | Review report for P13-T6 |
187188
| [REVIEW_FU-P13-T4-1_broker_proxy_loop.md](_Historical/REVIEW_FU-P13-T4-1_broker_proxy_loop.md) | Review report for FU-P13-T4-1 |
189+
| [REVIEW_FU-P13-T4-2_broker_proxy_reconnect.md](_Historical/REVIEW_FU-P13-T4-2_broker_proxy_reconnect.md) | Review report for FU-P13-T4-2 |
188190

189191
## Archive Log
190192

@@ -324,3 +326,5 @@
324326
| 2026-02-18 | P13-T6 | Archived REVIEW_P13-T6_broker_mode_configuration_migration report |
325327
| 2026-02-18 | FU-P13-T4-1 | Archived Fix_asyncio_get_event_loop_deprecation_in_BrokerProxy (PASS) |
326328
| 2026-02-18 | FU-P13-T4-1 | Archived REVIEW_FU-P13-T4-1_broker_proxy_loop report |
329+
| 2026-02-18 | FU-P13-T4-2 | Archived Implement_or_remove_reconnect_parameter_in_BrokerProxy (PASS) |
330+
| 2026-02-18 | FU-P13-T4-2 | Archived REVIEW_FU-P13-T4-2_broker_proxy_reconnect report |
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
## REVIEW REPORT — FU-P13-T4-2 broker proxy reconnect
2+
3+
**Scope:** origin/main..HEAD
4+
**Files:** 8
5+
6+
### Summary Verdict
7+
- [x] Approve
8+
- [ ] Approve with comments
9+
- [ ] Request changes
10+
- [ ] Block
11+
12+
### Critical Issues
13+
- None.
14+
15+
### Secondary Issues
16+
- None.
17+
18+
### Architectural Notes
19+
- Removing the unused `reconnect` constructor argument reduces API surface area and aligns implementation, docs, and call sites.
20+
- The constructor-signature test in `tests/unit/test_broker_proxy.py` guards against accidental reintroduction of dead parameters.
21+
22+
### Tests
23+
- Required quality gates were run during EXECUTE and all passed:
24+
- `pytest`
25+
- `ruff check src/`
26+
- `mypy src/`
27+
- `pytest --cov` (92.31%, threshold 90%)
28+
29+
### Next Steps
30+
- No actionable review findings.
31+
- FOLLOW-UP step is skipped per FLOW rules.

SPECS/INPROGRESS/next.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
## Recently Archived
44

5+
- 2026-02-18 — FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy (PASS)
56
- 2026-02-18 — FU-P13-T4-1: Fix asyncio.get_event_loop() deprecation in BrokerProxy (PASS)
67
- 2026-02-18 — P13-T6: Document broker mode configuration, migration, and rollback (PASS)
78
- 2026-02-18 — P13-T5: Validate prompt reduction and multi-client stability (PARTIAL)
89
- 2026-02-18 — P13-T4: Add stdio proxy mode for compatibility with existing MCP clients (PASS)
910
- 2026-02-18 — P13-T3: Implement multi-client transport and JSON-RPC multiplexing (PASS)
10-
- 2026-02-17 — P13-T2: Implement persistent broker daemon with single upstream Xcode bridge (PASS)
1111

1212
## Suggested Next Tasks
1313

14-
- FU-P13-T4-2 — Implement or remove `reconnect` parameter in `BrokerProxy` (P2)
1514
- P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1)
15+
- FU-P13-T2-1 — Replace run_forever() polling loop with asyncio.Event-based wait (P3)
1616
- FU-BUG-T7-1 — Cap `pending_methods` map to guard against unbounded growth (P3)

SPECS/Workplan.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,7 +2080,8 @@ Phase 9 Follow-up Backlog
20802080

20812081
---
20822082

2083-
#### FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy
2083+
#### ✅ FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy
2084+
- **Status:** ✅ Completed (2026-02-18)
20842085
- **Description:** The `reconnect: bool` parameter is stored but never used in `_run_bridge`. Either implement the reconnect loop (retry once on broken socket before stdin EOF) or remove the parameter entirely and add a comment referencing P13-T5.
20852086
- **Priority:** P2
20862087
- **Dependencies:** P13-T4
@@ -2089,9 +2090,9 @@ Phase 9 Follow-up Backlog
20892090
- Updated `src/mcpbridge_wrapper/broker/proxy.py`
20902091
- Updated `tests/unit/test_broker_proxy.py`
20912092
- **Acceptance Criteria:**
2092-
- [ ] `reconnect=True` either reconnects on broken socket or the parameter is removed
2093-
- [ ] No dead/unused code remains
2094-
- [ ] Tests pass
2093+
- [x] `reconnect=True` either reconnects on broken socket or the parameter is removed
2094+
- [x] No dead/unused code remains
2095+
- [x] Tests pass
20952096

20962097
---
20972098

src/mcpbridge_wrapper/__main__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,6 @@ def main() -> int:
255255
broker_config,
256256
auto_spawn=broker_spawn,
257257
connect_timeout=10.0,
258-
reconnect=False,
259258
)
260259
try:
261260
asyncio.run(proxy.run())

src/mcpbridge_wrapper/broker/proxy.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,6 @@ class BrokerProxy:
3838
flag.
3939
connect_timeout:
4040
Maximum seconds to wait for the broker socket to become available.
41-
reconnect:
42-
When ``True``, attempt to reconnect once if the socket connection is
43-
broken before stdin EOF (covers broker RECONNECTING window).
4441
stdin:
4542
Asyncio stream to read from (defaults to ``sys.stdin.buffer``).
4643
stdout:
@@ -53,15 +50,13 @@ def __init__(
5350
*,
5451
auto_spawn: bool = False,
5552
connect_timeout: float = 10.0,
56-
reconnect: bool = False,
5753
stdin: asyncio.StreamReader | None = None,
5854
stdout: asyncio.StreamWriter | None = None,
5955
) -> None:
6056
"""Initialise the proxy with the given broker configuration."""
6157
self._config = config
6258
self._auto_spawn = auto_spawn
6359
self._connect_timeout = connect_timeout
64-
self._reconnect = reconnect
6560
self._stdin = stdin
6661
self._stdout = stdout
6762

tests/unit/test_broker_proxy.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
- EOF on socket causes clean exit
88
- auto_spawn: spawns broker subprocess when socket absent
99
- auto_spawn: no-op when broker already running (PID file present)
10-
- reconnect=False: broken connection is not retried
1110
- _parse_broker_args: --broker-connect flag
1211
- _parse_broker_args: --broker-spawn implies --broker-connect
1312
- _parse_broker_args: unknown flags pass through
@@ -16,6 +15,7 @@
1615
from __future__ import annotations
1716

1817
import asyncio
18+
import inspect
1919
import os
2020
from pathlib import Path
2121
from unittest.mock import AsyncMock, MagicMock, patch
@@ -72,6 +72,10 @@ def _make_writer() -> MagicMock:
7272

7373

7474
class TestBrokerProxyConnectTimeout:
75+
def test_constructor_has_no_reconnect_parameter(self) -> None:
76+
params = inspect.signature(BrokerProxy.__init__).parameters
77+
assert "reconnect" not in params
78+
7579
@pytest.mark.asyncio
7680
async def test_raises_timeout_when_no_socket(self, tmp_path: Path) -> None:
7781
cfg = _make_config(tmp_path)

0 commit comments

Comments
 (0)