fix(cli): build \os dev\'s MCP connect hint from the resolved origin, not the listen socket - #16812
Conversation
… 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
… origin fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
📓 Docs Drift CheckThis PR changes 1 package(s): 30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8652f200e599c4247aa1a160e3c43a7c27165fba && git checkout 8652f200e599c4247aa1a160e3c43a7c27165fba
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0 b466fc5d90ec4621d2e5ced3bd9b004a92563389 && git checkout -B drift-repro 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0 && git merge --no-ff b466fc5d90ec4621d2e5ced3bd9b004a92563389
node scripts/docs-audit/affected-docs.mjs --json 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0
|
Accepted by the Gate ① — CI36 raw check runs → 33 after latest-per-name collapse: 30 success, 3 skipped, 0 red, 0 pending; Gate ② — clause ②, verified on CONTENT, not spelling
⭐ The one thing that could have rebutted ⇒ the Acceptance — the card's three, read in the DIFF
⭐ The judgement call beyond the card — accepted, and whyWhen the chain yields no parseable origin (set-but-empty ⇒ It is entailed by item 1. If the banner refuses to print an origin, the only way for the block to agree is to print nothing — and a Serial — re-measured at accept time, ⛔ not carried over from dispatch17 open PRs read, 0 empty file lists, 526 distinct files. ⭐ The ablation is why this acceptance is cheapTwo legs with different predicted signatures, and both matched. Reverting the origin to the listen socket reddened the three Docs-drift advisory — answered without widening the PRThe bot listed 30 pages and truncated at 15 rows. The delivery re-derived the list (34 docs, 4 anchors) instead of reading the truncated comment, and:
⇒ 0 of 34 pages edited; the PR is still 3 files. Deferred, ⛔ not filed on the delivery's wordThree observations were declined and argued (a now-readerless Landing: marked ready and routed to the merge queue. ⛔ Not merged outside the queue; ⛔ no governed surface in this diff. Generated by Claude Code |
Fixes #16734
Clause-②: no
os devprinted two MCP addresses in one boot output, from two origin resolvers, and only one of them knew the canonical origin. The ready banner's➜ MCP:row (printServerReady,packages/cli/src/utils/format.ts) goes throughresolveAuthBaseUrl—OS_AUTH_URL→ legacyBETTER_AUTH_URL→OS_BASE_URL→ thehttp://localhost:PORTtail. The🤖 MCP server — connect a coding agentblock inpackages/cli/src/commands/dev.tsbuilt its base from the serve child'sobjectstack:listeningurl, which is the socket the child BOUND, by construction.OS_AUTH_URLnever entered that expression.That block's
Connectline is a command the reader pastes, so the wrong origin is not cosmetic:claude mcp addregisters an entry against an address discovery never advertises and, behind a TLS proxy, nothing can reach — and the two rows disagreeing made the correct one look like the typo.What changed
packages/cli/src/commands/dev.ts— the block moves into an exportedprintMcpConnectHint({ boundPort, name })that resolves its origin throughresolveAuthBaseUrl, the same functionserve's banner call site uses, with the port the child ACTUALLY bound. The precedence chain is not restated anywhere indev.objectstack:listeninghandler now passesboundPort: actualand nothing else; thebaseexpression built frommsg.urlis gone.baseOrigin === null— a set-but-emptyOS_AUTH_URL=, or a value with no scheme) the block is omitted. The banner's documented rule for that case is "print the paths with no origin and name the variable that fixes it, never guess"; aclaude mcp addline has no paths-only form, so the honest output is no block rather than reprinting, on the same screen, the exact address the banner just refused to print.packages/cli/src/commands/dev-mcp-connect-hint-origin.test.ts— new, 11 cases..changeset/dev-mcp-connect-hint-origin.md—@objectstack/cli: patch.packages/cli/src/utils/format.tsis untouched: it is the reference implementation, and nothing in it needed to change. TheEndpoint/Skill/Connectwording is unchanged.start.tsalready carries the rule this PR applies todev, in its own words: "⛔ Do not reintroduce an address here fed from the child'sipcobjectstack:listeningmessage".devwas the one command still doing it.Anchors, re-derived on the current base
The card's line numbers were written against
5e53d73d; base here isdd2184ae25.dd2184ae25dev.ts:504-505— thebaseexpressiondev.ts:500-501dev.ts:503-508— the printed blockdev.ts:503-508format.ts:816— the➜ MCP:rowformat.ts:816验收备注
1. With
OS_AUTH_URLset, all three block lines and the➜ MCP:row agreeAsserted on both rows of one captured boot output, never on the block alone: each case installs one buffer over
console.error(the banner) andconsole.log(the hint), in call order, then drives both printers under one environment and one bound port. The card's own reason — "Two printers that disagree cannot be pinned by reading one of them."The measured repro is a case verbatim:
dev -p 4001underOS_AUTH_URL=https://localhost:4443asserts the banner row,Endpoint,SkillandConnectall carryhttps://localhost:4443, thatlocalhost:4001appears nowhere in the capture, and — independent of which rows were named — that the set of distinct MCP origins in the whole capture is exactly one. Two further cases carry the same assertion onto a public origin (with and without a port) and onto the rest of the chain (BETTER_AUTH_URL, thenOS_BASE_URL).2. With
OS_AUTH_URLunset, the block still prints the LISTEN originThis falls out of the resolver rather than from a second fallback: with none of the three variables set,
resolveAuthBaseUrl's own built-in tail answershttp://localhost:PORTfor the port it is handed, and the hint is handed the port the child actually bound.Three cases: an ordinary local boot on 3000; the auto-shifted port (3000 busy, bound 3001) which additionally asserts
3000appears nowhere in the capture; and an ephemeral bound port (45064), the--port 0shape.That control is measured, not asserted — ablation leg 2 below is exactly the "reach for a canonical origin and ignore the bound port" fix, and these are the cases that catch it.
3. The precedence chain is reused, not reimplemented; the existing pins are unchanged and still green
resolveAuthBaseUrlandformatUnusableAuthBaseUrlDiagnosticare untouched — the diff adds an import of the first and changes no line ofserve.ts. The set-but-empty behaviour keeps its existing owner; what this PR adds is a pin that both printers obey it (the banner prints paths only, the hint prints nothing, andnever-consulted.example.comfrom a setOS_BASE_URLappears in neither).packages/cli/src/commands/serve-auth-base-url-diagnostic.test.ts— 25 passed, unchanged file. Alongside it,serve-banner-external-base-url.test.ts6 passed,serve-bound-port-publication.test.ts21 passed,test/vitest-tiers-partition.test.ts22 passed, and the new file 11 passed — 85 of 85 in one run atb466fc5d90.Reverse verification — two legs, both restored byte-identical
Committed first, mutated on disk, proven on disk (occurrence counts for the removed text and the injected marker, plus
git hash-object), run, then restored withgit checkout HEAD --and re-proven (git diff HEADempty, blob equal to the HEAD blob). Both legs ran with atrap ... EXIT INT TERMrestore.8bb27f19db→a2980ca600OS_AUTH_URL-set cases and both unusable-value cases red; the three unset cases stayed green, as they mustresolveAuthBaseUrl(3000)instead of the bound port8bb27f19db→56d7bfd6e4Restore both times: blob back to
8bb27f19db,git diff HEADempty. Leg 1 shows the pin can see the reported defect; leg 2 shows acceptance 2 is a real control and not decoration. No permanent test file carries either mutation.Resolution note: both printers and the test resolve
./dev.js/./serve.jsto source insidepackages/cli(the unit project transforms TS directly), so neither leg is dist-mediated and no rebuild sits between the mutation and the reading.Checks run
All at
b466fc5d90unless noted. Exit codes captured before any pipe.pnpm --filter '@objectstack/cli^...' build— exit 0 (the dependency closure).pnpm --filter @objectstack/cli exec vitest run --project unit— 186 files, 2555 passed + 6 expected-fail, exit 0. First attempt reported 2 suites failing to collect withpackages/cli is not built (./dist/index.js is absent)— a stated prerequisite, not a finding; both green afterpnpm --filter @objectstack/cli buildand in the full re-run above.pnpm --filter @objectstack/cli typecheck— exit 0 (tsc --noEmitpluscheck:test-typecheck, the test-layer debt ledger held).pnpm lint(eslint . --no-inline-config, the whole repo, no narrowing) — exit 0, 1m22s.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, then reconciled with--ran: 57 derived, 57 run, 0 NOT-MEASURED, 0 UNRUN. 54 exit 0. The other three arePREREQUISITE NOT MET(exit 3, the gates' own distinct code) and say nothing in either direction — each needs a full workspace build, which is CI's run:check:dual-build-cjs-loads,check:i18n-coverage,check:type-check-debt. The first half of that last one,check:type-check-coverage, ran and passed.check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parity,check:scaffold-emission-policy— all exit 0.check:dts-closureneeded a second look: exit 0 during the sweep, exit 1 after the unit tier had run, naming 7 packages with JS-onlydist/and no declarations — none of them in this diff, none of them downstream of it. Rebuilding exactly those 7 restored it to 63 packages swept, 155/155 declarations present, exit 0. Container tree state, not this change.pnpm check:nul-bytesexit 0, plus a direct control-character scan over both edited files (grep -naP) — no hits.STALE TREEagainstorigin/main78bc4ad587, naming.github/workflows/publish-smoke.ymlandscripts/pm/check-skill-line-ratchet.mjs. Re-deriving aftergit fetch origin mainproduced a byte-identical command list. Neither can add a family for these paths — publish-smoke isworkflow_run/schedule-driven with no PR path filter, and the skill ratchet's population isskills/**, which this diff does not touch.Tier placement
Both tiers are derived by
packages/cli/vitest-tiers.ts, and the new file lands in unit (it spawns nothing and boots no kernel). The integration tier's include list names no file this diff touches, so the integration tier is declared to CI.test/vitest-tiers-partition.test.tswas run and is green.Docs drift advisory — answered, 0 pages edited
The bot's Docs Drift Check on this PR truncated its list at 15 rows, so it was re-derived locally rather than read off the comment:
34 docs, 4 anchors, 1 changed package. Provenance differs from the bot's on purpose and both readings are kept: the bot computed on merge commit
8652f200e599and said outright that "That checkout carried uncommitted changes, so the commit above does not fully identify what was read"; this run reportshead b466fc5d90ec,diffBase dd2184ae25,dirty: false, samesinceRef. The anchors areprintMcpConnectHint(symbol),os dev(command),/api/v1/mcpand/api/v1/mcp/skill(routes).The 4 release-owned pages —
content/docs/releases/v13, v14, v16, v17 — were not edited and are not this PR's to edit. Release notes are written centrally at release time; this PR's only input to them is its changeset. For the record, the two rows that actually describe this block (v16.mdxlines 75 and 468 — "theos devbanner prints the MCP endpoint, the agent-skill URL, and a ready-to-pasteclaude mcp addcommand") remain true after this change: all three lines still print, they simply carry the reachable origin now. Nothing to correct there even in a dedicated docs PR.The other 30: verified unaffected, grouped by why. Every one of them was listed through a generic anchor, not through anything this diff altered:
os dev(the command name, which many pages mention in passing) — no flag, no default, no output contract ofos devchanged;/api/v1/mcpor/api/v1/mcp/skill— both are path literals this diff moves between two files and does not change; no route moved, no route was added or removed;printMcpConnectHint— a symbol introduced by this PR, so no page can be naming it.30 rows is what
@objectstack/cliplus the MCP route path always produces; the count is not evidence of breakage, and none of the 30 was edited.Hand-read for the bot's declared blind spot — a page stating the rule by its INPUTS while the diff changed the EMITTER. This change does carry a rule (the connect hint is built from the resolved origin, not the listen socket), so
content/was grepped by hand for that rule's own vocabulary, whether or not the bot listed the page:grep -rn "objectstack:listening|listen origin|bound socket" content/— zero hits. No page anywhere states which origin the hint is built from, or mentions the listen socket or the two-row disagreement. There is no page for this rule to falsify.content/docs/ai/connect-mcp.mdxandcontent/docs/getting-started/build-with-claude-code.mdxare the two pages that print aclaude mcp addline:http://localhost:3000/api/v1/mcplabelled "local dev server" andhttps://your-deployment.example.com/api/v1/mcplabelled "a deployed instance". That is exactly what the fixed hint now emits in each case — the first is the resolver's unset tail, the second is whatOS_AUTH_URLresolves to. The change moves the code toward these pages; before it, only the first form could ever be printed. Verified unaffected, unedited.content/docs/deployment/cli.mdx:45— "The boot banner also prints the app's MCP endpoint (/api/v1/mcp)". It names the PATH and no origin, and it is about the ready banner informat.ts, which this diff does not touch. Still true.MCP:row, which is unconditional and unchanged.Outcome: 0 of 34 pages needed a change, 0 were edited, and this stays a 3-file PR.
Out of scope, noted and not filed
ListeningMessage.urlnow has no reader inpackages/cli/src:devwas its last consumer, andstartdeclines it on purpose. The field is still published and still pinned byserve-bound-port-publication.test.ts, and the sameurlis written to the runtime state file where it is the right value. Dead-ish code, not a defect. Carrier: the next PR touchingpublishBoundPort/ListeningMessage.🤖 Generated with Claude Code
https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Generated by Claude Code