diff --git a/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency.md b/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency.md new file mode 100644 index 00000000..b1273b95 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency.md @@ -0,0 +1,61 @@ +# PRD: FU-P13-T13-FU-1 — Set _stopped_event and _stop_event in _rollback_startup for defensive consistency + +**Status:** INPROGRESS +**Priority:** P3 +**Phase:** Phase 13 — Persistent Broker & Shared Xcode Session +**Dependencies:** FU-P13-T13 (✅) + +--- + +## 1. Objective + +Ensure `_rollback_startup()` leaves event flags fully consistent with STOPPED +state by setting both `_stop_event` and `_stopped_event` during rollback. + +--- + +## 2. Background + +`_rollback_startup()` currently: +- Cancels read task +- Terminates upstream +- Cleans files +- Sets `self._state = BrokerState.STOPPED` + +It does not explicitly set `_stop_event` / `_stopped_event` in this path. While +current call paths do not await these events after startup rollback, setting them +is a defensive consistency improvement and prevents future state/event mismatch. + +--- + +## 3. Design + +1. Update `BrokerDaemon._rollback_startup()` to call: + - `self._stop_event.set()` + - `self._stopped_event.set()` +2. Keep existing rollback sequence and error behavior unchanged. +3. Add regression test asserting both events are set after a startup failure + that triggers rollback. + +--- + +## 4. Files To Change + +| File | Change | +|------|--------| +| `src/mcpbridge_wrapper/broker/daemon.py` | Set stop/stopped events in `_rollback_startup()` | +| `tests/unit/test_broker_daemon.py` | Add assertion coverage for event state after rollback | +| `SPECS/INPROGRESS/FU-P13-T13-FU-1_Validation_Report.md` | Record validation and quality-gate outcomes | + +--- + +## 5. Acceptance Criteria + +- [ ] `_stopped_event.set()` called in `_rollback_startup()` +- [ ] `_stop_event.set()` called in `_rollback_startup()` +- [ ] Tests verify event states are set after a failed startup +- [ ] Quality gates are executed and recorded + +--- +**Archived:** 2026-02-19 +**Verdict:** PASS diff --git a/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Validation_Report.md b/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Validation_Report.md new file mode 100644 index 00000000..7819e247 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/FU-P13-T13-FU-1_Validation_Report.md @@ -0,0 +1,52 @@ +# Validation Report: FU-P13-T13-FU-1 — Set _stopped_event and _stop_event in _rollback_startup for defensive consistency + +**Date:** 2026-02-19 +**Verdict:** PASS + +--- + +## Acceptance Criteria + +| # | Criterion | Status | +|---|-----------|--------| +| 1 | `_stopped_event.set()` is called in `_rollback_startup()` | ✅ PASS | +| 2 | `_stop_event.set()` is called in `_rollback_startup()` | ✅ PASS | +| 3 | Tests verify event states are set after startup rollback | ✅ PASS | +| 4 | Quality gates are executed and documented | ✅ PASS | + +--- + +## Evidence + +### Implementation evidence + +`BrokerDaemon._rollback_startup()` now sets both stop events after transitioning to `BrokerState.STOPPED`: + +- `self._stop_event.set()` +- `self._stopped_event.set()` + +### Test evidence + +Added unit test: + +- `tests/unit/test_broker_daemon.py::TestStartupRollback::test_transport_start_failure_sets_stop_events` + +The test verifies that when startup fails and rollback runs, both event flags are set and daemon state is `STOPPED`. + +--- + +## Quality Gates + +| Gate | Result | Notes | +|------|--------|-------| +| `TMPDIR=/tmp pytest` | ✅ PASS | 624 passed, 5 skipped. | +| `ruff check src/` | ✅ PASS | All checks passed. | +| `mypy src/` | ✅ PASS | Success: no issues found in 18 source files. | +| `TMPDIR=/tmp pytest --cov` | ✅ PASS | 624 passed, 5 skipped; total coverage 91.70% (>=90%). | + +--- + +## Changed Files + +- `src/mcpbridge_wrapper/broker/daemon.py` +- `tests/unit/test_broker_daemon.py` diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 73c6e228..fdd1c7df 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-19 (FU-P13-T15_Restore_broker_same-UID_client_acceptance_when_peer_credential_APIs_are_unavailable) +**Last Updated:** 2026-02-19 (FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency) ## Archived Tasks @@ -129,6 +129,7 @@ | FU-P13-T13 | [FU-P13-T13_Make_broker_startup_transactional_when_transport_bind_start_fails/](FU-P13-T13_Make_broker_startup_transactional_when_transport_bind_start_fails/) | 2026-02-19 | PASS | | FU-P13-T14 | [FU-P13-T14_Complete_interactive_Xcode_prompt_verification_and_close_P13-T5/](FU-P13-T14_Complete_interactive_Xcode_prompt_verification_and_close_P13-T5/) | 2026-02-19 | FAIL | | FU-P13-T15 | [FU-P13-T15_Restore_broker_same-UID_client_acceptance_when_peer_credential_APIs_are_unavailable/](FU-P13-T15_Restore_broker_same-UID_client_acceptance_when_peer_credential_APIs_are_unavailable/) | 2026-02-19 | PASS | +| FU-P13-T13-FU-1 | [FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/](FU-P13-T13-FU-1_Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency/) | 2026-02-19 | PASS | ## Historical Artifacts @@ -223,6 +224,7 @@ | [REVIEW_FU-P13-T13_transactional_startup.md](_Historical/REVIEW_FU-P13-T13_transactional_startup.md) | Review report for FU-P13-T13 | | [REVIEW_FU-P13-T14_prompt_validation_closeout.md](_Historical/REVIEW_FU-P13-T14_prompt_validation_closeout.md) | Review report for FU-P13-T14 | | [REVIEW_FU-P13-T15_peer_credential_fallback.md](_Historical/REVIEW_FU-P13-T15_peer_credential_fallback.md) | Review report for FU-P13-T15 | +| [REVIEW_FU-P13-T13-FU-1_rollback_event_consistency.md](_Historical/REVIEW_FU-P13-T13-FU-1_rollback_event_consistency.md) | Review report for FU-P13-T13-FU-1 | ## Archive Log @@ -401,3 +403,5 @@ | 2026-02-19 | FU-P13-T14 | Archived REVIEW_FU-P13-T14_prompt_validation_closeout report | | 2026-02-19 | FU-P13-T15 | Archived Restore_broker_same-UID_client_acceptance_when_peer_credential_APIs_are_unavailable (PASS) | | 2026-02-19 | FU-P13-T15 | Archived REVIEW_FU-P13-T15_peer_credential_fallback report | +| 2026-02-19 | FU-P13-T13-FU-1 | Archived Set__stopped_event_and__stop_event_in__rollback_startup_for_defensive_consistency (PASS) | +| 2026-02-19 | FU-P13-T13-FU-1 | Archived REVIEW_FU-P13-T13-FU-1_rollback_event_consistency report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T13-FU-1_rollback_event_consistency.md b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T13-FU-1_rollback_event_consistency.md new file mode 100644 index 00000000..109249fa --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P13-T13-FU-1_rollback_event_consistency.md @@ -0,0 +1,34 @@ +## REVIEW REPORT — FU-P13-T13-FU-1 rollback event consistency + +**Scope:** `origin/main..HEAD` +**Files:** 7 + +### Summary Verdict +- [x] Approve +- [ ] Approve with comments +- [ ] Request changes +- [ ] Block + +### Critical Issues + +None. + +### Secondary Issues + +None. + +### Architectural Notes + +- `_rollback_startup()` now aligns state and signaling contracts by setting both `_stop_event` and `_stopped_event` when rollback transitions the daemon to `STOPPED`. +- The added regression test directly exercises transport startup failure and validates defensive event consistency. + +### Tests + +- `TMPDIR=/tmp pytest` → 624 passed, 5 skipped +- `ruff check src/` → pass +- `mypy src/` → pass +- `TMPDIR=/tmp pytest --cov` → 91.70% total coverage (>=90%) + +### Next Steps + +- FOLLOW-UP skipped: no actionable review findings. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index e1d3a7c5..306e9ac6 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -1,10 +1,11 @@ -# No Active Task +# Next Task: None — All Tasks Completed + +**Status:** Waiting for new tasks ## Recently Archived -- **FU-P13-T15** — Restore broker same-UID client acceptance when peer credential APIs are unavailable (2026-02-19, PASS) -- **FU-P13-T14** — Complete interactive Xcode prompt verification and close P13-T5 (2026-02-19, FAIL) +- `FU-P13-T13-FU-1` — Set _stopped_event and _stop_event in _rollback_startup for defensive consistency (2026-02-19, PASS) -## Suggested Next Tasks +## Next Step -- **FU-P13-T13-FU-1** — Set _stopped_event and _stop_event in _rollback_startup for defensive consistency (P3) +Add or prioritize follow-up work in `SPECS/Workplan.md`, then run SELECT. diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index b53dc118..e9bb0c20 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -2259,7 +2259,7 @@ Phase 9 Follow-up Backlog --- -#### ⬜️ FU-P13-T13-FU-1: Set _stopped_event and _stop_event in _rollback_startup for defensive consistency +#### ✅ FU-P13-T13-FU-1: Set _stopped_event and _stop_event in _rollback_startup for defensive consistency — Completed (2026-02-19, PASS) - **Description:** After `_rollback_startup()` sets state to STOPPED, also call `self._stopped_event.set()` and `self._stop_event.set()` so all event states are consistent with the STOPPED contract. These paths are currently unreachable by callers but the defensive fix ensures correctness if future callers are added. - **Priority:** P3 (Low — optional defensibility improvement) - **Dependencies:** FU-P13-T13 (✅) @@ -2268,9 +2268,9 @@ Phase 9 Follow-up Backlog - Updated `src/mcpbridge_wrapper/broker/daemon.py` `_rollback_startup()` method - Updated tests asserting event state after rollback - **Acceptance Criteria:** - - [ ] `_stopped_event.set()` called in `_rollback_startup()` - - [ ] `_stop_event.set()` called in `_rollback_startup()` - - [ ] Tests verify event states are set after a failed startup + - [x] `_stopped_event.set()` called in `_rollback_startup()` + - [x] `_stop_event.set()` called in `_rollback_startup()` + - [x] Tests verify event states are set after a failed startup --- diff --git a/src/mcpbridge_wrapper/broker/daemon.py b/src/mcpbridge_wrapper/broker/daemon.py index 5f5e6cdd..3bd553cc 100644 --- a/src/mcpbridge_wrapper/broker/daemon.py +++ b/src/mcpbridge_wrapper/broker/daemon.py @@ -261,6 +261,9 @@ async def _rollback_startup(self) -> None: # Remove stale files and mark daemon as stopped self._cleanup_files() self._state = BrokerState.STOPPED + # Keep event flags consistent with STOPPED state for future callers. + self._stop_event.set() + self._stopped_event.set() logger.info("Startup rollback complete — broker STOPPED.") def _check_and_clear_stale_lock(self) -> None: diff --git a/tests/unit/test_broker_daemon.py b/tests/unit/test_broker_daemon.py index 6f783f7e..a3642b45 100644 --- a/tests/unit/test_broker_daemon.py +++ b/tests/unit/test_broker_daemon.py @@ -936,3 +936,25 @@ async def test_pid_write_failure_terminates_upstream(self, tmp_path: Path) -> No proc.terminate.assert_called_once() assert daemon.state == BrokerState.STOPPED + + @pytest.mark.asyncio + async def test_transport_start_failure_sets_stop_events(self, tmp_path: Path) -> None: + """Rollback sets both stop events to match STOPPED state.""" + cfg = _make_config(tmp_path) + transport = MagicMock() + transport.start = AsyncMock(side_effect=OSError("addr in use")) + transport.stop = AsyncMock() + daemon = BrokerDaemon(cfg, transport=transport) + + proc = _make_mock_process() + proc.terminate = MagicMock() + + with patch( + "mcpbridge_wrapper.broker.daemon.asyncio.create_subprocess_exec", + new=AsyncMock(return_value=proc), + ), pytest.raises(OSError): + await daemon.start() + + assert daemon.state == BrokerState.STOPPED + assert daemon._stop_event.is_set() + assert daemon._stopped_event.is_set()