Skip to content

Commit 812e0bb

Browse files
committed
Quiet noisy dev-run logs: fix a settings init race and right-size MCP token rejections
The dev-run terminal carried warnings that were either a real ordering bug or mislevelled noise. Two fixes plus a doc: - **Fix pre-init settings reads.** The main window now gates its children render on `settingsReady`, which flips only after `initReactiveSettings()` + `initSettingsApplier()` resolve. `FilePane` / `BriefList` read `getSetting()` synchronously at mount; before this they mounted before the store loaded and got registry defaults, logging `getSetting(...) called before settings were initialized` and (worse) risking a default getting pushed to the backend as if chosen (the `ai.provider` "off" trap). The flag flips in `finally` so a settings-load failure still mounts on defaults rather than a blank window. Guardrail added to `(main)/CLAUDE.md`. - **Demote the bearer-token rejection** (`mcp::server`) from WARN to INFO: a token-gated call arriving without the token is expected protocol flow (the client gets the friendly "here's where the token lives" response and retries), not an anomaly. Kept at INFO, not debug, so a non-Cmdr process probing the gate stays visible in terminal and error bundles. The fail-closed "no token configured" case stays WARN. - **Document MCP auth** in `docs/tooling/mcp.md` (which tools are token-gated, and that `mcp-call.sh` sends the token automatically) so agents reach for the right path on the first try instead of tripping the rejection.
1 parent b84d687 commit 812e0bb

4 files changed

Lines changed: 61 additions & 10 deletions

File tree

