Skip to content

Commit fa32a41

Browse files
committed
fix(daemon): recover od mcp install from bare-od fallback via IPC discovery
`od mcp install <agent>` degrades to a broken `command: 'od'` launch spec whenever resolveMcpLaunchSpec() can't reach the running daemon's /api/mcp/install-info. In practice this triggers on every plain terminal invocation against a packaged install, since OD_SIDECAR_IPC_PATH is only ever stamped into the packaged app's own spawned children, and the packaged daemon binds an ephemeral port rather than the legacy 7456 default. On macOS/Linux the resulting bare `od` then collides with the system octal-dump utility (#5120/#5219, docs-only fix, root cause untouched). resolveDaemonUrl() gains an opt-in allowConventionalIpcDiscovery option: when OD_SIDECAR_IPC_PATH is absent, it also probes the conventional per-release-channel sidecar socket path(s) (stable first, deterministic even when multiple channels are live). Default stays false so every other `od` subcommand keeps its existing behavior unchanged; only resolveMcpLaunchSpec() opts in. Probed responses are also required to be loopback URLs before being trusted, since the JSON-IPC protocol has no responder-identity check and this is the first caller to persist a probed response's command/args into a coding agent's config. The remaining fallback (daemon truly unreachable) now self-reinvokes via process.execPath + process.argv[1], matching the existing pattern used elsewhere in cli.ts, instead of the bare 'od' string. Fixes #6424
1 parent fe1231e commit fa32a41

3 files changed

Lines changed: 366 additions & 17 deletions

File tree

apps/daemon/src/cli.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,12 +1402,12 @@ function positionalArgs(argv, stringFlags = new Set()) {
14021402
return out;
14031403
}
14041404

