Skip to content
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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`
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-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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 |
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 6 additions & 5 deletions SPECS/INPROGRESS/next.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 4 additions & 4 deletions SPECS/Workplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 (✅)
Expand All @@ -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

---

Expand Down
3 changes: 3 additions & 0 deletions src/mcpbridge_wrapper/broker/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/test_broker_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()