apps/desktop/src-tauri/src/mcp/server.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,13 @@ pub fn validate_token(headers: &HeaderMap) -> Result<(), ()> {
604604
.unwrap_or("");
605605

606606
if presented.is_empty() || !constant_time_eq(presented.as_bytes(), expected.as_bytes()) {
607-
log::warn!(target: "mcp::server", "MCP: rejected request with missing/invalid bearer token");
607+
// INFO, not WARN: a token-gated call arriving without the token is expected protocol
608+
// flow, not an anomaly. Agents reach a gated tool (`set_setting`, auto-confirm
609+
// delete/move/copy, `dialog confirm`) before reading `<data_dir>/mcp.token`, get the
610+
// friendly `auto_confirm_token_required_response` telling them where the token lives,
611+
// and retry. We still log it (not `debug`) so the security-relevant case — a local
612+
// non-Cmdr process probing the gate — stays visible in terminal and error bundles.
613+
log::info!(target: "mcp::server", "MCP: rejected request with missing/invalid bearer token");
608614
return Err(());
609615
}
610616

apps/desktop/src/routes/(main)/+layout.svelte

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@
6363
6464
const { children }: Props = $props()
6565
66+
// Gates the children render until settings are loaded and applied. File-explorer
67+
// components (FilePane, BriefList, ...) read getSetting() synchronously at mount; if
68+
// they mount before the store finishes loading they get the registry default, which
69+
// logs a pre-init-read warning and, worse, can push a default back to the backend as
70+
// if the user chose it (this is how a pre-init read of `ai.provider` could quietly set
71+
// AI to "off"). Mounting the page only once settings are ready closes that race for the
72+
// whole subtree. See settings-store.ts § getSetting and (main)/CLAUDE.md § Gotchas.
73+
let settingsReady = $state(false)
74+
6675
// State for crash report dialog
6776
let showCrashReportDialog = $state(false)
6877
let pendingCrashReport = $state<CrashReport | null>(null)
@@ -178,14 +187,24 @@
178187
179188
// Initialize all async setup
180189
void (async () => {
181-
// Initialize reactive settings for UI components
182-
await initReactiveSettings()
183-
184-
// Initialize settings and apply them to CSS variables
185-
await initSettingsApplier()
186-
187-
// Subscribe to volume-tint settings so FilePane bg updates live
188-
initVolumeTints()
190+
try {
191+
// Initialize reactive settings for UI components
192+
await initReactiveSettings()
193+
194+
// Initialize settings and apply them to CSS variables
195+
await initSettingsApplier()
196+
197+
// Subscribe to volume-tint settings so FilePane bg updates live
198+
initVolumeTints()
199+
} finally {
200+
// Settings are now loaded, applied to CSS, and volume tints are wired, so the
201+
// file-explorer subtree can mount without any pre-init getSetting() reads (and
202+
// without a flash of default git chip / volume tint). In `finally` so a settings
203+
// load failure (which logs its own error in initializeSettings) still mounts the
204+
// page on registry defaults rather than leaving a blank window. Everything below
205+
// is independent of the children mounting, so it keeps running in the background.
206+
settingsReady = true
207+
}
189208
190209
// Log once whether this WebKit supports the modern CSS we lean on
191210
// (`color-mix()`). Old Safari versions on macOS 12 Monterey fall
@@ -314,7 +333,9 @@
314333
<MtpPermissionDialog onClose={closePermissionDialog} onRetry={retryPermissionConnection} />
315334
{/if}
316335
<div class="page-wrapper">
317-
{@render children?.()}
336+
{#if settingsReady}
337+
{@render children?.()}
338+
{/if}
318339
</div>
319340

320341
<style>

apps/desktop/src/routes/(main)/CLAUDE.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ through the bus would pollute the `CommandId` union with dev-only ids for zero g
119119

120120
## Gotchas
121121

122+
- **`+layout.svelte` gates the children render on `settingsReady`.** The page subtree mounts only after
123+
`initReactiveSettings()` + `initSettingsApplier()` resolve in `onMount`. File-explorer components read `getSetting()`
124+
synchronously at mount; without the gate they mount before the store loads and get registry defaults (a logged
125+
pre-init read, and a default that can be pushed back to the backend as if chosen — see `settings-store.ts` §
126+
`getSetting`). Don't remove the `{#if settingsReady}` wrapper, and don't move setting-reading work ahead of the flag.
122127
- **`+page.svelte` is >900 lines, flagged by `file-length`.** Don't pile new state into the page — extract another
123128
`setupXxxListeners(ctx)` module like `mcp-listeners.ts`. The dispatch core (`command-dispatch.ts`) is small (~170
124129
lines): new commands get a handler in the relevant `command-handlers/` family module, NOT a branch in the core. If a

docs/tooling/mcp.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,25 @@ connects independently and works regardless of the session's MCP state.
7575
./scripts/mcp-call.sh --list-tools
7676
```
7777

78+
## Authentication (token-gated tools)
79+
80+
Most tools (resource reads, nav, search, dialog-prompting ops) need no auth. A bearer token is required ONLY for the
81+
calls that bypass the user's confirmation dialog: `set_setting`, `delete` / `move` / `copy` with `autoConfirm: true`,
82+
and `dialog` with `action: "confirm"`. Calling one of these without the token logs
83+
`MCP: rejected request with missing/invalid bearer token` and returns a JSON-RPC error pointing at the token file. To
84+
get it right on the first try:
85+
86+
- **`./scripts/mcp-call.sh` handles the token for you.** It reads `<data_dir>/mcp.token` (or `CMDR_MCP_TOKEN`) and sends
87+
`Authorization: Bearer <token>` on every request. Prefer it for any gated call: `./scripts/mcp-call.sh set_setting …`
88+
just works.
89+
- **The wired-up `mcp__cmdr-*__*` tools can't add headers**, so gated ops through them fail unless you start Cmdr with
90+
`CMDR_MCP_TOKEN` exported and add `"headers": { "Authorization": "Bearer ${CMDR_MCP_TOKEN}" }` to the server entry in
91+
`.mcp.json`. Without that setup, route gated ops through `mcp-call.sh` instead. Read-only / nav / search tools work
92+
through the wired-up tools regardless.
93+
94+
Full token model (why a per-launch CSPRNG token, the `CMDR_MCP_TOKEN` override, why rejection is HTTP 200 not 401):
95+
`apps/desktop/src-tauri/src/mcp/DETAILS.md` § Authentication.
96+
7897
## Action-tool ack contract
7998

8099
Action tools (`copy`, `move`, `delete`, `mkdir`, `mkfile`, `select`, `toggle_hidden`, `set_view_mode`, `sort`, `tab`,

0 commit comments

Comments
 (0)