1405-
async function cliDaemonUrl(flags) {
1406-
return resolveDaemonUrl({ flagUrl: flags?.['daemon-url'] });
1405+
async function cliDaemonUrl(flags, resolveOptions = {}) {
1406+
return resolveDaemonUrl({ flagUrl: flags?.['daemon-url'], ...resolveOptions });
14071407
}
14081408

1409-
async function cliDaemonBaseUrl(flags) {
1410-
return (await cliDaemonUrl(flags)).replace(/\/$/, '');
1409+
async function cliDaemonBaseUrl(flags, resolveOptions = {}) {
1410+
return (await cliDaemonUrl(flags, resolveOptions)).replace(/\/$/, '');
14111411
}
14121412

14131413
function printMediaHelp() {
@@ -1575,8 +1575,14 @@ To register this server into a coding agent's own config automatically:
15751575
// Codex one-click install use), so every install path configures byte-for-
15761576
// byte the same command. Falls back to a minimal `od mcp --daemon-url`
15771577
// spec when the daemon is unreachable.
1578+
//
1579+
// Opts into conventional per-channel IPC discovery (daemon-url.ts) — unlike
1580+
// every other `od` subcommand, a bare terminal invocation of `mcp install
1581+
// <agent>` against a packaged install has no other way to find the running
1582+
// daemon, since OD_SIDECAR_IPC_PATH is only ever stamped into the packaged
1583+
// app's own spawned children. See issue #6424.
15781584
async function resolveMcpLaunchSpec(flags) {
1579-
const base = await cliDaemonBaseUrl(flags);
1585+
const base = await cliDaemonBaseUrl(flags, { allowConventionalIpcDiscovery: true });
15801586
try {
15811587
const resp = await fetch(`${base}/api/mcp/install-info`);
15821588
if (resp.ok) {
@@ -1592,9 +1598,14 @@ async function resolveMcpLaunchSpec(flags) {
15921598
} catch {
15931599
// daemon not running / unreachable — fall through to the minimal spec
15941600
}
1601+
// Bare `od` collides with the system octal-dump utility on macOS/Linux
1602+
// (issue #5120). Self-reinvoke via the absolute interpreter + entry-point
1603+
// paths this very process was launched with instead — the same pattern
1604+
// already used for plugin-validate above — so the degraded spec still
1605+
// resolves to a real executable even when discovery keeps failing.
15951606
return {
1596-
command: 'od',
1597-
args: ['mcp', '--daemon-url', base],
1607+
command: process.execPath,
1608+
args: [process.argv[1], 'mcp', '--daemon-url', base],
15981609
env: {},
15991610
};
16001611
}

apps/daemon/src/daemon-url.ts

Lines changed: 138 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,18 @@ import { spawn } from "node:child_process";
22
import path from "node:path";
33
import { fileURLToPath } from "node:url";
44
import {
5+
RELEASE_CHANNELS,
6+
releaseNamespace,
7+
type ReleasePlatform,
8+
} from "@open-design/release";
9+
import {
10+
APP_KEYS,
11+
OPEN_DESIGN_SIDECAR_CONTRACT,
512
SIDECAR_ENV,
613
SIDECAR_MESSAGES,
714
type DaemonStatusSnapshot,
815
} from "@open-design/sidecar-proto";
9-
import { requestJsonIpc } from "@open-design/sidecar";
16+
import { requestJsonIpc, resolveAppIpcPath } from "@open-design/sidecar";
1017

1118
export const DEFAULT_DAEMON_URL = "http://127.0.0.1:7456";
1219
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../../..");
@@ -18,16 +25,32 @@ export interface ResolveDaemonUrlOptions {
1825
env?: NodeJS.ProcessEnv;
1926
/** IPC discovery timeout. Short by default so an absent daemon does not stall CLI startup. */
2027
timeoutMs?: number;
28+
/**
29+
* Opt-in: when `OD_SIDECAR_IPC_PATH` is absent, also probe the
30+
* conventional per-release-channel sidecar socket path(s) (see
31+
* `conventionalIpcSocketPaths`) before falling through to `tools-dev`
32+
* discovery and the legacy default. Defaults to `false` so every existing
33+
* `resolveDaemonUrl` caller (media generate, project list, run start, …)
34+
* keeps its current behavior unchanged — an unrelated already-running
35+
* packaged daemon must not silently start answering for commands that
36+
* never asked for daemon auto-discovery beyond an explicit IPC path.
37+
* `resolveMcpLaunchSpec` (cli.ts, `od mcp install <agent>`) is the one
38+
* caller that opts in: a plain terminal invocation of that command has no
39+
* other way to find a packaged install's daemon. See issue #6424.
40+
*/
41+
allowConventionalIpcDiscovery?: boolean;
2142
}
2243

2344
/**
2445
* Resolve the daemon HTTP base URL for `od` client commands.
2546
*
2647
* Spawn order: explicit `--daemon-url` flag, `OD_DAEMON_URL` env, then
2748
* a STATUS roundtrip to the concrete sidecar IPC endpoint supplied by
28-
* the lifecycle owner in `OD_SIDECAR_IPC_PATH`, then the default
29-
* `tools-dev status --json` runtime. Falls back to the legacy default
30-
* for direct `od` launches that do not run as a sidecar.
49+
* the lifecycle owner in `OD_SIDECAR_IPC_PATH` (optionally falling back to
50+
* the conventional per-channel socket path(s) when that env var is absent —
51+
* see `allowConventionalIpcDiscovery` / `conventionalIpcSocketPaths`), then
52+
* the default `tools-dev status --json` runtime. Falls back to the legacy
53+
* default for direct `od` launches that do not run as a sidecar.
3154
*/
3255
export async function resolveDaemonUrl(
3356
options: ResolveDaemonUrlOptions = {},
@@ -37,7 +60,11 @@ export async function resolveDaemonUrl(
3760
if (flagUrl != null && flagUrl.length > 0) return flagUrl;
3861
const envUrl = env.OD_DAEMON_URL;
3962
if (envUrl != null && envUrl.length > 0) return envUrl;
40-
const discovered = await discoverDaemonUrlFromIpc(env, options.timeoutMs ?? 800);
63+
const discovered = await discoverDaemonUrlFromIpc(
64+
env,
65+
options.timeoutMs ?? 800,
66+
options.allowConventionalIpcDiscovery ?? false,
67+
);
4168
if (discovered != null) return discovered;
4269
const toolsDevUrl = await discoverDaemonUrlFromToolsDev(env, options.timeoutMs ?? 800);
4370
if (toolsDevUrl != null) return toolsDevUrl;
@@ -47,21 +74,124 @@ export async function resolveDaemonUrl(
4774
async function discoverDaemonUrlFromIpc(
4875
env: NodeJS.ProcessEnv,
4976
timeoutMs: number,
77+
allowConventionalIpcDiscovery: boolean,
78+
): Promise<string | null> {
79+
const explicitSocketPath = env[SIDECAR_ENV.IPC_PATH];
80+
if (explicitSocketPath != null && explicitSocketPath.length > 0) {
81+
return await probeIpcSocket(explicitSocketPath, timeoutMs);
82+
}
83+
if (!allowConventionalIpcDiscovery) return null;
84+
// `OD_SIDECAR_IPC_PATH` is only ever stamped by the packaged app into its
85+
// OWN spawned child processes (see apps/packaged/src/sidecars.ts) — an
86+
// ordinary user terminal never has it set. Without this fallback, `od mcp
87+
// install <agent>` run from a plain shell against a running packaged
88+
// install can never find `/api/mcp/install-info` and always degrades to
89+
// the broken bare-`od` launch spec in cli.ts's resolveMcpLaunchSpec, even
90+
// though a live daemon is reachable at a well-known socket path. Gated
91+
// behind `allowConventionalIpcDiscovery` so every other `od` subcommand
92+
// keeps requiring an explicit IPC path / --daemon-url instead of silently
93+
// latching onto an unrelated already-running packaged daemon. See #6424.
94+
const candidates = conventionalIpcSocketPaths(env);
95+
if (candidates.length === 0) return null;
96+
const results = await Promise.allSettled(
97+
candidates.map((socketPath) => probeIpcSocket(socketPath, timeoutMs)),
98+
);
99+
for (const result of results) {
100+
if (result.status === "fulfilled" && result.value != null) return result.value;
101+
}
102+
return null;
103+
}
104+
105+
async function probeIpcSocket(
106+
socketPath: string,
107+
timeoutMs: number,
50108
): Promise<string | null> {
51-
const socketPath = env[SIDECAR_ENV.IPC_PATH];
52-
if (socketPath == null || socketPath.length === 0) return null;
53109
try {
54110
const status = await requestJsonIpc<DaemonStatusSnapshot>(
55111
socketPath,
56112
{ type: SIDECAR_MESSAGES.STATUS },
57113
{ timeoutMs },
58114
);
59-
return status?.url ?? null;
115+
const url = status?.url ?? null;
116+
return url != null && isLoopbackHttpUrl(url) ? url : null;
60117
} catch {
61118
return null;
62119
}
63120
}
64121

122+
/**
123+
* Whether `url` is an http(s) URL whose host is loopback. The JSON-IPC
124+
* protocol has no responder-identity check (no peer-credential/uid
125+
* verification, no shared secret — see #6424 discussion), so a STATUS
126+
* response is not proof the daemon is who it claims to be. This does not
127+
* close that gap — the sidecar IPC endpoint itself would need to add
128+
* authentication for that — but it does stop a responder on a predictable
129+
* socket/pipe path from redirecting daemon discovery off-host, which is the
130+
* one part of "what can this URL make us do" this module can cheaply rule
131+
* out before the caller `fetch()`s `/api/mcp/install-info` from it and
132+
* potentially persists whatever `command`/`args` come back into a coding
133+
* agent's config.
134+
*/
135+
function isLoopbackHttpUrl(url: string): boolean {
136+
let parsed: URL;
137+
try {
138+
parsed = new URL(url);
139+
} catch {
140+
return false;
141+
}
142+
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return false;
143+
return parsed.hostname === "127.0.0.1" || parsed.hostname === "::1" || parsed.hostname === "localhost";
144+
}
145+
146+
/**
147+
* Conventional per-release-channel sidecar IPC socket paths, stable-channel
148+
* first. Bounded to the product's own known channels (`@open-design/release`)
149+
* so an absent daemon still fails fast — probes run concurrently via
150+
* `Promise.allSettled` in the caller, so the wall-clock cost stays bounded by
151+
* a single timeout regardless of candidate count, not their sum.
152+
*
153+
* Honors an explicit `OD_SIDECAR_NAMESPACE` when present (cheap extra check,
154+
* mirrors the explicit-namespace precedence `resolveNamespace` already uses
155+
* elsewhere); otherwise derives the current platform's namespace suffix from
156+
* `process.platform`/`process.arch` and tries every known channel.
157+
*/
158+
function conventionalIpcSocketPaths(env: NodeJS.ProcessEnv): string[] {
159+
const explicitNamespace = env[SIDECAR_ENV.NAMESPACE];
160+
if (explicitNamespace != null && explicitNamespace.length > 0) {
161+
return [
162+
resolveAppIpcPath({
163+
app: APP_KEYS.DAEMON,
164+
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
165+
env,
166+
namespace: explicitNamespace,
167+
}),
168+
];
169+
}
170+
const platform: ReleasePlatform =
171+
process.platform === "darwin"
172+
? process.arch === "arm64"
173+
? "mac"
174+
: "macIntel"
175+
: process.platform === "win32"
176+
? "win"
177+
: "linux";
178+
const orderedChannels = [
179+
RELEASE_CHANNELS.STABLE,
180+
RELEASE_CHANNELS.BETA,
181+
RELEASE_CHANNELS.BETAS,
182+
RELEASE_CHANNELS.PRERELEASE,
183+
RELEASE_CHANNELS.PREVIEW,
184+
] as const;
185+
return orderedChannels.map((channel) =>
186+
resolveAppIpcPath({
187+
app: APP_KEYS.DAEMON,
188+
contract: OPEN_DESIGN_SIDECAR_CONTRACT,
189+
env,
190+
namespace: releaseNamespace(channel, platform),
191+
}),
192+
);
193+
}
194+
65195
async function discoverDaemonUrlFromToolsDev(
66196
env: NodeJS.ProcessEnv,
67197
timeoutMs: number,

0 commit comments

Comments
 (0)