From 95b95b6f7c6a0448f5b1eb709ad921b525306fab Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 10:51:55 +0900 Subject: [PATCH 1/8] fix(gui): open the local management ingress on a hub instead of the tailnet-bound proxy origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ocx gui` derived the URL from the proxy bind, so a hub whose `hostname` is a Tailscale address opened a browser at that tailnet origin — a page the local browser cannot authenticate against the management plane, while the hub's own loopback management ingress was sitting there unused. Prefer the ingress when `runtimeRole: "hub"` has it enabled; every other topology keeps the previous URL derivation exactly. Co-Authored-By: Claude Fable 5.1 --- src/cli/dispatch.ts | 23 +++++++++++++++++++---- tests/cli/cli-dispatch.test.ts | 23 ++++++++++++++++++++++- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index e85de1c05f..94f0c14766 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -54,6 +54,24 @@ export interface CliDispatchDeps { type CommandRunner = (deps: CliDispatchDeps) => Promise; +/** + * The hub's management ingress is deliberately loopback-only. Prefer it for + * a browser opened on the hub itself: the proxy listener may be restricted to + * a Tailscale address, while the ingress is the local authenticated dashboard. + */ +export function selectDefaultGuiUrl( + config: Pick, + live: Pick | null, + probeHostname: (hostname: string | undefined) => string, +): string { + const ingress = config.runtimeRole === "hub" ? config.hub?.managementIngress : undefined; + if (ingress?.enabled) return `http://localhost:${ingress.port}`; + + const guiHost = probeHostname(live?.hostname ?? config.hostname); + const hostname = guiHost === "127.0.0.1" ? "localhost" : guiHost; + return `http://${hostname}:${live?.port ?? config.port ?? 10100}`; +} + const commandRunners: Record = { init: async () => { const { runInit } = await import("./init"); @@ -535,10 +553,7 @@ const commandRunners: Record = { return 1; } } - // Open the host the proxy actually binds — `localhost` only answers for - // loopback/wildcard binds, not a concrete LAN/IPv6 hostname. - const guiHost = deps.probeHostname(live?.hostname ?? config.hostname); - const guiUrl = `http://${guiHost === "127.0.0.1" ? "localhost" : guiHost}:${live?.port ?? config.port}`; + const guiUrl = selectDefaultGuiUrl(config, live, deps.probeHostname); console.log(`Opening ${guiUrl}`); const { openUrl } = await import("../lib/open-url"); openUrl(guiUrl); diff --git a/tests/cli/cli-dispatch.test.ts b/tests/cli/cli-dispatch.test.ts index f3d102d1b9..ef940dcf83 100644 --- a/tests/cli/cli-dispatch.test.ts +++ b/tests/cli/cli-dispatch.test.ts @@ -1,7 +1,8 @@ import { describe, expect, spyOn, test } from "bun:test"; import { CLI_COMMANDS } from "../../src/cli/registry"; -import { DISPATCH_ALIASES, DISPATCH_COMMANDS, dispatchCommand, resolveDispatchCommand, decideStartWithLiveOwner } from "../../src/cli/dispatch"; +import { DISPATCH_ALIASES, DISPATCH_COMMANDS, dispatchCommand, resolveDispatchCommand, decideStartWithLiveOwner, selectDefaultGuiUrl } from "../../src/cli/dispatch"; import type { CliDispatchDeps } from "../../src/cli/dispatch"; +import type { OcxConfig } from "../../src/types"; import { runGuiCommand } from "../../src/cli/gui"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -652,6 +653,26 @@ describe("GUI command delegation", () => { defaultProvider: "openai", }; + test("opens the loopback management ingress from the hub", () => { + const hubConfig = { + port: 10100, + hostname: "100.76.170.81", + runtimeRole: "hub" as const, + hub: { + managementPublicOrigin: "https://hub.example.test", + managementIngress: { enabled: true as const, port: 10102 }, + }, + } as Pick; + const live = { hostname: "100.76.170.81", port: 10100 }; + + expect(selectDefaultGuiUrl(hubConfig, live, hostname => hostname ?? "127.0.0.1")) + .toBe("http://localhost:10102"); + + const withoutIngress = { ...hubConfig, hub: { managementPublicOrigin: "https://hub.example.test" } }; + expect(selectDefaultGuiUrl(withoutIngress, live, hostname => hostname ?? "127.0.0.1")) + .toBe("http://100.76.170.81:10100"); + }); + test("keeps the default open behavior and requires an explicit pairing origin", async () => { let opens = 0; const deps = { From cdd64f3034417a616cc060f664a1a3ae632b2cd3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:11:10 +0900 Subject: [PATCH 2/8] feat(server): bind a same-port loopback companion on a non-loopback hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `unauthenticatedLoopbackListener.port` becomes optional. `{ "enabled": true }` with no port is the companion form: bind `127.0.0.1:` with the existing loopback policy view. That is the one-port hub topology — remote clients dial `hostname:port` with a credential, local processes dial `127.0.0.1:port` without one — and it is what lets the integrations that hardcode `http://127.0.0.1:` keep working on a tailnet-bound hub whose public address they cannot reach (#4236). The form is legal only when `hostname` is a specific non-loopback, non-wildcard address; on `127.0.0.1`/`localhost`/`0.0.0.0`/`::` the public listener already holds that loopback address. `loopbackCompanionBindError` is the one sentence for that collision, naming the port and both fixes (set a distinct `port`, or drop the listener because a loopback bind already admits local callers). It runs at the write boundary over both `hostname` and the listener — so `ocx config set hostname 127.0.0.1` on a companion host is refused there rather than breaking the next start — and again in `startServer` before any bind, so a hand edit that skipped validation reads the same diagnosis instead of EADDRINUSE from a rolled-back transaction. `effectiveLoopbackListenerPort` joins `isLoopbackHostname` as the single answer to "where do local callers dial", so no reader repeats `?? port`. The startup line distinguishes the two forms: the companion states where local processes go, the ported form keeps today's unauthenticated-surface warning verbatim. The listener transaction, its rollback, the route allowlist and `startServer`'s synchronous window are unchanged: this moves a bind address, not an admission decision. Co-Authored-By: Claude Fable 5.1 --- .../docs/reference/configuration/server.md | 21 +++++- src/codex/loopback-target.ts | 40 ++++++++++++ src/config.ts | 64 +++++++++++++++---- src/server/index.ts | 27 ++++++-- src/types/config.ts | 19 ++++-- 5 files changed, 149 insertions(+), 22 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index 6a3533f10c..4fbbeb9e8e 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -173,9 +173,28 @@ credential. The main listener is untouched — remote callers still need the tok `ocx sync` then writes `base_url = "http://127.0.0.1:10200/v1"` into the managed Codex provider block and omits the auth header, so a directly spawned app-server works without any credential plumbing. -The port is required and must differ from the proxy port. It is never OS-assigned: an ephemeral port +When you set `port`, it must differ from the proxy port. It is never OS-assigned: an ephemeral port would change across restarts while already-running app-servers kept the previous `base_url`. +Omitting `port` selects the **companion** form — the listener binds the proxy port on `127.0.0.1`: + +```json +{ + "hostname": "100.76.170.81", + "port": 10100, + "unauthenticatedLoopbackListener": { "enabled": true } +} +``` + +Remote clients dial `100.76.170.81:10100` with a credential; local processes dial +`127.0.0.1:10100` without one. That is the address every local integration already writes, so +`ocx claude`, Claude Desktop, Cursor and the system-env injection keep working on a host whose +public bind they cannot reach. The companion form is accepted only when `hostname` is a specific +non-loopback, non-wildcard address: on `127.0.0.1`, `localhost` or `0.0.0.0` the public listener +already holds that loopback address, so OpenCodex refuses the pair at write time and at startup +rather than failing the second bind. On those binds you do not need the listener at all — a +loopback bind already admits local callers. + The listener serves only `POST /v1/responses`, its WebSocket upgrade, `POST /v1/responses/compact`, `POST /v1/alpha/search` (the native Codex web-search relay), `GET /v1/models`, and the realtime voice surface: the standalone WebSocket upgrades, WebRTC call creation (`POST /v1/live`, diff --git a/src/codex/loopback-target.ts b/src/codex/loopback-target.ts index 94e81d42c3..1540dfca68 100644 --- a/src/codex/loopback-target.ts +++ b/src/codex/loopback-target.ts @@ -36,6 +36,46 @@ export function isLoopbackHostname(hostname: string | undefined): boolean { ); } +/** + * Bind scope again, from the other side: a wildcard listener already answers on 127.0.0.1. + * + * This matters only for the port-less "companion" loopback listener. On `0.0.0.0`/`::` the + * public socket owns loopback on that port, so a companion bound to the same port would + * collide; on a specific non-loopback address (a tailnet or LAN IP) 127.0.0.1 is free. + */ +export function isWildcardHostname(hostname: string | undefined): boolean { + const normalized = (hostname ?? "").trim().toLowerCase().replace(/\.$/, ""); + return normalized === "0.0.0.0" || normalized === "::" || normalized === "[::]" || normalized === "*"; +} + +/** + * Is `{ enabled: true }` with no port legal for this bind address? + * + * The companion form binds `127.0.0.1:`, which is exactly the one-port hub shape: + * remote clients dial `hostname:port`, local processes dial `127.0.0.1:port`, and every + * hardcoded `http://127.0.0.1:` integration works with no rewriting. It is legal + * only when the public listener is NOT already holding that loopback address. + */ +export function loopbackCompanionAllowed(hostname: string | undefined): boolean { + return !isLoopbackHostname(hostname) && !isWildcardHostname(hostname); +} + +/** + * The port local callers reach the unauthenticated listener on, or null when it is off. + * + * One resolver for every reader (#4236): an enabled listener with no `port` is the companion + * form and answers on the public port. Callers must not repeat `?? port` — the day the default + * changes, a forgotten site points a client config at a closed socket. + */ +export function effectiveLoopbackListenerPort( + config: Pick | undefined, + publicPort: number, +): number | null { + const listener = config?.unauthenticatedLoopbackListener; + if (!listener?.enabled) return null; + return listener.port ?? publicPort; +} + export function shouldInjectApiAuthHeader( config: Pick | undefined, ): boolean { diff --git a/src/config.ts b/src/config.ts index bda3b8a8ae..3fc48312c8 100644 --- a/src/config.ts +++ b/src/config.ts @@ -46,6 +46,7 @@ import { MAIN_CODEX_ACCOUNT_NAMESPACE_TARGET, } from "./codex/account-namespace-match"; import { isCodexAccountPriorityKey } from "./codex/account-priority"; +import { loopbackCompanionAllowed } from "./codex/loopback-target"; import { UPSTREAM_HOST_CIRCUIT_MAX_THRESHOLD } from "./codex/upstream-host-health"; import { adoptCustomModelCatalogMigration, @@ -1199,13 +1200,16 @@ const configSchema = z.object({ // is safe: startServer() already falls back to 127.0.0.1 for a missing hostname. Write-time // rejection lives in validateConfigCandidate() so bad values still surface to the caller. hostname: z.string().trim().min(1).optional().catch(undefined), - // Discriminated on `enabled` so a disabled entry cannot be forced to carry a port, and an - // enabled one cannot omit it (#1102). A malformed value degrades to undefined rather than - // failing the whole parse: this is an opt-in convenience surface, and a hand-edit typo here - // must never reset providers/apiKeys through the backup-and-defaults repair path. + // Discriminated on `enabled` so a disabled entry cannot be forced to carry a port (#1102). + // An enabled one MAY omit it: that is the companion form, which binds 127.0.0.1 on the proxy + // port and is legal only off a loopback/wildcard bind — a relationship between two fields, so + // it is enforced in validateConfigCandidate() and again at startup, not here (#4236). + // A malformed value degrades to undefined rather than failing the whole parse: this is an + // opt-in convenience surface, and a hand-edit typo here must never reset providers/apiKeys + // through the backup-and-defaults repair path. unauthenticatedLoopbackListener: z.union([ z.object({ enabled: z.literal(false) }), - z.object({ enabled: z.literal(true), port: z.number().int().min(1).max(65535) }), + z.object({ enabled: z.literal(true), port: z.number().int().min(1).max(65535).optional() }), ]).optional().catch(undefined), providers: z.record(z.string(), providerConfigSchema), modelPinnedEfforts: modelPinnedEffortsSchema.optional(), @@ -2820,15 +2824,22 @@ function oauthOpenBrowserError(value: unknown): string | null { /** Validate an in-memory config candidate without touching disk. Used by headless CLI import/set. */ /** - * Reject a loopback-listener port that collides with the proxy port (#1102). + * Reject a loopback-listener port that collides with the proxy port (#1102), and a port-less + * companion listener on a bind address that already owns 127.0.0.1 (#4236). * - * The schema can only check the shape of each field on its own; the two ports being distinct - * is a relationship between them. Letting the pair through would surface as a startup failure - * after the public listener already bound, which reads like an unrelated port conflict. + * The schema can only check the shape of each field on its own; the two ports being distinct — + * and the port-less form being compatible with `hostname` — are relationships between fields. + * Letting either through would surface as a startup failure after the public listener already + * bound, which reads like an unrelated port conflict. + * + * Both keys are read from the same candidate, so `ocx config set hostname 127.0.0.1` on a host + * whose listener is already the companion form is refused by this same check, with the same + * message, rather than breaking the next start. * * This is write-time only, matching `blankHostnameError`: a live caller can be told the value * is wrong, whereas a hand-edited config on the read path degrades to undefined rather than - * resetting the whole file. + * resetting the whole file. `assertLoopbackListenerBindable` repeats the decision at startup so + * a hand edit that skipped this boundary fails with the same sentence instead of EADDRINUSE. */ function loopbackListenerPortError(value: unknown): string | null { if (!value || typeof value !== "object" || Array.isArray(value)) return null; @@ -2846,17 +2857,46 @@ function loopbackListenerPortError(value: unknown): string | null { return "schema_invalid: unauthenticatedLoopbackListener.enabled: must be a boolean"; } if (entry.enabled !== true) return null; + const hostname = typeof (value as Record).hostname === "string" + ? (value as Record).hostname as string + : undefined; + const proxyPort = (value as Record).port; const listenerPort = entry.port; + // The companion form. `port` omitted means "same port as the public listener, on 127.0.0.1", + // which only exists as a free address when the public listener is bound somewhere else. + if (listenerPort === undefined) { + return loopbackCompanionBindError( + hostname, + typeof proxyPort === "number" ? proxyPort : 10100, + ); + } if (typeof listenerPort !== "number" || !Number.isInteger(listenerPort) || listenerPort < 1 || listenerPort > 65535) { - return "schema_invalid: unauthenticatedLoopbackListener.port: must be an integer port when enabled"; + return "schema_invalid: unauthenticatedLoopbackListener.port: must be an integer port when enabled, or omitted to share the proxy port"; } - const proxyPort = (value as Record).port; if (typeof proxyPort === "number" && proxyPort === listenerPort) { return "schema_invalid: unauthenticatedLoopbackListener.port: must differ from the proxy port"; } return null; } +/** + * The one sentence both the write boundary and startup use for an impossible companion bind. + * + * Exported so `startServer` can fail with the identical text: an operator who hand-edited the + * file past `validateConfigCandidate` must read the same diagnosis, not EADDRINUSE. + */ +export function loopbackCompanionBindError( + hostname: string | undefined, + proxyPort: number, +): string | null { + if (loopbackCompanionAllowed(hostname)) return null; + const bind = (hostname ?? "").trim() || "127.0.0.1"; + return "schema_invalid: unauthenticatedLoopbackListener: a port-less listener binds " + + `127.0.0.1:${proxyPort}, which the public listener on hostname "${bind}" already holds. ` + + "Either set a distinct unauthenticatedLoopbackListener.port, or remove the listener — a " + + "loopback bind already admits local callers without a credential."; +} + /** * Validate the hub management ingress at the live-write boundary. * diff --git a/src/server/index.ts b/src/server/index.ts index dc3bc2561d..106f34069a 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -17,6 +17,7 @@ import { loadConfig, saveConfig, getConfigDir, + loopbackCompanionBindError, websocketsEnabled, } from "../config"; import { grokDefaultReasoningEffort } from "../grok/effort"; @@ -28,6 +29,7 @@ import { withCatalogWriteSerialization } from "../codex/catalog-write-serializat import { invalidateCodexModelsCacheWithPermit } from "../codex/catalog/sync"; import { currentServiceHomes, serviceStatePathsForOpenCodexHome } from "../service"; import { shouldSyncCodexOnStart } from "../codex/desired-state"; +import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; import { createWindowsTaskListingCache, inspectNativeCodexOwnership, @@ -773,8 +775,16 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server`. Legal only + * when `hostname` is a specific non-loopback, non-wildcard address (a tailnet or LAN IP), + * because otherwise the public socket already owns that loopback address. This is the + * one-port hub shape: remote clients dial `hostname:port`, local processes dial + * `127.0.0.1:port`, and every integration that hardcodes `http://127.0.0.1:` + * keeps working on a hub whose public bind they cannot reach (#4236). + * + * Neither form is OS-assigned. A changing port would break already-running app-servers + * holding the previous `base_url` — the exact symptom #1102 reported and we disproved for + * token rotation. */ unauthenticatedLoopbackListener?: | { enabled: false } - | { enabled: true; port: number }; + | { enabled: true; port?: number }; /** * Outbound HTTP(S) proxy URL for provider requests (e.g. "http://user:pass@proxy:8080", or * "${HTTPS_PROXY}"-style env reference). Mirrored into HTTP_PROXY/HTTPS_PROXY at startup when From 06a506c6be747c08bd3141b5f1b1ffa20cc27e55 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:11:31 +0900 Subject: [PATCH 3/8] fix(codex): honor the companion port everywhere, and name the hub gate instead of the toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three consequences of the companion form, all in the readers (#4236): 1. `standaloneCodexRoutingTarget` resolves the listener through `effectiveLoopbackListenerPort`, so an omitted port means the public port. Every writer that already goes through it — the Codex provider block, `opencodeProxyBaseUrl`, `syncGrokConfig`, the integrations exporter — lands on `127.0.0.1:` with no admission header, which is the same origin the hardcoded local integrations write. `chooseListenPort` deliberately keeps reading `.port` directly: only an explicitly ported listener reserves a port, because reserving the shared one would refuse every start on a one-port hub. 2. The Grok fence drift check compares against a SET — `{listen.port} ∪ {effective loopback port}`. `ocx sync` writes the listener's port into the fence, so comparing against the public port alone told every hub operator that their freshly synced, working config pointed at a closed port, and to run the command that had just written it (issue defect 4). A third port still warns, still against the public listener. 3. The hub-role gate gets its own skip reason (`"hub-gated"`) and its own sentence: "This machine is a hub; it does not rewrite its own Codex/Grok/Claude configs unless unauthenticatedLoopbackListener is enabled." It travels the existing linearized channel (`CodexWriteLockSkipReason` → `codexInjectLockOutcome` → `CodexInjectResult` → `CodexSyncResult`) so the reason cannot disagree with the write. `ocx restore back` is the worst of the old reports: it committed the toggle ON, got a gated skip, and told the operator to "retry after the competing integration change finishes" — there is no competing writer. `ocx ensure` was the most destructive: it stripped the managed Grok block as if Grok had been switched off. Only an explicit `clientIntegrations.grok === false` authorizes that strip now; a gated hub is told why nothing was written and `~/.grok/config.toml` is left exactly as it is. `ocx sync`, `ocx sync-cache` and the three "startup left Codex native" lines say the same honest thing, and an explicit OFF keeps its existing wording byte for byte. Co-Authored-By: Claude Fable 5.1 --- src/cli/dispatch.ts | 32 ++++++++++++++++++---- src/cli/ensure-desired-integrations.ts | 10 +++++++ src/cli/index.ts | 30 ++++++++++++++++++-- src/cli/status.ts | 9 +++++- src/codex/codex-write-lock.ts | 13 +++++++-- src/codex/desired-state.ts | 36 +++++++++++++++++++++++- src/codex/inject-coordination.ts | 15 ++++++---- src/codex/inject.ts | 36 +++++++++++++++++------- src/codex/sync.ts | 38 ++++++++++++++++++++------ src/grok/status.ts | 10 ++++++- 10 files changed, 192 insertions(+), 37 deletions(-) diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index 94f0c14766..a02f1d6f41 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -16,7 +16,12 @@ import type { LivenessIo, LiveProxy } from "../server/proxy-liveness"; import type { OcxConfig } from "../types"; import type { OwnedIntegrationRefreshOutcome } from "../integrations/owned-refresh"; import { hasHelpFlag, printSubcommandUsage, printUsage } from "./help"; -import { setIntegrationEnabled, shouldSyncCodexOnStart } from "../codex/desired-state"; +import { + HUB_GATED_SKIP_MESSAGE, + localClientSkipMessage, + setIntegrationEnabled, + shouldSyncCodexOnStart, +} from "../codex/desired-state"; import { syncModelsToCodex } from "../codex/sync"; import { collectOrcaCodexHomeDiagnostic } from "../codex/home"; import { restoreNativeCodexAsync } from "../codex/inject"; @@ -123,7 +128,17 @@ const commandRunners: Record = { } const synced = await syncModelsToCodex(live.port); if (synced.status === "skipped") { - return emitBack(false, "Codex integration is OFF; restore back did not change Codex. Retry after the competing integration change finishes.", 2); + // `setIntegrationEnabled` above just committed ON, so a skip here is NOT the toggle and + // is not a competing writer either — on a hub it is the role gate. Telling the operator + // to "retry after the competing integration change finishes" sent them waiting for a + // writer that does not exist (#4236). + return emitBack( + false, + synced.skippedReason === "hub-gated" + ? `${HUB_GATED_SKIP_MESSAGE} restore back did not change Codex.` + : "Codex integration is OFF; restore back did not change Codex. Retry after the competing integration change finishes.", + 2, + ); } if (!synced.ok) { return emitBack(false, "Plain `codex` was not switched back to opencodex. Fix the reported Codex config issue and retry.", 1); @@ -390,7 +405,9 @@ const commandRunners: Record = { ); let code = 0; if (synced.status === "skipped") { - console.log("Codex integration is OFF; sync skipped and no Codex files changed."); + console.log(synced.skippedReason === "hub-gated" + ? `${HUB_GATED_SKIP_MESSAGE} sync skipped and no Codex files changed.` + : "Codex integration is OFF; sync skipped and no Codex files changed."); } else if (synced.status === "catalog-only") { // Explicit sync with the integration OFF still refreshes the catalog/cache // for side profiles that consume the proxy without injection. @@ -466,7 +483,8 @@ const commandRunners: Record = { const { readCodexCatalogPathForHome } = await import("../codex/catalog/parsing"); const { existsSync } = await import("node:fs"); const owningCodexHome = getCodexHome(); - const desiredDisabled = !shouldSyncCodexOnStart(deps.loadConfig()); + const cacheGateSnapshot = deps.loadConfig(); + const desiredDisabled = !shouldSyncCodexOnStart(cacheGateSnapshot); const invalidated = withCatalogWriteSerialization(owningCodexHome, permit => invalidateCodexModelsCacheWithPermit(permit, owningCodexHome, { allowWhenDesiredDisabled: true })); const cacheJson = cacheArgs.includes("--json"); @@ -480,7 +498,11 @@ const commandRunners: Record = { } else if (desiredDisabled && !cacheJson) { // Worth saying in the human path, because it explains why nothing was written. // Under --json this belongs on the envelope, not as a second stdout line. - console.log("Codex integration is OFF; no catalog or cache write resulted."); + console.log(localClientSkipMessage( + cacheGateSnapshot, + "Codex integration is OFF; no catalog or cache write resulted.", + "No catalog or cache write resulted.", + )); } // `completed` with a falsy value means the cache was NOT rewritten. Previously every // outcome exited 0, so a script could not tell a refreshed cache from a skipped one. diff --git a/src/cli/ensure-desired-integrations.ts b/src/cli/ensure-desired-integrations.ts index cb55eda15d..fe94a0b76e 100644 --- a/src/cli/ensure-desired-integrations.ts +++ b/src/cli/ensure-desired-integrations.ts @@ -13,6 +13,8 @@ import { stripGrokConfig, type GrokInjectResult } from "../grok/inject"; import { removeDesktop3pStandardPivot } from "../claude/desktop-3p"; import { claudeDesktopIntegrationEnabled, + grokIntegrationEnabled, + HUB_GATED_SKIP_MESSAGE, shouldSyncGrokOnStart, } from "../codex/desired-state"; import type { OcxConfig } from "../types"; @@ -78,6 +80,14 @@ export async function ensureGrokFenceMatchesDesired( ): Promise { const config = deps.loadConfig(); const { log, error } = io(deps); + // A hub-gated skip is NOT "the user turned Grok off" (#4236). Stripping the managed block + // there deleted a fence the operator still wants — and `ocx ensure` reported it as the + // Grok toggle doing its job. Only an explicit OFF authorizes the strip; the gate just + // declines to write, and says which key would let it. + if (!shouldSyncGrokOnStart(config) && grokIntegrationEnabled(config)) { + log(` ${HUB_GATED_SKIP_MESSAGE} ~/.grok/config.toml was left exactly as it is.`); + return; + } if (!shouldSyncGrokOnStart(config)) { try { const grok = deps.stripGrokConfig(); diff --git a/src/cli/index.ts b/src/cli/index.ts index 2589f1eb5d..4cb1e68ba9 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -81,8 +81,11 @@ import { scheduleCatalogPrewarm } from "./catalog-prewarm"; import { maybeShowUpdatePrompt } from "../update/notify"; import { syncModelsToCodex } from "../codex/sync"; import { + HUB_GATED_SKIP_MESSAGE, + localClientSkipReason, shouldSyncGrokOnStart, syncCodexOnStartIfEnabled, + type LocalClientSkipReason, } from "../codex/desired-state"; import { reconcileClientStartupBeforeReady, @@ -174,6 +177,19 @@ async function waitForProxy(timeoutMs = 8_000): Promise { return null; } +/** + * The one startup line for "nothing was written to Codex". + * + * Two very different facts reached it: the user's own OFF switch, and a hub declining to + * rewrite its own local clients. Printing the toggle's wording for the gate is what made + * operators hunt for a switch they never set (#4236). + */ +function startupLeftCodexNativeLine(reason: LocalClientSkipReason): string { + return reason === "hub-gated" + ? ` ${HUB_GATED_SKIP_MESSAGE} Startup left Codex native.` + : " Codex integration OFF; startup left Codex native."; +} + /** Argv for detached `start`, optionally hard-pinning the listen port. */ function startArgv(port?: number): string[] { const args = ["start"]; @@ -190,6 +206,10 @@ async function chooseListenPort( const config = loadConfig(); const preferred = requestedPort ?? config.port ?? 10100; const hardPin = requestedPort !== undefined && requestedPort > 0; + // Only an EXPLICITLY ported listener reserves a port. The companion form (`{enabled:true}` + // with no port) deliberately shares the public port on 127.0.0.1, so treating it as a + // reservation — via `effectiveLoopbackListenerPort` — would refuse every start on a + // one-port hub and hop the public listener onto an ephemeral port (#4236). const reservedLoopbackPort = config.unauthenticatedLoopbackListener?.enabled ? config.unauthenticatedLoopbackListener.port : undefined; @@ -513,7 +533,7 @@ async function handleStart(options: { block?: boolean } = {}) { } }, ); - if (!startupSync.ran) console.log(" Codex integration OFF; startup left Codex native."); + if (!startupSync.ran) console.log(startupLeftCodexNativeLine(localClientSkipReason(config))); await refreshOwnedRaycastCatalog(config, port); // #1046: one warning per startup, after BOTH writes. The server's cache // invalidation happens first and the catalog sync second, so the mtime is only @@ -579,7 +599,9 @@ async function handleEnsure(options: { existingIsSuccess?: boolean } = {}): Prom console.error(`⚠️ Model sync skipped: ${e instanceof Error ? e.message : String(e)}`); return null; }); - if (synced?.status === "skipped") console.log(" Codex integration OFF; startup left Codex native."); + if (synced?.status === "skipped") { + console.log(startupLeftCodexNativeLine(synced.skippedReason ?? "desired_disabled")); + } // Do not refresh Raycast from saved config here: live bind/admission and // secondary-listener settings may differ. Explicit sync or server startup // owns catalog refresh; ensure must not overwrite a working destination. @@ -626,7 +648,9 @@ async function handleEnsure(options: { existingIsSuccess?: boolean } = {}): Prom console.error(`⚠️ Model sync skipped: ${e instanceof Error ? e.message : String(e)}`); return null; }); - if (synced?.status === "skipped") console.log(" Codex integration OFF; startup left Codex native."); + if (synced?.status === "skipped") { + console.log(startupLeftCodexNativeLine(synced.skippedReason ?? "desired_disabled")); + } // The child performs Raycast refresh with its actual startup config. The // parent's pre-spawn snapshot is not authoritative for a client-file write. // The child opens /healthz before its best-effort roster reconcile. Await the same idempotent diff --git a/src/cli/status.ts b/src/cli/status.ts index 02e39f7fbe..8590ba38ec 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -14,6 +14,7 @@ import { computeVersionSkew, type VersionSkew } from "./version-skew"; import { redactSecretString, redactUserPath } from "../lib/redact"; import { collectOrcaCodexHomeDiagnostic, type OrcaCodexHomeDiagnostic } from "../codex/home"; import { grokFenceEndpointDrift, readGrokStatus } from "../grok/status"; +import { effectiveLoopbackListenerPort } from "../codex/loopback-target"; import { claudeDesktopIntegrationEnabled } from "../codex/desired-state"; import { claudeDesktopPolicyHealth, probeClaudeDesktopPolicy, type ClaudeDesktopPolicyHealth } from "../claude/desktop-policy"; import { collectClientConnectionStatus } from "./connect"; @@ -282,7 +283,13 @@ export async function collectStatus(): Promise { // no log line — ever reaches us. Surface it here, where the live port is already known. const grokDrift = (() => { try { - return grokFenceEndpointDrift(readGrokStatus(), health.ok ? listen.port : undefined); + // Locally reachable is a set: the public port plus the unauthenticated loopback + // listener's port. A fence naming either is correct; only a third port is drift (#4236). + return grokFenceEndpointDrift( + readGrokStatus(), + health.ok ? listen.port : undefined, + health.ok ? effectiveLoopbackListenerPort(config, listen.port) : null, + ); } catch { return null; // reading grok's config must never break `ocx status` } diff --git a/src/codex/codex-write-lock.ts b/src/codex/codex-write-lock.ts index c05b687190..13dd4c0b7a 100644 --- a/src/codex/codex-write-lock.ts +++ b/src/codex/codex-write-lock.ts @@ -66,7 +66,7 @@ export type CodexWriteLockRefusalReason = export type CodexWriteLockResult = | { status: "acquired"; value: T; waitedMs: number; lockId: string } - | { status: "skipped"; reason: "desired_disabled" | "desired_enabled"; waitedMs: number } + | { status: "skipped"; reason: CodexWriteLockSkipReason; waitedMs: number } | { status: "busy"; reason: "deadline" | "cancelled"; retryable: true; waitedMs: number; lockId: string } | { status: "refused"; @@ -127,9 +127,18 @@ export interface CodexWriteCommitContext { readonly coordinator: CodexCoordinatorTransaction; } +/** + * Why an under-lock policy re-read refused the write. + * + * `hub-gated` is not the user's switch: a hub declines to rewrite its own local clients, and + * reporting that as "integration is OFF" sent operators hunting for a toggle they never set + * (#4236). + */ +export type CodexWriteLockSkipReason = "desired_disabled" | "desired_enabled" | "hub-gated"; + /** A synchronous under-lock policy re-read proved the requested apply stale. */ export class CodexWriteLockSkipped extends Error { - constructor(readonly reason: "desired_disabled" | "desired_enabled") { + constructor(readonly reason: CodexWriteLockSkipReason) { super(reason); this.name = "CodexWriteLockSkipped"; } diff --git a/src/codex/desired-state.ts b/src/codex/desired-state.ts index d8ca52a84c..1fad88e82a 100644 --- a/src/codex/desired-state.ts +++ b/src/codex/desired-state.ts @@ -76,11 +76,45 @@ type LocalClientSyncConfig = Pick< "clientIntegrations" | "runtimeRole" | "unauthenticatedLoopbackListener" >; -function localClientSyncAllowed(config: LocalClientSyncConfig): boolean { +export function localClientSyncAllowed(config: LocalClientSyncConfig): boolean { return config.runtimeRole !== "hub" || config.unauthenticatedLoopbackListener?.enabled === true; } +/** + * The one sentence every hub-gated skip says (#4236). + * + * The gate is a reasonable decision; reporting it as "Codex integration is OFF" was not. An + * operator whose `clientIntegrations` says nothing — or says `true` — was told their own switch + * was off, and `ocx restore back` went further and blamed a competing writer that did not + * exist. Name the gate and name the key that opens it. + */ +export const HUB_GATED_SKIP_MESSAGE = + "This machine is a hub; it does not rewrite its own Codex/Grok/Claude configs unless " + + "unauthenticatedLoopbackListener is enabled."; + +/** Why a local-client write was skipped. The gate outranks the toggle: it is the surprising one. */ +export type LocalClientSkipReason = "desired_disabled" | "hub-gated"; + +export function localClientSkipReason(config: LocalClientSyncConfig): LocalClientSkipReason { + return localClientSyncAllowed(config) ? "desired_disabled" : "hub-gated"; +} + +/** + * Pick the skip message for a snapshot: today's toggle text, or the hub-gate sentence. + * + * `hubSuffix` states what the hub-gated path still did (a catalog refresh, say) so the + * composed line stays as informative as the toggle one it replaces. + */ +export function localClientSkipMessage( + config: LocalClientSyncConfig, + integrationOffMessage: string, + hubSuffix?: string, +): string { + if (localClientSyncAllowed(config)) return integrationOffMessage; + return hubSuffix ? `${HUB_GATED_SKIP_MESSAGE} ${hubSuffix}` : HUB_GATED_SKIP_MESSAGE; +} + export function shouldSyncCodexOnStart(config: LocalClientSyncConfig): boolean { // A hub is a server for OTHER machines: it must not rewrite its own host's // Codex/Claude/Grok client configs on startup (interview decision Q6, and the diff --git a/src/codex/inject-coordination.ts b/src/codex/inject-coordination.ts index e91e9f339b..55be8b81a1 100644 --- a/src/codex/inject-coordination.ts +++ b/src/codex/inject-coordination.ts @@ -8,7 +8,8 @@ import { createHash } from "node:crypto"; import { existsSync, lstatSync, readFileSync } from "node:fs"; import { atomicWriteFile } from "../config"; -import type { CodexWriteLockResult } from "./codex-write-lock"; +import type { CodexWriteLockResult, CodexWriteLockSkipReason } from "./codex-write-lock"; +import { HUB_GATED_SKIP_MESSAGE } from "./desired-state"; import { inspectCodexCoordinatorPath } from "./coordinator-doctor"; import { JOURNAL_PATH } from "./journal"; import { updateIntegrationRecord } from "./integration-record"; @@ -443,16 +444,20 @@ export function recomputeInjectWitness(options: { export function codexInjectLockOutcome( result: Exclude, { status: "acquired" }>, ): { success: false; message: string; retryable: boolean } | { - success: true; status: "skipped"; skippedReason: "desired_disabled" | "desired_enabled"; message: string; + success: true; status: "skipped"; skippedReason: CodexWriteLockSkipReason; message: string; } { if (result.status === "skipped") { return { success: true, status: "skipped", skippedReason: result.reason, - message: result.reason === "desired_disabled" - ? "Codex integration is OFF; no Codex config, catalog, cache, or history was changed." - : "Codex integration was re-enabled; native restore was skipped.", + // Three distinct facts, three sentences. The hub gate in particular must not borrow the + // toggle's wording — that is the phantom "integration is OFF" report from #4236. + message: result.reason === "hub-gated" + ? `${HUB_GATED_SKIP_MESSAGE} No Codex config, catalog, cache, or history was changed.` + : result.reason === "desired_disabled" + ? "Codex integration is OFF; no Codex config, catalog, cache, or history was changed." + : "Codex integration was re-enabled; native restore was skipped.", }; } if (result.status === "busy") { diff --git a/src/codex/inject.ts b/src/codex/inject.ts index cbcb3c671c..12ae087ac4 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -9,7 +9,11 @@ import { withConfigMutationLockSync, } from "../config"; import { CodexWriteLockSkipped, withCodexWriteLock } from "./codex-write-lock"; -import { shouldSyncCodexOnStart } from "./desired-state"; +import { + localClientSkipMessage, + localClientSkipReason, + shouldSyncCodexOnStart, +} from "./desired-state"; import { resolveCodexHistoryTransition } from "./history-transition"; import { buildInjectWitness, @@ -79,9 +83,9 @@ import { type ManagedSubagentDefaults, } from "./subagent-defaults"; import type { OcxConfig } from "../types"; -import { isLoopbackHostname, shouldInjectApiAuthHeader } from "./loopback-target"; +import { effectiveLoopbackListenerPort, isLoopbackHostname, shouldInjectApiAuthHeader } from "./loopback-target"; -export { isLoopbackHostname, shouldInjectApiAuthHeader } from "./loopback-target"; +export { effectiveLoopbackListenerPort, isLoopbackHostname, shouldInjectApiAuthHeader } from "./loopback-target"; // Ownership predicates live in `./injected-marker` so `journal.ts` can reach them // without importing this module back. Re-exported for existing external callers. @@ -214,8 +218,11 @@ export function standaloneCodexRoutingTarget( "hostname" | "unauthenticatedLoopbackListener" | "codexDesktopAuthless" | "codexClientCompaction" >, ): CodexRoutingTarget { + // An enabled listener with no `port` is the companion form: it answers on `port` itself, + // bound to 127.0.0.1 (#4236). Resolving it through the shared helper is what makes the + // one-port hub work without every writer repeating `?? port`. const loopback = config?.unauthenticatedLoopbackListener; - const effectivePort = loopback?.enabled ? loopback.port : port; + const effectivePort = effectiveLoopbackListenerPort(config, port) ?? port; const hostname = loopback?.enabled ? undefined : config?.hostname; const requiresAdmissionToken = loopback?.enabled ? false : shouldInjectApiAuthHeader(config); return { @@ -887,7 +894,8 @@ export interface CodexInjectResult { success: boolean; message: string; status?: "skipped"; - skippedReason?: "desired_disabled" | "desired_enabled"; + /** `hub-gated` is the hub-role gate (#4236), distinct from the user's own OFF switch. */ + skippedReason?: "desired_disabled" | "desired_enabled" | "hub-gated"; nativeSubagentDefaultsWarning?: string; } @@ -1261,12 +1269,17 @@ export async function injectCodexConfig( if (eligibility.kind === "legacy-uncoordinated") { const applyLegacy = (): CodexInjectResult | undefined => { - if (!shouldSyncCodexOnStart(loadConfig())) { + const legacyGateSnapshot = loadConfig(); + if (!shouldSyncCodexOnStart(legacyGateSnapshot)) { return { success: true, status: "skipped", - skippedReason: "desired_disabled", - message: "Codex integration is OFF; no Codex config, catalog, cache, or history was changed.", + skippedReason: localClientSkipReason(legacyGateSnapshot), + message: localClientSkipMessage( + legacyGateSnapshot, + "Codex integration is OFF; no Codex config, catalog, cache, or history was changed.", + "No Codex config, catalog, cache, or history was changed.", + ), }; } runClientWriteGuard(options.beforeClientWrite); @@ -1295,8 +1308,11 @@ export async function injectCodexConfig( }), }, (ctx) => { - if (!shouldSyncCodexOnStart(loadConfig())) { - throw new CodexWriteLockSkipped("desired_disabled"); + const gateSnapshot = loadConfig(); + if (!shouldSyncCodexOnStart(gateSnapshot)) { + // Carry WHY under the lock: "the hub does not write its own clients" and "the user + // turned Codex off" produce the same no-write and must not produce the same sentence. + throw new CodexWriteLockSkipped(localClientSkipReason(gateSnapshot)); } // N and C are held here. Reject stale client work before publishing a // transition or capturing preimages; rejection must not compensate over diff --git a/src/codex/sync.ts b/src/codex/sync.ts index 6d7008a01a..21056e7c11 100644 --- a/src/codex/sync.ts +++ b/src/codex/sync.ts @@ -5,7 +5,12 @@ import { applyProxyEnv, loadConfig } from "../config"; import type { OcxConfig } from "../types"; import { collectOrcaCodexHomeDiagnostic } from "./home"; import { summarizeComboCatalogOmissions, type ComboCatalogOmission } from "./catalog/aggregation"; -import { shouldSyncCodexOnStart } from "./desired-state"; +import { + localClientSkipMessage, + localClientSkipReason, + shouldSyncCodexOnStart, + type LocalClientSkipReason, +} from "./desired-state"; import { admitCodexWrite, type CodexAdmission } from "./admission"; import type { CodexCatalogSyncOptions } from "./catalog/sync"; import { resetCodexAppServerCatalogStateCache } from "./app-server-processes"; @@ -18,7 +23,8 @@ export interface CodexSyncResult { */ status: "applied" | "skipped" | "catalog-only" | "refused"; ok: boolean; - skippedReason?: "desired_disabled"; + /** `hub-gated` is the hub-role gate, not the user's toggle — the two read very differently. */ + skippedReason?: LocalClientSkipReason; /** Present when unattended convergence refused another service's native home. */ authority?: "service-home"; added: number; @@ -85,19 +91,24 @@ export async function syncModelsToCodex( // durable user switch and must be read again at this production boundary: a // PUT OFF while provider discovery is in flight cannot be allowed to commit // through an older captured object. - const desiredDisabled = !shouldSyncCodexOnStart(loadConfig()); + const gateSnapshot = loadConfig(); + const desiredDisabled = !shouldSyncCodexOnStart(gateSnapshot); const catalogEvenWhenNotInjected = options.catalogEvenWhenNotInjected === true; if (desiredDisabled && !catalogEvenWhenNotInjected) { return { status: "skipped", - skippedReason: "desired_disabled", + skippedReason: localClientSkipReason(gateSnapshot), ok: true, added: 0, catalogPath: null, catalogExists: false, catalogWritten: false, cacheSynced: false, - message: "Codex integration is OFF; no Codex config, catalog, cache, or history was changed.", + message: localClientSkipMessage( + gateSnapshot, + "Codex integration is OFF; no Codex config, catalog, cache, or history was changed.", + "No Codex config, catalog, cache, or history was changed.", + ), }; } // Catalog gathering precedes injection and can itself write the native @@ -131,8 +142,16 @@ export async function syncModelsToCodex( applyProxyEnv(config); const refreshed = await refreshCatalogForSync(config, deps, { allowWhenDesiredDisabled: true }, log); const message = refreshed.catalogWritten || refreshed.cacheSynced - ? "Codex integration is OFF; catalog and models cache refreshed, Codex config untouched." - : "Codex integration is OFF; catalog refresh skipped, Codex config untouched."; + ? localClientSkipMessage( + gateSnapshot, + "Codex integration is OFF; catalog and models cache refreshed, Codex config untouched.", + "Catalog and models cache refreshed, Codex config untouched.", + ) + : localClientSkipMessage( + gateSnapshot, + "Codex integration is OFF; catalog refresh skipped, Codex config untouched.", + "Catalog refresh skipped, Codex config untouched.", + ); return { status: "catalog-only", ok: true, @@ -242,8 +261,9 @@ export async function syncModelsToCodex( if (result.status === "skipped") { return { status: "skipped", - // The apply direction's only under-lock policy skip is desired OFF. - skippedReason: "desired_disabled", + // The apply direction's only under-lock policy skips are desired OFF and the hub gate; + // carry whichever the injector reported so the caller can say the honest thing. + skippedReason: result.skippedReason === "hub-gated" ? "hub-gated" : "desired_disabled", ok: true, added: 0, catalogPath: null, diff --git a/src/grok/status.ts b/src/grok/status.ts index 663c723a6f..948da8a237 100644 --- a/src/grok/status.ts +++ b/src/grok/status.ts @@ -111,11 +111,18 @@ export function readGrokStatus(opts: { grokHome?: string } = {}): GrokStatus { * `ocx status` is for. * * Returns null when there is nothing to say: no fence, an unparsable endpoint, or a - * fence that already agrees with the live listener. + * fence that already agrees with a port we are actually listening on. + * + * "Listening on" is a SET, not one number (#4236). A hub with an unauthenticated loopback + * listener answers on the public port and on the listener's port; a fence pointing at the + * latter is exactly what `ocx sync` wrote, so reporting it as drift told the operator their + * working config was broken. `loopbackPort` is that second reachable port, already resolved + * through `effectiveLoopbackListenerPort`, or null when no such listener is configured. */ export function grokFenceEndpointDrift( status: Pick, livePort: number | undefined, + loopbackPort?: number | null, ): { fencePort: number; livePort: number } | null { if (!status.present || !status.baseUrl) return null; if (typeof livePort !== "number" || !Number.isFinite(livePort) || livePort <= 0) return null; @@ -130,5 +137,6 @@ export function grokFenceEndpointDrift( return null; } if (!Number.isFinite(fencePort) || fencePort === livePort) return null; + if (typeof loopbackPort === "number" && fencePort === loopbackPort) return null; return { fencePort, livePort }; } From d015bc7f13a65928138725e66649475f3d77c1b6 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:11:48 +0900 Subject: [PATCH 4/8] test(server,cli): pin the companion listener, the drift set, and the hub-gate sentences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Config boundary: the port-less form is accepted on a specific non-loopback bind and the absent port SURVIVES the parse (a schema that helpfully filled it in would make the pair look like the #1102 collision on the next write); it is refused on loopback, localhost, ::1 and the wildcards, with a message that names `127.0.0.1:` and both fixes; and reverting `hostname` to loopback is refused by that same check, which is the `ocx config set hostname` path. Listener: a real companion start proves one port answers twice — 200 without a credential on 127.0.0.1, 401 on the tailnet address — and that the startup line says "companion" rather than the ported form's warning. A second case pins what did NOT change: `/api/*`, `/healthz`, `/` and `POST /v1/messages` still 404 on that socket, so sharing a port widens no surface. A third proves an impossible companion throws before the public listener opens, leaving the port bindable. Clients: `tests/server/loopback-companion-client-targets.test.ts` is the claim the whole PR rests on — `standaloneCodexRoutingTarget` and the untouched `buildClaudeEnv` resolve to the SAME origin on a companion hub, with no admission header — and keeps the ported form's split as a regression witness for the issue's table. Hub gate: `tests/cli/hub-gated-local-clients.test.ts` holds `restore back` to the gate's sentence instead of the phantom-conflict text (human and `--json`), holds `ocx ensure` to leaving a Grok block it did not own the decision to delete while still stripping on an explicit OFF, and pins the skip reason/message that every other caller prints. Both new files are registered in `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`. Co-Authored-By: Claude Fable 5.1 --- scripts/test-layout/layout.json | 2 + tests/cli/hub-gated-local-clients.test.ts | 226 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 2 + tests/providers/xai/grok-status.test.ts | 22 ++ tests/providers/xai/grok-sync.test.ts | 28 +++ .../loopback-companion-client-targets.test.ts | 85 +++++++ .../loopback-listener-admission.test.ts | 101 +++++++- .../loopback-listener-integration.test.ts | 94 ++++++++ 8 files changed, 557 insertions(+), 3 deletions(-) create mode 100644 tests/cli/hub-gated-local-clients.test.ts create mode 100644 tests/server/loopback-companion-client-targets.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 4d02cb0ea5..4aa9b3de01 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -703,6 +703,7 @@ "history-migration-guardian.test.ts": "codex-integration", "history-ocx-compaction-recovery.test.ts": "codex-integration", "hyperbolic-provider.test.ts": "providers", + "hub-gated-local-clients.test.ts": "cli", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", @@ -806,6 +807,7 @@ "logs-timezone.test.ts": "server", "loop-reasoning-replay.test.ts": "images", "loop.test.ts": "images", + "loopback-companion-client-targets.test.ts": "server", "loopback-listener-admission.test.ts": "server", "loopback-listener-integration.test.ts": "server", "macos-serial-lanes.test.ts": "ci-workflows", diff --git a/tests/cli/hub-gated-local-clients.test.ts b/tests/cli/hub-gated-local-clients.test.ts new file mode 100644 index 0000000000..5eca3af702 --- /dev/null +++ b/tests/cli/hub-gated-local-clients.test.ts @@ -0,0 +1,226 @@ +/** + * Hub-gate honesty (#4236, follow-up 2). + * + * `localClientSyncAllowed` refuses to rewrite a hub's OWN Codex/Grok/Claude configs unless the + * unauthenticated loopback listener is on. The gate is fine; the reporting was not. Every + * caller printed the *toggle's* message, so on a hub with `clientIntegrations` absent: + * + * - `ocx sync` said "Codex integration is OFF" about a switch the operator never set, + * - `ocx restore back` committed ON and then told the operator to "retry after the competing + * integration change finishes" — there was no competing writer, + * - `ocx ensure` removed the managed Grok block as if Grok had been switched off. + * + * These tests pin the distinct reason and its sentence at each of those boundaries. + */ +import { describe, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { dispatchCommand, type CliDispatchDeps } from "../../src/cli/dispatch"; +import { ensureGrokFenceMatchesDesired, type EnsureDesiredIntegrationsDeps } from "../../src/cli/ensure-desired-integrations"; +import { codexInjectLockOutcome } from "../../src/codex/inject-coordination"; +import { + HUB_GATED_SKIP_MESSAGE, + localClientSkipMessage, + localClientSkipReason, + localClientSyncAllowed, +} from "../../src/codex/desired-state"; +import { saveConfig } from "../../src/config"; +import type { GrokInjectResult } from "../../src/grok/inject"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const PHANTOM_CONFLICT = "Retry after the competing integration change finishes"; + +function hubConfig(overrides: Partial = {}): OcxConfig { + return { + port: 10_100, + hostname: "100.76.170.81", + runtimeRole: "hub", + providers: {}, + defaultProvider: "openai", + checkForUpdates: false, + ...overrides, + } as unknown as OcxConfig; +} + +describe("the hub gate has its own reason and its own sentence", () => { + test("a hub without the listener is gated; a listener or a non-hub role opens it", () => { + expect(localClientSyncAllowed(hubConfig())).toBe(false); + expect(localClientSkipReason(hubConfig())).toBe("hub-gated"); + // The companion form counts: it is exactly how a hub becomes its own local client. + expect(localClientSkipReason(hubConfig({ unauthenticatedLoopbackListener: { enabled: true } }))) + .toBe("desired_disabled"); + expect(localClientSkipReason(hubConfig({ unauthenticatedLoopbackListener: { enabled: true, port: 10_104 } }))) + .toBe("desired_disabled"); + expect(localClientSkipReason(hubConfig({ runtimeRole: undefined }))).toBe("desired_disabled"); + }); + + test("the message names the hub and the key that opens the gate", () => { + expect(HUB_GATED_SKIP_MESSAGE).toContain("This machine is a hub"); + expect(HUB_GATED_SKIP_MESSAGE).toContain("unauthenticatedLoopbackListener"); + // And it must not borrow the toggle's wording, which is the whole defect. + expect(HUB_GATED_SKIP_MESSAGE).not.toContain("integration is OFF"); + }); + + test("localClientSkipMessage keeps the toggle text for a real OFF", () => { + const off = hubConfig({ + unauthenticatedLoopbackListener: { enabled: true }, + clientIntegrations: { codex: false }, + }); + expect(localClientSkipMessage(off, "Codex integration is OFF; nothing changed.")) + .toBe("Codex integration is OFF; nothing changed."); + expect(localClientSkipMessage(hubConfig(), "Codex integration is OFF; nothing changed.", "Nothing changed.")) + .toBe(`${HUB_GATED_SKIP_MESSAGE} Nothing changed.`); + }); + + test("the under-lock skip projection reports the gate, not the toggle", () => { + const gated = codexInjectLockOutcome({ status: "skipped", reason: "hub-gated", waitedMs: 0 }); + expect(gated).toMatchObject({ success: true, status: "skipped", skippedReason: "hub-gated" }); + expect(gated.message).toContain(HUB_GATED_SKIP_MESSAGE); + + // The two pre-existing reasons keep their exact wording. + const off = codexInjectLockOutcome({ status: "skipped", reason: "desired_disabled", waitedMs: 0 }); + expect(off.message).toBe("Codex integration is OFF; no Codex config, catalog, cache, or history was changed."); + const reenabled = codexInjectLockOutcome({ status: "skipped", reason: "desired_enabled", waitedMs: 0 }); + expect(reenabled.message).toBe("Codex integration was re-enabled; native restore was skipped."); + }); +}); + +describe("ocx ensure does not strip a Grok block the operator still wants", () => { + function harness(config: OcxConfig) { + const actions: Array<"strip" | "sync"> = []; + const logs: string[] = []; + const deps: EnsureDesiredIntegrationsDeps = { + loadConfig: () => config, + stripGrokConfig: () => { + actions.push("strip"); + return { ok: true, changed: true, message: "Removed the opencodex managed block from Grok config." } as GrokInjectResult; + }, + syncGrokConfig: async () => { + actions.push("sync"); + return { ok: true, changed: true, message: "updated" } as GrokInjectResult; + }, + removeDesktop3pStandardPivot: () => ({ ok: true, changed: false, kind: "absent" as const, libraryPath: "/tmp/desktop" }), + log: message => { logs.push(message); }, + error: message => { logs.push(message); }, + }; + return { actions, logs, deps }; + } + + test("a hub-gated skip leaves ~/.grok/config.toml untouched and says why", async () => { + // The operator never turned Grok off. Deleting their fence and reporting it as the toggle + // working is the defect: it destroys a working config on every `ocx ensure`. + const h = harness(hubConfig()); + await ensureGrokFenceMatchesDesired(10_100, {}, h.deps); + expect(h.actions).toEqual([]); + expect(h.logs.join("\n")).toContain(HUB_GATED_SKIP_MESSAGE); + }); + + test("an explicit Grok OFF still strips, because that is the operator's own decision", async () => { + const h = harness(hubConfig({ + unauthenticatedLoopbackListener: { enabled: true }, + clientIntegrations: { grok: false }, + })); + await ensureGrokFenceMatchesDesired(10_100, {}, h.deps); + expect(h.actions).toEqual(["strip"]); + }); + + test("a hub with the listener on syncs the fence like any other host", async () => { + const h = harness(hubConfig({ unauthenticatedLoopbackListener: { enabled: true } })); + await ensureGrokFenceMatchesDesired(10_100, {}, h.deps); + expect(h.actions).toEqual(["sync"]); + }); + + test("an explicit OFF on a GATED hub still strips: the gate never overrides the operator", async () => { + const h = harness(hubConfig({ clientIntegrations: { grok: false } })); + await ensureGrokFenceMatchesDesired(10_100, {}, h.deps); + expect(h.actions).toEqual(["strip"]); + }); +}); + +describe("CLI output on a hub-gated host", () => { + /** + * `tests/preload.ts` sandboxes HOME/OPENCODEX_HOME, but these cases PERSIST a hub config and + * `restore back` mutates it (`setIntegrationEnabled`), so each gets its own home rather than + * leaving a hub role behind for the next test in the shard. + */ + async function runInHubHome( + command: string, + args: string[], + extraDeps: Partial = {}, + ): Promise<{ code: number; out: string[]; err: string[] }> { + const home = mkdtempSync(join(tmpdir(), "ocx-hub-gated-")); + const previous = process.env.OPENCODEX_HOME; + const out: string[] = []; + const err: string[] = []; + const log = console.log; + const error = console.error; + process.env.OPENCODEX_HOME = home; + console.log = (...values: unknown[]) => { out.push(values.join(" ")); }; + console.error = (...values: unknown[]) => { err.push(values.join(" ")); }; + try { + saveConfig(hubConfig()); + const deps = { + args, + ...extraDeps, + } as unknown as CliDispatchDeps; + const code = await dispatchCommand({ kind: "command", command, args }, deps); + return { code, out, err }; + } finally { + console.log = log; + console.error = error; + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + removeTreeWithRetry(home); + } + } + + test("restore back names the gate instead of blaming a competing writer", async () => { + const result = await runInHubHome("restore", ["restore", "back"], { + findLiveProxy: async () => ({ pid: 4242, port: 10_100, hostname: "100.76.170.81", source: "config" as const }), + }); + // Still a refusal — nothing was written — but for the stated reason. + expect(result.code).toBe(2); + const combined = [...result.out, ...result.err].join("\n"); + expect(combined).toContain(HUB_GATED_SKIP_MESSAGE); + expect(combined).toContain("restore back did not change Codex"); + expect(combined).not.toContain(PHANTOM_CONFLICT); + }); + + test("restore back --json carries the same sentence in its envelope", async () => { + const result = await runInHubHome("restore", ["restore", "back", "--json"], { + findLiveProxy: async () => ({ pid: 4242, port: 10_100, hostname: "100.76.170.81", source: "config" as const }), + }); + expect(result.code).toBe(2); + const envelope = JSON.parse(result.out.join("\n")) as { success: boolean; message?: string }; + expect(envelope.success).toBe(false); + expect(JSON.stringify(envelope)).toContain("This machine is a hub"); + expect(JSON.stringify(envelope)).not.toContain(PHANTOM_CONFLICT); + }); + + test("the sync result every caller prints carries the gate's reason and sentence", async () => { + // Through `syncModelsToCodex` rather than the `sync` runner: the runner's own output is a + // pass-through of these two fields, and reaching it for real would drag in the native + // ownership probe (a launchctl/systemd call) this assertion has nothing to do with. + const home = mkdtempSync(join(tmpdir(), "ocx-hub-gated-sync-")); + const previous = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + try { + saveConfig(hubConfig()); + const { syncModelsToCodex } = await import("../../src/codex/sync"); + const result = await syncModelsToCodex(10_100, hubConfig(), null); + expect(result.status).toBe("skipped"); + expect(result.skippedReason).toBe("hub-gated"); + expect(result.message).toContain(HUB_GATED_SKIP_MESSAGE); + expect(result.message).not.toContain("Codex integration is OFF"); + // A skip is still a success: the gate is policy, not a failure. + expect(result.ok).toBe(true); + expect(result.catalogWritten).toBe(false); + } finally { + if (previous === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previous; + removeTreeWithRetry(home); + } + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 2670f61761..1287b981ca 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -538,6 +538,7 @@ "history-migration-guardian.test.ts": "codex-integration", "history-ocx-compaction-recovery.test.ts": "codex-integration", "hyperbolic-provider.test.ts": "providers", + "hub-gated-local-clients.test.ts": "cli", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", @@ -641,6 +642,7 @@ "logs-timezone.test.ts": "server", "loop-reasoning-replay.test.ts": "images", "loop.test.ts": "images", + "loopback-companion-client-targets.test.ts": "server", "loopback-listener-admission.test.ts": "server", "loopback-listener-integration.test.ts": "server", "macos-serial-lanes.test.ts": "ci-workflows", diff --git a/tests/providers/xai/grok-status.test.ts b/tests/providers/xai/grok-status.test.ts index 355472798d..bb80349462 100644 --- a/tests/providers/xai/grok-status.test.ts +++ b/tests/providers/xai/grok-status.test.ts @@ -129,6 +129,28 @@ describe("Grok fence endpoint drift", () => { )).toBeNull(); }); + /** + * The unauthenticated loopback listener makes "the port we listen on" a SET (#4236). `ocx + * sync` writes the LISTENER's port into the fence, so comparing against the public port alone + * warned every hub operator that their freshly synced, working config pointed at a closed + * port — and told them to run the command that had just written it. + */ + test("accepts either port when a loopback listener is configured", () => { + const fence = { present: true, baseUrl: "http://127.0.0.1:10104/v1" } as const; + // Ported form: the fence names the listener, the public listener is elsewhere. + expect(grokFenceEndpointDrift(fence, 10100, 10104)).toBeNull(); + // The public port is equally correct. + expect(grokFenceEndpointDrift({ present: true, baseUrl: "http://127.0.0.1:10100/v1" }, 10100, 10104)).toBeNull(); + // Companion form: both members of the set are the same port. + expect(grokFenceEndpointDrift({ present: true, baseUrl: "http://127.0.0.1:10100/v1" }, 10100, 10100)).toBeNull(); + // A third port is still drift, and is still reported against the public listener. + expect(grokFenceEndpointDrift({ present: true, baseUrl: "http://127.0.0.1:4179/v1" }, 10100, 10104)) + .toEqual({ fencePort: 4179, livePort: 10100 }); + // Absent listener: unchanged behaviour, whichever way "absent" is spelled. + expect(grokFenceEndpointDrift(fence, 10100, null)).toEqual({ fencePort: 10104, livePort: 10100 }); + expect(grokFenceEndpointDrift(fence, 10100)).toEqual({ fencePort: 10104, livePort: 10100 }); + }); + // No fence, no live port, or an endpoint shape we never emit: there is nothing the user // could act on, and a false warning about their own config is worse than silence. test("stays quiet when there is nothing to compare", () => { diff --git a/tests/providers/xai/grok-sync.test.ts b/tests/providers/xai/grok-sync.test.ts index af67b4dff6..6ce02a973f 100644 --- a/tests/providers/xai/grok-sync.test.ts +++ b/tests/providers/xai/grok-sync.test.ts @@ -274,6 +274,34 @@ describe("syncGrokConfig", () => { } }); + test("the companion form writes the PUBLIC port on loopback, matching the one-port hub (#4236)", async () => { + const { root, grokHome } = tempGrokHome(); + try { + const config = { + ...baseConfig, + runtimeRole: "hub", + hostname: "100.64.0.10", + // No port: the listener shares the proxy port on 127.0.0.1, so the fence grok writes is + // the same origin `ocx claude` and Claude Desktop hardcode. + unauthenticatedLoopbackListener: { enabled: true }, + } as OcxConfig; + const result = await syncGrokConfig(10100, config, { + grokHome, + hostname: "100.64.0.10", + }, { + fetchAllModels: async () => [], + injectGrokConfig, + }); + + expect(result).toMatchObject({ ok: true, changed: true }); + const content = readFileSync(join(grokHome, "config.toml"), "utf8"); + expect(content).toContain('base_url = "http://127.0.0.1:10100/v1"'); + expect(content).not.toContain("100.64.0.10"); + } finally { + removeTreeWithRetry(root); + } + }); + test("catalog failure surfaces ok:false without touching the config", async () => { const { root, grokHome } = tempGrokHome(); try { diff --git a/tests/server/loopback-companion-client-targets.test.ts b/tests/server/loopback-companion-client-targets.test.ts new file mode 100644 index 0000000000..2b24fe4350 --- /dev/null +++ b/tests/server/loopback-companion-client-targets.test.ts @@ -0,0 +1,85 @@ +/** + * What the companion loopback listener is FOR: local clients keep working on a hub (#4236). + * + * With `hostname: "100.76.170.81"` and `unauthenticatedLoopbackListener: { enabled: true, port: + * 10104 }`, only the `ocx sync` writers honored the listener. Everything else — `ocx claude`, + * Claude Desktop, Cursor, `system-env`, the vision helper — hardcodes + * `http://127.0.0.1:`, a port that does not exist on a tailnet-bound hub. The + * reported symptom was "Codex works but nothing else does". + * + * The port-less companion form answers that without touching any of those call sites: the + * listener binds the proxy port on 127.0.0.1, so the URL they already write is live. These + * tests pin the two ends of that claim — the sync-writer target and the hardcoded one must be + * the SAME origin — because the fix is only real while those two agree. + */ +import { describe, expect, test } from "bun:test"; +import { buildClaudeEnv } from "../../src/cli/claude"; +import { opencodeProxyBaseUrl } from "../../src/clients/config-export"; +import { standaloneCodexRoutingTarget } from "../../src/codex/inject"; +import type { OcxConfig } from "../../src/types"; + +const HUB_PORT = 10_100; +const TAILNET_ADDRESS = "100.76.170.81"; + +function hubConfig(listener: OcxConfig["unauthenticatedLoopbackListener"]): OcxConfig { + return { + port: HUB_PORT, + hostname: TAILNET_ADDRESS, + runtimeRole: "hub", + defaultProvider: "openai", + providers: { + openai: { adapter: "openai-responses", baseUrl: "https://chatgpt.com/backend-api/codex", authMode: "forward" }, + }, + ...(listener === undefined ? {} : { unauthenticatedLoopbackListener: listener }), + } as unknown as OcxConfig; +} + +/** The origin a hardcoded local integration writes, spelled the way those call sites spell it. */ +function hardcodedLocalOrigin(port: number): string { + return `http://127.0.0.1:${port}`; +} + +describe("companion hub: sync-managed and hardcoded clients agree", () => { + test("the Codex target is the public port on loopback, with no admission header", () => { + const target = standaloneCodexRoutingTarget(HUB_PORT, hubConfig({ enabled: true })); + expect(target.baseUrl).toBe(`${hardcodedLocalOrigin(HUB_PORT)}/v1`); + // A directly spawned app-server has no OPENCODEX_API_AUTH_TOKEN to put in a header, so + // demanding one here is the #1102 failure in a new place. + expect(target.requiresAdmissionToken).toBe(false); + // And the tailnet address must not leak into a base_url a local client dials. + expect(target.baseUrl).not.toContain(TAILNET_ADDRESS); + }); + + test("buildClaudeEnv lands on that exact origin without being taught about the listener", () => { + // `ocx claude` passes the live PUBLIC port and hardcodes 127.0.0.1. It is not changed by + // this PR; the point is that it no longer has to be. + const config = hubConfig({ enabled: true }); + const env = buildClaudeEnv(config, HUB_PORT, {}); + expect(env.ANTHROPIC_BASE_URL).toBe(hardcodedLocalOrigin(HUB_PORT)); + + const codexOrigin = new URL(standaloneCodexRoutingTarget(HUB_PORT, config).baseUrl).origin; + expect(env.ANTHROPIC_BASE_URL).toBe(codexOrigin); + }); + + test("the opencode/OMP exporter resolves to the same origin", () => { + const config = hubConfig({ enabled: true }); + expect(opencodeProxyBaseUrl(HUB_PORT, config.hostname, config)) + .toBe(`${hardcodedLocalOrigin(HUB_PORT)}/v1`); + }); + + test("the ported form still splits the two, which is why the companion exists", () => { + // Kept as a regression witness for the issue's table: with a distinct port the sync + // writers move and the hardcoded callers do not, so they disagree by construction. + const config = hubConfig({ enabled: true, port: 10_104 }); + const codexOrigin = new URL(standaloneCodexRoutingTarget(HUB_PORT, config).baseUrl).origin; + expect(codexOrigin).toBe(hardcodedLocalOrigin(10_104)); + expect(buildClaudeEnv(config, HUB_PORT, {}).ANTHROPIC_BASE_URL).toBe(hardcodedLocalOrigin(HUB_PORT)); + expect(codexOrigin).not.toBe(hardcodedLocalOrigin(HUB_PORT)); + }); + + test("with no listener a hub keeps demanding admission on its public address", () => { + const target = standaloneCodexRoutingTarget(HUB_PORT, hubConfig(undefined)); + expect(target.baseUrl).toBe(`http://${TAILNET_ADDRESS}:${HUB_PORT}/v1`); + expect(target.requiresAdmissionToken).toBe(true); + }); +}); diff --git a/tests/server/loopback-listener-admission.test.ts b/tests/server/loopback-listener-admission.test.ts index e7e676fef9..6ec1bd5028 100644 --- a/tests/server/loopback-listener-admission.test.ts +++ b/tests/server/loopback-listener-admission.test.ts @@ -19,6 +19,7 @@ import { resolveResponsesApiAuth, } from "../../src/server/auth-cors"; import { buildProviderTableBlock, shouldInjectApiAuthHeader } from "../../src/codex/inject"; +import { effectiveLoopbackListenerPort, loopbackCompanionAllowed } from "../../src/codex/loopback-target"; import { validateConfigCandidate } from "../../src/config"; import type { OcxConfig } from "../../src/types"; @@ -133,9 +134,11 @@ describe("loopback listener configuration", () => { if (!result.ok) expect(result.error).toContain("must differ from the proxy port"); }); - test("an enabled listener without a port is rejected", () => { - // An OS-assigned port would change across restarts and strand app-servers holding the - // previous base_url — the symptom #1102 reported and we disproved for token rotation. + test("an enabled listener without a port is rejected on a loopback bind", () => { + // The port-less form means "same port, on 127.0.0.1". With the public listener already on + // 127.0.0.1 there is no such address to take, and an OS-assigned port is not the fallback: + // it would change across restarts and strand app-servers holding the previous base_url — + // the symptom #1102 reported and we disproved for token rotation. const result = validateConfigCandidate({ port: 10100, providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, @@ -169,6 +172,98 @@ describe("loopback listener configuration", () => { }); }); +/** + * The one-port hub (#4236). `{ enabled: true }` with no port binds 127.0.0.1:, so a + * tailnet-bound hub serves remote clients on its public address and its own local processes on + * loopback — including every integration that hardcodes `http://127.0.0.1:`. + */ +describe("loopback companion listener configuration", () => { + const candidate = (overrides: Record = {}) => ({ + port: 10100, + providers: { openai: { adapter: "openai", baseUrl: "https://chatgpt.com/backend-api/codex" } }, + defaultProvider: "openai", + unauthenticatedLoopbackListener: { enabled: true }, + ...overrides, + }); + + test("a port-less listener is accepted on a specific non-loopback bind and survives the parse", () => { + for (const hostname of ["100.76.170.81", "192.168.1.40", "fd7a:115c:a1e0::1", "macmini.tail19a2d7.ts.net"]) { + const result = validateConfigCandidate(candidate({ hostname })); + expect({ hostname, ok: result.ok }).toEqual({ hostname, ok: true }); + // The absent port must SURVIVE. A schema that helpfully filled in the proxy port would + // make the pair look like the #1102 collision on the next write. + if (result.ok) { + expect(result.config.unauthenticatedLoopbackListener).toEqual({ enabled: true }); + } + } + }); + + test("a port-less listener is refused wherever the public listener already holds loopback", () => { + // Wildcards included: 0.0.0.0 answers on 127.0.0.1 too, so the companion would collide + // there just as surely as on an explicit loopback bind. + for (const hostname of [undefined, "127.0.0.1", "localhost", "::1", "0.0.0.0", "::", "[::]"]) { + const result = validateConfigCandidate(candidate(hostname === undefined ? {} : { hostname })); + expect({ hostname, ok: result.ok }).toEqual({ hostname, ok: false }); + if (!result.ok) { + // The message has to name the collision AND both ways out, because an operator who + // only hears "invalid" will try the other illegal shape next. + expect(result.error).toContain("127.0.0.1:10100"); + expect(result.error).toContain("set a distinct unauthenticatedLoopbackListener.port"); + expect(result.error).toContain("remove the listener"); + } + } + }); + + test("the proxy port named in the refusal is the configured one", () => { + const result = validateConfigCandidate(candidate({ port: 8080, hostname: "0.0.0.0" })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toContain("127.0.0.1:8080"); + }); + + test("pointing hostname back at loopback is refused by the same check, not by the next start", () => { + // `ocx config set hostname 127.0.0.1` on a host whose listener is already the companion + // form writes a candidate carrying BOTH keys. Validating only the key being written would + // let this through and turn the next `ocx start` into an EADDRINUSE rollback. + const enabled = validateConfigCandidate(candidate({ hostname: "100.76.170.81" })); + expect(enabled.ok).toBe(true); + const reverted = validateConfigCandidate(candidate({ hostname: "127.0.0.1" })); + expect(reverted.ok).toBe(false); + if (!reverted.ok) expect(reverted.error).toContain("127.0.0.1:10100"); + }); + + test("an explicit port is still required to differ, and still wins over the companion form", () => { + const collision = validateConfigCandidate(candidate({ + hostname: "100.76.170.81", + unauthenticatedLoopbackListener: { enabled: true, port: 10100 }, + })); + expect(collision.ok).toBe(false); + if (!collision.ok) expect(collision.error).toContain("must differ from the proxy port"); + + const ported = validateConfigCandidate(candidate({ + hostname: "100.76.170.81", + unauthenticatedLoopbackListener: { enabled: true, port: 10104 }, + })); + expect(ported.ok).toBe(true); + }); + + test("effectiveLoopbackListenerPort is the single answer to \"where do local callers dial\"", () => { + expect(effectiveLoopbackListenerPort({ unauthenticatedLoopbackListener: { enabled: true } }, 10100)).toBe(10100); + expect(effectiveLoopbackListenerPort({ unauthenticatedLoopbackListener: { enabled: true, port: 10104 } }, 10100)).toBe(10104); + expect(effectiveLoopbackListenerPort({ unauthenticatedLoopbackListener: { enabled: false } }, 10100)).toBeNull(); + expect(effectiveLoopbackListenerPort({}, 10100)).toBeNull(); + expect(effectiveLoopbackListenerPort(undefined, 10100)).toBeNull(); + }); + + test("loopbackCompanionAllowed is the bind-scope half of that decision", () => { + for (const hostname of ["100.76.170.81", "10.0.0.5", "hub.example.test"]) { + expect({ hostname, allowed: loopbackCompanionAllowed(hostname) }).toEqual({ hostname, allowed: true }); + } + for (const hostname of [undefined, "", "localhost", "127.0.0.1", "::1", "[::1]", "0.0.0.0", "::", "[::]", "*"]) { + expect({ hostname, allowed: loopbackCompanionAllowed(hostname) }).toEqual({ hostname, allowed: false }); + } + }); +}); + describe("hub management ingress configuration", () => { const candidate = (overrides: Record = {}) => ({ port: 10100, diff --git a/tests/server/loopback-listener-integration.test.ts b/tests/server/loopback-listener-integration.test.ts index 416fea6f4c..a98c6607a2 100644 --- a/tests/server/loopback-listener-integration.test.ts +++ b/tests/server/loopback-listener-integration.test.ts @@ -865,3 +865,97 @@ describe("Codex injection targets the loopback listener", () => { } }); }); + +/** + * The companion form: `{ enabled: true }` with no port, on a hub bound to a tailnet/LAN + * address (#4236). One port, two sockets — remote clients reach `hostname:port` with a + * credential, local processes reach `127.0.0.1:port` without one. + */ +describe("loopback companion listener", () => { + function companionConfig(hostname: string): OcxConfig { + const config = baseConfig(null) as Record; + config.hostname = hostname; + config.unauthenticatedLoopbackListener = { enabled: true }; + return config as unknown as OcxConfig; + } + + test("binds 127.0.0.1 on the public port, and only loopback is credential-free", async () => { + const address = firstNonLoopbackIPv4(); + if (!address) { + // Without a second address there is no way to distinguish the two sockets, and a silent + // pass would hide exactly the regression this test exists for. + console.warn("[loopback-companion] no non-loopback IPv4 interface; same-port check not run"); + return; + } + const port = await freePort(); + saveConfig(companionConfig(address)); + const logs: string[] = []; + const log = console.log; + const warn = console.warn; + console.log = (...values: unknown[]) => { logs.push(values.join(" ")); }; + console.warn = (...values: unknown[]) => { logs.push(values.join(" ")); }; + let server: ReturnType | null = null; + try { + server = startServer(port); + } finally { + console.log = log; + console.warn = warn; + } + try { + expect(server!.port).toBe(port); + // The same port, two answers: that is the whole feature. + expect((await fetch(`http://127.0.0.1:${port}/v1/models`)).status).toBe(200); + expect((await fetch(`http://${address}:${port}/v1/models`)).status).toBe(401); + + // The startup line has to say "companion" rather than warn about a surprise second + // port: the operator chose this topology, and the ported form's warning misdescribes it. + const startup = logs.join("\n"); + expect(startup).toContain(`Loopback companion active on http://127.0.0.1:${port}`); + expect(startup).toContain("same port as the public listener; local processes need no credential"); + expect(startup).not.toContain("Unauthenticated loopback listener active"); + } finally { + await server!.stop(true); + } + }, SERVER_BUDGET_MS); + + test("the route allowlist is unchanged: sharing a port widens no surface", async () => { + const address = firstNonLoopbackIPv4(); + if (!address) { + console.warn("[loopback-companion] no non-loopback IPv4 interface; allowlist check not run"); + return; + } + const port = await freePort(); + saveConfig(companionConfig(address)); + const server = startServer(port); + try { + // Same socket semantics as the ported form, same default-deny. A companion is a bind + // address change, never an admission change — `/api/*` in particular stays unreachable + // without a management credential, and the Anthropic wire stays off this listener. + for (const path of ["/api/config", "/healthz", "/"]) { + const response = await fetch(`http://127.0.0.1:${port}${path}`); + expect({ path, status: response.status }).toEqual({ path, status: 404 }); + } + const messages = await fetch(`http://127.0.0.1:${port}/v1/messages`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: '{"model":"x","messages":[]}', + }); + expect(messages.status).toBe(404); + expect((await fetch(`http://127.0.0.1:${port}/v1/models`)).status).toBe(200); + } finally { + await server.stop(true); + } + }, SERVER_BUDGET_MS); + + test("an impossible companion fails before any socket opens", async () => { + // A hand edit that skipped validateConfigCandidate must read the same sentence the write + // boundary gives, not EADDRINUSE from a rolled-back startup that looks like a foreign + // process holding the port. + const port = await freePort(); + saveConfig(companionConfig("127.0.0.1")); + expect(() => startServer(port)).toThrow(/a port-less listener binds 127\.0\.0\.1/); + // Nothing was bound: the refusal lands before the public listener opens. + const rebound = Bun.serve({ port, hostname: "127.0.0.1", fetch: () => new Response("ok") }); + await rebound.stop(true); + }, SERVER_BUDGET_MS); +}); From 98dc39b16a03d9d8be32dc70c4b77b4d49ebda4f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:18:27 +0900 Subject: [PATCH 5/8] test(clients): give the extracted handleEnsure harness its new startup-line helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `already-running ensure leaves Raycast untouched…` transpiles the real `handleEnsure` body and evaluates it with every free identifier injected. Reporting a skipped sync now goes through `startupLeftCodexNativeLine` — the hub gate and the Codex toggle must not print the same sentence — so the harness has to supply that name too, or the case fails with a ReferenceError that says nothing about Raycast. Co-Authored-By: Claude Fable 5.1 --- tests/clients/sync-client-integrations.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/clients/sync-client-integrations.test.ts b/tests/clients/sync-client-integrations.test.ts index 65dc41a2b3..e051afb2f4 100644 --- a/tests/clients/sync-client-integrations.test.ts +++ b/tests/clients/sync-client-integrations.test.ts @@ -693,6 +693,10 @@ test("already-running ensure leaves Raycast untouched when saved host and listen loadConfig: () => savedConfig, codexAutoStartEnabled: () => true, syncModelsToCodex: async () => ({ status: "skipped" }), + // handleEnsure reports a skipped sync through this helper (the hub gate and the Codex + // toggle produce different sentences). This harness supplies every free identifier the + // extracted body reads, so it supplies that one too. + startupLeftCodexNativeLine: () => "", refreshOwnedRaycastCatalog: async () => { refreshCalls += 1; writeFileSync(configPath, "wrong saved destination"); From 62b905eeab9c6f29f2e1d8432c27dcc5a782df81 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:19:00 +0900 Subject: [PATCH 6/8] docs(devlog): record the hub single-port companion unit (PR2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit What shipped, the decisions behind it — including the one this PR deliberately did NOT make, widening the loopback route allowlist — and the exact verification commands with their counts. Co-Authored-By: Claude Fable 5.1 --- .../020_loopback_companion.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 devlog/_plan/260911_hub_single_port/020_loopback_companion.md diff --git a/devlog/_plan/260911_hub_single_port/020_loopback_companion.md b/devlog/_plan/260911_hub_single_port/020_loopback_companion.md new file mode 100644 index 0000000000..99a4393076 --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/020_loopback_companion.md @@ -0,0 +1,164 @@ +# 020 — PR2: same-port loopback companion, honest hub-gate messages + +Unit: `devlog/_plan/260911_hub_single_port`. Stack position 2 of 4 (PR1 = launchd repair, +PR3 = hub token UX, PR4 = docs/skill). Branch `codex/260911-l4-hub-loopback-companion`, +based on `dev` = `babb76449` (`Merge pull request #4240 … codex/260911-l4-client-catalog`); it +will be rebased onto PR1's branch. Issue: lidge-jun/opencodex#4236 (follow-ups 1 and 2, +defect 4). + +The assigned worktree was created at `origin/main` (`06ec55363`), 1071 commits behind `dev`, so +the whole branch was rebased onto `dev` before verification. Conflicts were only in +`src/cli/index.ts` (three "startup left Codex native" call sites that `dev` had grown Raycast +comments around) and `src/codex/inject.ts` (the legacy-uncoordinated skip now lives inside +`applyLegacy`, under the config mutation lock); both were resolved by keeping `dev`'s structure +and putting the honest message inside it. Every count below is from the rebased tree. + +## What shipped + +### 1. `ocx gui` on a hub opens the local management ingress (first commit) + +`ocx gui` derived its URL from the proxy bind, so a hub with `hostname: ""` opened a +browser at the tailnet origin while the hub's own loopback management ingress sat unused. +`selectDefaultGuiUrl` now prefers `http://localhost:` when `runtimeRole: "hub"` has +`hub.managementIngress.enabled`; every other topology keeps the previous derivation byte for byte. + +### 2. The companion form of `unauthenticatedLoopbackListener` + +`port` is now optional. `{ "enabled": true }` means "bind `127.0.0.1:`" — one port for +the whole hub: remote clients dial `hostname:port` with a credential, local processes dial +`127.0.0.1:port` without one, and every integration that hardcodes `http://127.0.0.1:` +(`ocx claude`, Claude Desktop, Cursor, `system-env`, the vision helper — the eight sites in the +issue's table) keeps working with **no edit to those call sites**. That was the whole point: the +PR changes where the socket is, not what the clients write. + +Legal only when `hostname` is a specific non-loopback, non-wildcard address. On +`127.0.0.1`/`localhost`/`::1`/`0.0.0.0`/`::` the public listener already owns that loopback +address, so the pair is refused: + +- at the write boundary (`loopbackListenerPortError` → `loopbackCompanionBindError`), reading both + `hostname` and the listener from the same candidate, so `ocx config set hostname 127.0.0.1` on a + companion host is refused by the same check rather than breaking the next start; +- at startup in `startServer`, before any bind, with the identical sentence — a hand edit that + skipped the boundary must not surface as EADDRINUSE from a rolled-back transaction. + +The message names the collision (`127.0.0.1:`) and both fixes: set a distinct +`port`, or drop the listener because a loopback bind already admits local callers. + +### 3. One resolver, every reader + +`effectiveLoopbackListenerPort(config, publicPort)` in `src/codex/loopback-target.ts`, next to +`isLoopbackHostname`, plus `isWildcardHostname` / `loopbackCompanionAllowed`. Used by +`standaloneCodexRoutingTarget` (which `opencodeProxyBaseUrl`, `syncGrokConfig` and the integration +state exporter already go through), the server's listener transaction, and the `ocx status` drift +check. Deliberately NOT used in `chooseListenPort`: only an explicitly ported listener reserves a +port, because the companion form shares the public one and reserving it would refuse every start. + +Startup log now distinguishes the two forms — companion: +`🔁 Loopback companion active on http://127.0.0.1: — same port as the public listener; local +processes need no credential`; ported: today's four-line unauthenticated-surface warning, verbatim. + +### 4. Drift warnings accept the whole locally reachable set (defect 4) + +`grokFenceEndpointDrift` takes an optional second reachable port. `ocx status` passes +`effectiveLoopbackListenerPort(config, listen.port)`, so a fence naming the listener's port — the +port `ocx sync` itself wrote — is no longer reported as drift against a closed port. A third port +still warns, still against the public listener. + +### 5. Hub-gate honesty (follow-up 2) + +`localClientSyncAllowed` refusing to rewrite a hub's own clients is the right decision; reporting +it as the user's toggle was not. New `"hub-gated"` reason and one sentence: + +> This machine is a hub; it does not rewrite its own Codex/Grok/Claude configs unless +> unauthenticatedLoopbackListener is enabled. + +Threaded through `CodexWriteLockSkipReason` → `codexInjectLockOutcome` → `CodexInjectResult` → +`CodexSyncResult`, and used by `ocx sync`, `ocx sync-cache`, the three `startup left Codex native` +lines, and `ocx restore back` — which previously committed the toggle ON and then told the operator +to "retry after the competing integration change finishes", a writer that does not exist. + +`ocx ensure` no longer strips the managed Grok block on a hub-gated skip: only an explicit +`clientIntegrations.grok === false` authorizes the strip. A gated hub with Grok ON is told why +nothing was written and `~/.grok/config.toml` is left exactly as it is. + +## Decisions + +- **The route allowlist was NOT widened.** `loopbackRouteAllowed` still serves only the Codex + data-plane set, so on a companion hub `http://127.0.0.1:/v1/messages` and `/api/*` still + return 404. A companion is a bind-address change, never an admission change (maintainer review + on #4236: "Do not add `/api/*` to the unauthenticated listener"). A test pins this. + **Consequence, recorded as open work:** `ocx claude` and `fetchClaudeCodeState` on a + tailnet-bound hub reach a live socket but get 404 on the Anthropic wire and on + `/api/claude-code`. Closing that needs the two destination contracts the reviewer described — + authenticated local management discovery vs. per-wire inference — which is its own change, not + this one. +- **`port` stays non-OS-assigned in both forms.** An ephemeral port would change across restarts + while running app-servers held the previous `base_url` (#1102). +- **`startServer` stays synchronous** and the companion check is a plain throw before the first + `Bun.serve`, so the listener transaction and its rollback are untouched. +- **The hub-gate reason rides the existing lock skip channel** rather than a parallel one: the + skip is already linearized under the Codex write lock, and a second channel would let the + reason and the write disagree. +- Docs: the English `reference/configuration/server.md` paragraph that said "the port is required" + was false after this change, so it now documents both forms. The full docs/skill rewrite + (en + ko, remote-hub guide) is PR4; the other locale copies still describe only the ported form. + +## Verification (exact commands, this branch) + +``` +bun run typecheck # clean +bun run privacy:scan # Privacy scan passed +bun test tests/server/loopback-listener-admission.test.ts \ + tests/server/loopback-companion-client-targets.test.ts \ + tests/server/server-loopback-host-gate.test.ts # 44 pass +bun test tests/server/loopback-listener-integration.test.ts # 34 pass +bun test tests/cli/hub-gated-local-clients.test.ts \ + tests/cli/cli-dispatch.test.ts # 50 pass +bun test tests/providers/xai/grok-status.test.ts tests/providers/xai/grok-sync.test.ts \ + tests/providers/xai/grok-lifecycle.test.ts \ + tests/codex-integration/codex-desired-state.test.ts \ + tests/codex-integration/codex-inject.test.ts \ + tests/codex-integration/codex-sync-api.test.ts \ + tests/cli/ensure-desired-integrations-race.test.ts # 149 pass +bun test tests/config/config-user-edits.test.ts tests/config/config-load-degrade.test.ts \ + tests/cli/cli-json-contract.test.ts tests/cli/cli-restore-back.test.ts \ + tests/clients/integrations-writer.test.ts tests/clients/sync-client-integrations.test.ts \ + tests/server/startup-prompt.test.ts # 177 pass +bun test tests/cli/cli-transport-honesty.test.ts tests/cli/cli-status-json.test.ts \ + tests/cli/cli-config-command.test.ts tests/cli/cli-start-journal-order.test.ts \ + tests/cli/cli-capabilities.test.ts tests/cli/cli-help.test.ts # 108 pass +bun test tests/codex-integration/codex-write-lock.test.ts \ + tests/codex-integration/codex-inject-write-lock.test.ts \ + tests/codex-integration/codex-composed-acceptance.test.ts \ + tests/codex-integration/codex-history-lock.test.ts \ + tests/lab/lab-activation.test.ts tests/lab/core-lab-boundary.test.ts \ + tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 98 pass +bun test tests/update/update-stop-first.test.ts # 23 pass +``` + +One pre-existing test needed a harness line: `tests/clients/sync-client-integrations.test.ts` +transpiles the real `handleEnsure` body and evaluates it with every free identifier injected, so +the new `startupLeftCodexNativeLine` had to be added to that injection map. + +The GUI does not render this field (`grep -rni loopback gui/src` finds only unrelated copy), so +there is no GUI change and `lint:gui` was not required. + +No repository-wide suite (operator instruction); hosted CI at exact head is the proof. + +New test files registered in `scripts/test-layout/layout.json` and +`tests/fixtures/test-layout-expected.json`: +`tests/server/loopback-companion-client-targets.test.ts`, +`tests/cli/hub-gated-local-clients.test.ts`. + +The same-port integration case needs a non-loopback IPv4 interface to tell the two sockets apart; +on a host without one it warns and returns rather than passing silently (matching the existing +bind-scope case in that file). + +## Left for the rest of the stack + +- PR3: data-plane token auto-provisioning, `ocx hub invite`, the `ocx status` hub block (which + should print the companion state this PR introduces). +- PR4: `guides/remote-hub.md` en + ko around the one-port recipe, the other locale copies of + `reference/configuration/server.md`, `skills/ocx`. +- Follow-up (not in this stack as scoped): the Claude/management destination split described + above, i.e. what `ocx claude` should dial on a companion hub. From e958961b2113c3d058cd4066cfc0e0d0f14dfe60 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 11:32:19 +0900 Subject: [PATCH 7/8] fix(codex): report the hub gate only when the toggle is on, and refuse every wildcard spelling `localClientSkipReason` named the hub gate even when `clientIntegrations.codex` was false, pointing that operator at a listener that would not make the sync happen. It is now the conjunction the Grok path already used (toggle on AND gate closed), per client id. `isWildcardHostname` accepted `::0`, `[::0]`, `0::`, `0:0:0:0:0:0:0:0`, bare `0` and padded IPv4 zeros as specific addresses, so a port-less companion on those binds passed validation and then rolled back with EADDRINUSE. Every all-zero spelling is refused up front now. Co-Authored-By: Claude Fable 5.1 --- .../020_loopback_companion.md | 15 +++++++++++++++ src/codex/desired-state.ts | 18 +++++++++++++++--- src/codex/loopback-target.ts | 9 +++++++-- tests/cli/hub-gated-local-clients.test.ts | 12 ++++++++++++ .../server/loopback-listener-admission.test.ts | 5 ++++- 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/devlog/_plan/260911_hub_single_port/020_loopback_companion.md b/devlog/_plan/260911_hub_single_port/020_loopback_companion.md index 99a4393076..8ee9d64bf2 100644 --- a/devlog/_plan/260911_hub_single_port/020_loopback_companion.md +++ b/devlog/_plan/260911_hub_single_port/020_loopback_companion.md @@ -162,3 +162,18 @@ bind-scope case in that file). `reference/configuration/server.md`, `skills/ocx`. - Follow-up (not in this stack as scoped): the Claude/management destination split described above, i.e. what `ocx claude` should dial on a companion hub. + +## Review round (coordinator) + +Two findings from the read-only review, both fixed in a follow-up commit: + +- `localClientSkipReason` claimed `"hub-gated"` even when the operator's own toggle was OFF, which + would have sent that operator to enable a listener that cannot make the sync happen. The reason is + now the conjunction the Grok path already used: toggle ON **and** gate closed. It takes the client + id (default `codex`) so a Grok OFF does not silence the Codex gate. +- `isWildcardHostname` missed the IPv6 unspecified aliases (`::0`, `[::0]`, `0::`, + `0:0:0:0:0:0:0:0`), bare `0`, and padded IPv4 zeros. A `hostname: "::0"` companion would have + passed both checks and then rolled back with EADDRINUSE — the exact misdiagnosis the check exists + to prevent. Normalisation now strips brackets and matches any all-zero spelling. + +Verification: `bun test tests/cli/hub-gated-local-clients.test.ts tests/server/loopback-listener-admission.test.ts tests/server/loopback-listener-integration.test.ts tests/cli/cli-dispatch.test.ts tests/clients/sync-client-integrations.test.ts` → 144 pass / 0 fail; `bun run typecheck` clean. diff --git a/src/codex/desired-state.ts b/src/codex/desired-state.ts index 1fad88e82a..8f34c65585 100644 --- a/src/codex/desired-state.ts +++ b/src/codex/desired-state.ts @@ -96,8 +96,19 @@ export const HUB_GATED_SKIP_MESSAGE = /** Why a local-client write was skipped. The gate outranks the toggle: it is the surprising one. */ export type LocalClientSkipReason = "desired_disabled" | "hub-gated"; -export function localClientSkipReason(config: LocalClientSyncConfig): LocalClientSkipReason { - return localClientSyncAllowed(config) ? "desired_disabled" : "hub-gated"; +/** + * "hub-gated" is claimed only when the toggle is ON and the gate is what stopped the write. + * With the toggle OFF the gate is moot: telling that operator to enable the loopback listener + * would send them to a key that cannot make the sync happen — the mirror image of the defect + * this reason exists to fix. + */ +export function localClientSkipReason( + config: LocalClientSyncConfig, + client: DurableIntentClientId = "codex", +): LocalClientSkipReason { + return integrationEnabled(config, client) && !localClientSyncAllowed(config) + ? "hub-gated" + : "desired_disabled"; } /** @@ -110,8 +121,9 @@ export function localClientSkipMessage( config: LocalClientSyncConfig, integrationOffMessage: string, hubSuffix?: string, + client: DurableIntentClientId = "codex", ): string { - if (localClientSyncAllowed(config)) return integrationOffMessage; + if (localClientSkipReason(config, client) !== "hub-gated") return integrationOffMessage; return hubSuffix ? `${HUB_GATED_SKIP_MESSAGE} ${hubSuffix}` : HUB_GATED_SKIP_MESSAGE; } diff --git a/src/codex/loopback-target.ts b/src/codex/loopback-target.ts index 1540dfca68..6c4f242e09 100644 --- a/src/codex/loopback-target.ts +++ b/src/codex/loopback-target.ts @@ -44,8 +44,13 @@ export function isLoopbackHostname(hostname: string | undefined): boolean { * collide; on a specific non-loopback address (a tailnet or LAN IP) 127.0.0.1 is free. */ export function isWildcardHostname(hostname: string | undefined): boolean { - const normalized = (hostname ?? "").trim().toLowerCase().replace(/\.$/, ""); - return normalized === "0.0.0.0" || normalized === "::" || normalized === "[::]" || normalized === "*"; + const normalized = (hostname ?? "").trim().toLowerCase().replace(/\.$/, "").replace(/^\[(.*)\]$/, "$1"); + if (normalized === "*" || normalized === "0") return true; + // Every spelling of the IPv4 unspecified address ("0.0.0.0", "00.0.0.000", …). + if (/^(0+\.){3}0+$/.test(normalized)) return true; + // Every spelling of the IPv6 unspecified address ("::", "::0", "0::", "0:0:0:0:0:0:0:0", …): + // nothing but zero groups and colons. A dual-stack `::` bind answers on 127.0.0.1 as well. + return normalized.includes(":") && /^[0:]+$/.test(normalized); } /** diff --git a/tests/cli/hub-gated-local-clients.test.ts b/tests/cli/hub-gated-local-clients.test.ts index 5eca3af702..9c7ad0f05e 100644 --- a/tests/cli/hub-gated-local-clients.test.ts +++ b/tests/cli/hub-gated-local-clients.test.ts @@ -56,6 +56,18 @@ describe("the hub gate has its own reason and its own sentence", () => { expect(localClientSkipReason(hubConfig({ runtimeRole: undefined }))).toBe("desired_disabled"); }); + test("a real OFF on a gated hub is reported as the toggle, not the gate", () => { + // Enabling the listener would not make this sync happen, so naming the gate here would + // send the operator to the wrong key — the mirror image of the defect the reason fixes. + const off = hubConfig({ clientIntegrations: { codex: false } }); + expect(localClientSyncAllowed(off)).toBe(false); + expect(localClientSkipReason(off)).toBe("desired_disabled"); + expect(localClientSkipMessage(off, "Codex integration is OFF")).toBe("Codex integration is OFF"); + // Per-client: a Grok OFF does not silence the Codex gate and vice versa. + expect(localClientSkipReason(off, "grok")).toBe("hub-gated"); + expect(localClientSkipReason(hubConfig({ clientIntegrations: { grok: false } }))).toBe("hub-gated"); + }); + test("the message names the hub and the key that opens the gate", () => { expect(HUB_GATED_SKIP_MESSAGE).toContain("This machine is a hub"); expect(HUB_GATED_SKIP_MESSAGE).toContain("unauthenticatedLoopbackListener"); diff --git a/tests/server/loopback-listener-admission.test.ts b/tests/server/loopback-listener-admission.test.ts index 6ec1bd5028..d6a88e7408 100644 --- a/tests/server/loopback-listener-admission.test.ts +++ b/tests/server/loopback-listener-admission.test.ts @@ -201,7 +201,10 @@ describe("loopback companion listener configuration", () => { test("a port-less listener is refused wherever the public listener already holds loopback", () => { // Wildcards included: 0.0.0.0 answers on 127.0.0.1 too, so the companion would collide // there just as surely as on an explicit loopback bind. - for (const hostname of [undefined, "127.0.0.1", "localhost", "::1", "0.0.0.0", "::", "[::]"]) { + for (const hostname of [ + undefined, "127.0.0.1", "localhost", "::1", "[::1]", "0.0.0.0", "00.0.0.000", "0", "*", + "::", "[::]", "::0", "[::0]", "0::", "0:0:0:0:0:0:0:0", + ]) { const result = validateConfigCandidate(candidate(hostname === undefined ? {} : { hostname })); expect({ hostname, ok: result.ok }).toEqual({ hostname, ok: false }); if (!result.ok) { From 216032d89dff5ae400c9afb46a4674a6b8e01e4e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:20:04 +0900 Subject: [PATCH 8/8] test(service): point the gui-url source oracle at selectDefaultGuiUrl The oracle pinned the pre-#4236 line; the hub-aware selector keeps the bind-host branch, so pin both the call and the retained branch. Co-Authored-By: Claude Fable 5.1 --- tests/service/stale-state-purge.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/service/stale-state-purge.test.ts b/tests/service/stale-state-purge.test.ts index ceac7e5c77..9646a618f4 100644 --- a/tests/service/stale-state-purge.test.ts +++ b/tests/service/stale-state-purge.test.ts @@ -70,7 +70,10 @@ describe("snapshot-guarded stale-state purge", () => { test("gui opens the actual bind host and recover-history surfaces a locked DB", () => { const cliSource = readFileSync(repoPath("src", "cli", "index.ts"), "utf8"); const dispatchSource = readFileSync(repoPath("src", "cli", "dispatch.ts"), "utf8"); - expect(dispatchSource).toContain("const guiHost = deps.probeHostname(live?.hostname ?? config.hostname)"); + // The gui URL is chosen by selectDefaultGuiUrl: a hub with the management ingress opens the + // loopback-only dashboard, everything else still opens the actual bind host (#4236). + expect(dispatchSource).toContain("const guiUrl = selectDefaultGuiUrl(config, live, deps.probeHostname)"); + expect(dispatchSource).toContain("const guiHost = probeHostname(live?.hostname ?? config.hostname)"); const recoverFn = cliSource.slice(cliSource.indexOf("function handleRecoverHistory()"), cliSource.indexOf("await dispatchCommand(head")); expect(recoverFn).toContain("if (r.failed)"); expect(recoverFn).toContain("process.exit(1)");