Skip to content

Commit 2f8a5d2

Browse files
SoundBlasterSoundBlaster
authored
Merge pull request #165 from SoundBlaster/codex/p2-t8-broker-tools-catalog-gate
P2-T8: Gate broker tools/list on warmed tool catalog
2 parents 5cdcc42 + 10b4976 commit 2f8a5d2

12 files changed

Lines changed: 589 additions & 64 deletions

File tree

‎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
3+
**Last Updated:** 2026-03-10
44

55
## Archived Tasks
66

77
| Task ID | Folder | Archived | Verdict |
88
|---------|--------|----------|---------|
9+
| P2-T8 | [P2-T8_Gate_broker_tools_list_on_warmed_tool_catalog/](P2-T8_Gate_broker_tools_list_on_warmed_tool_catalog/) | 2026-03-10 | PASS |
910
| P8-T1 | [P8-T1_Release_version_0.4.2_to_PyPI_and_MCP_Registry/](P8-T1_Release_version_0.4.2_to_PyPI_and_MCP_Registry/) | 2026-03-07 | PASS |
1011
| P7-T5 | [P7-T5_Document_broker_UX/](P7-T5_Document_broker_UX/) | 2026-03-07 | PASS |
1112
| P7-T4 | [P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/](P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/) | 2026-03-07 | PASS |
@@ -205,6 +206,7 @@
205206

206207
| File | Description |
207208
|------|-------------|
209+
| [REVIEW_p2_t8_tools_catalog_gate.md](_Historical/REVIEW_p2_t8_tools_catalog_gate.md) | Review report for P2-T8 |
208210
| [REVIEW_broker_ux_docs.md](_Historical/REVIEW_broker_ux_docs.md) | Review report for P7-T5 |
209211
| [REVIEW_tui_local_status_fallback.md](P7-T4_Add_direct_local-status_fallback_for_TUI_when_dashboard_API_is_unavailable/REVIEW_tui_local_status_fallback.md) | Review report for P7-T4 |
210212
| [REVIEW_broker_owned_listener_guidance.md](_Historical/REVIEW_broker_owned_listener_guidance.md) | Review report for FU-P7-T3-2 |
@@ -636,3 +638,5 @@
636638
| 2026-03-07 | P7-T2 | Archived REVIEW_broker_doctor_diagnostics report |
637639
| 2026-03-07 | P7-T3 | Archived Auto-recover_or_guide_on_dashboard_port_ownership_conflicts (PASS) |
638640
| 2026-03-07 | P7-T3 | Archived REVIEW_dashboard_port_ownership_conflicts report |
641+
| 2026-03-10 | P2-T8 | Archived Gate_broker_tools_list_on_warmed_tool_catalog (PASS) |
642+
| 2026-03-10 | P2-T8 | Archived REVIEW_p2_t8_tools_catalog_gate report |

