Skip to content

Commit 68fd85a

Browse files
claude[bot]claude
andauthored
fix(cli): build \os dev\'s MCP connect hint from the resolved origin, not the listen socket (#16812)
* fix(cli): build `os dev`'s MCP connect hint from the resolved origin, not the listen socket `os dev` printed two MCP addresses in one boot output. The ready banner's `MCP:` row goes through `resolveAuthBaseUrl` (`OS_AUTH_URL` -> legacy `BETTER_AUTH_URL` -> `OS_BASE_URL` -> `http://localhost:<port>`); the `MCP server - connect a coding agent` block built its base from the serve child's `objectstack:listening` url, i.e. the socket the child bound. Under `OS_AUTH_URL=https://localhost:4443` with `dev -p 4001` the two disagreed, and the block's `Connect` line is a command the reader pastes: it registered an MCP entry against an origin discovery never advertises and a TLS proxy never exposes. The hint now resolves through the same function `serve` calls, with the port the child ACTUALLY bound - so the no-variable-set case still answers `http://localhost:<boundPort>` (auto-shifted port included) from the resolver's own tail rather than from a second fallback. When the chain yields no parseable origin the block is omitted rather than reprinting the address the banner just refused to print. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * chore(changeset): patch @objectstack/cli for the dev MCP connect-hint origin fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 30b0990 commit 68fd85a

3 files changed

