Skip to content

Commit 0c1503b

Browse files
committed
feat(gateway): diagnose set-header assumptions; refresh routing docs
The folder-set constraint rests on two assumptions that can only be checked against live traffic: that mcp-remote expands ${WORKSPACE_FOLDER_PATHS}, and that the active folder is always a member of the resulting set (212/212 in the probe). Both now report failure instead of degrading quietly. Adds warns for an unexpanded template on either header, a pinned root absent from the reported set (the invariant the constraint depends on), an ambiguous multi-folder session, and a refused set_workspace_root call. The candidate store is now idempotent so these audit once per change rather than per request. Also drops unexpanded ${...} entries during parsing. Keeping one would have matched no real folder and turned the membership check into a blanket refusal of every legitimate root, so this closes a real failure mode rather than only reporting it. Docs corrected where they still blamed the Agents window or called ${workspaceFolder}-via-args reliable: it fails 21% overall, and editor windows (29%) are worse than Agents windows (4%). Records the ruled-out alternatives so the first-entry heuristic doesn't get proposed again, and flags that the per-repo installer writes a bearer token into the repo with no gitignore entry. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent efabe48 commit 0c1503b

9 files changed

Lines changed: 243 additions & 40 deletions

crates/mcpmux-gateway/src/mcp/oauth_middleware.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,19 @@ pub async fn mcp_oauth_middleware(
262262
(sid, ws, ws_set)
263263
};
264264
match (&session_id_header, &workspace_header) {
265+
// Neither the editor nor mcp-remote expanded the template, so the
266+
// literal reached us. Distinct from the empty case below: it means
267+
// mcp-remote's own `${ENV}` substitution changed behavior, since today
268+
// it rewrites an unknown variable to an empty string.
269+
(_, Some(ws)) if ws.contains("${") => {
270+
warn!(
271+
trace_id = %trace_id,
272+
session_id = session_id_header.as_deref().unwrap_or("<none>"),
273+
workspace_header = %ws,
274+
"[SessionRoots] X-Mcpmux-Workspace arrived as an unexpanded template — \
275+
pin skipped rather than pinning a literal",
276+
);
277+
}
265278
(_, Some(ws)) if ws.trim().is_empty() => {
266279
warn!(
267280
trace_id = %trace_id,
@@ -303,6 +316,21 @@ pub async fn mcp_oauth_middleware(
303316
// because one candidate cannot be ambiguous. Held across initialize like
304317
// the workspace header, since both arrive before `mcp-session-id`.
305318
match (&session_id_header, &workspace_set_header) {
319+
// The bridge config assumes Cursor leaves `${WORKSPACE_FOLDER_PATHS}`
320+
// alone (it isn't a Cursor variable) so mcp-remote expands it from the
321+
// child environment. This warn is how that assumption reports failure:
322+
// the set is dropped rather than parsed, so routing degrades to the
323+
// pre-set-header behavior instead of misrouting.
324+
(_, Some(set)) if set.contains("${") => {
325+
warn!(
326+
trace_id = %trace_id,
327+
session_id = session_id_header.as_deref().unwrap_or("<none>"),
328+
workspace_set_header = %set,
329+
"[SessionRoots] X-Mcpmux-Workspace-Set arrived unexpanded — no candidate \
330+
set for this session; check that WORKSPACE_FOLDER_PATHS is present in \
331+
the mcp-remote child environment",
332+
);
333+
}
306334
(_, Some(set)) if set.trim().is_empty() => {}
307335
(Some(sid), Some(set)) => {
308336
services.session_roots.set_candidates(sid, set);

crates/mcpmux-gateway/src/services/meta_tools/set_workspace_root.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use async_trait::async_trait;
1414
use mcpmux_core::normalize_workspace_root;
1515
use rmcp::model::CallToolResult;
1616
use serde_json::{json, Value};
17+
use tracing::warn;
1718

1819
use super::meta_tool_common::{emit_tools_list_changed, text_result};
1920
use super::registry::{MetaTool, MetaToolCall, MetaToolError};
@@ -81,6 +82,16 @@ impl MetaTool for SetWorkspaceRootTool {
8182
.session_roots
8283
.get_candidates(session_id)
8384
.unwrap_or_default();
85+
// Expected when an agent guesses; a burst of these against roots
86+
// the user believes are open points at the membership invariant
87+
// (see the audit warn in `session_roots::store_candidates`).
88+
warn!(
89+
%session_id,
90+
client_id = %call.client_id,
91+
requested_root = %normalized,
92+
candidates = ?candidates,
93+
"[meta_tools] set_workspace_root refused — root not open in this window",
94+
);
8495
return Err(MetaToolError::InvalidArgument(format!(
8596
"workspace_root `{normalized}` is not open in this window. \
8697
Declare one of: {}",

crates/mcpmux-gateway/src/services/session_roots.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ pub struct SessionRootsRegistry {
120120
fn parse_candidate_set(raw_set: &str) -> Vec<String> {
121121
let mut parsed: Vec<String> = raw_set
122122
.split(',')
123+
.filter(|entry| !is_unexpanded_variable(entry))
123124
.map(normalize_workspace_root)
124125
.filter(|path| !path.is_empty())
125126
.collect();
@@ -128,6 +129,16 @@ fn parse_candidate_set(raw_set: &str) -> Vec<String> {
128129
parsed
129130
}
130131

132+
/// Whether a header value still carries a `${...}` template, meaning neither
133+
/// the editor nor `mcp-remote` expanded it.
134+
///
135+
/// Such a value must never become a candidate: it can't match any real folder,
136+
/// so it would turn the membership check into a blanket rejection of every
137+
/// legitimate root the caller might declare.
138+
fn is_unexpanded_variable(value: &str) -> bool {
139+
value.contains("${")
140+
}
141+
131142
impl SessionRootsRegistry {
132143
pub fn new() -> Arc<Self> {
133144
Arc::new(Self {
@@ -339,6 +350,18 @@ impl SessionRootsRegistry {
339350
if parsed.is_empty() {
340351
return;
341352
}
353+
// The set header rides every request, so skip the audit and the write
354+
// when nothing changed. Read the comparison into an owned bool so the
355+
// DashMap guard drops before the insert below (see `record_resolution`
356+
// for the self-deadlock this avoids).
357+
let unchanged = self
358+
.candidates
359+
.get(session_id)
360+
.is_some_and(|existing| *existing == parsed);
361+
if unchanged {
362+
return;
363+
}
364+
342365
if let [only] = parsed.as_slice() {
343366
// The active folder is always a member of the set, so a set of one
344367
// names it outright — no guessing involved.
@@ -349,6 +372,41 @@ impl SessionRootsRegistry {
349372
);
350373
self.set_pinned(session_id, only);
351374
}
375+
376+
match self.get_pinned(session_id) {
377+
// Audits the invariant the whole design rests on: the active
378+
// folder was a member of the reported set in 212 of 212 sampled
379+
// multi-folder spawns. If this ever fires, membership is no longer
380+
// safe as a constraint and `is_candidate` will start refusing
381+
// legitimate roots — treat it as a design regression, not noise.
382+
Some(pinned) if !parsed.iter().any(|c| c == &pinned) => {
383+
warn!(
384+
%session_id,
385+
pinned_root = %pinned,
386+
candidates = ?parsed,
387+
"[SessionRoots] pinned root is absent from X-Mcpmux-Workspace-Set — \
388+
membership invariant violated; set_workspace_root may now reject \
389+
valid roots for this session",
390+
);
391+
}
392+
Some(_) => {}
393+
// The active-folder header failed and more than one folder is
394+
// open, so this session cannot be routed without the caller
395+
// declaring which folder it is in. Logged at warn because it is
396+
// the case that costs the user an extra round trip.
397+
None if parsed.len() > 1 => {
398+
warn!(
399+
%session_id,
400+
candidate_count = parsed.len(),
401+
candidates = ?parsed,
402+
"[SessionRoots] no pinned root and multiple folders open — session must \
403+
call mcpmux_set_workspace_root to disambiguate (a per-repo static \
404+
header install avoids this entirely)",
405+
);
406+
}
407+
None => {}
408+
}
409+
352410
self.candidates.insert(session_id.to_string(), parsed);
353411
}
354412

@@ -775,6 +833,26 @@ mod tests {
775833
assert!(parse_candidate_set(" , ").is_empty());
776834
}
777835

836+
#[test]
837+
fn unexpanded_template_never_becomes_a_candidate() {
838+
// If mcp-remote stops expanding the variable, the literal must be
839+
// dropped. Keeping it would match no real folder and so turn the
840+
// membership check into a blanket refusal.
841+
assert!(parse_candidate_set("${WORKSPACE_FOLDER_PATHS}").is_empty());
842+
843+
let reg = SessionRootsRegistry::default();
844+
reg.set_candidates("sess-1", "${WORKSPACE_FOLDER_PATHS}");
845+
assert!(reg.get_candidates("sess-1").is_none());
846+
assert!(
847+
reg.is_candidate("sess-1", CANDIDATES[0]),
848+
"an unexpanded set must leave the session unconstrained, not deny it"
849+
);
850+
851+
// A partially expanded value keeps the real folders and drops the rest.
852+
let mixed = format!("{},${{WORKSPACE_FOLDER_PATHS}}", CANDIDATES[0]);
853+
assert_eq!(parse_candidate_set(&mixed), vec![CANDIDATES[0].to_string()]);
854+
}
855+
778856
#[test]
779857
fn pending_candidates_apply_when_session_id_arrives() {
780858
let reg = SessionRootsRegistry::default();

docs/manual/cursor-workspace-bridge.md

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ Regression check for the global `~/.cursor/mcp.json` bridge (see
66
This path replaces per-repo `.cursor/mcp.json` header installs for Cursor by
77
routing through `npx mcp-remote` with `${workspaceFolder}` in the bridge args.
88

9+
Cursor resolves that variable unreliably (~21% failure, measured), so the bridge
10+
also sends `${WORKSPACE_FOLDER_PATHS}` as `X-Mcpmux-Workspace-Set`. That set is a
11+
constraint on which folder a session may claim, never a way to pick one — see
12+
[Fallback](#fallback) for why inference is off the table.
13+
914
## Prerequisites
1015

1116
- `pnpm dev:admin` (or production McpMux) with gateway on `localhost:45818`.
@@ -48,38 +53,76 @@ Check the McpMux log (macOS:
4853
- `[FeatureSetResolver] resolved via WorkspaceBinding workspace_root=…` matching
4954
each window's folder.
5055

56+
Three warns exist to report that the bridge's assumptions broke. None should
57+
appear in a healthy two-window run:
58+
59+
| Log line | Means |
60+
| --------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
61+
| `X-Mcpmux-Workspace-Set arrived unexpanded` | Cursor mangled `${WORKSPACE_FOLDER_PATHS}` instead of passing it to `mcp-remote`. No candidate set, so routing degrades to pre-set-header behavior. |
62+
| `pinned root is absent from X-Mcpmux-Workspace-Set` | The active folder isn't a member of the reported set. This violates the invariant the constraint rests on; `set_workspace_root` will start refusing valid roots. |
63+
| `no pinned root and multiple folders open` | The 16% case. Expected occasionally; the session needs one `mcpmux_set_workspace_root` call. |
64+
5165
## 4. Bridge flags sanity check
5266

5367
Confirm the generated config includes:
5468

5569
- `--allow-http` (gateway is loopback HTTP, not TLS).
5670
- `--header` with **no space** after the colon:
5771
`X-Mcpmux-Workspace:${workspaceFolder}`.
72+
- `--header X-Mcpmux-Workspace-Set:${WORKSPACE_FOLDER_PATHS}`. Not a Cursor
73+
variable, so it passes through untouched and `mcp-remote` expands it from the
74+
child environment. Carries every folder open in the calling window.
5875
- `Authorization:Bearer ${MCPMUX_API_KEY}` with the key in `env.MCPMUX_API_KEY`.
5976

6077
To verify `mcp-remote` accepts these flags outside Cursor:
6178

6279
```bash
6380
npx -y mcp-remote http://127.0.0.1:45818/mcp --allow-http \
6481
--header "X-Mcpmux-Workspace:/path/to/folder" \
82+
--header "X-Mcpmux-Workspace-Set:/path/to/folder,/path/to/other" \
6583
--header "Authorization:Bearer mcpk_…"
6684
```
6785

6886
The process should stay up and the gateway should log an incoming MCP session.
6987

7088
## Fallback
7189

72-
Cursor's Agents window sometimes spawns the `mcp-remote` bridge without
73-
resolving `${workspaceFolder}`. The gateway then sees
74-
`X-Mcpmux-Workspace` present but empty, skips the pin, and the session
75-
stays at `PendingRoots` (or routes via the full multi-folder `roots/list`).
76-
77-
Look for this log line:
90+
Cursor fails to substitute `${workspaceFolder}` before spawning `mcp-remote` in
91+
roughly 21% of spawns. `mcp-remote` then expands the leftover literal to an
92+
empty string, so the gateway sees `X-Mcpmux-Workspace` present but empty and
93+
skips the pin.
7894

7995
```
8096
[SessionRoots] X-Mcpmux-Workspace present but empty — pin skipped
8197
```
8298

83-
When that happens, use the per-repo install path in
84-
[`workspace-header-routing.md`](./workspace-header-routing.md) section B
85-
(static `X-Mcpmux-Workspace` header in `.cursor/mcp.json`, no variable).
99+
This is **not** an Agents-window problem, despite what earlier revisions of this
100+
doc claimed. A 282-spawn probe measured editor windows failing at 29% and Agents
101+
windows at 4%, across folder counts from zero to five. It's a flaky
102+
substitution, not a surface-specific one.
103+
104+
There is no fallback signal for the active folder. The probe checked all 22
105+
Cursor and VS Code environment variables in the child process:
106+
`CURSOR_WORKSPACE_LABEL` is stale (it names the window that started the
107+
extension host, often a folder not even in the set), `VSCODE_PID` and
108+
`VSCODE_IPC_HOOK` are app-level rather than per-window, `cwd` is always the home
109+
directory, and no `.code-workspace` file exists for ad-hoc multi-root windows.
110+
111+
`WORKSPACE_FOLDER_PATHS` is the one usable signal, and only as a constraint. The
112+
active folder was a member of it in 212 of 212 resolved multi-folder spawns, but
113+
its position identified the active folder in only 70% of them. A 30% misroute
114+
rate would hand one project's credentials to another, so the gateway does not
115+
infer from position. What it does instead:
116+
117+
- **One folder in the set:** pins outright. Unambiguous by construction.
118+
- **No folders:** no workspace to route to; falls through to grants.
119+
- **Two or more:** refuses to guess. The session gets meta tools only until it
120+
calls `mcpmux_set_workspace_root`, which is now validated against the set so a
121+
caller can't declare a folder its window doesn't have open.
122+
123+
To avoid the whole class of problem, use the per-repo install in
124+
[`workspace-header-routing.md`](./workspace-header-routing.md) section B. It
125+
writes a literal path into `.cursor/mcp.json` with no variable to substitute,
126+
which is why it never flakes. Note that it also writes the bearer token into a
127+
file inside the repo and does not add a `.gitignore` entry, so exclude it
128+
yourself before committing.

docs/manual/workspace-header-routing.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@ This avoids maintaining a `.cursor/mcp.json` + `.gitignore` entry in every
2828
repo. The per-repo install path below (sections A–B) remains a supported
2929
fallback when you cannot use `npx`/`mcp-remote`.
3030

31+
**Multi-root windows should prefer the per-repo path.** Cursor fails to
32+
substitute `${workspaceFolder}` in about 21% of bridge spawns, and when the
33+
window has several folders open there is no sound way to recover which one is
34+
active (see [`cursor-workspace-bridge.md`](./cursor-workspace-bridge.md)
35+
Fallback). The per-repo install writes a literal path, so it never flakes.
36+
37+
**Before committing:** the installer writes your gateway bearer token into the
38+
repo-local config and does **not** add a `.gitignore` entry. Exclude the file
39+
yourself, or `git add .` will commit an access key.
40+
3141
## Prerequisites
3242

3343
- `pnpm dev` (desktop app + gateway) running.

docs/planning/aug14-gateway-ops-bugs.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,16 @@ These stay on the branch. They are not the dig targets.
2727

2828
Severity is "how much it hurts today," not "how hard to fix."
2929

30-
### B1. Empty `${workspaceFolder}` header (Agents window)
30+
### B1. Empty `${workspaceFolder}` header
3131

3232
**Severity:** High (wrong or stuck routing)
33-
**Status:** Telemetry shipped; no client-side fix
34-
**Symptom:** Cursor Agents window (and some other spawn paths) send `X-Mcpmux-Workspace` present but empty. `set_pinned` no-ops. Session falls through to multi-folder `roots/list``PendingRoots` or the wrong space.
35-
**Evidence:** 477 empty-header warns in ~40 min of new-binary uptime. Session `896e45f3…` fires it on every `tools/list` / `prompts/list` / `resources/list`.
36-
**Likely cause:** Cursor leaves `${workspaceFolder}` unresolved; `mcp-remote` then treats `${…}` as an env var and substitutes empty.
37-
**Related:** [`cursor-workspace-routing-bridge.md`](./cursor-workspace-routing-bridge.md) Open question (Aug 14). Fallback is per-repo static header in [`cursor-workspace-bridge.md`](../manual/cursor-workspace-bridge.md).
38-
**Decision already made:** no auto-disambiguation, no agent-facing UI, no client workaround this pass. Dig is "when/why + is the warn too loud."
33+
**Status:** Root-caused and bounded (`efabe48`); residual ~16% is inherent
34+
**Symptom:** Cursor sends `X-Mcpmux-Workspace` present but empty. `set_pinned` no-ops. Session falls through to multi-folder `roots/list``PendingRoots` or the wrong space.
35+
**Evidence:** 477 empty-header warns in ~40 min of new-binary uptime. Session `896e45f3…` fires it on every `tools/list` / `prompts/list` / `resources/list`. A later 282-spawn `env-probe` put the substitution failure at 21% overall.
36+
**Cause (confirmed):** Cursor leaves `${workspaceFolder}` unresolved; `mcp-remote` treats `${…}` as an env var and substitutes empty.
37+
**Correction:** this was filed as an Agents-window bug. It isn't. Editor windows fail at 29%, Agents windows at 4%. The `oauth_middleware` warn that blamed the Agents window has been reworded.
38+
**Related:** [`cursor-workspace-routing-bridge.md`](./cursor-workspace-routing-bridge.md) resolved question (Aug 20) and [`resilience-routing-leftovers.md`](./resilience-routing-leftovers.md) item 1. Fully immune path is the per-repo static header in [`cursor-workspace-bridge.md`](../manual/cursor-workspace-bridge.md).
39+
**Decision:** still no auto-disambiguation — a first-entry heuristic on `WORKSPACE_FOLDER_PATHS` would misroute 30% of the time. The window's folder set is now carried as a constraint instead, which bounds `set_workspace_root` rather than guessing.
3940

4041
### B2. Empty-header warn is per-request, not per-session
4142

docs/planning/backend-connection-resilience-test.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Follow-on (`dcc2977`) playbook: [`pool-invalidation-and-session-survival-test.md
2222

2323
**Inbound 404 after gateway rebuild:** process death drops `LocalSessionManager`. `POST /mcp` with a stale `Mcp-Session-Id` is a spec-correct 404. [`mcp-remote`](https://www.npmjs.com/package/mcp-remote) and the [TypeScript SDK](https://github.com/modelcontextprotocol/typescript-sdk/issues/1708) do **not** re-`initialize` on that 404. Do not persist sessions. Recovery is Reload MCP once. `/health` staying 200 means the gateway is up.
2424

25-
**Header pin after reload:** a non-empty `X-Mcpmux-Workspace` is held across initialize and applied when `mcp-session-id` appears. Empty `${workspaceFolder}` is still the Agents-window hole.
25+
**Header pin after reload:** a non-empty `X-Mcpmux-Workspace` is held across initialize and applied when `mcp-session-id` appears. An empty `${workspaceFolder}` is a ~21% flake in Cursor's substitution, not an Agents-window-specific hole (editor windows are the worse offender at 29%); the window's folder set now arrives separately as `X-Mcpmux-Workspace-Set` and collapses the single-folder case. See [`cursor-workspace-routing-bridge.md`](./cursor-workspace-routing-bridge.md).
2626

2727
---
2828

0 commit comments

Comments
 (0)