‎SPECS/ARCHIVE/P2-T8_Gate_broker_tools_list_on_warmed_tool_catalog/P2-T8_Gate_broker_tools_list_on_warmed_tool_catalog.md‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# P2-T8: Gate broker tools/list on warmed tool catalog
2+
3+
**Status:** In Progress
4+
**Priority:** P0
5+
**Dependencies:** BUG-T9, P4-T2
6+
**Branch:** `codex/p2-t8-broker-tools-catalog-gate`
7+
8+
---
9+
10+
## Problem Statement
11+
12+
Strict MCP clients such as Cursor and Zed cache the first successful `tools/list`
13+
response they receive from a server. In broker mode, `mcpbridge-wrapper` currently
14+
lets external client `tools/list` requests pass as soon as the upstream
15+
`initialize` round-trip finishes.
16+
17+
That is too early.
18+
19+
The broker still has a second startup phase after `initialize`:
20+
21+
1. Send `notifications/initialized` to `xcrun mcpbridge`
22+
2. Probe `tools/list` internally
23+
3. Cache the resulting tool catalog for later clients
24+
25+
If an external client sends `tools/list` during that warm-up gap, it can observe an
26+
empty or invalid tools list and cache that broken success locally. The user then
27+
sees a green MCP indicator but fewer than the full 20 Xcode tools until they toggle
28+
the server multiple times.
29+
30+
---
31+
32+
## Root Cause
33+
34+
In `UnixSocketServer._process_client_line`, the only readiness gate for all
35+
request/response traffic is `daemon.upstream_initialized`. That event becomes set
36+
immediately after the broker receives the upstream `initialize` response.
37+
38+
At that moment, however:
39+
40+
- the broker's internal `tools/list` probe may still be in flight
41+
- `_tools_list_cache` may still be `None`
42+
- the upstream may still return an empty or malformed tools payload during cold-start
43+
44+
Because `tools/list` follows the same gate as every other method, the first client
45+
tool-discovery request can race ahead of the broker's own cache warm-up.
46+
47+
---
48+
49+
## Fix
50+
51+
Introduce a second broker readiness concept dedicated to tool discovery:
52+
53+
- add a `tools_catalog_ready` event on the daemon
54+
- set it only after the broker receives a non-empty, structurally valid
55+
`tools/list` probe result
56+
- clear it on reconnect or when the internal probe returns an empty/invalid catalog
57+
- make external client `tools/list` wait on `tools_catalog_ready` instead of only
58+
`upstream_initialized`
59+
60+
This preserves existing behavior for non-`tools/list` methods while making the
61+
first tool-discovery handshake safe for strict clients.
62+
63+
---
64+
65+
## Deliverables
66+
67+
| File | Change |
68+
|------|--------|
69+
| `src/mcpbridge_wrapper/broker/daemon.py` | Add explicit tools-catalog readiness event and reject empty/invalid warm-up results |
70+
| `src/mcpbridge_wrapper/broker/transport.py` | Hold client `tools/list` until broker tool catalog is ready |
71+
| `tests/unit/test_broker_daemon.py` | Cover ready/non-ready broker tool catalog transitions |
72+
| `tests/unit/test_broker_transport.py` | Cover client `tools/list` wait/error/cache paths |
73+
| `tests/integration/test_broker_multi_client.py` | Keep integration coverage aligned with the stronger broker contract |
74+
75+
---
76+
77+
## Acceptance Criteria
78+
79+
- [ ] Broker does not forward external `tools/list` while the internal tools cache is still cold
80+
- [ ] Empty or invalid internal `tools/list` probe results do not open the client-facing readiness gate
81+
- [ ] Client `tools/list` returns either a warmed catalog or a clear TTL error, never a premature empty success
82+
- [ ] Existing non-`tools/list` broker traffic still flows after `upstream_initialized`
83+
- [ ] `pytest` passes
84+
- [ ] `ruff check src/` passes
85+
- [ ] `mypy src/` passes
86+
- [ ] `pytest --cov` remains at or above 90%
87+
88+
89+
---
90+
**Archived:** 2026-03-10
91+
**Verdict:** PASS

