Skip to content

Commit 9306bcf

Browse files
crimsonsunsetcrimsonsunset
committed
fix(gateway): restore advertised tools filter + add surfacing regression plan
Partial Phase 1 work: list_tools uses get_advertised_tools_for_grants again. Includes database.rs formatting cleanup and the planning doc. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent 9bb6e7a commit 9306bcf

3 files changed

Lines changed: 223 additions & 17 deletions

File tree

‎crates/mcpmux-gateway/src/mcp/handler.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,12 +736,13 @@ impl ServerHandler for McpMuxGatewayHandler {
736736
.resolve_routing(session_id_owned.as_deref(), &oauth_ctx.client_id)
737737
.await?;
738738

739-
// Get tools via FeatureService — using the *resolved* space.
739+
// Get advertised (surfaced) tools only — full invokable set is reachable
740+
// via mcpmux_invoke_tool; non-surfaced tools stay off tools/list.
740741
let tools = self
741742
.services
742743
.pool_services
743744
.feature_service
744-
.get_tools_for_grants(&space_id.to_string(), &feature_set_ids)
745+
.get_advertised_tools_for_grants(&space_id.to_string(), &feature_set_ids)
745746
.await
746747
.map_err(|e| McpError::internal_error(format!("Failed to get tools: {}", e), None))?;
747748

‎crates/mcpmux-storage/src/database.rs‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -430,9 +430,7 @@ impl Database {
430430
return Ok(());
431431
}
432432

433-
info!(
434-
"Detected fork-era migration numbering; reconciling to upstream+port schema..."
435-
);
433+
info!("Detected fork-era migration numbering; reconciling to upstream+port schema...");
436434

437435
let fk_was_on = self.foreign_keys_enabled();
438436
if fk_was_on {
@@ -441,22 +439,17 @@ impl Database {
441439

442440
let tx = self.conn.unchecked_transaction()?;
443441

444-
for migration in MIGRATIONS
445-
.iter()
446-
.filter(|m| (16..=19).contains(&m.version))
447-
{
442+
for migration in MIGRATIONS.iter().filter(|m| (16..=19).contains(&m.version)) {
448443
info!(
449444
"Applying upstream migration {} ({}) during fork reconcile...",
450445
migration.version, migration.name
451446
);
452-
self.conn
453-
.execute_batch(migration.sql)
454-
.with_context(|| {
455-
format!(
456-
"Failed upstream migration {} ({}) during fork reconcile",
457-
migration.version, migration.name
458-
)
459-
})?;
447+
self.conn.execute_batch(migration.sql).with_context(|| {
448+
format!(
449+
"Failed upstream migration {} ({}) during fork reconcile",
450+
migration.version, migration.name
451+
)
452+
})?;
460453
}
461454

462455
self.conn

‎docs/planning/dev-rebased-surfacing-regressions.md‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
# dev-rebased Surfacing Regression Fix
2+
3+
**Last Updated:** Jun 24, 2026
4+
**Status:** Active — in progress
5+
**Branch:** `dev-rebased`
6+
**Depends on:** Working tree clean at `d2307d9` (docs planning commit)
7+
**Unblocks:** Gateway surfaces ~5 meta tools instead of 2228; hard-cut model restored; data-integrity regressions resolved; phase hand-off to `dev-rebased-post-port-completion.md`
8+
9+
---
10+
11+
## Problem
12+
13+
A full diff audit between `dev` (reference) and `dev-rebased` HEAD surfaced 7 confirmed regressions introduced during the rebase. The branches share no merge base, so the comparison is file-level. One regression (`list_tools` exposing all resolved tools instead of surfaced ones) was found and patched before the audit; 6 more were found during it.
14+
15+
**1. Surfacing model completely stripped.** `dev` uses `get_advertised_*` methods on `list_tools`, `list_prompts`, `list_resources` — these filter to only features in the `surfaced` flag set. `dev-rebased` replaced all three with unfiltered `get_*_for_grants` calls. `get_advertised_prompts_for_grants` and `get_advertised_resources_for_grants` don't even exist in `facade.rs` anymore.
16+
17+
**2. Hard-cut guards gone from call_* paths.** `dev` rejects direct calls to non-surfaced tools with a `use_invoke_tool` / `bind_feature_set` redirect hint and a `list_inactive_discovery_tools` lookup. `dev-rebased` dropped the entire block — any invokable tool can be called directly, bypassing the meta-tool-first UX intent. Same pattern applied to `get_prompt` (lost `format_direct_fetch_prompt_redirect`) and `read_resource` (lost surfaced-only gate).
18+
19+
**3. `structured_content` dropped from `call_tool` result.** `dev` passes `result.structured_content` through to the client. `dev-rebased` removed the assignment. MCP clients expecting typed structured output silently get nothing.
20+
21+
**4. `WorkspaceNeedsBinding` event field broken.** Backend changed the event shape from `collision_client_id: Option<String>` to `space_locked: bool`. The frontend (`WorkspaceBindingSheet.tsx`, `useWorkspaceEvents.ts`) still keys collision badge copy off `collision_client_id` — that entire UX path is silently dead.
22+
23+
**5. Credential key migration removed.** `crates/mcpmux-storage/src/key_migration.rs` and its startup call in `apps/desktop/src-tauri/src/state/mod.rs` were deleted. Users with OS-keychain-dismiss-fallback encrypted credentials lose access after a keychain recovery.
24+
25+
**6. OAuth refresh dedup singleton removed.** `gateway.ts` inlined the token refresh logic and dropped the module-level singleton promise that prevented duplicate startup refresh calls when `useDataSync` triggers more than once.
26+
27+
---
28+
29+
## Decisions
30+
31+
| # | Decision | Choice | Rationale |
32+
| - | -------- | ------ | --------- |
33+
| 1 | Surfacing model for list_* | Restore full `dev` model: `get_advertised_*` on all three list paths | Keeps `tools/list` lean (~5 meta + surfaced only); full set reachable via `mcpmux_invoke_tool`. Consistent with the product intent of FeatureSet surfacing. |
34+
| 2 | Hard-cut guard model | Restore `dev`'s call_tool / get_prompt / read_resource hard-cut with redirect hints | "If it's not in your list, use the meta-tool path" enforces the UX contract. Guards are callable via `is_meta_tool()` which already exists. |
35+
| 3 | WorkspaceNeedsBinding event | Keep both `collision_client_id` AND `space_locked` fields | `space_locked` encodes the new scoped-Space behavior; `collision_client_id` drives existing frontend collision UX. Both are independent semantics. |
36+
| 4 | structured_content | Restore passthrough | MCP 2025-11-05+ clients expect structured output. Removing it breaks a spec-defined field. |
37+
| 5 | Credential key migration | Restore both the file and the startup call | No data on how many users hit the fallback path; safe to assume some did. Migration is idempotent. |
38+
| 6 | OAuth refresh dedup | Restore singleton promise guard | `useDataSync` fires on mount and on reconnect. Duplicate refreshes are wasted work and could race the token store. |
39+
40+
---
41+
42+
## Scope
43+
44+
**In:**
45+
- Restore `get_advertised_prompts_for_grants` and `get_advertised_resources_for_grants` in `facade.rs`
46+
- Wire `list_prompts`, `get_prompt`, `list_resources`, `read_resource` to use advertised methods
47+
- Restore `call_tool` hard-cut guard + `list_inactive_discovery_tools` redirect flow
48+
- Restore `get_prompt` and `read_resource` hard-cut redirect helpers
49+
- Restore `structured_content` passthrough on `call_tool`
50+
- Restore `collision_client_id` field on `WorkspaceNeedsBinding` domain event
51+
- Update frontend to render collision messaging using the restored field
52+
- Restore `key_migration.rs` + startup call
53+
- Restore OAuth refresh singleton in `gateway.ts`
54+
- Commit the already-applied `list_tools` fix
55+
- `pnpm validate` clean after each phase
56+
57+
**Out:**
58+
59+
| Item | Reason |
60+
| ---- | ------ |
61+
| dev delta cherry-picks (10 commits) | Tracked in [`dev-rebased-post-port-completion.md`](./dev-rebased-post-port-completion.md) Phase 1 — begin after this doc's phases are complete |
62+
| lib/api invoke → apiCall migration | Same — tracked in `dev-rebased-post-port-completion.md` Phase 2 |
63+
| Feature-by-feature verification pass | Same — tracked in `dev-rebased-post-port-completion.md` Phase 3 |
64+
| `should_prompt` on `SpaceDefault` reversal | Intentional improvement on `dev-rebased` — not a regression |
65+
| Roots-capability probe ordering fix | Intentional improvement on `dev-rebased` — not a regression |
66+
| New meta-tools (diagnose_view, etc.) | Not regressed — present on both branches |
67+
68+
---
69+
70+
## Architecture
71+
72+
### Surfacing model (restored)
73+
74+
```
75+
tools/list, list_prompts, list_resources
76+
└─ get_advertised_*_for_grants()
77+
├─ get_invokable_*_for_grants() ← full grant-resolved set (invoke ACL)
78+
└─ resolve_surfaced_feature_ids() ← surfaced flag filter
79+
→ intersection: only surfaced features hit the wire
80+
81+
call_tool / get_prompt / read_resource
82+
├─ if name starts with mcpmux_ → meta tool dispatch (no filter)
83+
├─ if name in advertised set → route to backend (existing behavior)
84+
├─ if name in invokable set → hard-cut: return use_invoke_tool redirect hint
85+
└─ else → tool not found
86+
```
87+
88+
The `list_inactive_discovery_tools` call in the hard-cut path lets the redirect hint name the bundle that contains the tool, so the agent can bind it if needed.
89+
90+
### WorkspaceNeedsBinding event shape (restored)
91+
92+
```rust
93+
WorkspaceNeedsBinding {
94+
client_id: String,
95+
session_id: String,
96+
space_id: Uuid,
97+
workspace_root: String,
98+
collision_client_id: Option<String>, // restored — existing client already bound this root
99+
space_locked: bool, // new — Space picker should be locked in binding sheet
100+
}
101+
```
102+
103+
---
104+
105+
## Phases
106+
107+
### Phase 1 — Surfacing: list_* paths (~2 hours)
108+
109+
- Commit the already-applied `list_tools` → `get_advertised_tools_for_grants` fix
110+
- Add `get_advertised_prompts_for_grants` to `facade.rs` (mirror the tools method: `get_prompts_for_grants` + `resolve_surfaced_feature_ids` filter)
111+
- Add `get_advertised_resources_for_grants` to `facade.rs` (same pattern)
112+
- Wire `list_prompts` (~941) to `get_advertised_prompts_for_grants`
113+
- Wire `get_prompt` (~998) to `get_advertised_prompts_for_grants` for the allow-list check
114+
- Wire `list_resources` (~1055) to `get_advertised_resources_for_grants`
115+
- Wire `read_resource` (~1109) to `get_advertised_resources_for_grants` for the allow-list check
116+
- `cargo check -p mcpmux-gateway` clean
117+
118+
**Outcome:** `tools/list`, `prompts/list`, and `resources/list` all return only surfaced features. Cursor shows ~5 meta tools. A fresh MCP session connecting to the gateway no longer dumps thousands of entries.
119+
120+
---
121+
122+
### Phase 2 — Surfacing: call_* hard-cut guards (~3 hours)
123+
124+
Restore the hard-cut logic that was in `dev`'s `call_tool`, `get_prompt`, and `read_resource` handlers. The call_tool guard structure on `dev` (lines ~773–865) is the canonical reference.
125+
126+
- Restore `call_tool` hard-cut block:
127+
- after advertised-set hit → route normally (unchanged)
128+
- if in invokable but not surfaced → `list_inactive_discovery_tools` lookup, return `use_invoke_tool` redirect hint with `bindable_feature_set_id`
129+
- else → tool not found error
130+
- Restore `structured_content` assignment on `call_tool` result (`result.structured_content = tool_result.structured_content`)
131+
- Restore `get_prompt` hard-cut: if prompt not in advertised set but in invokable → `format_direct_fetch_prompt_redirect` response
132+
- Restore `read_resource` hard-cut: if resource not in advertised set but readable → `format_direct_read_resource_redirect` response (or equivalent)
133+
- Verify `routing.rs` still has the `format_direct_*_redirect` helpers (subagent noted they're orphaned but still present)
134+
- `cargo check -p mcpmux-gateway` clean; `pnpm test:rust:unit` passes
135+
136+
**Outcome:** Calling a non-surfaced tool directly returns a structured hint pointing to `mcpmux_invoke_tool` and the bindable FeatureSet. The hard-cut is testable by invoking a tool from a connected server that isn't in the active FeatureSet's surfaced list. `structured_content` passes through correctly on tools that return it.
137+
138+
---
139+
140+
### Phase 3 — Data integrity regressions (~1.5 hours)
141+
142+
Three changes that could silently corrupt state or break user data:
143+
144+
**Credential key migration:**
145+
- Restore `crates/mcpmux-storage/src/key_migration.rs` from `dev` (`git show dev:crates/mcpmux-storage/src/key_migration.rs`)
146+
- Restore the `migrate_file_key_encrypted_fields` startup call in `apps/desktop/src-tauri/src/state/mod.rs` at its original location (~line 90)
147+
- Confirm migration is idempotent (no double-migration risk on a fresh install)
148+
149+
**WorkspaceNeedsBinding event:**
150+
- Add `collision_client_id: Option<String>` back to `DomainEvent::WorkspaceNeedsBinding` in `crates/mcpmux-core/src/domain/event.rs`
151+
- Restore population of `collision_client_id` in `handler.rs` at the `emit_domain_event` call site — requires looking up whether any existing session for this space already holds the root
152+
- Update `WorkspaceBindingSheet.tsx` to render collision badge when `collision_client_id` is `Some` (existing logic, just re-wire)
153+
- Update `useWorkspaceEvents.ts` to pass both `collision_client_id` and `space_locked` through to the sheet
154+
155+
**OAuth refresh dedup:**
156+
- Restore module-level singleton promise in `apps/desktop/src/lib/api/gateway.ts` that coalesces concurrent `refreshOAuthTokensOnStartup` calls to a single in-flight promise
157+
158+
**Outcome:** Tauri `cargo check` and `pnpm typecheck` both pass. A simulated double-mount of `useDataSync` produces one OAuth refresh call in the network tab. Opening the app after a keychain recovery does not permanently lose credential access for users who previously hit the OS-keychain-dismiss fallback path.
159+
160+
---
161+
162+
### Phase 4 — Hand off to post-port completion doc (~ongoing)
163+
164+
After Phases 1–3 are confirmed working (desktop Tauri app loads, gateway serves ~5 meta tools, `pnpm validate` clean), pick up the remaining port work from the existing plan:
165+
166+
- [`dev-rebased-post-port-completion.md`](./dev-rebased-post-port-completion.md) **Phase 1** — audit + cherry-pick 10 `dev`-only commits (meta-tool / server-update fixes)
167+
- [`dev-rebased-post-port-completion.md`](./dev-rebased-post-port-completion.md) **Phase 2** — `lib/api` `invoke` → `apiCall` migration (12 remaining files)
168+
- [`dev-rebased-post-port-completion.md`](./dev-rebased-post-port-completion.md) **Phase 3** — feature-by-feature verification
169+
170+
**Outcome:** `dev-rebased` reaches full feature parity with `dev` tip. Web admin loads cleanly. All verification items in the post-port completion doc are checked off.
171+
172+
---
173+
174+
## Files to create / modify
175+
176+
| Phase | File | Action |
177+
| ----- | ---- | ------ |
178+
| 1 | `crates/mcpmux-gateway/src/mcp/handler.rs` | Commit list_tools fix; wire list_prompts, get_prompt, list_resources, read_resource to advertised methods |
179+
| 1 | `crates/mcpmux-gateway/src/pool/features/facade.rs` | Add `get_advertised_prompts_for_grants`, `get_advertised_resources_for_grants` |
180+
| 2 | `crates/mcpmux-gateway/src/mcp/handler.rs` | Restore call_tool hard-cut block, structured_content, get_prompt redirect, read_resource redirect |
181+
| 2 | `crates/mcpmux-gateway/src/pool/routing.rs` | Verify format_direct_fetch_prompt_redirect / format_direct_read_resource_redirect present; restore if missing |
182+
| 3 | `crates/mcpmux-storage/src/key_migration.rs` | Restore from `dev` (deleted on dev-rebased) |
183+
| 3 | `crates/mcpmux-storage/src/lib.rs` | Re-export `key_migration` module |
184+
| 3 | `apps/desktop/src-tauri/src/state/mod.rs` | Restore `migrate_file_key_encrypted_fields` startup call |
185+
| 3 | `crates/mcpmux-core/src/domain/event.rs` | Add `collision_client_id: Option<String>` back to `WorkspaceNeedsBinding` |
186+
| 3 | `crates/mcpmux-gateway/src/mcp/handler.rs` | Restore `collision_client_id` population at `emit_domain_event` site |
187+
| 3 | `apps/desktop/src/features/workspaces/WorkspaceBindingSheet.tsx` | Re-wire collision badge to `collision_client_id` |
188+
| 3 | `apps/desktop/src/hooks/useWorkspaceEvents.ts` | Pass both `collision_client_id` and `space_locked` to sheet |
189+
| 3 | `apps/desktop/src/lib/api/gateway.ts` | Restore singleton dedup promise for `refreshOAuthTokensOnStartup` |
190+
191+
---
192+
193+
## Key files referenced
194+
195+
| File | Note |
196+
| ---- | ---- |
197+
| [`crates/mcpmux-gateway/src/mcp/handler.rs`](../../crates/mcpmux-gateway/src/mcp/handler.rs) | Primary regression surface — all list/call/fetch MCP paths |
198+
| [`crates/mcpmux-gateway/src/pool/features/facade.rs`](../../crates/mcpmux-gateway/src/pool/features/facade.rs) | Missing `get_advertised_prompts/resources_for_grants`; `get_advertised_tools_for_grants` already exists here |
199+
| [`crates/mcpmux-gateway/src/pool/routing.rs`](../../crates/mcpmux-gateway/src/pool/routing.rs) | `format_direct_*_redirect` helpers — orphaned but still present; needed by Phase 2 |
200+
| [`crates/mcpmux-core/src/domain/event.rs`](../../crates/mcpmux-core/src/domain/event.rs) | `WorkspaceNeedsBinding` shape — missing `collision_client_id` |
201+
| [`apps/desktop/src/features/workspaces/WorkspaceBindingSheet.tsx`](../../apps/desktop/src/features/workspaces/WorkspaceBindingSheet.tsx) | Collision badge/copy wired to `collision_client_id` (~209–215) |
202+
| [`apps/desktop/src/hooks/useWorkspaceEvents.ts`](../../apps/desktop/src/hooks/useWorkspaceEvents.ts) | Event handler that needs to forward both event fields (~36) |
203+
| [`apps/desktop/src-tauri/src/state/mod.rs`](../../apps/desktop/src-tauri/src/state/mod.rs) | Startup init — migration call goes at ~line 90 |
204+
| [`apps/desktop/src/lib/api/gateway.ts`](../../apps/desktop/src/lib/api/gateway.ts) | OAuth refresh dedup singleton — currently inlined without coalescing guard |
205+
206+
---
207+
208+
## Related documentation
209+
210+
- [`docs/planning/dev-rebased-post-port-completion.md`](./dev-rebased-post-port-completion.md) — Phase 4 of this doc hands off to Phases 1–3 of this doc
211+
- [`docs/planning/dev-to-main-port.md`](./dev-to-main-port.md) — the 8-phase port that produced the dev-rebased branch
212+
- [`docs/planning/web-admin-completion.md`](./web-admin-completion.md) — web admin gaps; unblocked after lib/api migration (post-port Phase 2)

0 commit comments

Comments
 (0)