Skip to content

Commit 47fcd17

Browse files
committed
fix(daemon): start daemon on MCP request, harden idle park and config watcher lifecycle
registration.ts: replace passive getDaemonStatus with ensureDaemon({implicit:true}) so MCP tool calls can bring the daemon up, reducing profile contention (one shared daemon vs stdio-per-window). Fall through to stdio on failure so MCP still works. auth.js: resolveIdleParkMs now returns the default on malformed input instead of disabling parking (a typo — "30m", "1_800_000" — used to pin ~300-600MB Chromium
1 parent 08274e3 commit 47fcd17

11 files changed

Lines changed: 1640 additions & 23 deletions

File tree

packages/extension/src/mcp/registration.ts

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,47 @@ export function registerMcpProvider(
7474
try {
7575
// ── HTTP definition branch (D-03) ──────────────────────────────
7676
const settings = getSettings();
77+
// Declared out here so the stdio fallback below can tell "the daemon is
78+
// serving us" from "we fell through" — the credential branch depends on it.
79+
let daemon: { port: number; bearerToken: string } | null = null;
7780
if (settings.mcp.useDaemon && daemonManager) {
78-
const daemonStatus = await daemonManager.getDaemonStatus();
79-
if (daemonStatus.healthy && daemonStatus.port != null && daemonStatus.bearerToken != null) {
81+
// START the daemon, don't just look for one. A passive getDaemonStatus()
82+
// meant an MCP request could never bring the daemon up, so unless the user
83+
// had opened the dashboard (or run a formula command) EVERY session fell
84+
// through to the stdio branch below — and that branch is the one that
85+
// contends for the shared .chrome-profile, once per VS Code window. One
86+
// shared daemon is the configuration that avoids Chrome exit 21, so
87+
// starting it here makes profile collisions LESS likely, not more.
88+
//
89+
// implicit:true is load-bearing: it keeps the _userStopped latch (a
90+
// deliberate Stop must not be undone by the next tool call) and the
91+
// SPAWN_SUPPRESS_MS window that closed the runaway-spawn OOM documented
92+
// in daemon-manager.ts. Never pass implicit:false from here.
93+
//
94+
// Timeout matches ensureDaemon's own default and SPAWN_SUPPRESS_MS. It is
95+
// a ceiling, not a cost: a healthy daemon returns on the first poll.
96+
// ponytail: on failure we fall through to stdio rather than returning [],
97+
// so MCP still works when the daemon can't start. That leaves a narrow
98+
// window where a late-arriving daemon and the stdio server both drive the
99+
// same profile; give the fallback its own AIRTABLE_PROFILE_DIR if that
100+
// ever shows up in the wild.
101+
try {
102+
const info = await daemonManager.ensureDaemon({ implicit: true, timeoutMs: 15_000 });
103+
if (info?.port != null && info.bearerToken != null) {
104+
daemon = { port: info.port, bearerToken: info.bearerToken };
105+
}
106+
} catch {
107+
daemon = null;
108+
}
109+
if (daemon) {
80110
const httpDef = createHttpDefinition(
81-
`http://127.0.0.1:${daemonStatus.port}/mcp`,
82-
`Bearer ${daemonStatus.bearerToken}`,
111+
`http://127.0.0.1:${daemon.port}/mcp`,
112+
`Bearer ${daemon.bearerToken}`,
83113
);
84114
if (httpDef) return [httpDef];
115+
// A definition we could not construct is not a served daemon — fall
116+
// through to stdio AND let the credential branch know it must inject.
117+
daemon = null;
85118
}
86119
}
87120

@@ -124,11 +157,17 @@ export function registerMcpProvider(
124157
if (authManager) {
125158
const authMode = settings.mcp.authMode;
126159
const isCredMode = authMode === 'byo' || authMode === 'direct-login';
127-
if (isCredMode && !settings.mcp.useDaemon) {
128-
// Pure stdio mode (no daemon): byo/direct-login REQUIRE credentials
129-
// (no browser), and env is the only channel VS Code gives a stdio
130-
// spawn. In daemon mode the daemon endpoint (/daemon/auth-credentials)
131-
// is the SINGLE credential channel — never duplicate secrets into env.
160+
if (isCredMode && !daemon) {
161+
// Gate on "did we actually fall through to stdio", NOT on the useDaemon
162+
// SETTING. Those used to be the same thing; they no longer are. useDaemon
163+
// defaults true, so with the setting-based test a byo/direct-login user
164+
// who reached this branch — daemon failed to start, or they pressed Stop —
165+
// got a stdio server with NO credentials and no browser to fall back on:
166+
// *_CREDENTIALS_MISSING on every tool call. `daemon` is null here exactly
167+
// when the daemon is not serving us, which is precisely when env is once
168+
// again the only credential channel VS Code gives a stdio spawn.
169+
// When the daemon IS serving, /daemon/auth-credentials stays the SINGLE
170+
// channel — secrets are still never duplicated into env on that path.
132171
const credEnv = await authManager.getCredentialsEnv(authMode);
133172
if (credEnv) Object.assign(env, credEnv);
134173
} else if (!isCredMode && settings.auth.loginMode === 'auto') {

packages/mcp-server/src/auth.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,13 @@ function resolveIdleParkMs() {
7777
const trimmed = String(raw).trim();
7878
if (trimmed === '') return 30 * 60_000;
7979
const parsed = Number(trimmed);
80-
if (!Number.isInteger(parsed) || parsed <= 0) return 0;
80+
// Only an explicit, well-formed 0 (or negative) disables parking. Garbage used to
81+
// land here too, so a typo — "30m", "1_800_000", a stray quote — silently pinned a
82+
// ~300-600MB Chromium tree resident forever, and the env var that was meant to tune
83+
// parking switched it off instead. Unparseable now means "I didn't understand you,
84+
// keep the default", which is the safe direction to fail.
85+
if (!Number.isInteger(parsed)) return 30 * 60_000;
86+
if (parsed <= 0) return 0;
8187
return parsed;
8288
}
8389

@@ -836,7 +842,15 @@ export class AirtableAuth {
836842
// Without a cookie header, cookie-only park would leave isLoggedIn true
837843
// with no HTTP path and force a full Chromium relaunch next call.
838844
if (!this._credentials?.cookieHeader) {
839-
console.error('[auth] idle park aborted — no cookie snapshot available; keeping browser up.');
845+
// Bailing here does NOT strand the browser, though it reads like it does:
846+
// scheduleIdlePark() nulled _parkTimer before calling us, so nothing is armed
847+
// at this instant. The re-arm comes from _subscribeBusy — we are inside
848+
// runBarrier, so the scheduler is busy now and fires a busy→idle edge the
849+
// moment this returns, which calls scheduleIdlePark() again. Retry is
850+
// automatic; an explicit re-arm here is redundant, not defensive.
851+
// Pinned by "a cookie-less park RE-ARMS the timer" in test-auth-idle-park —
852+
// that test fails only if BOTH paths are gone, which is the real invariant.
853+
console.error('[auth] idle park deferred — no cookie snapshot available; will retry when idle again.');
840854
return;
841855
}
842856

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* One-slot handoff from a tool handler to the express /mcp route.
3+
*
4+
* A `manage_daemon` stop/restart arriving over /mcp CANNOT shut the process down
5+
* from inside the handler. Traced through the bundled SDK
6+
* (@modelcontextprotocol/sdk/dist/cjs/server/webStandardStreamableHttp.js):
7+
* with `enableJsonResponse`, `handlePostRequest` returns a Promise that is only
8+
* ever settled by `resolveJson`, which lives in `_streamMapping`. `close()`
9+
* runs each entry's `cleanup()` — which merely DELETES the map entry, never
10+
* calling `resolveJson` — then clears the map. The still-pending tool call then
11+
* returns, `send()` finds the request id in `_requestToStreamMapping` (close()
12+
* does not clear that one) but no stream, and throws
13+
* `No connection established for request ID: …`. The handleRequest promise stays
14+
* unsettled forever, `res` never ends, and `httpServer.close()` blocks on the
15+
* open socket. A `setTimeout` inside the handler is the same race with extra
16+
* steps: the handler has no signal that its JSON was flushed.
17+
*
18+
* So the handler only STATES the intent here; server.js consumes it from the
19+
* response's own 'finish' event, which fires strictly after the body is handed
20+
* to the OS. Express owns the exit, the handler owns the answer.
21+
*
22+
* ponytail: a module-global single slot, not a per-request map — an exit is
23+
* process-wide by definition and there is exactly one daemon per process, so
24+
* two concurrent stop requests collapsing into one exit is the correct outcome.
25+
*/
26+
27+
/** @typedef {{ action: 'stop'|'restart', by?: string, reason?: string|null }} DaemonExitIntent */
28+
29+
/** @type {DaemonExitIntent|null} */
30+
let pending = null;
31+
32+
/** @param {DaemonExitIntent} intent */
33+
export function requestDaemonExit(intent) {
34+
pending = intent;
35+
}
36+
37+
/** Read-and-clear. @returns {DaemonExitIntent|null} */
38+
export function takeDaemonExit() {
39+
const intent = pending;
40+
pending = null;
41+
return intent;
42+
}
43+
44+
/** Non-consuming peek — for tests and for a handler that wants to report what it just staged. */
45+
export function peekDaemonExit() {
46+
return pending;
47+
}
48+
49+
/** Drop a staged intent (e.g. the action failed validation after staging). */
50+
export function clearDaemonExit() {
51+
pending = null;
52+
}

packages/mcp-server/src/daemon/launcher.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { startDaemonServer } from './server.js';
1010
import { acquire, getLockfilePath, isStale, read, release, replace } from './lockfile.js';
1111
import { ensureToken, getTokenPath, readToken } from './token.js';
1212
import { readTunnelSettings, writeTunnelSettings, getTunnelProvider } from './tunnel-providers/index.js';
13+
import { clearStopSentinel } from './stop-sentinel.js';
1314

1415
const __dirname = dirname(fileURLToPath(import.meta.url));
1516
const require = createRequire(import.meta.url);
@@ -60,7 +61,15 @@ async function probeHealth(record, options = {}) {
6061
}
6162
}
6263

63-
async function adminRequest(record, path, options) {
64+
/**
65+
* Authenticated loopback call to a daemon's own /daemon/* route, using the
66+
* bearer from its lockfile. Exported so `manage_daemon` can drive tunnel and
67+
* token administration through the SERVER's routes instead of reimplementing
68+
* them — `activeTunnel` and `currentToken` are closure state inside
69+
* startDaemonServer, and anything that mutates them from outside leaves the
70+
* running server holding a stale handle.
71+
*/
72+
export async function adminRequest(record, path, options) {
6473
const response = await fetch(`http://127.0.0.1:${record.port}${path}`, {
6574
method: options.method,
6675
headers: {
@@ -268,6 +277,12 @@ export async function startDaemon(options = {}) {
268277
continue;
269278
}
270279

280+
// A daemon is starting on purpose, so any "the user stopped this deliberately"
281+
// marker is now spent. Clearing it HERE — on the one path every intentional
282+
// start goes through — is what stops a stale sentinel from wedging startup
283+
// forever; see stop-sentinel.js.
284+
clearStopSentinel({ configDir });
285+
271286
let server;
272287
let lspChild = null;
273288
let activeTunnel = null;

0 commit comments

Comments
 (0)