‎SPECS/ARCHIVE/P2-T8_Gate_broker_tools_list_on_warmed_tool_catalog/P2-T8_Validation_Report.md‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Validation Report: P2-T8 — Gate broker tools/list on warmed tool catalog
2+
3+
**Date:** 2026-03-10
4+
**Verdict:** PASS
5+
6+
---
7+
8+
## Acceptance Criteria
9+
10+
| # | Criterion | Status |
11+
|---|-----------|--------|
12+
| 1 | Broker does not forward external `tools/list` while the internal tools cache is still cold | ✅ PASS |
13+
| 2 | Empty or invalid internal `tools/list` probe results do not open the client-facing readiness gate | ✅ PASS |
14+
| 3 | Client `tools/list` returns either a warmed catalog or a clear TTL error, never a premature empty success | ✅ PASS |
15+
| 4 | Existing non-`tools/list` broker traffic still flows after `upstream_initialized` | ✅ PASS |
16+
| 5 | `pytest` passes | ✅ PASS |
17+
| 6 | `ruff check src/` passes | ✅ PASS |
18+
| 7 | `mypy src/` passes | ✅ PASS |
19+
| 8 | `pytest --cov` remains at or above 90% | ✅ PASS |
20+
21+
---
22+
23+
## Evidence
24+
25+
### Functional behavior
26+
27+
- Added a dedicated `tools_catalog_ready` event in the broker daemon so tool discovery
28+
is gated separately from the upstream `initialize` round-trip.
29+
- The broker now treats only non-empty, structurally valid internal `tools/list`
30+
probe results as a ready catalog; empty or invalid results keep the gate closed,
31+
clear the cache, and schedule another broker-internal warm-up probe instead of
32+
requiring a reconnect or manual restart.
33+
- External client `tools/list` now waits on the warmed catalog gate and returns a
34+
deterministic `-32001` TTL error if the broker never reaches a safe ready state.
35+
- Non-`tools/list` methods still wait only on `upstream_initialized`, preserving the
36+
existing broker contract for normal request forwarding.
37+
38+
### Regression coverage
39+
40+
- `tests/unit/test_broker_daemon.py`
41+
- verifies catalog readiness opens only for a valid non-empty probe result
42+
- verifies empty probe results keep the catalog gate closed
43+
- verifies an empty first probe retries until a valid tool catalog becomes available
44+
- verifies reconnect clears both cache and readiness state
45+
- `tests/unit/test_broker_transport.py`
46+
- verifies `tools/list` times out with a catalog-specific readiness error
47+
- verifies non-`tools/list` requests still wait on upstream initialization only
48+
- verifies `tools/list` resumes from the warmed cache instead of racing upstream
49+
- `tests/integration/test_broker_multi_client.py`
50+
- keeps concurrent multi-client coverage aligned with the stronger broker warm-up
51+
contract by exercising a normal forwarded tool call path instead of the special
52+
cached `tools/list` path
53+
54+
### Validation environment hardening
55+
56+
- Added `pythonpath = ["src"]` to `pyproject.toml` so pytest in a clean worktree
57+
imports the local checkout instead of an unrelated editable install from another
58+
repository path. This makes FLOW validation deterministic and fixes `pytest --cov`
59+
reporting in multi-worktree setups.
60+
61+
### Command results
62+
63+
- `pytest` → **901 passed, 5 skipped, 2 warnings**
64+
- `ruff check src/` → **All checks passed**
65+
- `mypy src/` → **Success: no issues found in 20 source files**
66+
- `pytest --cov` → **901 passed, 5 skipped, 2 warnings; coverage 91.58%**
67+
68+
---
69+
70+
## Changed Files
71+
72+
- `pyproject.toml`
73+
- `src/mcpbridge_wrapper/broker/daemon.py`
74+
- `src/mcpbridge_wrapper/broker/transport.py`
75+
- `tests/integration/test_broker_multi_client.py`
76+
- `tests/unit/test_broker_daemon.py`
77+
- `tests/unit/test_broker_transport.py`

‎SPECS/ARCHIVE/_Historical/REVIEW_p2_t8_tools_catalog_gate.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
## REVIEW REPORT — P2-T8 Tools Catalog Gate
2+
3+
**Scope:** `origin/main..HEAD`
4+
**Files:** 12
5+
6+
### Summary Verdict
7+
- [x] Approve
8+
- [ ] Approve with comments
9+
- [ ] Request changes
10+
- [ ] Block
11+
12+
### Critical Issues
13+
14+
- None.
15+
16+
### Secondary Issues
17+
18+
- None.
19+
20+
### Architectural Notes
21+
22+
- Separating `tools_catalog_ready` from `upstream_initialized` closes the broker's
23+
startup race without delaying unrelated non-`tools/list` traffic.
24+
- Review of the first P2-T8 revision uncovered one high-risk hole: an empty first
25+
broker `tools/list` probe could leave the catalog gate closed indefinitely until
26+
reconnect or restart. The final branch resolves that by retrying the broker-internal
27+
warm-up probe until a valid non-empty catalog arrives or the daemon transitions.
28+
- Adding `pythonpath = ["src"]` to `pytest` config is a valid repo-level hardening
29+
step because this project is actively developed from multiple local worktrees and
30+
otherwise can import an unrelated editable install.
31+
32+
### Tests
33+
34+
- `pytest` — PASS (`901 passed, 5 skipped, 2 warnings`)
35+
- `ruff check src/` — PASS
36+
- `mypy src/` — PASS
37+
- `pytest --cov` — PASS (`91.58%`)
38+
39+
### Next Steps
40+
41+
- No actionable review findings. FOLLOW-UP is skipped.

‎SPECS/INPROGRESS/next.md‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,23 @@
1-
# Next Task: (none pending)
1+
# Next Task: None
22

