Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# PRD: FU-P13-T2-1 — Replace run_forever() polling loop with asyncio.Event-based wait

**Created:** 2026-02-18
**Priority:** P3
**Branch:** `codex/feature/FU-P13-T2-1-event-wait-shutdown`
**Status:** PLAN

---

## 1. Problem Statement

`BrokerDaemon.run_forever()` currently waits for shutdown by polling `asyncio.sleep(0.1)`. This adds up to 100ms latency to stop handling and introduces unnecessary wakeups.

---

## 2. Scope

### In Scope
- Replace polling-based wait in `run_forever()` with an `asyncio.Event`-driven wait.
- Preserve startup/shutdown semantics and existing lifecycle behavior.
- Keep current external API and tests intact.

### Out of Scope
- Refactoring broker daemon startup/lock management.
- Changes to proxy or transport behavior.
- New CLI options.

---

## 3. Deliverables

1. `src/mcpbridge_wrapper/broker/daemon.py`
- Introduce event-based shutdown signaling for `run_forever()`.
- Ensure `stop()` triggers the event and remains safe when called multiple times.

2. `tests/unit/test_broker_daemon.py`
- Keep existing behavior checks passing.
- Add or adjust assertions if needed to validate event-wait shutdown behavior.

3. `SPECS/INPROGRESS/FU-P13-T2-1_Validation_Report.md`
- Record quality-gate results and acceptance evidence.

---

## 4. Acceptance Criteria

- [ ] `run_forever()` responds to stop signal within one event loop tick.
- [ ] Existing `test_run_forever_starts_and_stops` passes without behavioral regressions.
- [ ] Full quality gates pass:
- `pytest`
- `ruff check src/`
- `mypy src/`
- `pytest --cov` (coverage >= 90%)

---

## 5. Dependencies

- P13-T2 ✅

---

## 6. Risks and Mitigations

- **Risk:** Event lifecycle may leak across multiple broker runs.
- **Mitigation:** Reinitialize/reset the event at broker startup boundaries and validate with unit tests.

---

## 7. Validation Plan

1. Implement event-based wait and stop signaling.
2. Run targeted daemon unit tests.
3. Run required quality gates and record outcomes in the validation report.

---
**Archived:** 2026-02-18
**Verdict:** PASS
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Validation Report: FU-P13-T2-1

**Task:** FU-P13-T2-1 — Replace run_forever() polling loop with asyncio.Event-based wait
**Date:** 2026-02-18
**Branch:** `codex/feature/FU-P13-T2-1-event-wait-shutdown`

## Scope validated

- Replaced `run_forever()` fixed-interval polling with event-based waiting.
- Added explicit stop-completion signaling so concurrent stop waiters and `run_forever()` unblock only after shutdown completes.
- Added a unit test confirming `run_forever()` no longer relies on a fixed `0.1s` polling sleep.

## Quality gates

### 1) Targeted task tests

Command:

```bash
pytest tests/unit/test_broker_daemon.py -q
```

Result: **PASS** (`27 passed`)

### 2) Full test suite

Command:

```bash
pytest
```

Result: **PASS** (`579 passed, 5 skipped`)

### 3) Lint

Command:

```bash
ruff check src/
```

Result: **PASS** (`All checks passed!`)

### 4) Type checks

Command:

```bash
mypy src/
```

Result: **PASS** (`Success: no issues found in 18 source files`)

### 5) Coverage

Command:

```bash
pytest --cov
```

Result: **PASS** (`Total coverage: 92.25%`, threshold: `>= 90%`)

## Acceptance criteria evidence

- [x] `run_forever()` responds to stop signal within one event loop tick.
- Evidence: `run_forever()` now waits on `self._stop_event.wait()` instead of sleep polling and exits after `self._stopped_event.wait()` completion.
- [x] Existing `test_run_forever_starts_and_stops` passes without behavioral regressions.
- Evidence: `pytest tests/unit/test_broker_daemon.py -q` includes this test and passed.
- [x] Full quality gates pass.
- Evidence: `pytest`, `ruff check src/`, `mypy src/`, and `pytest --cov` all passed; coverage `92.25%`.

## Notes

- Existing `websockets` deprecation warnings in Web UI tests remain unchanged and are 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,6 +1,6 @@
# mcpbridge-wrapper Tasks Archive

**Last Updated:** 2026-02-18 (FU-P13-T4-2)
**Last Updated:** 2026-02-18 (FU-P13-T2-1)

## Archived Tasks