Lines changed: 303 additions & 9 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os dev`'s MCP connect hint is built from the origin the deployment is REACHABLE on, not from the socket the serve child bound.
6+
7+
A dev boot printed two MCP addresses. The ready banner's `➜ MCP:` row goes through `resolveAuthBaseUrl``OS_AUTH_URL` → legacy `BETTER_AUTH_URL``OS_BASE_URL``http://localhost:<port>` — while the `🤖 MCP server — connect a coding agent` block below it derived its base from the child's `objectstack:listening` `url`, which is the bound socket by construction. `OS_AUTH_URL` never entered that expression, so anything sitting in front of the app split the two apart: measured on `objectstack dev -p 4001` under `OS_AUTH_URL=https://localhost:4443` behind a TLS reverse proxy, the banner said `https://localhost:4443/…` and the block said `http://localhost:4001/…` in the same output.
8+
9+
That block's `Connect` line is a command the reader pastes, so the wrong origin was not cosmetic: `claude mcp add` registered an entry against an address discovery never advertises and, behind the proxy, nothing can reach — and the two rows disagreeing made the correct one look like the typo.
10+
11+
- **One resolver, not two.** The hint now calls the same `resolveAuthBaseUrl` the banner's call site does, with the port the child ACTUALLY bound. The precedence chain is not restated anywhere in `dev`.
12+
- **The ordinary local boot is unchanged, by the resolver's own tail.** With none of the three variables set the chain answers `http://localhost:<boundPort>` — including dev's auto-shift (`3000` busy → `3001`) and an ephemeral port — so the local case needs no second fallback and cannot be broken by a canonical origin being hardcoded in front of it.
13+
- **An unusable base URL now prints no block at all.** When the chain yields nothing parseable — a set-but-empty `OS_AUTH_URL=`, which does not fall through to the rest of the chain, or a value with no scheme — the banner's rule is to print paths with no origin and name the variable that fixes it. A `claude mcp add` line has no paths-only form, so the block is omitted instead of reprinting, on the same screen, the exact address the banner just refused to print.
14+
15+
`resolveAuthBaseUrl` itself is untouched, including its set-but-empty behaviour; the `Endpoint` / `Skill` / `Connect` wording is unchanged.
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// framework#16734 — `os dev`'s MCP connect hint names the origin an MCP client
4+
// can REACH, and the ready banner printed in the same boot agrees with it.
5+
//
6+
// ## Why both printers are driven into ONE buffer
7+
//
8+
// The defect was never visible inside either printer. `os dev` prints two MCP
9+
// addresses from two processes: the serve child's ready banner (`➜ MCP:`,
10+
// `printServerReady`, resolved through `resolveAuthBaseUrl`) and the parent's
11+
// `🤖 MCP server — connect a coding agent` block, which used to be built from
12+
// the child's `objectstack:listening` `url` — the socket it BOUND. Each was
13+
// self-consistent; the bug was that one boot output carried both, saying
14+
// `https://localhost:4443/…` on one row and `http://localhost:4001/…` on the
15+
// next. A pin that reads only the block cannot see that, so every case below
16+
// captures BOTH printers into one ordered buffer and asserts on the whole
17+
// thing — `console.error` (the banner) and `console.log` (the hint) in call
18+
// order, exactly as a terminal renders them.
19+
//
20+
// ## The chain is not restated here
21+
//
22+
// No case below writes the precedence order down as a literal. Both printers
23+
// go through `resolveAuthBaseUrl`, whose own pins live in
24+
// `serve-auth-base-url-diagnostic.test.ts`; what this file asserts is that the
25+
// two printers consult THAT function and therefore cannot disagree.
26+
//
27+
// The parent resolves in its own process, which is sound because it hands the
28+
// child its own `process.env` (plus internal keys) and both run
29+
// `dotenvFlow.config({ node_env: 'development' })` over the same files before
30+
// any lookup. The last case pins the half of that a runtime assertion cannot:
31+
// that dev's child-env literal never sets a chain variable.
32+
33+
import { readFileSync } from 'node:fs';
34+
import { dirname, resolve } from 'node:path';
35+
import { fileURLToPath } from 'node:url';
36+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
37+
import { printMcpConnectHint } from './dev.js';
38+
import { AUTH_BASE_URL_ENV_NAMES, resolveAuthBaseUrl } from './serve.js';
39+
import { printServerReady, type ServerReadyOptions } from '../utils/format.js';
40+
41+
const HERE = dirname(fileURLToPath(import.meta.url));
42+
const DEV_SOURCE = readFileSync(resolve(HERE, 'dev.ts'), 'utf8');
43+
44+
const bannerOpts: Omit<ServerReadyOptions, 'externalBaseOrigin'> = {
45+
configFile: 'objectstack.config.ts',
46+
isDev: true,
47+
pluginCount: 1,
48+
uiEnabled: true,
49+
consolePath: '/_console',
50+
mcpEnabled: true,
51+
};
52+
53+
/** Every absolute MCP address anywhere in the captured output, deduplicated. */
54+
const mcpOrigins = (output: string): string[] =>
55+
[...new Set([...output.matchAll(/(https?:\/\/[^\s]+?)\/api\/v1\/mcp/g)].map((m) => m[1]))];
56+
57+
describe('os dev MCP connect hint — origin (#16734)', () => {
58+
const saved: Partial<Record<(typeof AUTH_BASE_URL_ENV_NAMES)[number], string | undefined>> = {};
59+
60+
let lines: string[];
61+
let errSpy: ReturnType<typeof vi.spyOn>;
62+
let logSpy: ReturnType<typeof vi.spyOn>;
63+
64+
const record = (...args: unknown[]) => {
65+
lines.push(args.join(' ').replace(/\u001b\[[0-9;]*m/g, ''));
66+
};
67+
68+
beforeEach(() => {
69+
for (const n of AUTH_BASE_URL_ENV_NAMES) {
70+
saved[n] = process.env[n];
71+
delete process.env[n];
72+
}
73+
lines = [];
74+
errSpy = vi.spyOn(console, 'error').mockImplementation(record);
75+
logSpy = vi.spyOn(console, 'log').mockImplementation(record);
76+
});
77+
78+
afterEach(() => {
79+
errSpy.mockRestore();
80+
logSpy.mockRestore();
81+
for (const n of AUTH_BASE_URL_ENV_NAMES) {
82+
if (saved[n] === undefined) delete process.env[n];
83+
else process.env[n] = saved[n];
84+
}
85+
});
86+
87+
/**
88+
* One `os dev` boot, both printers, in the order a reader sees them: the
89+
* child's banner (its own call site's expression, verbatim) and then the
90+
* parent's connect hint, both told the port the server ACTUALLY bound.
91+
*/
92+
const boot = (boundPort: number, name = 'hotcrm') => {
93+
printServerReady({ ...bannerOpts, externalBaseOrigin: resolveAuthBaseUrl(boundPort).baseOrigin });
94+
printMcpConnectHint({ boundPort, name });
95+
return lines.join('\n');
96+
};
97+
98+
// ── Acceptance 1 ────────────────────────────────────────────────────────
99+
describe('with OS_AUTH_URL set, all three hint lines and the banner row agree', () => {
100+
it('reproduces #16530: dev -p 4001 behind OS_AUTH_URL=https://localhost:4443', () => {
101+
process.env.OS_AUTH_URL = 'https://localhost:4443';
102+
103+
const output = boot(4001);
104+
105+
// The banner row and each of the three block lines, from ONE capture.
106+
expect(output).toContain('MCP: https://localhost:4443/api/v1/mcp');
107+
expect(output).toContain('Endpoint https://localhost:4443/api/v1/mcp');
108+
expect(output).toContain('Skill https://localhost:4443/api/v1/mcp/skill');
109+
expect(output).toContain(
110+
'Connect claude mcp add --transport http hotcrm https://localhost:4443/api/v1/mcp',
111+
);
112+
113+
// The reported symptom, stated as the absence it is: the bound socket
114+
// must not appear anywhere in the boot output that advertises MCP.
115+
expect(output).not.toContain('localhost:4001');
116+
// And the agreement itself, independent of which rows were named above.
117+
expect(mcpOrigins(output)).toEqual(['https://localhost:4443']);
118+
});
119+
120+
it('follows the deployment onto its public origin, port and all', () => {
121+
process.env.OS_AUTH_URL = 'https://app.example.com';
122+
expect(mcpOrigins(boot(3000))).toEqual(['https://app.example.com']);
123+
124+
lines.length = 0;
125+
process.env.OS_AUTH_URL = 'https://app.example.com:8443';
126+
expect(mcpOrigins(boot(3000))).toEqual(['https://app.example.com:8443']);
127+
});
128+
129+
it('honours the rest of the chain — the legacy name, then OS_BASE_URL', () => {
130+
process.env.BETTER_AUTH_URL = 'https://legacy.example.com';
131+
process.env.OS_BASE_URL = 'https://base.example.com';
132+
expect(mcpOrigins(boot(3000))).toEqual(['https://legacy.example.com']);
133+
134+
lines.length = 0;
135+
delete process.env.BETTER_AUTH_URL;
136+
expect(mcpOrigins(boot(3000))).toEqual(['https://base.example.com']);
137+
});
138+
});
139+
140+
// ── Acceptance 2 — the negative control ─────────────────────────────────
141+
describe('with OS_AUTH_URL unset, the hint still prints the LISTEN origin', () => {
142+
it('names the bound port on an ordinary local boot', () => {
143+
const output = boot(3000, 'my-app');
144+
145+
expect(output).toContain('MCP: http://localhost:3000/api/v1/mcp');
146+
expect(output).toContain('Endpoint http://localhost:3000/api/v1/mcp');
147+
expect(output).toContain('Skill http://localhost:3000/api/v1/mcp/skill');
148+
expect(output).toContain(
149+
'Connect claude mcp add --transport http my-app http://localhost:3000/api/v1/mcp',
150+
);
151+
expect(mcpOrigins(output)).toEqual(['http://localhost:3000']);
152+
});
153+
154+
it("follows dev's auto-shifted port — 3000 busy, bound 3001", () => {
155+
// The case the surrounding code exists to handle. A fix that reached for
156+
// a canonical origin instead of the resolver would print :3000 here, or
157+
// nothing at all; both are worse than the behaviour being repaired.
158+
const output = boot(3001, 'my-app');
159+
160+
expect(output).toContain('Endpoint http://localhost:3001/api/v1/mcp');
161+
expect(output).toContain(
162+
'Connect claude mcp add --transport http my-app http://localhost:3001/api/v1/mcp',
163+
);
164+
expect(output).not.toContain('3000');
165+
expect(mcpOrigins(output)).toEqual(['http://localhost:3001']);
166+
});
167+
168+
it('names an ephemeral bound port, never the 0 that was requested', () => {
169+
expect(mcpOrigins(boot(45064))).toEqual(['http://localhost:45064']);
170+
});
171+
});
172+
173+
// ── Acceptance 3 — the unusable-value cases stay the resolver's, unchanged ─
174+
describe('an unusable base URL prints no connect command, and no guess', () => {
175+
it('set-but-empty OS_AUTH_URL: banner prints paths only, the hint prints nothing', () => {
176+
// Empty is not unset — the chain stops there, so neither OS_BASE_URL nor
177+
// the localhost tail is consulted. `resolveAuthBaseUrl` reports that as
178+
// `baseOrigin: null` (its own pins own that behaviour); what this asserts
179+
// is that BOTH printers obey it. A `claude mcp add` line has no
180+
// paths-only form, so the block is omitted rather than fabricated.
181+
process.env.OS_AUTH_URL = '';
182+
process.env.OS_BASE_URL = 'https://never-consulted.example.com';
183+
184+
const output = boot(3000);
185+
186+
expect(output).toContain('/api/v1/mcp');
187+
expect(mcpOrigins(output)).toEqual([]);
188+
expect(output).not.toContain('http://localhost:3000');
189+
expect(output).not.toContain('never-consulted');
190+
expect(output).not.toContain('claude mcp add');
191+
expect(output).toContain('OS_AUTH_URL');
192+
});
193+
194+
it('a value with no scheme is not smuggled in as an origin either', () => {
195+
process.env.OS_AUTH_URL = 'app.example.com';
196+
197+
const output = boot(3000);
198+
199+
expect(mcpOrigins(output)).toEqual([]);
200+
expect(output).not.toContain('claude mcp add');
201+
expect(output).not.toContain('http://localhost:3000');
202+
expect(output).not.toContain('app.example.com/api/v1/mcp');
203+
});
204+
});
205+
206+
// ── What only the source can say ────────────────────────────────────────
207+
describe('the call site feeds the printer the bound port, and nothing else', () => {
208+
it('hands `printMcpConnectHint` the ACTUALLY BOUND port', () => {
209+
expect(DEV_SOURCE).toContain('printMcpConnectHint({ boundPort: actual,');
210+
});
211+
212+
it('builds no address out of the listening message any more', () => {
213+
// The defect in one line: `base` came from `msg.url`, the bound socket.
214+
expect(DEV_SOURCE).not.toMatch(/msg\.url/);
215+
});
216+
217+
it('never sets a base-URL chain variable in the child env it spawns', () => {
218+
// The parent resolves the chain in ITS process and the child resolves it
219+
// again in its own; the two answers are the same value only while dev
220+
// passes these variables through untouched.
221+
const start = DEV_SOURCE.indexOf('const localEnv: NodeJS.ProcessEnv = {');
222+
expect(start).toBeGreaterThan(-1);
223+
const childEnvLiteral = DEV_SOURCE.slice(start, DEV_SOURCE.indexOf('\n };', start));
224+
for (const name of AUTH_BASE_URL_ENV_NAMES) {
225+
expect(childEnvLiteral).not.toContain(name);
226+
}
227+
});
228+
});
229+
});

packages/cli/src/commands/dev.ts

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
2424
// no range, no reader, no wording; a second copy of the bound is exactly what
2525
// #12620 and #12662 protected against.
2626
import { describePortSource, parseRequestedPort, formatInvalidPortNotice } from '../utils/port-contract.js';
27+
// The auth base-URL precedence chain, borrowed from the command that owns it
28+
// (#16734). ⛔ `dev` declares no chain of its own — see printMcpConnectHint.
29+
import { resolveAuthBaseUrl } from './serve.js';
2730
import type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime';
2831

2932
/**
@@ -63,6 +66,57 @@ export async function resolveDevDatabase(opts: {
6366
});
6467
}
6568

69+
/**
70+
* The `os dev` MCP connect hint — the three lines a reader PASTES (#3167,
71+
* #16734).
72+
*
73+
* ## The origin these lines carry is the one an MCP client can REACH
74+
*
75+
* `Connect` is a COMMAND, not documentation: whatever address it names is the
76+
* address `claude mcp add` registers. So the block has to print the origin the
77+
* deployment is reachable on — which is exactly what the ready banner the
78+
* serve child prints in the same boot output already carries, resolved through
79+
* {@link resolveAuthBaseUrl} (`OS_AUTH_URL` → legacy `BETTER_AUTH_URL` →
80+
* `OS_BASE_URL` → `http://localhost:<port>`).
81+
*
82+
* It used to be built from the child's `objectstack:listening` `url`, which is
83+
* the socket the child BOUND, by construction. `OS_AUTH_URL` never entered
84+
* that expression, so behind anything at all — a TLS reverse proxy, a compose
85+
* stack that only `expose`s the app port — one boot printed two origins.
86+
* MEASURED with `objectstack dev -p 4001` under
87+
* `OS_AUTH_URL=https://localhost:4443`: the banner's `➜ MCP:` row said
88+
* `https://localhost:4443/…` and this block said `http://localhost:4001/…`.
89+
* Pasting the latter registers an MCP entry against an origin discovery never
90+
* advertises and the proxy never exposes — and makes the correct row look
91+
* like the typo.
92+
*
93+
* ⛔ The chain is NOT re-derived here: this calls the same function `serve`
94+
* calls, with the port the child ACTUALLY bound. That is also why the ordinary
95+
* local case needs no fallback of its own — with none of the three variables
96+
* set, the resolver's own built-in tail answers `http://localhost:<boundPort>`,
97+
* dev's auto-shifted port (3000 busy → 3001) included.
98+
*
99+
* ## `null` means print nothing, never guess
100+
*
101+
* `baseOrigin` is `null` when the chain produced a value that will not parse —
102+
* a set-but-empty `OS_AUTH_URL=` (which does NOT fall through to the rest of
103+
* the chain), or a value with no scheme. The banner's rule for that case is to
104+
* print the paths with no origin in front of them and name the variable that
105+
* fixes it. A connect COMMAND has no paths-only form, so the honest output
106+
* here is no block at all: falling back to the bound socket would reprint, on
107+
* the same screen, the exact address the banner just refused to print.
108+
*/
109+
export function printMcpConnectHint(opts: { boundPort: number | string; name: string }): void {
110+
const { baseOrigin } = resolveAuthBaseUrl(opts.boundPort);
111+
if (baseOrigin === null) return;
112+
console.log();
113+
console.log(chalk.cyan(' 🤖 MCP server — connect a coding agent:'));
114+
console.log(` Endpoint ${baseOrigin}/api/v1/mcp`);
115+
console.log(` Skill ${baseOrigin}/api/v1/mcp/skill`);
116+
console.log(chalk.dim(` Connect claude mcp add --transport http ${opts.name} ${baseOrigin}/api/v1/mcp`));
117+
console.log(chalk.dim(' Disable OS_MCP_SERVER_ENABLED=false'));
118+
}
119+
66120
export default class Dev extends Command {
67121
static override description =
68122
'Start development mode — watch sources, rebuild the artifact, and restart the server on change';
@@ -497,15 +551,11 @@ export default class Dev extends Command {
497551
// (OS_MCP_SERVER_ENABLED=false) advertises nothing, mirroring the
498552
// connect-UI / discovery gates that follow the same switch.
499553
if (isMcpServerEnabled()) {
500-
const base =
501-
typeof msg.url === 'string' && msg.url ? msg.url.replace(/\/+$/, '') : `http://localhost:${actual}`;
502-
const name = path.basename(process.cwd()) || 'objectstack';
503-
console.log();
504-
console.log(chalk.cyan(' 🤖 MCP server — connect a coding agent:'));
505-
console.log(` Endpoint ${base}/api/v1/mcp`);
506-
console.log(` Skill ${base}/api/v1/mcp/skill`);
507-
console.log(chalk.dim(` Connect claude mcp add --transport http ${name} ${base}/api/v1/mcp`));
508-
console.log(chalk.dim(' Disable OS_MCP_SERVER_ENABLED=false'));
554+
// ⛔ The ACTUALLY BOUND port, never the child's listen URL: the
555+
// origin these lines carry is resolved from the runtime's own
556+
// precedence chain inside the printer, so the banner the child
557+
// prints and this block cannot name two different deployments.
558+
printMcpConnectHint({ boundPort: actual, name: path.basename(process.cwd()) || 'objectstack' });
509559
}
510560
}
511561
});

0 commit comments

Comments
 (0)