3-
All current workplan tasks are complete.
3+
## Selected Task
4+
5+
- No active task selected.
6+
7+
## Description
8+
9+
All tasks currently tracked in `SPECS/Workplan.md` are archived or completed. Select a new
10+
task only after a follow-up or new workplan entry is created.
11+
12+
## Outputs
13+
14+
- None.
415

516
## Recently Archived
617

18+
- `2026-03-10` — `P2-T8` archived with verdict `PASS`
719
- `2026-03-07` — `P8-T1` archived with verdict `PASS`
820
- `2026-03-07` — `P7-T5` archived with verdict `PASS`
921
- `2026-03-07` — `P7-T4` archived with verdict `PASS`
1022
- `2026-03-07` — `FU-P7-T3-2` archived with verdict `PASS`
1123
- `2026-03-07` — `FU-P7-T3-1` archived with verdict `PASS`
12-
13-
## Next Step
14-
15-
All tasks in the current workplan cycle have been completed. Add new tasks to
16-
`SPECS/Workplan.md` to begin the next cycle.
17-
18-
## Post-Merge Action Required
19-
20-
After the P8-T1 PR merges to `main`, push the release tag to trigger publishing:
21-
22-
```bash
23-
git checkout main && git pull origin main
24-
git tag v0.4.2
25-
git push origin v0.4.2
26-
```
27-
28-
Then verify GitHub Actions `publish-mcp.yml` completes successfully.

‎SPECS/Workplan.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,23 @@ Add new tasks using the canonical template in [TASK_TEMPLATE.md](TASK_TEMPLATE.m
274274
- [x] Broker mode guidance remains clear with `--broker` (proxy) and `--broker-daemon` (host)
275275
- [x] Required quality gates pass (`pytest`, `ruff check src/`, `mypy src/`, `pytest --cov` with coverage >=90%)
276276

277+
#### ✅ P2-T8: Gate broker tools/list on warmed tool catalog
278+
- **Status:** ✅ Completed (2026-03-10)
279+
- **Description:** Cursor and Zed can cache the first successful `tools/list` response they receive from `mcpbridge-wrapper`. Today the broker releases client `tools/list` requests immediately after upstream `initialize`, even if the broker has not yet completed its own `notifications/initialized` + `tools/list` warm-up and populated a stable tool cache. During cold-start or Xcode approval timing, that lets strict clients see an empty or invalid tool list and forces users to toggle the server several times before all 20 Xcode tools appear. Fix the broker so external `tools/list` waits for a warmed non-empty catalog instead of racing the warm-up path.
280+
- **Priority:** P0
281+
- **Dependencies:** BUG-T9, P4-T2
282+
- **Parallelizable:** no
283+
- **Outputs/Artifacts:**
284+
- `src/mcpbridge_wrapper/broker/daemon.py` — explicit tool-catalog readiness gate and empty-catalog handling
285+
- `src/mcpbridge_wrapper/broker/transport.py` — hold client `tools/list` until broker cache is ready
286+
- `tests/unit/test_broker_daemon.py`, `tests/unit/test_broker_transport.py` — broker warm-up regression coverage
287+
- `tests/integration/test_broker_multi_client.py` — integration coverage updated for the new broker contract
288+
- **Acceptance Criteria:**
289+
- [x] Broker does not forward client `tools/list` while its internal tool catalog is still cold
290+
- [x] Empty or invalid broker `tools/list` probe results do not open the client-facing readiness gate
291+
- [x] Cursor/Zed-style first `tools/list` requests receive either a warmed catalog or a clear TTL error, never a prematurely cached empty success
292+
- [x] Required quality gates pass (`pytest`, `ruff check src/`, `mypy src/`, `pytest --cov` with coverage >=90%)
293+
277294
### Phase 3: Web UI Controls
278295

279296
#### ✅ P3-T11: Add Stop broker/service control button to Web UI

‎pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ include-package-data = true
7171

7272
[tool.pytest.ini_options]
7373
testpaths = ["tests"]
74+
pythonpath = ["src"]
7475
python_files = ["test_*.py", "*_test.py"]
7576
python_classes = ["Test*"]
7677
python_functions = ["test_*"]

0 commit comments

Comments
 (0)