Expand Down Expand Up @@ -112,6 +112,7 @@
| P13-T6 | [P13-T6_Document_broker_mode_configuration_migration_and_rollback/](P13-T6_Document_broker_mode_configuration_migration_and_rollback/) | 2026-02-18 | PASS |
| 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 |
| 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 |
| FU-P13-T2-1 | [FU-P13-T2-1_Replace_run_forever_polling_with_Event_wait/](FU-P13-T2-1_Replace_run_forever_polling_with_Event_wait/) | 2026-02-18 | PASS |

## Historical Artifacts

Expand Down Expand Up @@ -187,6 +188,7 @@
| [REVIEW_P13-T6_broker_mode_configuration_migration.md](_Historical/REVIEW_P13-T6_broker_mode_configuration_migration.md) | Review report for P13-T6 |
| [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 |
| [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 |
| [REVIEW_FU-P13-T2-1_event_wait_shutdown.md](_Historical/REVIEW_FU-P13-T2-1_event_wait_shutdown.md) | Review report for FU-P13-T2-1 |

## Archive Log

Expand Down Expand Up @@ -328,3 +330,5 @@
| 2026-02-18 | FU-P13-T4-1 | Archived REVIEW_FU-P13-T4-1_broker_proxy_loop report |
| 2026-02-18 | FU-P13-T4-2 | Archived Implement_or_remove_reconnect_parameter_in_BrokerProxy (PASS) |
| 2026-02-18 | FU-P13-T4-2 | Archived REVIEW_FU-P13-T4-2_broker_proxy_reconnect report |
| 2026-02-18 | FU-P13-T2-1 | Archived Replace_run_forever_polling_with_Event_wait (PASS) |
| 2026-02-18 | FU-P13-T2-1 | Archived REVIEW_FU-P13-T2-1_event_wait_shutdown report |
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
## REVIEW REPORT — FU-P13-T2-1 event wait shutdown

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

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

### Critical Issues
- None.

### Secondary Issues
- None.

### Architectural Notes
- Replacing polling with event waits improves shutdown responsiveness and removes unnecessary wakeups in `run_forever()`.
- Adding `_stopped_event` ensures concurrent `stop()` callers and `run_forever()` wait for full shutdown completion.

### Tests
- Required quality gates were run during EXECUTE and all passed:
- `pytest`
- `ruff check src/`
- `mypy src/`
- `pytest --cov` (92.25%, threshold 90%)

### Next Steps
- No actionable review findings.
- FOLLOW-UP step is skipped per FLOW rules.
4 changes: 2 additions & 2 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@

## Recently Archived

- 2026-02-18 — FU-P13-T2-1: Replace run_forever() polling loop with asyncio.Event-based wait (PASS)
- 2026-02-18 — FU-P13-T4-2: Implement or remove reconnect parameter in BrokerProxy (PASS)
- 2026-02-18 — FU-P13-T4-1: Fix asyncio.get_event_loop() deprecation in BrokerProxy (PASS)
- 2026-02-18 — P13-T6: Document broker mode configuration, migration, and rollback (PASS)
- 2026-02-18 — P13-T5: Validate prompt reduction and multi-client stability (PARTIAL)
- 2026-02-18 — P13-T4: Add stdio proxy mode for compatibility with existing MCP clients (PASS)
- 2026-02-18 — P13-T3: Implement multi-client transport and JSON-RPC multiplexing (PASS)

## Suggested Next Tasks

- P13-T5 follow-up — Complete interactive prompt verification in a desktop session (P1)
- FU-P13-T2-1Replace run_forever() polling loop with asyncio.Event-based wait (P3)
- FU-P13-T2-2Move PID file write to after successful upstream launch (P3)
- FU-BUG-T7-1 — Cap `pending_methods` map to guard against unbounded growth (P3)
7 changes: 4 additions & 3 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -2007,15 +2007,16 @@ Phase 9 Follow-up Backlog

---

#### FU-P13-T2-1: Replace run_forever() polling loop with asyncio.Event-based wait
#### ✅ FU-P13-T2-1: Replace run_forever() polling loop with asyncio.Event-based wait
- **Status:** ✅ Completed (2026-02-18)
- **Type:** Enhancement
- **Priority:** P3
- **Discovered:** 2026-02-17 (REVIEW_P13-T2)
- **Component:** BrokerDaemon.run_forever()
- **Description:** Current implementation uses `asyncio.sleep(0.1)` polling which introduces up to 100ms stop-signal latency. Replace with `asyncio.Event.wait()` for idiomatic zero-latency shutdown.
- **Acceptance Criteria:**
- [ ] `run_forever()` responds to stop signal within one event loop tick
- [ ] Existing `test_run_forever_starts_and_stops` passes without change
- [x] `run_forever()` responds to stop signal within one event loop tick
- [x] Existing `test_run_forever_starts_and_stops` passes without change

---

Expand Down
57 changes: 30 additions & 27 deletions src/mcpbridge_wrapper/broker/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def __init__(
self._read_task: asyncio.Task[None] | None = None
self._reconnect_attempt: int = 0
self._stop_event: asyncio.Event = asyncio.Event()
self._stopped_event: asyncio.Event = asyncio.Event()

# ------------------------------------------------------------------
# Public API
Expand Down Expand Up @@ -118,6 +119,7 @@ async def start(self) -> None:

# Background reader
self._stop_event.clear()
self._stopped_event.clear()
self._read_task = asyncio.ensure_future(self._read_upstream_loop())

# Start transport (Unix socket server) if provided
Expand All @@ -130,7 +132,10 @@ async def stop(self) -> None:
Drains in-flight requests up to ``config.graceful_shutdown_timeout``
seconds, then terminates the upstream subprocess and removes socket/PID.
"""
if self._state in (BrokerState.STOPPED, BrokerState.STOPPING):
if self._state == BrokerState.STOPPED:
return
if self._state == BrokerState.STOPPING:
await self._stopped_event.wait()
return

self._state = BrokerState.STOPPING
Expand All @@ -153,25 +158,27 @@ async def stop(self) -> None:
timeout=self._config.graceful_shutdown_timeout,
)

# Terminate upstream
if self._upstream is not None and self._upstream.returncode is None:
with contextlib.suppress(Exception):
if self._upstream.stdin is not None:
self._upstream.stdin.close()
try:
await asyncio.wait_for(
self._upstream.wait(),
timeout=self._config.graceful_shutdown_timeout,
)
except asyncio.TimeoutError:
logger.warning("Upstream did not exit cleanly; killing.")
self._upstream.kill()
await self._upstream.wait()

# Cleanup files
self._cleanup_files()
self._state = BrokerState.STOPPED
logger.info("Broker STOPPED")
try:
# Terminate upstream
if self._upstream is not None and self._upstream.returncode is None:
with contextlib.suppress(Exception):
if self._upstream.stdin is not None:
self._upstream.stdin.close()
try:
await asyncio.wait_for(
self._upstream.wait(),
timeout=self._config.graceful_shutdown_timeout,
)
except asyncio.TimeoutError:
logger.warning("Upstream did not exit cleanly; killing.")
self._upstream.kill()
await self._upstream.wait()
finally:
# Always mark shutdown complete so run_forever/stop waiters unblock.
self._cleanup_files()
self._state = BrokerState.STOPPED
self._stopped_event.set()
logger.info("Broker STOPPED")

async def run_forever(self) -> None:
"""Start and block until a shutdown signal is received."""
Expand All @@ -194,13 +201,9 @@ def _sync_signal_handler() -> None:
with contextlib.suppress(NotImplementedError, RuntimeError):
loop.add_signal_handler(sig, _sync_signal_handler)

# Wait until stopped
while self._state not in (BrokerState.STOPPED, BrokerState.STOPPING):
await asyncio.sleep(0.1)

# Ensure stop completes if STOPPING
if self._state == BrokerState.STOPPING:
await self.stop()
# Wait for shutdown to be requested and fully completed.
await self._stop_event.wait()
await self._stopped_event.wait()

# ------------------------------------------------------------------
# Internal helpers
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/test_broker_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,43 @@ async def _do_stop() -> None:

assert daemon.state == BrokerState.STOPPED

@pytest.mark.asyncio
async def test_run_forever_does_not_poll_with_fixed_sleep(self, tmp_path: Path) -> None:
"""run_forever waits on events and does not use fixed-interval polling."""
cfg = _make_config(tmp_path)
daemon = BrokerDaemon(cfg)

async def _block(*a, **kw) -> bytes: # type: ignore[no-untyped-def]
await daemon._stop_event.wait()
return b""

proc = _make_mock_process()
proc.stdout.readline = _block

sleep_calls: list[float] = []
original_sleep = asyncio.sleep

async def _tracked_sleep(delay: float) -> None:
sleep_calls.append(delay)
await original_sleep(delay)

async def _do_stop() -> None:
await original_sleep(0.05)
await daemon.stop()

with patch(
"mcpbridge_wrapper.broker.daemon.asyncio.create_subprocess_exec",
new=AsyncMock(return_value=proc),
), patch(
"mcpbridge_wrapper.broker.daemon.asyncio.sleep",
side_effect=_tracked_sleep,
):
stopper = asyncio.ensure_future(_do_stop())
await daemon.run_forever()
await stopper

assert all(delay != 0.1 for delay in sleep_calls)


# ---------------------------------------------------------------------------
# _check_and_clear_stale_lock — edge cases
Expand Down