From 2a53d7e7aa5d37e0311bf491ab0a8518e181816f Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:05:14 +0900 Subject: [PATCH 01/10] feat(server): serve the hub's provider, login and roster state on the data plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connected client had no way to ask the hub what it can actually serve, so `ocx status` on the client reported the client's own empty credential store as if it were the truth: `xai ✗ not logged in` on a machine whose hub has xAI logged in and is serving grok. An agent working on such a client read that output and concluded the hub could not serve grok. `GET|HEAD /v1/hub-state` is the least-privilege read that fixes it, built in the `/v1/catalog` (#809) tradition: the same `resolveApiAuth` admission set, the same origin check, no parameters, `no-store`, no validator, and a fixed bounded body of booleans plus model ids. The alternative operators reach for — handing the client an admin token so it can call `GET /api/providers` — is exactly the trade #809 already refused, and widening `/api/*` to the data plane would be worse. What crosses the boundary is deliberately narrow. `hasCredential` is the same `!!p.apiKey` presence projection `GET /api/providers` ships; `loggedIn` is `oauthLoginSummary`'s boolean with the email and account id dropped rather than masked. Provider names already leak through `/v1/catalog` slugs, so the delta is those two booleans. The projection builds every row field by field for that reason: a spread would silently start exporting whatever field is added to a provider or a login record next. The route 404s with its own `hub_state_not_a_hub` code unless `runtimeRole === "hub"`, so a standalone install gains no surface at all, and that gate runs AFTER admission on purpose — answering an anonymous caller would turn the route into a free "is that host a hub?" probe. Co-Authored-By: Claude Fable 5.1 --- scripts/test-layout/layout.json | 7 +- src/remote/hub-state.ts | 155 ++++++++++++++++ src/server/auth-cors.ts | 5 + src/server/hub-state.ts | 79 ++++++++ src/server/index.ts | 70 +++++++ tests/fixtures/test-layout-expected.json | 7 +- tests/server/api-key-attribution.test.ts | 7 +- tests/server/v1-hub-state.test.ts | 225 +++++++++++++++++++++++ 8 files changed, 552 insertions(+), 3 deletions(-) create mode 100644 src/remote/hub-state.ts create mode 100644 src/server/hub-state.ts create mode 100644 tests/server/v1-hub-state.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index ad361fc548..fad27db650 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -290,6 +290,7 @@ "cl01-review-regressions.test.ts": "routing", "claude-529-mapping.test.ts": "claude-integration", "claude-agent-startup-sync.test.ts": "claude-integration", + "claude-agents-inject-client.test.ts": "claude-integration", "claude-agents-inject.test.ts": "claude-integration", "claude-alias.test.ts": "claude-integration", "claude-auth-detect.test.ts": "claude-integration", @@ -332,6 +333,7 @@ "cli-codex-log-guard-protection.test.ts": "cli", "cli-codex-log-guard.test.ts": "cli", "cli-config-command.test.ts": "cli", + "cli-config-show-client.test.ts": "cli", "cli-dispatch.test.ts": "cli", "cli-dto-fidelity.test.ts": "cli", "cli-export-command.test.ts": "cli", @@ -354,6 +356,7 @@ "cli-restart-health.test.ts": "cli", "cli-restore-back.test.ts": "cli", "cli-start-journal-order.test.ts": "cli", + "cli-status-hub-state.test.ts": "cli", "cli-status-json.test.ts": "cli", "cli-status-oauth-health.test.ts": "cli", "cli-storage-inspect.test.ts": "cli", @@ -368,6 +371,7 @@ "client-export-modality-enum.test.ts": "clients", "client-fingerprint.test.ts": "clients", "client-hub-relay.test.ts": "clients", + "client-hub-state.test.ts": "clients", "client-injection-guard.test.ts": "codex-integration", "client-lifecycle-lock.test.ts": "clients", "client-machine-listener.test.ts": "clients", @@ -702,9 +706,9 @@ "health-scoring.test.ts": "server", "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", "hub-invite.test.ts": "cli", + "hyperbolic-provider.test.ts": "providers", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", @@ -1270,6 +1274,7 @@ "user-cost-overlay-coderabbit-regressions.test.ts": "usage", "user-cost-overlay-live-reconcile.test.ts": "usage", "user-cost-overlay-provider-delete.test.ts": "usage", + "v1-hub-state.test.ts": "server", "v2-agent-message-failfast.test.ts": "server", "vercel-gateway-provider-routing.test.ts": "providers", "version-line.test.ts": "ci-workflows", diff --git a/src/remote/hub-state.ts b/src/remote/hub-state.ts new file mode 100644 index 0000000000..7ac48b7bc6 --- /dev/null +++ b/src/remote/hub-state.ts @@ -0,0 +1,155 @@ +/** + * The hub-state contract shared by `GET|HEAD /v1/hub-state` and every connected client. + * + * Why it exists (#4236): an agent working on a connected client machine read that machine's + * own `~/.opencodex/config.json` and `ocx status`, saw `xai ✗ not logged in`, no grok provider + * and five delegable models, and concluded the hub could not serve grok — while the hub had + * xAI logged in and was serving grok all along. A client's local credential store is empty BY + * DESIGN, so reporting it as the truth is not a cosmetic defect: it makes the client lie about + * the only machine that has the facts. + * + * What may cross this boundary is deliberately narrow. The client holds a per-client DATA key + * and no management credential, so this is a least-privilege data-plane read in the `/v1/catalog` + * (#809) tradition rather than a widened `/api/*` boundary. The payload is BOOLEANS and model + * ids: `hasCredential` is the same `!!p.apiKey` projection `GET /api/providers` already ships, + * and `loggedIn` is `oauthLoginSummary`'s boolean with the email and account id dropped + * entirely. No keys, no tokens, no quotas, no usage, no account identity — and nothing of that + * shape may be added later, because this surface is reachable with a data key. + * + * Provider NAMES already leak through `/v1/catalog` slugs, so the delta this adds is only the + * two booleans. + */ + +export const HUB_STATE_SCHEMA_VERSION = 1; + +/** Hard caps, so the serialized body is bounded by construction rather than by hope. */ +export const MAX_HUB_STATE_PROVIDERS = 200; +export const MAX_HUB_STATE_SUBAGENT_MODELS = 32; +export const MAX_HUB_STATE_OAUTH_PROVIDERS = 200; +export const MAX_HUB_STATE_STRING_CHARS = 200; +/** Response/transfer ceiling. The caps above keep a realistic body two orders below this. */ +export const MAX_HUB_STATE_BYTES = 64 * 1024; + +/** + * Mirrors `OcxProviderConfig.authMode` (src/types/provider.ts); default `"key"`. + * + * It is shape, not secret: it says HOW a provider authenticates, which is what lets a client + * explain `hasCredential: false` on an `oauth` provider without claiming nothing is configured. + */ +export const HUB_STATE_AUTH_MODES = ["key", "forward", "oauth", "local"] as const; +export type HubStateAuthMode = (typeof HUB_STATE_AUTH_MODES)[number] | null; + +export interface HubStateProvider { + name: string; + adapter: string; + authMode: HubStateAuthMode; + /** Presence only — the same `!!p.apiKey` projection `GET /api/providers` ships. */ + hasCredential: boolean; + disabled: boolean; +} + +export interface HubStateOAuthEntry { + provider: string; + /** `oauthLoginSummary().loggedIn`. The email and account id are dropped, not masked. */ + loggedIn: boolean; +} + +export interface HubStateDTO { + schemaVersion: typeof HUB_STATE_SCHEMA_VERSION; + /** Always "hub": the route 404s on any other role, so a client can trust what it reads. */ + runtimeRole: "hub"; + hubVersion: string; + /** `hub.dataPublicOrigin` when the operator set one; null rather than a guess. */ + origin: string | null; + providers: HubStateProvider[]; + oauth: HubStateOAuthEntry[]; + /** The hub's effective featured subagent roster — what a client should delegate to. */ + subagentModels: string[]; + claudeCode: { enabled: boolean }; +} + +function boundedString(value: unknown, max = MAX_HUB_STATE_STRING_CHARS): string | null { + if (typeof value !== "string") return null; + const trimmed = value.trim(); + if (!trimmed || trimmed.length > max || /[\x00-\x1f\x7f]/.test(trimmed)) return null; + return trimmed; +} + +/** + * Validate a hub-state body received over the wire. + * + * Returns null instead of throwing so both the live fetch and the on-disk cache can reject a + * malformed document the same way, and so a client NEVER degrades to its own local login state + * on a shape it does not recognize — degrading quietly is the defect being fixed. + * + * Unknown keys are dropped rather than refused: a newer hub must be readable by an older + * client, and the fields this projection reads are all required. + */ +export function parseHubStateBody(value: unknown): HubStateDTO | null { + if (!value || typeof value !== "object" || Array.isArray(value)) return null; + const raw = value as Record; + if (raw.schemaVersion !== HUB_STATE_SCHEMA_VERSION) return null; + if (raw.runtimeRole !== "hub") return null; + const hubVersion = boundedString(raw.hubVersion, 64); + if (!hubVersion) return null; + const origin = raw.origin === null || raw.origin === undefined ? null : boundedString(raw.origin, 512); + if (raw.origin !== null && raw.origin !== undefined && origin === null) return null; + if (!Array.isArray(raw.providers) || raw.providers.length > MAX_HUB_STATE_PROVIDERS) return null; + if (!Array.isArray(raw.oauth) || raw.oauth.length > MAX_HUB_STATE_OAUTH_PROVIDERS) return null; + if (!Array.isArray(raw.subagentModels) || raw.subagentModels.length > MAX_HUB_STATE_SUBAGENT_MODELS) return null; + const claudeCode = raw.claudeCode; + if (!claudeCode || typeof claudeCode !== "object" || Array.isArray(claudeCode)) return null; + const enabled = (claudeCode as Record).enabled; + if (typeof enabled !== "boolean") return null; + + const providers: HubStateProvider[] = []; + for (const row of raw.providers) { + if (!row || typeof row !== "object" || Array.isArray(row)) return null; + const entry = row as Record; + const name = boundedString(entry.name); + const adapter = boundedString(entry.adapter); + if (!name || !adapter) return null; + if (typeof entry.hasCredential !== "boolean" || typeof entry.disabled !== "boolean") return null; + const authMode = entry.authMode; + const normalizedAuthMode = typeof authMode === "string" && (HUB_STATE_AUTH_MODES as readonly string[]).includes(authMode) + ? authMode as NonNullable + : null; + // An unrecognized authMode is refused rather than nulled: nulling it would let a newer hub's + // new mode read as "unset", which is a different claim about the provider. + if (authMode !== null && authMode !== undefined && normalizedAuthMode === null) return null; + providers.push({ + name, + adapter, + authMode: normalizedAuthMode, + hasCredential: entry.hasCredential, + disabled: entry.disabled, + }); + } + + const oauth: HubStateOAuthEntry[] = []; + for (const row of raw.oauth) { + if (!row || typeof row !== "object" || Array.isArray(row)) return null; + const entry = row as Record; + const provider = boundedString(entry.provider); + if (!provider || typeof entry.loggedIn !== "boolean") return null; + oauth.push({ provider, loggedIn: entry.loggedIn }); + } + + const subagentModels: string[] = []; + for (const row of raw.subagentModels) { + const model = boundedString(row); + if (!model) return null; + subagentModels.push(model); + } + + return { + schemaVersion: HUB_STATE_SCHEMA_VERSION, + runtimeRole: "hub", + hubVersion, + origin, + providers, + oauth, + subagentModels, + claudeCode: { enabled }, + }; +} diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index d103721f35..8b845c8460 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -439,6 +439,11 @@ export const AUTH_MATRIX: readonly ApiAuthMatrixRow[] = [ // /v1/models and for the same reason — it forwards no caller credential upstream — so a // remote client no longer needs an admin token just to read the model catalog. { endpoint: "/v1/catalog", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, + // #4236: the hub-state read a connected client uses instead of reporting its own empty + // credential store. Same admission set and the same justification as the two rows above — + // it forwards no caller credential upstream and its body is booleans plus model ids — and it + // 404s on any host whose runtimeRole is not "hub", so no standalone install gains a surface. + { endpoint: "/v1/hub-state", bearer: "accepted", dedicated: "accepted", xApiKey: "accepted" }, ]; /** Whether `token` is the environment-provided management secret. */ diff --git a/src/server/hub-state.ts b/src/server/hub-state.ts new file mode 100644 index 0000000000..8ff30f568d --- /dev/null +++ b/src/server/hub-state.ts @@ -0,0 +1,79 @@ +/** + * The hub's own projection for `GET|HEAD /v1/hub-state` (#4236). + * + * Pure: it takes the config and an already-computed login summary and returns the DTO. The + * route owns admission, the role gate and the size ceiling; this module owns what a client is + * allowed to learn. Keeping the projection here — and building each row field by field rather + * than spreading a provider or a login summary — is what makes "no keys, no emails, no account + * ids" checkable by reading one function. A spread would silently start exporting whatever the + * next field added to those records happens to be. + * + * Contract and caps live in `src/remote/hub-state.ts` so the client validates the same shape. + */ +import { + HUB_STATE_SCHEMA_VERSION, + MAX_HUB_STATE_OAUTH_PROVIDERS, + MAX_HUB_STATE_PROVIDERS, + MAX_HUB_STATE_SUBAGENT_MODELS, + HUB_STATE_AUTH_MODES, + type HubStateDTO, + type HubStateOAuthEntry, + type HubStateProvider, +} from "../remote/hub-state"; +import { DEFAULT_SUBAGENT_MODELS } from "../config/subagent-models"; +import type { OcxConfig } from "../types"; + +export type HubStateConfigView = Pick; + +/** A login summary row as `oauthLoginSummary()` returns it; extra fields are never read. */ +export interface HubStateLoginRow { + provider: string; + loggedIn: boolean; +} + +/** + * The hub's effective featured roster: the same "unset means the defaults, an explicit `[]` + * means none" rule `buildClaudeAgentDefs` applies, so a client that delegates from this list + * sees exactly what the hub itself would offer. + */ +export function hubSubagentRoster(config: Pick): string[] { + const roster = config.subagentModels === undefined ? DEFAULT_SUBAGENT_MODELS : config.subagentModels; + return roster + .filter((entry): entry is string => typeof entry === "string" && entry.trim() !== "") + .map(entry => entry.trim()) + .slice(0, MAX_HUB_STATE_SUBAGENT_MODELS); +} + +export function buildHubState( + config: HubStateConfigView, + logins: readonly HubStateLoginRow[], + hubVersion: string, +): HubStateDTO { + const providers: HubStateProvider[] = Object.entries(config.providers ?? {}) + .slice(0, MAX_HUB_STATE_PROVIDERS) + .map(([name, provider]) => ({ + name, + adapter: provider.adapter, + authMode: provider.authMode !== undefined && HUB_STATE_AUTH_MODES.includes(provider.authMode) + ? provider.authMode + : null, + // Presence only. Identical to the projection GET /api/providers already ships. + hasCredential: Boolean(provider.apiKey), + disabled: provider.disabled === true, + })); + // Field-by-field, never a spread: oauthLoginSummary also carries the operator's email. + const oauth: HubStateOAuthEntry[] = logins + .slice(0, MAX_HUB_STATE_OAUTH_PROVIDERS) + .map(entry => ({ provider: entry.provider, loggedIn: entry.loggedIn === true })); + return { + schemaVersion: HUB_STATE_SCHEMA_VERSION, + runtimeRole: "hub", + hubVersion, + origin: config.hub?.dataPublicOrigin ?? null, + providers, + oauth, + subagentModels: hubSubagentRoster(config), + // Same predicate the launch path uses: absence means enabled. + claudeCode: { enabled: config.claudeCode?.enabled !== false }, + }; +} diff --git a/src/server/index.ts b/src/server/index.ts index e5d6b182f6..b0f57eadbe 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -1401,6 +1401,76 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server MAX_HUB_STATE_BYTES) { + return withCors( + new Response(JSON.stringify({ + error: { type: "server_error", code: "hub_state_too_large", message: "hub state exceeds the maximum served size" }, + }), { status: 507, headers: { "content-type": "application/json" } }), + req, + policy, + ); + } + return withCors( + new Response(req.method === "HEAD" ? null : body, { + status: 200, + headers: { + "content-type": "application/json", + // Varies by credential-bearing identity and by live login state: never cached, + // and no validator to revalidate with (same rule as /v1/catalog). + "cache-control": "no-store", + "content-length": String(bytes), + }, + }), + req, + policy, + ); + } + if (url.pathname === "/v1/models" && req.method === "GET") { // #809: the catalog read sits immediately before model discovery because it shares // that route's admission rationale exactly. Keep them adjacent so a future change to diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 034fb57f71..e8e712c839 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -125,6 +125,7 @@ "cl01-review-regressions.test.ts": "routing", "claude-529-mapping.test.ts": "claude-integration", "claude-agent-startup-sync.test.ts": "claude-integration", + "claude-agents-inject-client.test.ts": "claude-integration", "claude-agents-inject.test.ts": "claude-integration", "claude-alias.test.ts": "claude-integration", "claude-auth-detect.test.ts": "claude-integration", @@ -167,6 +168,7 @@ "cli-codex-log-guard-protection.test.ts": "cli", "cli-codex-log-guard.test.ts": "cli", "cli-config-command.test.ts": "cli", + "cli-config-show-client.test.ts": "cli", "cli-dispatch.test.ts": "cli", "cli-dto-fidelity.test.ts": "cli", "cli-export-command.test.ts": "cli", @@ -189,6 +191,7 @@ "cli-restart-health.test.ts": "cli", "cli-restore-back.test.ts": "cli", "cli-start-journal-order.test.ts": "cli", + "cli-status-hub-state.test.ts": "cli", "cli-status-json.test.ts": "cli", "cli-status-oauth-health.test.ts": "cli", "cli-storage-inspect.test.ts": "cli", @@ -203,6 +206,7 @@ "client-export-modality-enum.test.ts": "clients", "client-fingerprint.test.ts": "clients", "client-hub-relay.test.ts": "clients", + "client-hub-state.test.ts": "clients", "client-injection-guard.test.ts": "codex-integration", "client-lifecycle-lock.test.ts": "clients", "client-machine-listener.test.ts": "clients", @@ -537,9 +541,9 @@ "health-scoring.test.ts": "server", "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", "hub-invite.test.ts": "cli", + "hyperbolic-provider.test.ts": "providers", "identity-neutralize.test.ts": "adapters", "init-backup-cleanup.test.ts": "service", "init-eof.test.ts": "service", @@ -1105,6 +1109,7 @@ "user-cost-overlay-coderabbit-regressions.test.ts": "usage", "user-cost-overlay-live-reconcile.test.ts": "usage", "user-cost-overlay-provider-delete.test.ts": "usage", + "v1-hub-state.test.ts": "server", "v2-agent-message-failfast.test.ts": "server", "vercel-gateway-provider-routing.test.ts": "providers", "version-line.test.ts": "ci-workflows", diff --git a/tests/server/api-key-attribution.test.ts b/tests/server/api-key-attribution.test.ts index 66c0a24759..6be92c4652 100644 --- a/tests/server/api-key-attribution.test.ts +++ b/tests/server/api-key-attribution.test.ts @@ -632,7 +632,8 @@ describe("AUTH_MATRIX is true of the running server", () => { // Read-only endpoints must be exercised with GET: sending POST would draw a 405 // from routing and the assertions below would be testing the method guard rather // than admission. /v1/catalog joined this set in #809. - const isGet = row.endpoint === "/v1/models" || row.endpoint === "/v1/catalog"; + const isGet = row.endpoint === "/v1/models" || row.endpoint === "/v1/catalog" + || row.endpoint === "/v1/hub-state"; const res = await fetch(new URL(row.endpoint, server.url), { method: isGet ? "GET" : "POST", headers: { "content-type": "application/json", ...headers }, @@ -660,6 +661,10 @@ describe("AUTH_MATRIX is true of the running server", () => { // which is admission proof rather than a missing route. Pin the distinguishing // code so a deleted route still cannot pass here. if (row.endpoint === "/v1/catalog") expect(body.error?.code).toBe("catalog_not_found"); + // /v1/hub-state 404s for the same kind of reason (#4236): this fixture is not a + // hub, and the role gate runs AFTER admission, so reaching the gate is itself the + // admission proof. Pin its distinguishing code too. + if (row.endpoint === "/v1/hub-state") expect(body.error?.code).toBe("hub_state_not_a_hub"); } const admitted = res.status !== 401; expect({ endpoint: row.endpoint, headers: Object.keys(headers)[0], admitted }) diff --git a/tests/server/v1-hub-state.test.ts b/tests/server/v1-hub-state.test.ts new file mode 100644 index 0000000000..215ab3ef39 --- /dev/null +++ b/tests/server/v1-hub-state.test.ts @@ -0,0 +1,225 @@ +/** + * `GET|HEAD /v1/hub-state` — the hub's answer to "what can you actually serve?" (#4236). + * + * Two things are being pinned, and they pull in opposite directions. The route must be + * REACHABLE with nothing but a per-client data key, because the client holds nothing else and + * the alternative an operator reaches for is handing out an admin token. And it must carry no + * credential, no email and no account id, because a data key is the weakest thing that opens + * it. The serialized-body scan below is the half that cannot be satisfied by reading the + * projection: it configures real-looking provider keys and a real-looking OAuth credential + * (access token, refresh token, email) and asserts none of those bytes appear in the response. + * + * The role gate gets its own cases because it is the reason a standalone install gains no new + * surface at all, and because it runs AFTER admission on purpose — answering an anonymous + * caller would turn the route into a free "is that host a hub?" probe. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../../src/config"; +import { startServer } from "../../src/server"; +import { MAX_HUB_STATE_BYTES, parseHubStateBody } from "../../src/remote/hub-state"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const DATA_KEY = "ocx_data_hubstatereader"; +const PROVIDER_KEY = "sk-hub-state-provider-secret-9e1f"; +const OAUTH_ACCESS = "oauth-access-hub-state-7c2a"; +const OAUTH_REFRESH = "oauth-refresh-hub-state-4b8d"; +const OAUTH_EMAIL = "hub-operator@example.test"; + +const previousHome = process.env.OPENCODEX_HOME; +const previousDataToken = process.env.OPENCODEX_API_AUTH_TOKEN; +let testHome = ""; + +function hubConfig(overrides: Partial = {}): OcxConfig { + return { + port: 0, + // Non-loopback so admission is required and the origin check is live. + hostname: "0.0.0.0", + defaultProvider: "xai", + runtimeRole: "hub", + hub: { dataPublicOrigin: "https://hub.example.test:8443" }, + providers: { + xai: { + adapter: "openai-chat", + baseUrl: "https://api.x.ai/v1", + apiKey: PROVIDER_KEY, + authMode: "oauth", + models: ["grok-4.6"], + }, + quiet: { adapter: "openai-chat", baseUrl: "https://example.test/v1", disabled: true, models: ["m"] }, + }, + subagentModels: ["xai/grok-4.6", "gpt-5.6-sol"], + // Pinned so the one-time roster migration does not prepend the native default and make the + // roster assertion below about migration rather than about what the hub reports. + subagentModelsVersion: 1, + apiKeys: [{ id: "client-one", name: "laptop", key: DATA_KEY, createdAt: "2026-09-01T00:00:00.000Z" }], + ...overrides, + } as OcxConfig; +} + +/** The legacy single-credential shape normalizes on load, which is all this needs. */ +function writeLoggedInXai(): void { + writeFileSync(join(testHome, "auth.json"), JSON.stringify({ + xai: { + access: OAUTH_ACCESS, + refresh: OAUTH_REFRESH, + expires: Date.now() + 3_600_000, + email: OAUTH_EMAIL, + }, + })); +} + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "ocx-hub-state-")); + process.env.OPENCODEX_HOME = testHome; + process.env.OPENCODEX_API_AUTH_TOKEN = "hub-admission-secret"; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (previousDataToken === undefined) delete process.env.OPENCODEX_API_AUTH_TOKEN; + else process.env.OPENCODEX_API_AUTH_TOKEN = previousDataToken; + if (testHome) removeTreeWithRetry(testHome); + testHome = ""; +}); + +describe("GET /v1/hub-state", () => { + test("refuses an unauthenticated read before it reveals whether this host is a hub", async () => { + saveConfig(hubConfig()); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url)); + expect(res.status).toBe(401); + // The role is not disclosed on the 401 path: the gate runs after admission. + const body = await res.text(); + expect(body).not.toContain("hub_state_not_a_hub"); + } finally { + await server.stop(true); + } + }); + + test("a per-client data key reads the hub's providers, logins and roster", async () => { + saveConfig(hubConfig()); + writeLoggedInXai(); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + headers: { "x-opencodex-api-key": DATA_KEY }, + }); + expect(res.status).toBe(200); + expect(res.headers.get("cache-control")).toBe("no-store"); + // No validator to revalidate with, for the same reason /v1/catalog emits none. + expect(res.headers.get("etag")).toBeNull(); + const text = await res.text(); + const state = parseHubStateBody(JSON.parse(text)); + expect(state).not.toBeNull(); + expect(state!.runtimeRole).toBe("hub"); + expect(state!.origin).toBe("https://hub.example.test:8443"); + expect(typeof state!.hubVersion).toBe("string"); + // The exact defect: the hub HAS xai and IS logged in, and a client must be able to see it. + expect(state!.providers.find(p => p.name === "xai")).toEqual({ + name: "xai", + adapter: "openai-chat", + authMode: "oauth", + hasCredential: true, + disabled: false, + }); + expect(state!.providers.find(p => p.name === "quiet")?.disabled).toBe(true); + expect(state!.oauth.find(entry => entry.provider === "xai")?.loggedIn).toBe(true); + expect(state!.subagentModels).toEqual(["xai/grok-4.6", "gpt-5.6-sol"]); + expect(state!.claudeCode.enabled).toBe(true); + expect(Number(res.headers.get("content-length"))).toBe(Buffer.byteLength(text)); + expect(Buffer.byteLength(text)).toBeLessThanOrEqual(MAX_HUB_STATE_BYTES); + } finally { + await server.stop(true); + } + }); + + test("the serialized body carries no key, token, email or account id", async () => { + saveConfig(hubConfig()); + writeLoggedInXai(); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + headers: { "x-opencodex-api-key": DATA_KEY }, + }); + expect(res.status).toBe(200); + const text = await res.text(); + for (const secret of [PROVIDER_KEY, OAUTH_ACCESS, OAUTH_REFRESH, OAUTH_EMAIL, DATA_KEY, "hub-admission-secret"]) { + expect(text).not.toContain(secret); + } + // Field names too: an accidental spread would bring the key along with its value. + for (const field of ["apiKey", "accessToken", "refreshToken", "\"access\"", "\"refresh\"", "email", "accountId", "activeAccountId"]) { + expect(text).not.toContain(field); + } + } finally { + await server.stop(true); + } + }); + + test("HEAD answers with the same status and headers and no body", async () => { + saveConfig(hubConfig()); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + method: "HEAD", + headers: { "x-opencodex-api-key": DATA_KEY }, + }); + expect(res.status).toBe(200); + expect(res.headers.get("content-type")).toBe("application/json"); + expect(Number(res.headers.get("content-length"))).toBeGreaterThan(0); + expect(await res.text()).toBe(""); + } finally { + await server.stop(true); + } + }); + + test("a cross-origin browser read is refused even with a valid key", async () => { + saveConfig(hubConfig()); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + headers: { "x-opencodex-api-key": DATA_KEY, origin: "https://attacker.test" }, + }); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ error: { code: "origin_rejected" } }); + } finally { + await server.stop(true); + } + }); + + test.each(["standalone", undefined] as const)("runtimeRole %s serves no hub state", async role => { + saveConfig(hubConfig(role === undefined ? { runtimeRole: undefined } : { runtimeRole: role })); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + headers: { "x-opencodex-api-key": DATA_KEY }, + }); + expect(res.status).toBe(404); + // A distinct code, not the generic not_found: it is what tells "this host is not a hub" + // apart from "this build has no such route", and the latter would pass vacuously. + expect(await res.json()).toMatchObject({ error: { code: "hub_state_not_a_hub" } }); + } finally { + await server.stop(true); + } + }); + + test("POST is not a hub-state verb", async () => { + saveConfig(hubConfig()); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + method: "POST", + headers: { "x-opencodex-api-key": DATA_KEY, "content-type": "application/json" }, + body: "{}", + }); + expect(res.status).not.toBe(200); + } finally { + await server.stop(true); + } + }); +}); From a9ea013786b23fa2b475e02f962d48898693db5e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:06:13 +0900 Subject: [PATCH 02/10] feat(client): read hub state over the data plane, and never degrade to local state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fetchHubState` sits beside `downloadClientCatalog` because it is the same kind of call: one bounded, schema-validated, unconditional GET with the per-client data key. There is deliberately no management variant — the client holds no hub management credential, and minting one to read a list of booleans is the trade #809 already refused. `resolveHubState` is where the actual fix lives. Every failure path — a 404 from a hub too old to serve the route, a 401, an unreachable host, a non-JSON or malformed or foreign-schema body, an oversized one — lands on `cache` or `unavailable` with a reason, and none of them reaches back into the client's own providers and logins. That fallback is invisible in output, because local state renders exactly like hub state, which is how a client came to report `xai ✗ not logged in` about a hub that had xAI logged in. The last good response is cached 0600 at `/hub-state.json`, stamped with the (serverUrl, apiKeyId, connectedAt) triple the rest of the client lifecycle compares on. A stale cache is still the HUB's state; an unstamped one would be a different hub's, which after a disconnect and reconnect is not staleness but a lie. Co-Authored-By: Claude Fable 5.1 --- src/client/hub-client.ts | 52 ++++++ src/client/hub-state.ts | 199 ++++++++++++++++++++ tests/clients/client-hub-state.test.ts | 239 +++++++++++++++++++++++++ 3 files changed, 490 insertions(+) create mode 100644 src/client/hub-state.ts create mode 100644 tests/clients/client-hub-state.test.ts diff --git a/src/client/hub-client.ts b/src/client/hub-client.ts index 43e7105e7e..707605f2bb 100644 --- a/src/client/hub-client.ts +++ b/src/client/hub-client.ts @@ -1,4 +1,5 @@ import { MAX_REMOTE_CATALOG_BYTES } from "../server/catalog-download"; +import { MAX_HUB_STATE_BYTES, parseHubStateBody, type HubStateDTO } from "../remote/hub-state"; import { readBoundedResponseBytes } from "../lib/bounded-body"; import { clearableDeadline } from "../lib/abort"; import type { Desktop3pModelEntry } from "../claude/desktop-3p"; @@ -471,6 +472,57 @@ export async function downloadClientCatalog( return { kind: "fresh", body, ...(keyId ? { keyId } : {}) }; } +/** + * Read the hub's provider/login/roster state with the per-client DATA key (#4236). + * + * Sits beside `downloadClientCatalog` because it is the same kind of call: one bounded, + * schema-validated, unconditional GET on the data plane with the credential the client already + * holds. It deliberately has no management variant — the client has no hub management + * credential, and handing it one to read a list of booleans is the trade #809 already refused. + * + * A hub too old to serve the route answers 404, which surfaces as `hub_state_unsupported`. The + * caller must report that as "state unavailable" and MUST NOT fall back to the client's own + * local provider/login state: that silent fallback is the defect this route exists to fix. + */ +export async function fetchHubState( + serverUrl: string, + admissionToken: string, + options: { timeoutMs?: number; fetchImpl?: typeof fetch } = {}, +): Promise { + const origin = normalizeHubOrigin(serverUrl); + const response = await fetchBounded(options.fetchImpl ?? fetch, `${origin}/v1/hub-state`, { + method: "GET", + headers: new Headers({ Accept: "application/json", "x-opencodex-api-key": admissionToken }), + }, options.timeoutMs, "headers"); + if (response.status === 404) { + try { await response.body?.cancel(); } catch { /* best effort */ } + throw new HubClientError("hub_state_unsupported", "Hub does not serve /v1/hub-state; upgrade the hub", 404); + } + if (!response.ok) { + const code = response.status === 401 ? "hub_state_unauthorized" : `hub_state_http_${response.status}`; + try { await response.body?.cancel(); } catch { /* best effort */ } + throw new HubClientError(code, `Hub state request failed (${response.status})`, response.status); + } + if (!jsonCompatibleContentType(response)) { + try { await response.body?.cancel(); } catch { /* best effort */ } + throw new HubClientError("hub_state_content_type_invalid", "Hub state response was not JSON", response.status); + } + let text: string; + try { + text = await boundedText(response, MAX_HUB_STATE_BYTES, { + inactivityTimeoutMs: safeTimeout(options.timeoutMs), + }); + } catch (error) { + if (error instanceof DOMException && error.name === "TimeoutError") { + throw new HubClientError("unreachable", "Hub state read stalled", undefined, { cause: error }); + } + throw error; + } + const parsed = parseHubStateBody(parseJson(text, "hub_state_invalid")); + if (!parsed) throw new HubClientError("hub_state_schema_invalid", "Hub state response was invalid", response.status); + return parsed; +} + function desktopSnapshotModels(value: unknown): Desktop3pModelEntry[] { const invalid = () => new HubClientError("desktop_snapshot_invalid", "Hub Desktop model snapshot was invalid"); if (!value || typeof value !== "object" || Array.isArray(value)) throw invalid(); diff --git a/src/client/hub-state.ts b/src/client/hub-state.ts new file mode 100644 index 0000000000..9ea5c3e0a8 --- /dev/null +++ b/src/client/hub-state.ts @@ -0,0 +1,199 @@ +/** + * A connected client's view of its hub's provider, login and roster state (#4236). + * + * The rule this module exists to enforce: on a connected client, the hub is the authority, and + * when the hub cannot be read the answer is "unavailable" — never the client's own local + * credential store. That store is empty by design, and reporting it as the truth is what made an + * agent on a connected machine conclude the hub could not serve grok while the hub was serving + * grok. Every failure path here therefore lands on `stateSource: "unavailable"` with a reason a + * human can act on, and none of them reaches back into local config. + * + * The last good response is cached at `/hub-state.json`, 0600, stamped with the + * connection that produced it. The owner stamp is not decoration: after `ocx disconnect` and a + * reconnect to a different hub (or a key rotation that changes `apiKeyId`), a stale file would + * otherwise be presented as this hub's state. `sameClientConnectionOwner` is the same triple + * (`serverUrl`, `apiKeyId`, `connectedAt`) the rest of the client lifecycle compares on. + */ +import { existsSync, lstatSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { getConfigDir } from "../config"; +import { atomicWriteFile } from "../config/atomic-write"; +import { parseHubStateBody, type HubStateDTO } from "../remote/hub-state"; +import type { OcxClientConnectionConfig } from "../types"; +import { fetchHubState, HubClientError } from "./hub-client"; +import { sameClientConnectionOwner } from "./state"; + +/** Bound the status path: `ocx status` must answer even when the hub is gone. */ +const DEFAULT_HUB_STATE_TIMEOUT_MS = 3_000; +/** The cache document plus its stamp; the DTO itself is already capped by its own contract. */ +const MAX_CACHE_BYTES = 128 * 1024; + +export type HubStateOwner = Pick; + +/** Where the state came from. "unavailable" is a reportable outcome, not a fallback to local. */ +export type HubStateSource = "hub" | "cache" | "unavailable"; + +export interface HubStateResolution { + stateSource: HubStateSource; + state: HubStateDTO | null; + /** Present whenever the live read did not succeed. Short, operator-facing. */ + reason?: string; + /** ISO timestamp of the response this state came from. */ + fetchedAt?: string; + ageSeconds?: number; +} + +export function hubStateCachePath(): string { + return join(getConfigDir(), "hub-state.json"); +} + +interface CacheDocument { + version: 1; + owner: HubStateOwner; + fetchedAt: string; + state: HubStateDTO; +} + +function readCacheDocument(): CacheDocument | null { + const path = hubStateCachePath(); + if (!existsSync(path)) return null; + try { + const stat = lstatSync(path); + // A symlink or an oversized file is refused rather than followed: this file is written + // 0600 by us, and anything else about it is someone else's doing. + if (stat.isSymbolicLink() || !stat.isFile() || stat.size > MAX_CACHE_BYTES) return null; + const raw = JSON.parse(readFileSync(path, "utf8")) as unknown; + if (!raw || typeof raw !== "object" || Array.isArray(raw)) return null; + const doc = raw as Record; + if (doc.version !== 1) return null; + const owner = doc.owner; + if (!owner || typeof owner !== "object" || Array.isArray(owner)) return null; + const ownerRow = owner as Record; + if (typeof ownerRow.serverUrl !== "string" || typeof ownerRow.apiKeyId !== "string" + || typeof ownerRow.connectedAt !== "string") return null; + if (typeof doc.fetchedAt !== "string" || Number.isNaN(Date.parse(doc.fetchedAt))) return null; + const state = parseHubStateBody(doc.state); + if (!state) return null; + return { + version: 1, + owner: { + serverUrl: ownerRow.serverUrl, + apiKeyId: ownerRow.apiKeyId, + connectedAt: ownerRow.connectedAt, + }, + fetchedAt: doc.fetchedAt, + state, + }; + } catch { + return null; + } +} + +/** The cached state for THIS connection, or null when absent, malformed, or another hub's. */ +export function readCachedHubState(owner: HubStateOwner): { state: HubStateDTO; fetchedAt: string } | null { + const doc = readCacheDocument(); + if (!doc) return null; + if (!sameClientConnectionOwner(doc.owner, owner)) return null; + return { state: doc.state, fetchedAt: doc.fetchedAt }; +} + +/** Best-effort: a cache that cannot be written must never fail the command that asked. */ +export function writeCachedHubState(owner: HubStateOwner, state: HubStateDTO, fetchedAt: string): boolean { + try { + const document: CacheDocument = { + version: 1, + owner: { serverUrl: owner.serverUrl, apiKeyId: owner.apiKeyId, connectedAt: owner.connectedAt }, + fetchedAt, + state, + }; + atomicWriteFile(hubStateCachePath(), `${JSON.stringify(document, null, 2)}\n`); + return true; + } catch { + return false; + } +} + +/** + * Why the live read did not land, in words an operator can act on. + * + * `hub_state_unsupported` is the version-skew case and gets an explicit upgrade instruction: + * left as a bare code it reads like a bug in the client. + */ +export function hubStateFailureReason(error: unknown): string { + if (error instanceof HubClientError) { + switch (error.code) { + case "hub_state_unsupported": + return "this hub is too old to report its state; upgrade the hub"; + case "hub_state_unauthorized": + return "the hub rejected this client's data key"; + case "hub_state_schema_invalid": + case "hub_state_invalid": + return "the hub returned an unreadable hub-state document"; + case "body_too_large": + return "the hub's state response exceeded the allowed size"; + case "unreachable": + return "the hub is unreachable"; + case "redirect_refused": + return "the hub redirected the state request"; + default: + return error.code; + } + } + return "the hub state could not be read"; +} + +export interface ResolveHubStateOptions { + owner: HubStateOwner; + /** The per-client data key. Null when the token file is missing or unsafe. */ + token: string | null; + timeoutMs?: number; + fetchImpl?: typeof fetch; + now?: number; + /** False reads only the cache — for paths that must not make a network call. */ + allowNetwork?: boolean; + /** False skips the cache write, for read-only callers. */ + persist?: boolean; +} + +function withAge( + source: HubStateSource, + state: HubStateDTO | null, + fetchedAt: string | undefined, + now: number, + reason?: string, +): HubStateResolution { + const ageSeconds = fetchedAt ? Math.max(0, Math.floor((now - Date.parse(fetchedAt)) / 1000)) : undefined; + return { + stateSource: source, + state, + ...(reason ? { reason } : {}), + ...(fetchedAt ? { fetchedAt } : {}), + ...(ageSeconds === undefined || Number.isNaN(ageSeconds) ? {} : { ageSeconds }), + }; +} + +export async function resolveHubState(options: ResolveHubStateOptions): Promise { + const now = options.now ?? Date.now(); + const fromCache = (reason: string): HubStateResolution => { + const cached = readCachedHubState(options.owner); + return cached + ? withAge("cache", cached.state, cached.fetchedAt, now, reason) + : withAge("unavailable", null, undefined, now, reason); + }; + if (!options.token) return fromCache("this client has no usable data-plane token"); + if (options.allowNetwork === false) return fromCache("a live hub read was not attempted"); + let state: HubStateDTO; + try { + state = await fetchHubState(options.owner.serverUrl, options.token, { + timeoutMs: options.timeoutMs ?? DEFAULT_HUB_STATE_TIMEOUT_MS, + ...(options.fetchImpl ? { fetchImpl: options.fetchImpl } : {}), + }); + } catch (error) { + // Deliberately no local-state fallback here. A stale cache is still the HUB's state; the + // client's own providers and logins are not, at any age. + return fromCache(hubStateFailureReason(error)); + } + const fetchedAt = new Date(now).toISOString(); + if (options.persist !== false) writeCachedHubState(options.owner, state, fetchedAt); + return withAge("hub", state, fetchedAt, now); +} diff --git a/tests/clients/client-hub-state.test.ts b/tests/clients/client-hub-state.test.ts new file mode 100644 index 0000000000..716bff2c83 --- /dev/null +++ b/tests/clients/client-hub-state.test.ts @@ -0,0 +1,239 @@ +/** + * A connected client's hub-state read and its 0600 cache (#4236). + * + * The invariant every case here defends is one sentence: when the hub cannot be read, the + * answer is "unavailable" — never the client's own local provider and login state. That silent + * degradation is the defect being fixed, and it is invisible in output, because local state + * renders exactly like hub state. So the cases enumerate every way the read can fail (404 from + * an old hub, 401, unreachable, non-JSON, malformed JSON, wrong schema, oversized body) and + * assert the resolution is `cache` or `unavailable` with a reason, with `state` null whenever + * there is nothing true to show. + * + * The owner stamp gets its own case because it is the difference between a stale file and a + * LIE: after a disconnect and a reconnect to a different hub, an unstamped cache would present + * the previous hub's providers as this one's. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { lstatSync, mkdtempSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fetchHubState, HubClientError } from "../../src/client/hub-client"; +import { + hubStateCachePath, + readCachedHubState, + resolveHubState, + writeCachedHubState, + type HubStateOwner, +} from "../../src/client/hub-state"; +import type { HubStateDTO } from "../../src/remote/hub-state"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const previousHome = process.env.OPENCODEX_HOME; +let testHome = ""; + +const OWNER: HubStateOwner = { + serverUrl: "https://hub.example.test:8443", + apiKeyId: "client-one", + connectedAt: "2026-09-01T00:00:00.000Z", +}; + +function hubState(overrides: Partial = {}): HubStateDTO { + return { + schemaVersion: 1, + runtimeRole: "hub", + hubVersion: "2.51.0", + origin: "https://hub.example.test:8443", + providers: [{ name: "xai", adapter: "openai-chat", authMode: "oauth", hasCredential: true, disabled: false }], + oauth: [{ provider: "xai", loggedIn: true }], + subagentModels: ["xai/grok-4.6"], + claudeCode: { enabled: true }, + ...overrides, + }; +} + +function jsonFetch(body: unknown, init: { status?: number; contentType?: string } = {}): typeof fetch { + return (async () => new Response(typeof body === "string" ? body : JSON.stringify(body), { + status: init.status ?? 200, + headers: { "content-type": init.contentType ?? "application/json" }, + })) as unknown as typeof fetch; +} + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "ocx-client-hub-state-")); + process.env.OPENCODEX_HOME = testHome; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (testHome) removeTreeWithRetry(testHome); + testHome = ""; +}); + +describe("fetchHubState", () => { + test("parses a well-formed hub response", async () => { + const state = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { fetchImpl: jsonFetch(hubState()) }); + expect(state.providers[0]?.name).toBe("xai"); + expect(state.subagentModels).toEqual(["xai/grok-4.6"]); + }); + + test("an old hub's 404 is version skew, not a missing hub", async () => { + // The distinct code is what lets the CLI say "upgrade the hub" instead of printing a + // generic failure that reads like a client bug. + const error = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch({ error: { code: "not_found" } }, { status: 404 }), + }).catch((e: unknown) => e); + expect(error).toBeInstanceOf(HubClientError); + expect((error as HubClientError).code).toBe("hub_state_unsupported"); + }); + + test.each([ + [401, "hub_state_unauthorized"], + [500, "hub_state_http_500"], + ] as const)("status %i surfaces as %s", async (status, code) => { + const error = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch({ error: {} }, { status }), + }).catch((e: unknown) => e); + expect((error as HubClientError).code).toBe(code); + }); + + test("a non-JSON content type is refused without reading the body", async () => { + const error = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch("hello", { contentType: "text/html" }), + }).catch((e: unknown) => e); + expect((error as HubClientError).code).toBe("hub_state_content_type_invalid"); + }); + + test.each([ + ["malformed JSON", "{not json", "hub_state_invalid"], + ["a foreign document", JSON.stringify({ schemaVersion: 1, models: [] }), "hub_state_schema_invalid"], + ["a future schema", JSON.stringify({ ...hubState(), schemaVersion: 2 }), "hub_state_schema_invalid"], + ["a non-hub role", JSON.stringify({ ...hubState(), runtimeRole: "standalone" }), "hub_state_schema_invalid"], + ["a provider row with no booleans", JSON.stringify({ ...hubState(), providers: [{ name: "x", adapter: "y" }] }), "hub_state_schema_invalid"], + ])("%s is refused (%#)", async (_label, body, code) => { + const error = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch(body), + }).catch((e: unknown) => e); + expect((error as HubClientError).code).toBe(code); + }); + + test("an oversized roster is refused rather than truncated", async () => { + const body = hubState({ subagentModels: Array.from({ length: 64 }, (_, i) => `m-${i}`) }); + const error = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch(body), + }).catch((e: unknown) => e); + expect((error as HubClientError).code).toBe("hub_state_schema_invalid"); + }); +}); + +describe("resolveHubState", () => { + test("a live read reports stateSource hub and writes an owner-stamped 0600 cache", async () => { + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: jsonFetch(hubState()), + now: Date.parse("2026-09-11T12:00:00.000Z"), + }); + expect(resolved.stateSource).toBe("hub"); + expect(resolved.reason).toBeUndefined(); + expect(resolved.state?.oauth[0]).toEqual({ provider: "xai", loggedIn: true }); + expect(resolved.fetchedAt).toBe("2026-09-11T12:00:00.000Z"); + const mode = lstatSync(hubStateCachePath()).mode & 0o777; + if (process.platform !== "win32") expect(mode).toBe(0o600); + expect(JSON.parse(readFileSync(hubStateCachePath(), "utf8")).owner).toEqual(OWNER); + }); + + test("an unreachable hub falls back to the cache and says why", async () => { + writeCachedHubState(OWNER, hubState(), "2026-09-11T11:00:00.000Z"); + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: (() => { throw new Error("ECONNREFUSED"); }) as unknown as typeof fetch, + now: Date.parse("2026-09-11T12:00:00.000Z"), + }); + expect(resolved.stateSource).toBe("cache"); + expect(resolved.reason).toBe("the hub is unreachable"); + expect(resolved.ageSeconds).toBe(3600); + // Still the HUB's providers. A cache is stale hub state; local state is not hub state. + expect(resolved.state?.providers[0]?.name).toBe("xai"); + }); + + test("with no cache an unreachable hub is unavailable, never local state", async () => { + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: (() => { throw new Error("ECONNREFUSED"); }) as unknown as typeof fetch, + }); + expect(resolved.stateSource).toBe("unavailable"); + expect(resolved.state).toBeNull(); + expect(resolved.reason).toBe("the hub is unreachable"); + }); + + test("an old hub is unavailable with an upgrade instruction", async () => { + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: jsonFetch({ error: {} }, { status: 404 }), + }); + expect(resolved.stateSource).toBe("unavailable"); + expect(resolved.reason).toContain("upgrade the hub"); + }); + + test("a missing data token never becomes a live read", async () => { + let called = false; + const resolved = await resolveHubState({ + owner: OWNER, + token: null, + fetchImpl: (() => { called = true; throw new Error("should not be called"); }) as unknown as typeof fetch, + }); + expect(called).toBe(false); + expect(resolved.stateSource).toBe("unavailable"); + expect(resolved.reason).toBe("this client has no usable data-plane token"); + }); + + test("allowNetwork false reads only the cache", async () => { + writeCachedHubState(OWNER, hubState(), "2026-09-11T11:59:30.000Z"); + let called = false; + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + allowNetwork: false, + fetchImpl: (() => { called = true; throw new Error("should not be called"); }) as unknown as typeof fetch, + now: Date.parse("2026-09-11T12:00:00.000Z"), + }); + expect(called).toBe(false); + expect(resolved.stateSource).toBe("cache"); + expect(resolved.ageSeconds).toBe(30); + }); + + test("another hub's cache is discarded rather than shown as this hub's", async () => { + writeCachedHubState( + { serverUrl: "https://other-hub.example.test", apiKeyId: "client-one", connectedAt: OWNER.connectedAt }, + hubState(), + "2026-09-11T11:00:00.000Z", + ); + expect(readCachedHubState(OWNER)).toBeNull(); + const resolved = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: (() => { throw new Error("ECONNREFUSED"); }) as unknown as typeof fetch, + }); + expect(resolved.stateSource).toBe("unavailable"); + }); + + test("a rotated apiKeyId invalidates the cache", () => { + writeCachedHubState(OWNER, hubState(), "2026-09-11T11:00:00.000Z"); + expect(readCachedHubState({ ...OWNER, apiKeyId: "client-two" })).toBeNull(); + }); + + test("a malformed or symlinked cache file is ignored", () => { + writeFileSync(hubStateCachePath(), "{not json"); + expect(readCachedHubState(OWNER)).toBeNull(); + writeFileSync(join(testHome, "elsewhere.json"), JSON.stringify({ + version: 1, owner: OWNER, fetchedAt: "2026-09-11T11:00:00.000Z", state: hubState(), + })); + removeTreeWithRetry(hubStateCachePath()); + symlinkSync(join(testHome, "elsewhere.json"), hubStateCachePath()); + expect(readCachedHubState(OWNER)).toBeNull(); + }); +}); From e832345017465168bd14f157924ba6807660841e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:10:16 +0900 Subject: [PATCH 03/10] feat(cli): ocx status on a connected client reports the hub, and labels what is local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The report was not incomplete, it was misattributed. On a connected client `ocx status` printed `OAuth logins:` from this machine's own credential store — empty by design — plus local providers and a local five-model roster, with one buried `Remote hub: connected ()` line as the only hint that none of it described the machine doing the work. An agent read `xai ✗ not logged in` off such a client and concluded the hub could not serve grok. Now a connected client leads with a banner naming the hub origin, prints the hub's OAuth logins, providers and delegable models above the local block, labels that block "not used for routing while connected", and tags the lines that really are about this machine — proxy, health, dashboard, config, service, shim, Codex runtime/version/source/home — `(local)`. The tag appears only while connected: on a standalone install every line is local and tagging them all would teach the reader to ignore the tag. `--json` gains `runtimeRole` and a `remoteHub` block with the same `stateSource: "hub" | "cache" | "unavailable"` honesty. `schemaVersion` stays 1 (additive, same rule as `versionSkew`), and `connection` is untouched: it describes the LINK, `remoteHub` describes what is on the other end of it. When the hub cannot be read the block is empty with a reason and the banner says so, because the one thing this must never do is answer from local state. Providers carry `authMode`, which is what stops "no API key" from reading as "not configured" for an `oauth` provider — the precise inference that went wrong. Co-Authored-By: Claude Fable 5.1 --- src/cli/index.ts | 61 +++-- src/cli/status.ts | 158 ++++++++++++- tests/cli/cli-status-hub-state.test.ts | 309 +++++++++++++++++++++++++ tests/server/v1-hub-state.test.ts | 4 +- 4 files changed, 510 insertions(+), 22 deletions(-) create mode 100644 tests/cli/cli-status-hub-state.test.ts diff --git a/src/cli/index.ts b/src/cli/index.ts index 730ec0c795..97a80f32c8 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -48,7 +48,7 @@ import { pendingTeardownPathFor, quarantinePendingTeardown, } from "../config/pending-teardown"; -import { collectStatus, hubStatusLines, unusedProxyWarningLines } from "./status"; +import { collectStatus, hubStatusLines, remoteHubBannerLine, remoteHubStatusLines, unusedProxyWarningLines } from "./status"; import { endpointsToProve, everyEndpointProvenDown, sharedTeardownAuthorized, type UninstallObservation } from "./uninstall-plan"; import { takeFlag } from "./runtime-api"; @@ -1383,12 +1383,22 @@ async function handleStatus() { return; } + // First line of the report, above the proxy line, deliberately (#4236): on a connected client + // the provider/login/model lines describe the HUB, and the lines that describe this machine + // are tagged `(local)`. A reader who sees neither draws the wrong conclusion from a correct + // report — an agent read `xai ✗ not logged in` off a client and decided the hub could not + // serve grok. + const remoteHubBanner = remoteHubBannerLine(status.json.remoteHub); + if (remoteHubBanner) console.log(remoteHubBanner); + // `(local)` only while connected: on a standalone install every line is local, and tagging + // them all would be noise that trains the reader to skip the tag. + const local = status.json.remoteHub.connected ? " (local)" : ""; if (status.json.proxy.pid || status.json.proxy.health.ok) { - console.log(`✅ Proxy: ${status.proxyLabel}`); + console.log(`✅ Proxy: ${status.proxyLabel}${local}`); } else { - console.log(`❌ Proxy: ${status.proxyLabel}`); + console.log(`❌ Proxy: ${status.proxyLabel}${local}`); } - console.log(` Health: ${status.healthLabel}`); + console.log(` Health: ${status.healthLabel}${local}`); if (status.json.claudeDesktop.desiredEnabled && !status.json.claudeDesktop.policy.ok) { console.log(` ⚠️ Claude Desktop 3P health: ${status.json.claudeDesktop.policy.status}`); console.log(` ${status.json.claudeDesktop.policy.message}`); @@ -1428,12 +1438,13 @@ async function handleStatus() { ? " Restart with 'ocx start', or refresh the installed service: 'ocx service repair'." : " Restart with 'ocx start', or install the persistent service: 'ocx service install'."); } - console.log(` Dashboard: ${status.json.dashboard.url}`); - console.log(` Config: ${status.json.paths.config}`); - console.log(` PID file: ${status.json.paths.pid}`); - console.log(` Runtime: ${status.json.paths.runtime}`); - console.log(` Runtime source: ${status.json.runtime.source}${status.json.runtime.overrideEnv ? ` (${status.json.runtime.overrideEnv})` : ""}`); - console.log(` Default provider: ${status.json.defaultProvider}`); + console.log(` Dashboard: ${status.json.dashboard.url}${local}`); + console.log(` Config: ${status.json.paths.config}${local}`); + console.log(` PID file: ${status.json.paths.pid}${local}`); + console.log(` Runtime: ${status.json.paths.runtime}${local}`); + console.log(` Runtime source: ${status.json.runtime.source}${status.json.runtime.overrideEnv ? ` (${status.json.runtime.overrideEnv})` : ""}${local}`); + // On a client this is the local default, which routing does not use — the hub applies its own. + console.log(` Default provider: ${status.json.defaultProvider}${local}`); // One block rather than six scattered lines, and only on a hub: `hubStatusLines` owns the // sentences so they are testable without spawning the CLI. It prints no token value. if (status.json.hub) { @@ -1443,15 +1454,15 @@ async function handleStatus() { if (status.json.connection.state === "invalid" || status.json.connection.state === "mismatched") { console.log(` ⚠️ ${status.json.connection.reason}`); } - console.log(` Codex autostart: ${status.json.codexAutostart ? "enabled" : "disabled"}`); - console.log(` Restart safety: ${startupHealthSummary(status.json.startup)}`); - console.log(` ${formatStartupRoutingDetail(status.json.startup)}`); - console.log(` Service: ${status.json.service.summary}`); - console.log(` ${status.json.codexShim.summary}`); - console.log(` Codex runtime: ${status.json.codexRuntime.path}`); - console.log(` Codex version: ${status.json.codexRuntime.version ?? "unknown"}`); - console.log(` Codex source: ${status.json.codexRuntime.source}`); - console.log(` Codex home: ${status.json.codexHome.effectiveCodexHome}`); + console.log(` Codex autostart: ${status.json.codexAutostart ? "enabled" : "disabled"}${local}`); + console.log(` Restart safety: ${startupHealthSummary(status.json.startup)}${local}`); + console.log(` ${formatStartupRoutingDetail(status.json.startup)}${local}`); + console.log(` Service: ${status.json.service.summary}${local}`); + console.log(` ${status.json.codexShim.summary}${local}`); + console.log(` Codex runtime: ${status.json.codexRuntime.path}${local}`); + console.log(` Codex version: ${status.json.codexRuntime.version ?? "unknown"}${local}`); + console.log(` Codex source: ${status.json.codexRuntime.source}${local}`); + console.log(` Codex home: ${status.json.codexHome.effectiveCodexHome}${local}`); if (status.json.codexHome.warning) { console.log(` ⚠️ ${status.json.codexHome.warning}`); console.log(` Action: ${status.json.codexHome.action}`); @@ -1473,7 +1484,17 @@ async function handleStatus() { const { collectOAuthHealthEntriesForCli, oauthLoginSummary } = await import("../oauth"); const { emailMaskingEnabled } = await import("../lib/privacy"); const { formatOAuthHealthForStatus } = await import("./status-oauth"); - console.log(` OAuth logins:`); + // On a connected client the HUB's providers and logins come first, because they are the ones + // that decide what a request can route to. The local block still prints — an operator debugging + // a half-migrated machine needs to see it — but under a heading that says it is not in use, and + // below the hub's, so the hub's is what a reader (or an agent) encounters first (#4236). + for (const line of remoteHubStatusLines(status.json.remoteHub)) console.log(` ${line}`); + if (status.json.remoteHub.connected) { + console.log(status.json.remoteHub.stateSource === "unavailable" + ? " Local-only credential state (this is NOT the hub's; the hub's state could not be read):" + : " Local-only (not used for routing while connected):"); + } + console.log(` OAuth logins${local}:`); // The operator's own `privacy.maskEmails` decision applies to the CLI too: `ocx status` is not // the dashboard, but it reads the same stored addresses, and a flag that only moved one of the // two would leave the operator unable to tell which surface they had configured. diff --git a/src/cli/status.ts b/src/cli/status.ts index 1b794eca25..b4c77bdfaa 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -17,7 +17,9 @@ 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"; +import { collectClientConnectionStatus, type ClientConnectionStatus } from "./connect"; +import type { HubStateOAuthEntry, HubStateProvider } from "../remote/hub-state"; +import type { HubStateSource } from "../client/hub-state"; import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets"; import { tokenCollidesWithAdmin } from "../lib/admin-secrets"; export { proxyHealthFailureReason, isConnectionRefused, isUncleanExitEvidence, probeUncleanExitState } from "./status-probes"; @@ -65,8 +67,42 @@ export type HubStatus = { dataTokenEnvInShell: boolean; }; +/** + * What a connected client learned from its hub, and how reliable that answer is (#4236). + * + * `stateSource` is the load-bearing field. "hub" is a live read; "cache" is the last good read + * from THIS connection, still the hub's state rather than this machine's; "unavailable" means + * nothing true is known, and the consumer must say so rather than substitute local facts. A + * client's own `providers`/`oauth` sections are empty by design, so presenting them as the + * answer is not a degraded report — it is a wrong one. + */ +export type CliRemoteHubStatus = { + connected: boolean; + /** The hub's advertised data origin when it publishes one, else the URL this client dials. */ + origin: string | null; + stateSource: HubStateSource; + /** Present whenever `stateSource` is not "hub". Operator-facing, never a bare error code. */ + reason?: string; + fetchedAt?: string; + ageSeconds?: number; + hubVersion: string | null; + providers: HubStateProvider[]; + oauth: HubStateOAuthEntry[]; + subagentModels: string[]; + /** The hub's own Claude Code toggle; a client cannot infer it from local config. */ + claudeCodeEnabled: boolean | null; +}; + export type CliStatusJson = { schemaVersion: 1; + /** + * This machine's topology role, named rather than inferred (#4236). + * + * Every other field in this report was already ambiguous without it: an agent reading + * `providers: {}` on a client could not tell "nothing is configured" from "the provider + * configuration lives on the hub". Additive, so `schemaVersion` stays 1. + */ + runtimeRole: "standalone" | "hub" | "client"; proxy: { running: boolean; pid: number | null; @@ -144,6 +180,14 @@ export type CliStatusJson = { * Never carries a token value; only which source holds one. */ hub: HubStatus | null; + /** + * The hub's answer on a connected client, or a not-connected placeholder (#4236). + * + * Separate from `connection`, which describes the LINK (is the key owned, is the catalog + * present). This describes what is on the other end of it. Additive and always present, so + * `schemaVersion` stays 1 and a consumer never has to branch on the key existing. + */ + remoteHub: CliRemoteHubStatus; /** * This CLI's version against the running proxy's (#2701). * @@ -255,6 +299,112 @@ export function hubStatusLines(hub: HubStatus): string[] { ]; } +/** The not-connected placeholder. Arrays are empty because nothing was asked, not because nothing exists. */ +export function disconnectedRemoteHubStatus(): CliRemoteHubStatus { + return { + connected: false, + origin: null, + stateSource: "unavailable", + hubVersion: null, + providers: [], + oauth: [], + subagentModels: [], + claudeCodeEnabled: null, + }; +} + +/** + * Ask the hub what it can serve, with a bounded read and a cache fallback. + * + * `ocx status` must answer while the hub is offline, so the fetch is bounded and a failure is + * reported rather than thrown. It must also never answer from local provider/login state — see + * `src/client/hub-state.ts` for why that substitution is the defect rather than a graceful + * degradation. + */ +export async function collectRemoteHubStatus( + connection: Pick, + options: { fetchImpl?: typeof fetch; timeoutMs?: number; now?: number } = {}, +): Promise { + if (connection.state !== "connected" || !connection.serverUrl || !connection.apiKeyId || !connection.connectedAt) { + return disconnectedRemoteHubStatus(); + } + const { resolveHubState } = await import("../client/hub-state"); + const token = readServiceApiTokenState(); + const resolved = await resolveHubState({ + owner: { + serverUrl: connection.serverUrl, + apiKeyId: connection.apiKeyId, + connectedAt: connection.connectedAt, + }, + token: token.kind === "present" ? token.token : null, + ...(options.fetchImpl ? { fetchImpl: options.fetchImpl } : {}), + ...(options.timeoutMs === undefined ? {} : { timeoutMs: options.timeoutMs }), + ...(options.now === undefined ? {} : { now: options.now }), + }); + return { + connected: true, + // The hub's own advertised origin when it publishes one; otherwise the URL this client + // dials, which is the origin the operator would recognize. + origin: resolved.state?.origin ?? connection.serverUrl, + stateSource: resolved.stateSource, + ...(resolved.reason ? { reason: resolved.reason } : {}), + ...(resolved.fetchedAt ? { fetchedAt: resolved.fetchedAt } : {}), + ...(resolved.ageSeconds === undefined ? {} : { ageSeconds: resolved.ageSeconds }), + hubVersion: resolved.state?.hubVersion ?? null, + providers: resolved.state?.providers ?? [], + oauth: resolved.state?.oauth ?? [], + subagentModels: resolved.state?.subagentModels ?? [], + claudeCodeEnabled: resolved.state ? resolved.state.claudeCode.enabled : null, + }; +} + +/** + * The one line that has to be read before anything else in the report. + * + * It goes FIRST, above the proxy line, because the failure mode is an agent or operator reading + * the provider and login lines in isolation and concluding the hub cannot do something it can. + * A buried `Remote hub: connected ()` line — which is all this report had — does not stop + * that, as #4236 demonstrated. + */ +export function remoteHubBannerLine(remoteHub: CliRemoteHubStatus): string | null { + if (!remoteHub.connected) return null; + const origin = remoteHub.origin ?? "the hub"; + if (remoteHub.stateSource === "unavailable") { + return `⚠️ Hub ${origin}: state unavailable (${remoteHub.reason ?? "unknown reason"}) — provider and login lines below are LOCAL and do not describe the hub.`; + } + const staleness = remoteHub.stateSource === "cache" + ? ` — cached ${remoteHub.ageSeconds ?? "?"}s ago (${remoteHub.reason ?? "live read failed"})` + : ""; + return `🔗 State from hub ${origin}${staleness}: provider credentials, logins and delegable models below are the HUB's, not this machine's.`; +} + +/** + * The hub-sourced provider/login/model block, owned here so the sentences are testable without + * spawning the CLI. Indentation is the caller's. Empty when nothing true is known. + */ +export function remoteHubStatusLines(remoteHub: CliRemoteHubStatus): string[] { + if (!remoteHub.connected || remoteHub.stateSource === "unavailable") return []; + const origin = remoteHub.origin ?? "hub"; + const lines = [ + `OAuth logins (hub ${origin}):`, + ...(remoteHub.oauth.length === 0 + ? [" (the hub reported no OAuth providers)"] + : remoteHub.oauth.map(entry => ` ${entry.provider.padEnd(10)} ${entry.loggedIn ? "✓ logged in" : "✗ not logged in"}`)), + `Providers (hub ${origin}):`, + ...(remoteHub.providers.length === 0 + ? [" (the hub reported no providers)"] + : remoteHub.providers.map(provider => { + // `authMode` is what keeps "no API key" from reading as "not configured": an `oauth` + // provider legitimately has no key and is still fully usable. + const credential = provider.hasCredential ? "credential stored" : `no API key (authMode ${provider.authMode ?? "key"})`; + return ` ${provider.name.padEnd(10)} ${provider.adapter} — ${credential}${provider.disabled ? ", disabled" : ""}`; + })), + `Delegable models (hub ${origin}): ${remoteHub.subagentModels.length === 0 ? "none" : remoteHub.subagentModels.join(", ")}`, + ]; + if (remoteHub.hubVersion) lines.push(`Hub version: ${remoteHub.hubVersion}`); + return lines; +} + export function selectListenTarget( config: StatusListenConfig, pid: number | null, @@ -311,6 +461,10 @@ export async function collectStatus(): Promise { policy: claudeDesktopPolicyHealth(probeClaudeDesktopPolicy()), }; const clientConnection = collectClientConnectionStatus(); + // Asked before the local probes below so a connected client's report is hub-sourced from its + // first line. Bounded and failure-tolerant: an offline hub degrades the remoteHub block, it + // does not fail `ocx status`. + const remoteHub = await collectRemoteHubStatus(clientConnection); // Prefer identity-verified liveness (runtime-port + /healthz) over ocx.pid alone (#618). // Pass the already-resolved diagnostics config so findLiveProxy does not re-load and // warn on malformed config.json (status --json must stay stderr-clean). @@ -467,6 +621,7 @@ export async function collectStatus(): Promise { healthLabel: health.label, json: { schemaVersion: 1, + runtimeRole: config.runtimeRole ?? "standalone", proxy: { running: Boolean(live) || Boolean(pid && health.ok), pid: live?.pid ?? pid, @@ -493,6 +648,7 @@ export async function collectStatus(): Promise { ...(bunRuntime.source === "override" ? { overrideEnv: bunRuntime.overrideEnv } : {}), }, hub: collectHubStatus(config, listen), + remoteHub, codexAutostart: codexAutoStartEnabled(config), startup, defaultProvider: typeof config.defaultProvider === "string" ? config.defaultProvider : null, diff --git a/tests/cli/cli-status-hub-state.test.ts b/tests/cli/cli-status-hub-state.test.ts new file mode 100644 index 0000000000..3a16a619e2 --- /dev/null +++ b/tests/cli/cli-status-hub-state.test.ts @@ -0,0 +1,309 @@ +/** + * `ocx status` on a connected client reports the HUB's state (#4236). + * + * The defect was not a missing field. `ocx status` printed a complete, internally consistent, + * entirely local report — `xai ✗ not logged in`, no grok provider, five delegable models — on a + * machine whose hub has xAI logged in and serves grok, and an agent reading it concluded the hub + * could not serve grok. Nothing in the output said which machine it described except one buried + * `Remote hub: connected ()` line. + * + * So these cases are about WHERE the reader's eye lands: the banner is the first line, the hub's + * providers and logins print above the local ones, the local block carries a heading that says + * it is not in use, and the lines that are genuinely about this machine are tagged `(local)`. The + * unreachable-hub case pins the other half — the report says "state unavailable" and names the + * reason instead of silently presenting local state as the answer. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { + collectRemoteHubStatus, + disconnectedRemoteHubStatus, + remoteHubBannerLine, + remoteHubStatusLines, + type CliRemoteHubStatus, +} from "../../src/cli/status"; +import type { HubStateDTO } from "../../src/remote/hub-state"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; + +const repoRoot = dirname(fileURLToPath(new URL("../../package.json", import.meta.url))); +const cliPath = join(repoRoot, "src", "cli", "index.ts"); +const FIXTURE_TOKEN = "status-hub-state-token"; + +const previousHome = process.env.OPENCODEX_HOME; +let testHome = ""; + +function hubState(overrides: Partial = {}): HubStateDTO { + return { + schemaVersion: 1, + runtimeRole: "hub", + hubVersion: "2.51.0", + origin: "https://hub.example.test:8443", + providers: [ + { name: "xai", adapter: "openai-chat", authMode: "oauth", hasCredential: false, disabled: false }, + { name: "openai", adapter: "openai-responses", authMode: "key", hasCredential: true, disabled: false }, + ], + oauth: [{ provider: "xai", loggedIn: true }, { provider: "anthropic", loggedIn: false }], + subagentModels: ["xai/grok-4.6", "gpt-5.6-sol"], + claudeCode: { enabled: true }, + ...overrides, + }; +} + +function connectedConfig(serverUrl: string) { + return { + port: 9, + defaultProvider: "openai", + providers: {}, + codexAutoStart: false, + runtimeRole: "client", + client: { + serverUrl, + managementUrl: serverUrl, + managementTransport: "direct", + selectedClients: ["claude"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", + apiKeyId: "status-hub-state", + tokenFingerprint: createHash("sha256").update(FIXTURE_TOKEN).digest("hex"), + protocolVersion: 1, + connectedAt: "2026-09-06T00:00:00.000Z", + }, + }; +} + +function writeConnectedHome(home: string, serverUrl: string): void { + writeFileSync(join(home, "config.json"), JSON.stringify(connectedConfig(serverUrl))); + writeFileSync(join(home, "service-api-token"), FIXTURE_TOKEN, { mode: 0o600 }); +} + +function jsonFetch(body: unknown): typeof fetch { + return (async () => new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + })) as unknown as typeof fetch; +} + +beforeEach(() => { + testHome = mkdtempSync(join(tmpdir(), "ocx-status-hub-")); + process.env.OPENCODEX_HOME = testHome; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (testHome) removeTreeWithRetry(testHome); + testHome = ""; +}); + +describe("collectRemoteHubStatus", () => { + test("a connected client with a live hub reports the hub's facts", async () => { + writeConnectedHome(testHome, "https://hub.example.test:8443"); + const remoteHub = await collectRemoteHubStatus( + { state: "connected", serverUrl: "https://hub.example.test:8443", apiKeyId: "status-hub-state", connectedAt: "2026-09-06T00:00:00.000Z" }, + { fetchImpl: jsonFetch(hubState()) }, + ); + expect(remoteHub.connected).toBe(true); + expect(remoteHub.stateSource).toBe("hub"); + expect(remoteHub.hubVersion).toBe("2.51.0"); + expect(remoteHub.oauth).toEqual([{ provider: "xai", loggedIn: true }, { provider: "anthropic", loggedIn: false }]); + expect(remoteHub.subagentModels).toEqual(["xai/grok-4.6", "gpt-5.6-sol"]); + expect(remoteHub.claudeCodeEnabled).toBe(true); + }); + + test("a client whose token file is missing is unavailable, not locally sourced", async () => { + writeFileSync(join(testHome, "config.json"), JSON.stringify(connectedConfig("https://hub.example.test:8443"))); + const remoteHub = await collectRemoteHubStatus( + { state: "connected", serverUrl: "https://hub.example.test:8443", apiKeyId: "status-hub-state", connectedAt: "2026-09-06T00:00:00.000Z" }, + { fetchImpl: jsonFetch(hubState()) }, + ); + expect(remoteHub.stateSource).toBe("unavailable"); + expect(remoteHub.providers).toEqual([]); + expect(remoteHub.reason).toContain("data-plane token"); + }); + + test("a disconnected machine asks the hub nothing", async () => { + const remoteHub = await collectRemoteHubStatus({ state: "disconnected" }); + expect(remoteHub).toEqual(disconnectedRemoteHubStatus()); + expect(remoteHub.connected).toBe(false); + }); +}); + +describe("the hub banner and block", () => { + const live: CliRemoteHubStatus = { + connected: true, + origin: "https://hub.example.test:8443", + stateSource: "hub", + hubVersion: "2.51.0", + providers: hubState().providers, + oauth: hubState().oauth, + subagentModels: hubState().subagentModels, + claudeCodeEnabled: true, + }; + + test("a live read leads with the hub origin and says the lines are the hub's", () => { + const banner = remoteHubBannerLine(live); + expect(banner).toContain("State from hub https://hub.example.test:8443"); + expect(banner).toContain("not this machine's"); + }); + + test("an unreachable hub names the reason and warns the lines below are local", () => { + const banner = remoteHubBannerLine({ + ...live, stateSource: "unavailable", reason: "the hub is unreachable", + hubVersion: null, providers: [], oauth: [], subagentModels: [], claudeCodeEnabled: null, + }); + expect(banner).toContain("state unavailable (the hub is unreachable)"); + expect(banner).toContain("LOCAL"); + }); + + test("a cached read is labelled cached with its age, not presented as live", () => { + const banner = remoteHubBannerLine({ ...live, stateSource: "cache", ageSeconds: 42, reason: "the hub is unreachable" }); + expect(banner).toContain("cached 42s ago"); + }); + + test("a standalone machine gets no banner at all", () => { + expect(remoteHubBannerLine(disconnectedRemoteHubStatus())).toBeNull(); + }); + + test("the block names the hub on every heading and explains a keyless oauth provider", () => { + const lines = remoteHubStatusLines(live); + expect(lines[0]).toBe("OAuth logins (hub https://hub.example.test:8443):"); + expect(lines.join("\n")).toContain("xai ✓ logged in"); + // The exact confusion being removed: no API key on an `oauth` provider is not "unconfigured". + expect(lines.join("\n")).toContain("no API key (authMode oauth)"); + expect(lines.join("\n")).toContain("Delegable models (hub https://hub.example.test:8443): xai/grok-4.6, gpt-5.6-sol"); + expect(lines.join("\n")).toContain("Hub version: 2.51.0"); + }); + + test("no hub state means no hub block, rather than an empty one that reads as 'nothing configured'", () => { + expect(remoteHubStatusLines({ ...live, stateSource: "unavailable", providers: [], oauth: [], subagentModels: [] })).toEqual([]); + expect(remoteHubStatusLines(disconnectedRemoteHubStatus())).toEqual([]); + }); +}); + +describe("ocx status end to end on a connected client", () => { + async function runStatus(home: string, codexHome: string, json: boolean) { + const child = Bun.spawn([process.execPath, cliPath, "status", ...(json ? ["--json"] : [])], { + cwd: repoRoot, + env: { ...process.env, OPENCODEX_HOME: home, CODEX_HOME: codexHome }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + return { stdout, stderr, exitCode }; + } + + test("a live hub drives the banner, the hub block and the (local) tags", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-status-hub-live-")); + const codexHome = join(home, "codex"); + mkdirSync(codexHome, { recursive: true }); + const hub = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + const path = new URL(request.url).pathname; + if (path !== "/v1/hub-state") return new Response("not found", { status: 404 }); + // Proof the client authenticates with its own data key and nothing else. + if (request.headers.get("x-opencodex-api-key") !== FIXTURE_TOKEN) { + return Response.json({ error: {} }, { status: 401 }); + } + return Response.json(hubState()); + }, + }); + try { + const origin = `http://127.0.0.1:${hub.port}`; + writeConnectedHome(home, origin); + const json = await runStatus(home, codexHome, true); + expect({ exitCode: json.exitCode, stderr: json.stderr }).toEqual({ exitCode: 0, stderr: "" }); + const parsed = JSON.parse(json.stdout); + // Additive by rule: the two new keys must not bump the schema. + expect(parsed.schemaVersion).toBe(1); + expect(parsed.runtimeRole).toBe("client"); + expect(parsed.remoteHub.connected).toBe(true); + expect(parsed.remoteHub.stateSource).toBe("hub"); + expect(parsed.remoteHub.hubVersion).toBe("2.51.0"); + expect(parsed.remoteHub.origin).toBe("https://hub.example.test:8443"); + expect(parsed.remoteHub.subagentModels).toEqual(["xai/grok-4.6", "gpt-5.6-sol"]); + expect(parsed.remoteHub.oauth).toEqual([{ provider: "xai", loggedIn: true }, { provider: "anthropic", loggedIn: false }]); + // The connection block is untouched; remoteHub describes the other end of the link. + expect(parsed.connection.state).toBe("connected"); + + const human = await runStatus(home, codexHome, false); + expect(human.exitCode).toBe(0); + const lines = human.stdout.split("\n"); + expect(lines[0]).toContain("State from hub"); + // The hub's logins print ABOVE the local block, which is labelled as unused. + const hubHeading = lines.findIndex(line => line.includes("OAuth logins (hub")); + const localHeading = lines.findIndex(line => line.includes("Local-only (not used for routing while connected)")); + expect(hubHeading).toBeGreaterThanOrEqual(0); + expect(localHeading).toBeGreaterThan(hubHeading); + expect(human.stdout).toContain("xai ✓ logged in"); + // Lines that really are about this machine say so. + expect(human.stdout).toMatch(/Codex runtime: .*\(local\)/); + expect(human.stdout).toMatch(/Service: .*\(local\)/); + } finally { + await hub.stop(true); + removeTreeWithRetry(home); + } + }, SPAWN_BUDGET_MS); + + test("an unreachable hub degrades to unavailable instead of to local state", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-status-hub-down-")); + const codexHome = join(home, "codex"); + mkdirSync(codexHome, { recursive: true }); + try { + // Port 1 on loopback: nothing listens, and the refusal is immediate. + writeConnectedHome(home, "http://127.0.0.1:1"); + const json = await runStatus(home, codexHome, true); + expect(json.exitCode).toBe(0); + const parsed = JSON.parse(json.stdout); + expect(parsed.runtimeRole).toBe("client"); + expect(parsed.remoteHub.stateSource).toBe("unavailable"); + expect(parsed.remoteHub.providers).toEqual([]); + expect(typeof parsed.remoteHub.reason).toBe("string"); + + const human = await runStatus(home, codexHome, false); + expect(human.stdout).toContain("state unavailable"); + // The local credential block is still printed, but it is explicitly not the hub's. + expect(human.stdout).toContain("Local-only credential state"); + } finally { + removeTreeWithRetry(home); + } + }, SPAWN_BUDGET_MS); + + test("a standalone machine's report gains no banner and no (local) tags", async () => { + const home = mkdtempSync(join(tmpdir(), "ocx-status-standalone-")); + const codexHome = join(home, "codex"); + mkdirSync(codexHome, { recursive: true }); + try { + writeFileSync(join(home, "config.json"), JSON.stringify({ port: 9, providers: {}, codexAutoStart: false })); + const json = await runStatus(home, codexHome, true); + expect(json.exitCode).toBe(0); + const parsed = JSON.parse(json.stdout); + expect(parsed.runtimeRole).toBe("standalone"); + expect(parsed.remoteHub).toEqual({ + connected: false, + origin: null, + stateSource: "unavailable", + hubVersion: null, + providers: [], + oauth: [], + subagentModels: [], + claudeCodeEnabled: null, + }); + const human = await runStatus(home, codexHome, false); + expect(human.stdout).not.toContain("(local)"); + expect(human.stdout).not.toContain("State from hub"); + expect(human.stdout).toContain("OAuth logins:"); + } finally { + removeTreeWithRetry(home); + } + }, SPAWN_BUDGET_MS); +}); diff --git a/tests/server/v1-hub-state.test.ts b/tests/server/v1-hub-state.test.ts index 215ab3ef39..7bfed259dd 100644 --- a/tests/server/v1-hub-state.test.ts +++ b/tests/server/v1-hub-state.test.ts @@ -24,7 +24,9 @@ import type { OcxConfig } from "../../src/types"; import { removeTreeWithRetry } from "../helpers/remove-tree"; const DATA_KEY = "ocx_data_hubstatereader"; -const PROVIDER_KEY = "sk-hub-state-provider-secret-9e1f"; +// Deliberately NOT an `sk-…` shape: the privacy scan refuses one in a tracked file, and the +// assertion below only needs a distinctive string to hunt for in the response bytes. +const PROVIDER_KEY = "provider-credential-hub-state-9e1f"; const OAUTH_ACCESS = "oauth-access-hub-state-7c2a"; const OAUTH_REFRESH = "oauth-refresh-hub-state-4b8d"; const OAUTH_EMAIL = "hub-operator@example.test"; From 60d9e5d535038c1dfa90f944f66894778058887c Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:12:48 +0900 Subject: [PATCH 04/10] fix(claude): write the roster on a connected client, from the hub's list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cmdClaude` gated the roster writer on `typeof route === "number"`, which is false on a connected client — `route` is a `ClaudeRoutingTarget` there. So `~/.claude/agents/ocx-*.md` on a client was whatever a previous standalone run had left, indefinitely. And when it did run, it built the roster from local `config.subagentModels`: the list this machine had before it joined the hub. The operator saw five delegable native models on a client whose hub serves grok, with nothing in the output admitting the list described a different machine. The gate is gone and `buildClaudeAgentDefs` takes an explicit roster argument, defaulting to today's behaviour including "unset means the defaults, an explicit `[]` means none". On a connected client `ocx claude` passes the hub's featured roster; when the hub cannot be read it falls back to the local list and SAYS so, because an unannounced fallback is indistinguishable from a correct answer, which is how this defect stayed invisible. The five-row cap stays. It is a Claude Code picker constraint, not the bug — sourcing the five from the wrong machine was. `entryParts` already kept the raw id when a provider is absent from local config, which is what lets `xai/grok-4.6` produce `ocx-grok-4-6.md` on a credential-less client instead of throwing and aborting the sync for every other model too; there is now a test holding that. `syncClaudeAgentDefsAtProxyStartup` uses the same roster from the on-disk cache only. Startup makes no hub round trip for it: the live read belongs on the `ocx claude` path, and an offline hub must not stand between an operator and a local proxy start. Behaviour change worth naming: the first `ocx claude` after this lands rewrites a previously frozen roster on a client. The `generated-by: opencodex` ownership marker is untouched, so a user-authored `ocx-*.md` is still never overwritten or pruned. Co-Authored-By: Claude Fable 5.1 --- src/claude/agents-inject.ts | 34 ++- src/cli/claude-agent-startup-sync.ts | 27 ++- src/cli/claude.ts | 67 +++++- .../claude-agents-inject-client.test.ts | 223 ++++++++++++++++++ 4 files changed, 336 insertions(+), 15 deletions(-) create mode 100644 tests/claude-integration/claude-agents-inject-client.test.ts diff --git a/src/claude/agents-inject.ts b/src/claude/agents-inject.ts index bcd8c7a044..9e78fd996a 100644 --- a/src/claude/agents-inject.ts +++ b/src/claude/agents-inject.ts @@ -94,7 +94,23 @@ function entryParts(entry: string, config: OcxConfig): { alias: string; id: stri return { alias: claudeCodeNativeAlias(entry), id: entry, provider: "native" }; } -export function buildClaudeAgentDefs(config: OcxConfig, windows: Record, configDir = claudeConfigDir()): ClaudeAgentDef[] { +export function buildClaudeAgentDefs( + config: OcxConfig, + windows: Record, + configDir = claudeConfigDir(), + /** + * The roster to generate defs from, overriding local `config.subagentModels` (#4236). + * + * A connected client's local roster is whatever it had before it joined the hub — on a fresh + * client, the five native defaults — while the hub's featured roster is the list that + * actually routes. Passing it in keeps this function pure and keeps the override visible at + * the call site instead of hidden behind a config read. + * + * Undefined preserves today's behaviour exactly, including "unset means the defaults, an + * explicit `[]` means none". + */ + rosterOverride?: readonly string[], +): ClaudeAgentDef[] { const blockedSkills = effectiveBlockedSkillNames(config.claudeCode); const blockedSkillsFor = (model: string): readonly string[] => { const unmarked = stripOneMillionMarker(model); @@ -137,7 +153,8 @@ export function buildClaudeAgentDefs(config: OcxConfig, windows: Record, configDir?: string): string[] | null { +export function injectClaudeAgentDefs( + config: OcxConfig, + windows: Record, + configDir?: string, + /** Hub-sourced roster on a connected client; see `buildClaudeAgentDefs`. */ + rosterOverride?: readonly string[], +): string[] | null { if (config.claudeCode?.enabled === false || config.claudeCode?.injectAgents === false) { // Disabled: prune verified-owned files so stale definitions stop loading - // in future sessions (audit 071 #3). + // in future sessions (audit 071 #3). The roster override is irrelevant here by + // construction: there is nothing to build. return syncClaudeAgentDefs([], configDir); } - return syncClaudeAgentDefs(buildClaudeAgentDefs(config, windows, configDir), configDir); + return syncClaudeAgentDefs(buildClaudeAgentDefs(config, windows, configDir, rosterOverride), configDir); } /** * Dispatcher directive appended to every ocx-* description. The ocx-route body diff --git a/src/cli/claude-agent-startup-sync.ts b/src/cli/claude-agent-startup-sync.ts index 71acbb1911..8dbe158ca8 100644 --- a/src/cli/claude-agent-startup-sync.ts +++ b/src/cli/claude-agent-startup-sync.ts @@ -1,5 +1,6 @@ import type { OcxConfig } from "../types"; import { injectClaudeAgentDefs } from "../claude/agents-inject"; +import { readCachedHubState } from "../client/hub-state"; import { fetchClaudeContextWindows } from "./claude"; import type { ReadinessGate } from "../server/readiness"; @@ -7,6 +8,30 @@ export interface ClaudeAgentStartupSyncDeps { fetchContextWindows?: typeof fetchClaudeContextWindows; injectAgentDefs?: typeof injectClaudeAgentDefs; warn?: (message: string) => void; + /** Seam for the hub roster lookup; the default reads only the on-disk cache. */ + readHubRoster?: (config: OcxConfig) => readonly string[] | undefined; +} + +/** + * The hub's roster for a connected client, from the CACHE only (#4236). + * + * Startup deliberately makes no network call for this. The roster is a convenience here — `ocx + * claude` does the live read on the path where it matters — and a hub round trip on every proxy + * start would put an offline hub in the way of a local launch. Undefined falls back to local + * `subagentModels`, which is what this path has always used. + */ +function cachedHubRoster(config: OcxConfig): readonly string[] | undefined { + if (config.runtimeRole !== "client" || !config.client) return undefined; + try { + const cached = readCachedHubState({ + serverUrl: config.client.serverUrl, + apiKeyId: config.client.apiKeyId, + connectedAt: config.client.connectedAt, + }); + return cached?.state.subagentModels; + } catch { + return undefined; + } } /** @@ -70,7 +95,7 @@ export async function syncClaudeAgentDefsAtProxyStartup( // Startup remains best-effort. The next management mutation or `ocx claude` launch can // restore context markers after a transient catalog/Management API failure. } - return inject(config, windows); + return inject(config, windows, undefined, (deps.readHubRoster ?? cachedHubRoster)(config)); } catch (error) { warn(`⚠ Claude agent definitions could not be synced at proxy startup: ${error instanceof Error ? error.message : String(error)}`); return null; diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 03261c016a..0a0b8a1c5c 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -25,6 +25,7 @@ import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; import { selfLaunchArgv } from "../lib/self-launch-argv"; import { ANTHROPIC_PARENT_ENV_SLOTS, trustedNodeLauncherContext, type AnthropicParentEnvSlot } from "./launcher-context"; import { readClientConnectionState, type ClientConnectionState } from "../client/state"; +import { resolveHubState } from "../client/hub-state"; import { readServiceApiTokenState, type ServiceApiTokenState } from "../lib/service-secrets"; import { DEFAULT_CATALOG_PATH } from "../codex/paths"; import { readFileSync } from "node:fs"; @@ -663,6 +664,43 @@ export function rootSkipPermissionsNotice(env: ClaudeLaunchEnv): string { return `⚠ Root --dangerously-skip-permissions requested: preserving user IS_SANDBOX=${env.IS_SANDBOX}; Claude Code's root guard remains in control.`; } +/** + * The hub's featured subagent roster, or undefined to fall back to local `subagentModels`. + * + * Best-effort by design: a launch must not fail because the hub is slow or old. But the + * fallback is ANNOUNCED (#4236) — a silently local roster is exactly how an operator came to + * believe a hub that serves grok could only delegate to five native models. + * + * An empty hub roster is honoured as empty, not treated as "no answer": an operator who cleared + * the hub's featured list meant it. + */ +export async function resolveHubRosterForClaude( + connection: { serverUrl: string; apiKeyId: string; connectedAt: string }, + token: string, + deps: { resolve?: typeof resolveHubState; warn?: (message: string) => void } = {}, +): Promise { + const warn = deps.warn ?? (message => console.error(message)); + const resolve = deps.resolve ?? resolveHubState; + try { + const resolved = await resolve({ + owner: { serverUrl: connection.serverUrl, apiKeyId: connection.apiKeyId, connectedAt: connection.connectedAt }, + token, + }); + if (!resolved.state) { + warn(`⚠ Hub roster unavailable (${resolved.reason ?? "unknown reason"}); using this machine's local subagentModels instead. The delegable agents below may not be what the hub can route.`); + return undefined; + } + if (resolved.stateSource === "cache") { + warn(`⚠ Hub roster came from a cached read ${resolved.ageSeconds ?? "?"}s old (${resolved.reason ?? "live read failed"}).`); + } + return resolved.state.subagentModels; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + warn(`⚠ Hub roster could not be read (${message}); using this machine's local subagentModels instead.`); + return undefined; + } +} + export async function cmdClaude(args: string[]): Promise { const config = loadConfig(); const clientState = readClientConnectionState(); @@ -675,10 +713,13 @@ export async function cmdClaude(args: string[]): Promise { if (preflight.kind === "native") return launchNativeClaude(config, args, preflight.notice); let route: number | ClaudeRoutingTarget; let contextWindows: Record; + /** The hub's featured roster on a connected client; undefined means "use local config". */ + let hubRoster: readonly string[] | undefined; if (clientState.kind === "connected") { if (tokenState?.kind !== "present") return 1; route = { baseUrl: clientState.value.serverUrl, admissionToken: tokenState.token }; contextWindows = readConnectedClaudeContextWindows(); + hubRoster = await resolveHubRosterForClaude(clientState.value, tokenState.token); } else { const port = await ensureProxyForClaude(); if (!port) { @@ -710,16 +751,24 @@ export async function cmdClaude(args: string[]): Promise { console.error(`⚠ Gateway model cache could not be refreshed: ${message}`); } // Sync roster agents (devlog 070): subagentModels + self -> ~/.claude/agents/ocx-*.md. - if (typeof route === "number") { - try { - const written = injectClaudeAgentDefs(config, contextWindows); - if (written === null) { - console.error("⚠ Claude agent definitions could not be synced; check ~/.claude/agents permissions."); - } - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - console.error(`⚠ Claude agent definitions could not be synced: ${message}`); + // + // This used to run only when `route` was a number — i.e. never on a connected client, where + // `route` is a ClaudeRoutingTarget (#4236). So `~/.claude/agents/ocx-*.md` on a client stayed + // whatever a previous standalone run had left, and the five delegable agents an operator saw + // were a frozen snapshot of a machine that no longer does the routing. Nothing in the output + // said so; the roster simply looked like the answer. + // + // On a client the roster comes from the hub, because the local `subagentModels` list is the + // one this machine had before it joined. The five-row cap stays: it is a Claude Code picker + // constraint, not the defect — sourcing the five from the wrong machine was. + try { + const written = injectClaudeAgentDefs(config, contextWindows, undefined, hubRoster); + if (written === null) { + console.error("⚠ Claude agent definitions could not be synced; check ~/.claude/agents permissions."); } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(`⚠ Claude agent definitions could not be synced: ${message}`); } return spawnClaude(args, env); } diff --git a/tests/claude-integration/claude-agents-inject-client.test.ts b/tests/claude-integration/claude-agents-inject-client.test.ts new file mode 100644 index 0000000000..1e201987cd --- /dev/null +++ b/tests/claude-integration/claude-agents-inject-client.test.ts @@ -0,0 +1,223 @@ +/** + * The Claude Code spawn surface on a CONNECTED client (#4236). + * + * Two defects met here. `cmdClaude` gated the roster writer on `typeof route === "number"`, + * which is false on a connected client, so `~/.claude/agents/ocx-*.md` stayed whatever a + * previous standalone run had left — and even when it did run, it built the roster from local + * `config.subagentModels`, the list this machine had before it joined the hub. The visible + * symptom was five delegable native models on a client whose hub serves grok, with nothing in + * the output admitting the roster described a different machine. + * + * So the cases below prove the roster can come from the hub, that a hub model whose provider is + * absent from local config still yields a def instead of aborting the whole sync, that the + * announced fallback is announced, and that none of this weakened the ownership marker — the one + * invariant that keeps a user-authored `ocx-*.md` from being overwritten. + */ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildClaudeAgentDefs, injectClaudeAgentDefs } from "../../src/claude/agents-inject"; +import { resolveHubRosterForClaude } from "../../src/cli/claude"; +import { syncClaudeAgentDefsAtProxyStartup } from "../../src/cli/claude-agent-startup-sync"; +import type { HubStateResolution } from "../../src/client/hub-state"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const dirs: string[] = []; +function tempDir(): string { + const d = mkdtempSync(join(tmpdir(), "ocx-agents-client-")); + dirs.push(d); + return d; +} +afterEach(() => { for (const d of dirs.splice(0)) removeTreeWithRetry(d); }); + +const CONNECTION = { + serverUrl: "https://hub.example.test:8443", + apiKeyId: "client-one", + connectedAt: "2026-09-01T00:00:00.000Z", +}; + +function clientConfig(extra?: Partial): OcxConfig { + return { + port: 10100, + defaultProvider: "mock", + // Empty on purpose: a client stores no provider credentials, which is why a hub model's + // provider is routinely absent from local config. + providers: {}, + runtimeRole: "client", + subagentModels: ["gpt-5.6-sol"], + ...extra, + } as OcxConfig; +} + +describe("a hub-sourced roster drives the generated defs", () => { + test("the hub's models become ocx-*.md even when their provider is absent locally", () => { + const dir = tempDir(); + const defs = buildClaudeAgentDefs(clientConfig(), {}, dir, ["xai/grok-4.6", "gpt-5.6-sol"]); + const names = defs.map(def => def.name); + // The exact model the operator was told the hub could not serve. + expect(names).toContain("ocx-grok-4-6"); + expect(names).toContain("ocx-gpt-5-6-sol"); + // No local `xai` provider means no routed-id decode, and the raw id must survive rather + // than throwing and aborting the sync for every other model too. + const grok = defs.find(def => def.name === "ocx-grok-4-6"); + expect(grok?.model).toContain("grok-4.6"); + expect(grok?.description).toContain("(xai)"); + }); + + test("the local roster is ignored entirely when a hub roster is supplied", () => { + const dir = tempDir(); + const defs = buildClaudeAgentDefs( + clientConfig({ subagentModels: ["gpt-5.5", "gpt-5.6-terra"] }), + {}, + dir, + ["xai/grok-4.6"], + ); + expect(defs.map(def => def.name)).toEqual(["ocx-grok-4-6"]); + }); + + test("an empty hub roster is honoured as empty, not read as 'no answer'", () => { + const dir = tempDir(); + expect(buildClaudeAgentDefs(clientConfig(), {}, dir, [])).toEqual([]); + }); + + test("no override still means local config, byte for byte", () => { + const dir = tempDir(); + const withoutOverride = buildClaudeAgentDefs(clientConfig(), {}, dir); + const withLocalList = buildClaudeAgentDefs(clientConfig(), {}, dir, ["gpt-5.6-sol"]); + expect(withoutOverride).toEqual(withLocalList); + }); + + test("the picker's five-row cap still applies to a longer hub roster", () => { + const dir = tempDir(); + const defs = buildClaudeAgentDefs(clientConfig(), {}, dir, [ + "xai/grok-4.6", "a/one", "b/two", "c/three", "d/four", "e/five-should-not-appear", + ]); + expect(defs).toHaveLength(5); + expect(defs.map(def => def.name)).not.toContain("ocx-five-should-not-appear"); + }); +}); + +describe("injectClaudeAgentDefs on a client", () => { + test("writes the hub roster and keeps the ownership marker", () => { + const dir = tempDir(); + const written = injectClaudeAgentDefs(clientConfig(), {}, dir, ["xai/grok-4.6"]); + expect(written).toEqual(["ocx-grok-4-6.md"]); + const body = readFileSync(join(dir, "agents", "ocx-grok-4-6.md"), "utf8"); + expect(body).toContain("generated-by: opencodex"); + }); + + test("a user-authored ocx-* file without the marker is never overwritten or pruned", () => { + const dir = tempDir(); + mkdirSync(join(dir, "agents"), { recursive: true }); + const mine = join(dir, "agents", "ocx-grok-4-6.md"); + writeFileSync(mine, "---\nname: ocx-grok-4-6\n---\nmy own agent\n"); + const stale = join(dir, "agents", "ocx-handwritten.md"); + writeFileSync(stale, "---\nname: ocx-handwritten\n---\nalso mine\n"); + injectClaudeAgentDefs(clientConfig(), {}, dir, ["xai/grok-4.6"]); + expect(readFileSync(mine, "utf8")).toContain("my own agent"); + expect(readFileSync(stale, "utf8")).toContain("also mine"); + }); + + test("injectAgents false still prunes owned files, roster override notwithstanding", () => { + const dir = tempDir(); + injectClaudeAgentDefs(clientConfig(), {}, dir, ["xai/grok-4.6"]); + expect(readdirSync(join(dir, "agents"))).toContain("ocx-grok-4-6.md"); + const pruned = injectClaudeAgentDefs( + clientConfig({ claudeCode: { injectAgents: false } }), + {}, + dir, + ["xai/grok-4.6"], + ); + expect(pruned).toEqual([]); + expect(existsSync(join(dir, "agents", "ocx-grok-4-6.md"))).toBe(false); + }); +}); + +describe("resolveHubRosterForClaude", () => { + function resolution(overrides: Partial): HubStateResolution { + return { + stateSource: "hub", + state: { + schemaVersion: 1, + runtimeRole: "hub", + hubVersion: "2.51.0", + origin: null, + providers: [], + oauth: [], + subagentModels: ["xai/grok-4.6"], + claudeCode: { enabled: true }, + }, + ...overrides, + }; + } + + test("a live read hands back the hub's roster with no warning", async () => { + const warnings: string[] = []; + const roster = await resolveHubRosterForClaude(CONNECTION, "ocx_data_x", { + resolve: async () => resolution({}), + warn: message => { warnings.push(message); }, + }); + expect(roster).toEqual(["xai/grok-4.6"]); + expect(warnings).toEqual([]); + }); + + test("an unreadable hub falls back to local config but SAYS so", async () => { + // An unannounced fallback is the whole defect: a locally sourced roster is indistinguishable + // from a hub-sourced one in the output. + const warnings: string[] = []; + const roster = await resolveHubRosterForClaude(CONNECTION, "ocx_data_x", { + resolve: async () => ({ stateSource: "unavailable", state: null, reason: "the hub is unreachable" }), + warn: message => { warnings.push(message); }, + }); + expect(roster).toBeUndefined(); + expect(warnings.join("\n")).toContain("Hub roster unavailable (the hub is unreachable)"); + expect(warnings.join("\n")).toContain("local subagentModels"); + }); + + test("a cached roster is used and labelled as cached", async () => { + const warnings: string[] = []; + const roster = await resolveHubRosterForClaude(CONNECTION, "ocx_data_x", { + resolve: async () => resolution({ stateSource: "cache", ageSeconds: 120, reason: "the hub is unreachable" }), + warn: message => { warnings.push(message); }, + }); + expect(roster).toEqual(["xai/grok-4.6"]); + expect(warnings.join("\n")).toContain("cached read 120s old"); + }); + + test("a thrown resolver never takes the launch down with it", async () => { + const warnings: string[] = []; + const roster = await resolveHubRosterForClaude(CONNECTION, "ocx_data_x", { + resolve: async () => { throw new Error("boom"); }, + warn: message => { warnings.push(message); }, + }); + expect(roster).toBeUndefined(); + expect(warnings.join("\n")).toContain("boom"); + }); +}); + +describe("the proxy-startup roster sync", () => { + test("uses the cached hub roster on a client and makes no network call", async () => { + const dir = tempDir(); + let fetchedWindows = false; + const written = await syncClaudeAgentDefsAtProxyStartup(clientConfig(), 10100, { + fetchContextWindows: async () => { fetchedWindows = true; return {}; }, + readHubRoster: () => ["xai/grok-4.6"], + injectAgentDefs: (config, windows, _configDir, roster) => injectClaudeAgentDefs(config, windows, dir, roster), + }); + expect(written).toEqual(["ocx-grok-4-6.md"]); + // The context-window read is the only live call this path has ever made; the roster itself + // comes off disk so an offline hub cannot stand in the way of a local proxy start. + expect(fetchedWindows).toBe(true); + }); + + test("a hub machine still writes nothing", async () => { + const written = await syncClaudeAgentDefsAtProxyStartup( + clientConfig({ runtimeRole: "hub", client: undefined }), + 10100, + { readHubRoster: () => ["xai/grok-4.6"], injectAgentDefs: () => ["should-not-happen.md"] }, + ); + expect(written).toBeNull(); + }); +}); From bb6737353f7d882674d99156b827fc9d479ffeef Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:14:03 +0900 Subject: [PATCH 05/10] feat(cli): label a client in ocx config show and stop printing the catalog blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runtimeRole: "client"` and the `client` block were already printed and were already missed. An agent read a client's `config.json`, saw `providers: {}` and no grok provider, and concluded the hub could not serve grok — two floors below the fact that this machine stores no provider credentials on purpose. Two changes, both about where a reader's eye lands. `_remoteHub` is now the FIRST key on a client, naming the hub origin and pointing at `ocx status`, which is the command that has the facts. And `client.priorCatalog` — the base64 snapshot connect takes before overwriting the local catalog, up to 64 MB of it — prints as ``, mirroring `sanitizeModelCostsForDisplay`, instead of burying every other field under a wall of base64. `_remoteHub` is synthetic and never persisted: `clientConnectionSchema` is `.strict()` so a real `client.note` would not validate, and persisted prose drifts from the behaviour it describes. `config export` emits the real config untouched — annotation and omission marker both absent — so round trips still validate. Co-Authored-By: Claude Fable 5.1 --- src/cli/config-command.ts | 36 ++++- tests/cli/cli-config-show-client.test.ts | 160 +++++++++++++++++++++++ 2 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 tests/cli/cli-config-show-client.test.ts diff --git a/src/cli/config-command.ts b/src/cli/config-command.ts index cd33fefd74..f3d662330a 100644 --- a/src/cli/config-command.ts +++ b/src/cli/config-command.ts @@ -26,8 +26,36 @@ const USAGE = `Usage: const SECRET_KEYS = /^(apiKey|key|accessToken|refreshToken|idToken|token|password|clientSecret|webhookUrl)$/i; const BLOCKED_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); +/** + * The synthetic `_remoteHub` note printed by `ocx config show` on a client (#4236). + * + * `runtimeRole: "client"` and the `client` block were already printed, and were already ignored: + * an agent read a client's `config.json`, saw an empty `providers` map and no grok, and concluded + * the hub could not serve grok. Naming the situation in the config output costs one key. + * + * Synthetic and NOT persisted, for two reasons. `clientConnectionSchema` is `.strict()`, so a + * `client.note` field would not validate; and persisted prose drifts from the behaviour it + * describes. The leading underscore marks it as an annotation rather than a setting, and + * `config export` emits the real config untouched so round-trips still validate. + */ +export function remoteHubConfigNote(config: OcxConfig): { connected: boolean; origin: string; note: string } | null { + if (config.runtimeRole !== "client" || !config.client) return null; + return { + connected: true, + origin: config.client.serverUrl, + note: "provider credentials and model availability live on the hub; run ocx status", + }; +} + function redact(value: unknown, key = ""): unknown { if (SECRET_KEYS.test(key) && typeof value === "string") return value ? "********" : value; + // `client.priorCatalog` is the base64 catalog snapshot connect took before overwriting the + // local one — up to 64 MB of it (src/config.ts). Printed in full it buried `runtimeRole` and + // the `client` block under a wall of base64, which is how a reader came to miss that this + // machine is a client at all. Size only, mirroring sanitizeModelCostsForDisplay. + if (key === "priorCatalog" && typeof value === "string") { + return value ? `` : value; + } // modelCosts rows are keyed by model id; a pasted API key in a key position // must not be echoed back by config show/get (values are already redacted). if (key === "modelCosts") return sanitizeModelCostsForDisplay(value); @@ -119,7 +147,13 @@ export async function handleConfigCommand(argv: string[]): Promise { const source = takeFlag(args, "--source"); rejectArgs(args, USAGE); const diagnostics = readConfigDiagnostics(); - const config = redact(diagnostics.config); + const redacted = redact(diagnostics.config); + const note = remoteHubConfigNote(diagnostics.config); + // First key, not last: it has to be read before the empty `providers` map that misled a + // reader into concluding nothing was configured anywhere. + const config = note && redacted && typeof redacted === "object" && !Array.isArray(redacted) + ? { _remoteHub: note, ...redacted as Record } + : redacted; const result = source ? { config, source: diagnostics.source, error: diagnostics.error, warnings: diagnostics.warnings ?? [] } : config; printData(result, true); return; diff --git a/tests/cli/cli-config-show-client.test.ts b/tests/cli/cli-config-show-client.test.ts new file mode 100644 index 0000000000..8b7b0532f2 --- /dev/null +++ b/tests/cli/cli-config-show-client.test.ts @@ -0,0 +1,160 @@ +/** + * `ocx config show` on a client says so, and stops burying the fact (#4236). + * + * `runtimeRole: "client"` and the `client` block were already in the output and were already + * missed: an agent read a client's config, saw `providers: {}` and no grok, and concluded the hub + * could not serve grok. Two things made that easy. Nothing labelled the situation, and + * `client.priorCatalog` — the base64 snapshot connect took before overwriting the local catalog, + * up to 64 MB of it — sat in the middle of the document. + * + * So the synthetic `_remoteHub` note is asserted to be the FIRST key (it has to be read before + * the empty `providers` map), `priorCatalog` is asserted to be a size marker, and `config export` + * is asserted to be untouched and still `config validate`-clean — because a synthetic annotation + * that leaked into a round trip would be a worse bug than the one it fixes. + */ +import { describe, expect, setDefaultTimeout, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { remoteHubConfigNote } from "../../src/cli/config-command"; +import type { OcxConfig } from "../../src/types"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { SPAWN_BUDGET_MS } from "../helpers/test-budget"; + +const repoRoot = dirname(fileURLToPath(new URL("../../package.json", import.meta.url))); +const cliPath = join(repoRoot, "src", "cli", "index.ts"); +const isolatedCodexHome = mkdtempSync(join(tmpdir(), "ocx-config-client-codex-")); + +setDefaultTimeout(SPAWN_BUDGET_MS); + +function runCli(args: string[], home: string) { + return spawnSync(process.execPath, [cliPath, ...args], { + cwd: repoRoot, + env: { ...process.env, CODEX_HOME: isolatedCodexHome, OPENCODEX_HOME: home }, + encoding: "utf8", + timeout: SPAWN_BUDGET_MS - 5_000, + }); +} + +/** 12 KB of base64 stands in for the real thing; the assertion is about the shape, not the size. */ +const PRIOR_CATALOG = "A".repeat(12_288); + +function clientHome(): string { + const home = mkdtempSync(join(tmpdir(), "ocx-config-client-")); + writeFileSync(join(home, "config.json"), JSON.stringify({ + port: 10100, + providers: {}, + runtimeRole: "client", + client: { + serverUrl: "https://hub.example.test:8443", + managementUrl: "https://hub.example.test", + managementTransport: "direct", + selectedClients: ["codex", "claude"], + tokenEnv: "OPENCODEX_API_AUTH_TOKEN", + apiKeyId: "client-one", + tokenFingerprint: createHash("sha256").update("fixture-token").digest("hex"), + protocolVersion: 1, + connectedAt: "2026-09-01T00:00:00.000Z", + priorCatalog: PRIOR_CATALOG, + }, + }, null, 2)); + return home; +} + +function standaloneHome(): string { + const home = mkdtempSync(join(tmpdir(), "ocx-config-standalone-")); + writeFileSync(join(home, "config.json"), JSON.stringify({ port: 10100, providers: {} })); + return home; +} + +describe("remoteHubConfigNote", () => { + test("only a client with a connection block gets a note", () => { + expect(remoteHubConfigNote({ runtimeRole: "client" } as OcxConfig)).toBeNull(); + expect(remoteHubConfigNote({ runtimeRole: "standalone" } as OcxConfig)).toBeNull(); + expect(remoteHubConfigNote({ runtimeRole: "hub" } as OcxConfig)).toBeNull(); + expect(remoteHubConfigNote({} as OcxConfig)).toBeNull(); + }); + + test("the note names the hub and points at the command that has the facts", () => { + const note = remoteHubConfigNote({ + runtimeRole: "client", + client: { serverUrl: "https://hub.example.test:8443" }, + } as OcxConfig); + expect(note).toEqual({ + connected: true, + origin: "https://hub.example.test:8443", + note: "provider credentials and model availability live on the hub; run ocx status", + }); + }); +}); + +describe("ocx config show on a client", () => { + test("leads with _remoteHub and omits the priorCatalog blob", () => { + const home = clientHome(); + try { + const result = runCli(["config", "show"], home); + expect(result.status).toBe(0); + const parsed = JSON.parse(result.stdout); + // First key: it must be read before the empty providers map, not after it. + expect(Object.keys(parsed)[0]).toBe("_remoteHub"); + expect(parsed._remoteHub).toEqual({ + connected: true, + origin: "https://hub.example.test:8443", + note: "provider credentials and model availability live on the hub; run ocx status", + }); + expect(parsed.client.priorCatalog).toBe(``); + expect(result.stdout).not.toContain(PRIOR_CATALOG.slice(0, 256)); + // Everything else is still there; this is an annotation, not a filter. + expect(parsed.runtimeRole).toBe("client"); + expect(parsed.client.apiKeyId).toBe("client-one"); + } finally { + removeTreeWithRetry(home); + } + }); + + test("config get on the blob is omitted too, not printed through a side door", () => { + const home = clientHome(); + try { + const result = runCli(["config", "get", "client.priorCatalog"], home); + expect(result.status).toBe(0); + expect(result.stdout.trim()).toBe(``); + } finally { + removeTreeWithRetry(home); + } + }); + + test("config export carries the real config and stays validate-clean", () => { + const home = clientHome(); + const exported = join(home, "exported.json"); + try { + const result = runCli(["config", "export", exported], home); + expect(result.status).toBe(0); + const text = readFileSync(exported, "utf8"); + // A synthetic annotation that leaked into an export would break the round trip. + expect(text).not.toContain("_remoteHub"); + // And the export is the REAL config: the omission marker is a display concern only. + const parsed = JSON.parse(text); + expect(parsed.client.priorCatalog).toBe(PRIOR_CATALOG); + const validated = runCli(["config", "validate", exported], home); + expect(validated.status).toBe(0); + expect(validated.stdout).toContain("Config is valid."); + } finally { + removeTreeWithRetry(home); + } + }); + + test("a standalone machine's output is unannotated", () => { + const home = standaloneHome(); + try { + const result = runCli(["config", "show"], home); + expect(result.status).toBe(0); + expect(result.stdout).not.toContain("_remoteHub"); + expect(Object.keys(JSON.parse(result.stdout))).not.toContain("_remoteHub"); + } finally { + removeTreeWithRetry(home); + } + }); +}); From da65f9c549931a92dd873c4e16796b88fc387a14 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:16:53 +0900 Subject: [PATCH 06/10] docs(devlog): record the client hub-state unit (PR6) Includes the en remote-hub guide paragraph on what a connected client shows: the `State from hub ` banner, the hub-sourced OAuth/provider/model lines, the `(local)` tags, `runtimeRole` and the `remoteHub` block with its three-valued `stateSource`, and that the read uses the per-client data key only. The ko copy is left for the docs lane and is named as undone in the devlog. Co-Authored-By: Claude Fable 5.1 --- .../060_client_hub_state.md | 262 ++++++++++++++++++ .../src/content/docs/guides/remote-hub.md | 15 + 2 files changed, 277 insertions(+) create mode 100644 devlog/_plan/260911_hub_single_port/060_client_hub_state.md diff --git a/devlog/_plan/260911_hub_single_port/060_client_hub_state.md b/devlog/_plan/260911_hub_single_port/060_client_hub_state.md new file mode 100644 index 0000000000..65c737f556 --- /dev/null +++ b/devlog/_plan/260911_hub_single_port/060_client_hub_state.md @@ -0,0 +1,262 @@ +# 060 — PR6: a connected client reports the hub's state, not its own + +Unit: `devlog/_plan/260911_hub_single_port`. Stack position 6, branch +`codex/260911-l4-client-hub-state`, based on `codex/260911-l4-hub-token-ux` = `8c277294c` +(`test(service): drop the installLaunchd import the restack left unused`), so PR1 launchd repair, +PR2 loopback companion, PR3 hub local clients and PR4 hub token UX are all in ancestry. Sibling of +the docs PR 5 on the same base. Issue: lidge-jun/opencodex#4236. + +## The incident this closes + +Verbatim from the operator: an agent running on a connected **client** machine read that +machine's local `~/.opencodex/config.json` and `ocx status`, saw `xai ✗ not logged in`, no grok +provider and only five delegable models, and concluded **the hub could not serve grok** — while +the hub has xAI logged in and serves grok. + +Nothing malfunctioned. Every number the agent read was correct *about the client*, and a client +stores no provider credentials and no featured roster by design. The defect is attribution: three +surfaces reported local facts in the voice of the system. + +1. **`ocx status`.** `collectStatus` read only `readConfigDiagnostics()` plus local probes, and + the human renderer printed `OAuth logins:` from `oauthLoginSummary()` — this machine's + credential store, empty on a client. The only hub-aware output was one buried + `Remote hub: connected ()` line. +2. **The Claude Code spawn surface.** `cmdClaude` gated the roster writer on + `typeof route === "number"`, false on a connected client (`route` is a `ClaudeRoutingTarget`), + so `~/.claude/agents/ocx-*.md` was whatever a previous standalone run had left — and when it + did run it built from local `config.subagentModels`, capped at five. That is the "only 5 + delegable models" the operator saw. +3. **`ocx config show`.** `runtimeRole: "client"` and the `client` block were printed but + unlabelled, and `client.priorCatalog` — a base64 catalog snapshot up to 64 MB — sat in the + middle of the document burying them. + +## What shipped + +### 1. `GET|HEAD /v1/hub-state` on the hub's data plane + +`src/remote/hub-state.ts` (contract + caps + `parseHubStateBody`), `src/server/hub-state.ts` +(`buildHubState`, pure), the route in `src/server/index.ts` immediately after `/v1/catalog`, and +one `AUTH_MATRIX` row. + +Admission is `resolveApiAuth` + `isAllowedRequestOrigin`, identical to `/v1/catalog` (#809) and +for the same reason: nothing here forwards a caller credential upstream. No query parameters, +`Cache-Control: no-store`, no validator (so no 304 can cross identities), `content-length` set, +HEAD identical minus the body. + +Body: + +```json +{ "schemaVersion": 1, "runtimeRole": "hub", "hubVersion": "…", "origin": "…|null", + "providers": [{ "name": "", "adapter": "", "authMode": "key|forward|oauth|local|null", + "hasCredential": false, "disabled": false }], + "oauth": [{ "provider": "", "loggedIn": false }], + "subagentModels": [], "claudeCode": { "enabled": true } } +``` + +`hasCredential` is the `!!p.apiKey` presence projection `GET /api/providers` already ships. +`loggedIn` is `oauthLoginSummary`'s boolean with the **email and account id dropped, not masked**. +`buildHubState` constructs every row field by field and never spreads a provider or a login +record, which is what makes "no keys, no emails, no account ids" checkable by reading one +function — a spread would silently begin exporting whatever field is added to those records next. + +The role gate 404s with its own `hub_state_not_a_hub` code unless `runtimeRole === "hub"`, so a +standalone or client install gains no surface at all. It runs **after** admission on purpose: +answering an anonymous caller would turn the route into a free "is that host a hub?" probe. The +distinct code also keeps `tests/server/api-key-attribution.test.ts` honest — it is what tells +"this host is not a hub" apart from "this build has no such route", and the latter would let every +accepted admission cell pass vacuously. + +Bounded by construction (≤200 providers, ≤200 oauth rows, ≤32 roster entries, ≤200 chars per +string) plus a 64 KB ceiling that returns 507 `hub_state_too_large`. Deliberately **not** on +`loopbackRouteAllowed`: the unauthenticated loopback listener exists for inference wires the hub's +own local clients speak, a hub's own `ocx status` reads its config directly, and Ingwannu's review +note on #4236 is explicit that local management discovery goes to the authenticated surface. + +### 2. Client side: `fetchHubState` + `resolveHubState` + a 0600 cache + +`src/client/hub-client.ts` gains `fetchHubState`, beside `downloadClientCatalog` because it is the +same kind of call: bounded, schema-validated, unconditional GET with the per-client data key. A +404 surfaces as `hub_state_unsupported` — the version-skew case, which the CLI renders as +"upgrade the hub" rather than a bare code that reads like a client bug. + +`src/client/hub-state.ts` owns the resolution and the cache. Every failure path — 404, 401, +unreachable, non-JSON, malformed JSON, foreign schema, future schema, oversized — lands on +`stateSource: "cache"` or `"unavailable"` with an operator-facing reason, and **none of them +reaches back into local config**. That substitution is the defect, not a graceful degradation: a +locally sourced report is byte-indistinguishable from a hub-sourced one. + +The last good response is cached at `/hub-state.json` through `atomicWriteFile` +(0600), stamped with the `(serverUrl, apiKeyId, connectedAt)` triple and compared with +`sameClientConnectionOwner`. A stale cache is still the *hub's* state; an unstamped one would be a +*different* hub's after a disconnect and reconnect, which is not staleness but a lie. Symlinked or +oversized cache files are refused rather than followed. + +### 3. `ocx status` + +`CliStatusJson` gains `runtimeRole` and an always-present `remoteHub` block — +`{ connected, origin, stateSource, reason?, fetchedAt?, ageSeconds?, hubVersion, providers, oauth, +subagentModels, claudeCodeEnabled }`. `schemaVersion` stays 1 (additive, same rule as +`versionSkew`), and `connection` is untouched: it describes the **link**, `remoteHub` describes +what is on the other end of it. + +Human output on a connected client: + +``` +🔗 State from hub https://hub…:8443: provider credentials, logins and delegable models below are the HUB's, not this machine's. +✅ Proxy: running (PID …) (local) + … + OAuth logins (hub https://hub…:8443): + xai ✓ logged in + Providers (hub https://hub…:8443): + xai openai-chat — no API key (authMode oauth) + Delegable models (hub https://hub…:8443): xai/grok-4.6, gpt-5.6-sol + Hub version: 2.51.0 + Local-only (not used for routing while connected): + OAuth logins (local): + xai ✗ not logged in +``` + +When the hub cannot be read the banner becomes +`⚠️ Hub : state unavailable () — provider and login lines below are LOCAL and do +not describe the hub.` and the local heading becomes `Local-only credential state (this is NOT the +hub's…)`. A cached read is labelled `cached Ns ago` rather than presented as live. + +The banner is the **first** line of the report, above the proxy line, because the failure mode is +a reader taking the provider/login lines in isolation. `(local)` tags go on proxy, health, +dashboard, config, PID file, runtime, runtime source, default provider, Codex autostart, restart +safety, routing detail, service, shim and Codex runtime/version/source/home — and only while +connected, because on a standalone install every line is local and tagging them all would train +the reader to skip the tag. + +`remoteHubBannerLine` and `remoteHubStatusLines` live in `src/cli/status.ts` so the sentences are +testable without spawning the CLI, matching `hubStatusLines` from PR4. + +### 4. The Claude Code spawn surface + +The `typeof route === "number"` gate is gone. `buildClaudeAgentDefs` and `injectClaudeAgentDefs` +take an explicit `rosterOverride`, defaulting to today's behaviour including "unset means the +defaults, an explicit `[]` means none". On a connected client `cmdClaude` passes the hub's roster +through `resolveHubRosterForClaude`; an unreadable hub falls back to the local list **and prints +a warning**, because an unannounced fallback is exactly how this stayed invisible. + +`entryParts` already kept the raw id when a provider is absent from local `config.providers`, so +`xai/grok-4.6` yields `ocx-grok-4-6.md` on a credential-less client instead of reaching +`decodeRoutedModelIdOrThrow` and aborting the whole sync. That was latent and untested; it now has +a test. + +`syncClaudeAgentDefsAtProxyStartup` uses the same roster from the **cache only** — startup makes +no hub round trip, so an offline hub cannot stand between an operator and a local proxy start. + +### 5. `ocx config show` + +`_remoteHub` is the **first** key on a client: +`{ connected, origin, note: "provider credentials and model availability live on the hub; run ocx +status" }`. It must be read before the empty `providers` map, not after it. `client.priorCatalog` +prints as ``, mirroring `sanitizeModelCostsForDisplay`. + +Both are display-only. `config export` emits the real config untouched, so round trips still +validate; a persisted `client.note` was rejected because `clientConnectionSchema` is `.strict()` +and persisted prose drifts. + +## Decisions + +- **One data-plane read, not a widened `/api/*`.** The client holds only the per-client data key. + The relay (`/api/machine/hub-relay/*`) is browser-only — it needs a local gui-session and + forwards whatever hub key the browser supplies — so it is not a CLI channel. Ingwannu's review + note forbids adding `/api/*` to the unauthenticated listener or copying an admin credential into + exported client configuration, and this does neither. +- **Booleans only, forever.** Provider *names* already leak through `/v1/catalog` slugs, so the + delta this route adds is `hasCredential` and `loggedIn`. Emails, account ids, quotas and usage + must never be added: a data key opens this. +- **`authMode` is included** even though it was not in the original sketch. Without it + `hasCredential: false` on an OAuth provider reads as "not configured" — the precise inference + that went wrong. It is shape, not secret. +- **The five-row roster cap stays.** It is a Claude Code picker constraint (the Agent tool's model + argument is a 4-alias enum and the picker shows five rows), not the bug; sourcing the five from + the wrong machine was. The operator's "only 5 delegable models" is fixed by making those five + the hub's, and the hub can now change which five without touching the client. +- **`stateSource` is three-valued, and "unavailable" is a reportable outcome.** A two-valued + ok/failed flag would have invited the same silent local fallback at the next call site. +- **An always-present `remoteHub` object** rather than `null` when disconnected, so a consumer + never branches on the key existing; `connected: false` carries it. +- **The cache write is a side effect of `ocx status`.** Accepted deliberately: without it an + offline hub leaves a client with no hub facts at all, and the alternative (fetch-on-demand only) + makes `ocx claude` useless on a flaky link. The file is 0600, owner-stamped, and holds nothing + secret. +- **No loopback-listener allowlist entry.** Default per the plan; a hub reads its own config + directly and has no use for the route. +- **`src/server/management/route-registry.ts` untouched.** That registry declares `/api/*` + management routes; `/v1/catalog` and `/v1/models` are not in it either. The `AUTH_MATRIX` row is + the data-plane declaration, and it is driven against a real request by + `tests/server/api-key-attribution.test.ts`. + +## Verification (exact commands, this branch) + +``` +bun x tsc --noEmit # clean +bun run privacy:scan # Privacy scan passed +bun test tests/server/v1-hub-state.test.ts # 8 pass 0 fail +bun test tests/server/api-key-attribution.test.ts # 25 pass 0 fail +bun test tests/clients/client-hub-state.test.ts # 20 pass 0 fail +bun test tests/cli/cli-status-hub-state.test.ts # 12 pass 0 fail +bun test tests/cli/cli-status-json.test.ts # 53 pass 0 fail +bun test tests/cli/cli-config-show-client.test.ts # 6 pass 0 fail +bun test tests/cli/cli-config-command.test.ts # 2 pass 0 fail +bun test tests/cli/cli-transport-honesty.test.ts # 22 pass 0 fail +bun test tests/claude-integration/claude-agents-inject-client.test.ts # 14 pass 0 fail +bun test tests/claude-integration/claude-agents-inject.test.ts # 20 pass 0 fail +bun test tests/claude-integration/claude-agent-startup-sync.test.ts # 9 pass 0 fail +bun test tests/claude-integration/claude-cli.test.ts # 51 pass 0 fail +bun test tests/server/management-route-registry.test.ts # 13 pass 0 fail +bun test tests/ci-workflows/docs-remote-hub-claims.test.ts # 7 pass 0 fail +bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 17 pass 0 fail +``` + +`bun test tests/server/server-auth.test.ts` is 111 pass / 1 fail — `native passthrough upstream +reset still logs 502 and penalizes the pool`, the same pre-existing failure PR4's devlog (040) +recorded on this exact base. Not a regression; this PR touches one declarative array in +`src/server/auth-cors.ts` and adds a route block to `src/server/index.ts`. + +Four new test files, registered in `scripts/test-layout/layout.json` (`explicit`) and +`tests/fixtures/test-layout-expected.json`: + +- `tests/server/v1-hub-state.test.ts` — 401 before the role is disclosed, 200 with the data key, + HEAD, cross-origin 403, 404 on `standalone` and on an absent role, POST refused, and a + **serialized-body secret scan** with a real-looking provider key and a real-looking OAuth + credential (access, refresh, email) configured. +- `tests/clients/client-hub-state.test.ts` — every failure mode of the fetch, cache freshness and + age, owner mismatch, rotated `apiKeyId`, malformed and symlinked cache files, and that a + missing token never becomes a live read. +- `tests/cli/cli-status-hub-state.test.ts` — the banner and block sentences, plus three spawned + `ocx status` runs: a live local fake hub (which asserts the client presents its own data key), + an unreachable hub, and a standalone machine whose output gains no banner and no `(local)`. +- `tests/claude-integration/claude-agents-inject-client.test.ts` — the hub roster drives the defs, + `xai/grok-4.6` with no local `xai` provider does not throw, the cap still applies, the + `generated-by: opencodex` marker still protects a user-authored `ocx-*.md`, `injectAgents: false` + still prunes, and the announced fallback is announced. + +No repository-wide suite (operator instruction); hosted CI at the pushed head is the proof. + +### Live-hub safety + +This machine is a live OpenCodex hub. `ocx service …`, `ocx start/stop/ensure/sync/restore/connect/ +disconnect` and `launchctl` were **not** run, and the real `~/.opencodex`, `~/.codex`, +`~/.claude/agents` and `~/Library/LaunchAgents` were not touched. Every test sets +`OPENCODEX_HOME` to a `mkdtemp` directory; `tests/preload.ts` arms `OCX_TEST_HOME_GUARD=1` for +every invocation including a bare `bun test `. + +## Left undone + +- **ko docs.** The paragraph landed in `docs-site/src/content/docs/guides/remote-hub.md` (en) + only, as scoped. The Korean copy still describes a client that reports its own state. +- **`GET /api/machine/hub-state` on the client's own listener**, so the local dashboard sees the + same data without a hub gui-session. Sketched in the plan as optional; not built. +- **A `loggedIn: false` hub provider cannot be distinguished from one the hub has never + configured** in the `oauth` array, because `oauthLoginSummary` enumerates every known OAuth + provider. That matches what a hub operator sees locally, so it is consistent rather than wrong, + but a `configured` boolean would be clearer. +- **Staleness policy.** A cached hub state has no expiry; it is reported with its age and the + reader decides. A TTL that flipped `cache` to `unavailable` after N minutes would need a + defensible N. +- **`ocx doctor`** still reports local provider/login state on a client. Same class of defect, + separate surface. diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 60331d9746..9950244ffb 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -66,6 +66,21 @@ the hub. `ocx connect revoke --admin-token-stdin` is available only while still the persisted `apiKeyId`; it accepts no id override. Browser session logout/expiry is separate from data-key rotation, revocation, and disconnect. +### What a connected client shows + +A client stores no provider credentials and no catalog of its own, so its local config and +credential store are empty by design — and reading them as the truth produces a confident, wrong +answer about what the hub can serve. On a connected client `ocx status` therefore leads with +`State from hub ` and sources the OAuth-logins, providers and delegable-models lines from +the hub over the data plane, tagging the lines that really describe this machine `(local)`: the +proxy, the service, the Codex binary and shim, and the local ports. `ocx status --json` carries the +same answer as `runtimeRole` plus a `remoteHub` block whose `stateSource` is `hub`, `cache`, or +`unavailable` — never the client's own state. An older hub that does not serve `/v1/hub-state` +reports `unavailable` with an instruction to upgrade the hub rather than silently falling back to +local login state, and `ocx config show` on a client prints a `_remoteHub` note saying the +credentials and model availability live on the hub. The hub read uses the per-client data key +only; no admin token and no provider secret ever reaches a client. + ## Linux systemd or macOS launchd Choose the hub's Tailscale address for the data listener and the exact browser-visible HTTPS origin From 44b256c032d87c48437c63daf207113937f6fe4b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:53:09 +0900 Subject: [PATCH 07/10] fix(server,client): hub state exports only enabled providers and names every failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildHubState` mapped every `config.providers` row, `disabled: true` included, while `/v1/catalog` and `/v1/models` both filter a disabled provider out — so this route was the only data-plane surface that named one, and three comments plus the devlog claimed the delta over `/v1/catalog` was "only the two booleans". A boundary comment that understates what the code discloses is worse than none: it is what the next reviewer checks against. A disabled provider is now dropped entirely (a client cannot route to it, and `authMode` already explains a keyless row), and the comments state the delta exactly, down to the name of an enabled provider the catalog omits for want of a credential. `disabled` stays in the contract because an older hub still sends `true` and a client must label that row. `truncated` joins the body so a cap is reported rather than silently clipping the lists, and `hubStateFailureReason` grows a sentence for `hub_state_content_type_invalid` and for the `hub_state_http_` family — the `ocx status` banner printed `state unavailable (hub_state_http_507)`, which reads like a client bug when the hub has in fact answered. Co-Authored-By: Claude Fable 5.1 --- src/cli/status.ts | 9 +++++ src/client/hub-state.ts | 19 ++++++++- src/remote/hub-state.ts | 31 ++++++++++++++- src/server/hub-state.ts | 27 +++++++++++-- src/server/index.ts | 7 ++++ tests/cli/cli-status-hub-state.test.ts | 10 +++++ tests/clients/client-hub-state.test.ts | 53 +++++++++++++++++++++++++- tests/server/v1-hub-state.test.ts | 29 +++++++++++++- 8 files changed, 175 insertions(+), 10 deletions(-) diff --git a/src/cli/status.ts b/src/cli/status.ts index b4c77bdfaa..1fcbe33466 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -89,6 +89,8 @@ export type CliRemoteHubStatus = { providers: HubStateProvider[]; oauth: HubStateOAuthEntry[]; subagentModels: string[]; + /** The hub hit one of its own caps, so the lists above are a prefix and the report says so. */ + truncated: boolean; /** The hub's own Claude Code toggle; a client cannot infer it from local config. */ claudeCodeEnabled: boolean | null; }; @@ -309,6 +311,7 @@ export function disconnectedRemoteHubStatus(): CliRemoteHubStatus { providers: [], oauth: [], subagentModels: [], + truncated: false, claudeCodeEnabled: null, }; } @@ -354,6 +357,7 @@ export async function collectRemoteHubStatus( providers: resolved.state?.providers ?? [], oauth: resolved.state?.oauth ?? [], subagentModels: resolved.state?.subagentModels ?? [], + truncated: resolved.state?.truncated === true, claudeCodeEnabled: resolved.state ? resolved.state.claudeCode.enabled : null, }; } @@ -402,6 +406,11 @@ export function remoteHubStatusLines(remoteHub: CliRemoteHubStatus): string[] { `Delegable models (hub ${origin}): ${remoteHub.subagentModels.length === 0 ? "none" : remoteHub.subagentModels.join(", ")}`, ]; if (remoteHub.hubVersion) lines.push(`Hub version: ${remoteHub.hubVersion}`); + // The hub told us its own lists are a prefix. Saying nothing here would make this report claim + // completeness it does not have — the same shape of confident-and-wrong answer as #4236. + if (remoteHub.truncated) { + lines.push(" (the hub truncated this state to fit its response caps; some rows are not listed)"); + } return lines; } diff --git a/src/client/hub-state.ts b/src/client/hub-state.ts index 9ea5c3e0a8..655bcbdd9c 100644 --- a/src/client/hub-state.ts +++ b/src/client/hub-state.ts @@ -118,6 +118,11 @@ export function writeCachedHubState(owner: HubStateOwner, state: HubStateDTO, fe * * `hub_state_unsupported` is the version-skew case and gets an explicit upgrade instruction: * left as a bare code it reads like a bug in the client. + * + * Every code `fetchHubState` can throw has a sentence here, including the open-ended + * `hub_state_http_` family. This reason is printed in the `ocx status` banner, and a + * banner reading `state unavailable (hub_state_http_507)` sends the reader looking for a client + * bug when the hub has in fact answered and said something. */ export function hubStateFailureReason(error: unknown): string { if (error instanceof HubClientError) { @@ -129,14 +134,24 @@ export function hubStateFailureReason(error: unknown): string { case "hub_state_schema_invalid": case "hub_state_invalid": return "the hub returned an unreadable hub-state document"; + case "hub_state_content_type_invalid": + // Usually a captive portal, a TLS-terminating proxy or an error page in front of the + // hub: the request reached SOMETHING, and that something is not the hub's API. + return "the hub's state response was not JSON"; case "body_too_large": return "the hub's state response exceeded the allowed size"; case "unreachable": return "the hub is unreachable"; case "redirect_refused": return "the hub redirected the state request"; - default: - return error.code; + default: { + const status = error.code.startsWith("hub_state_http_") + ? error.code.slice("hub_state_http_".length) + : null; + return status && /^\d+$/.test(status) + ? `the hub answered HTTP ${status} to the state request` + : error.code; + } } } return "the hub state could not be read"; diff --git a/src/remote/hub-state.ts b/src/remote/hub-state.ts index 7ac48b7bc6..b1c299f03f 100644 --- a/src/remote/hub-state.ts +++ b/src/remote/hub-state.ts @@ -16,8 +16,17 @@ * entirely. No keys, no tokens, no quotas, no usage, no account identity — and nothing of that * shape may be added later, because this surface is reachable with a data key. * - * Provider NAMES already leak through `/v1/catalog` slugs, so the delta this adds is only the - * two booleans. + * The delta over `/v1/catalog`, stated exactly, because "only two booleans" was wrong and a + * wrong boundary claim is worse than none. Provider names already leak through `/v1/catalog` + * slugs and `/v1/models` ids, but only for providers those routes list. What this route adds is: + * `hasCredential`, `loggedIn`, `authMode`, the featured roster — and the NAME and adapter of an + * ENABLED provider that the catalog omits for want of a usable credential. That last one is the + * point of the route (a client has to be able to say "the hub has xai configured and has no key + * for it" rather than "the hub cannot serve grok"), and it is the whole widening. + * + * A provider the operator marked `disabled` is NOT exported — see `src/server/hub-state.ts`. + * Naming it would tell a data-key holder about a provider no other data-plane route mentions, + * and a client has no use for it: it is not routable, so "absent" is the truthful report. */ export const HUB_STATE_SCHEMA_VERSION = 1; @@ -45,6 +54,11 @@ export interface HubStateProvider { authMode: HubStateAuthMode; /** Presence only — the same `!!p.apiKey` projection `GET /api/providers` ships. */ hasCredential: boolean; + /** + * Always `false` from a hub of this version, which does not export a disabled provider at all. + * The field stays in the contract because an OLDER hub does send `true`, and a client reading + * one must still be able to label the row rather than present it as routable. + */ disabled: boolean; } @@ -65,6 +79,13 @@ export interface HubStateDTO { oauth: HubStateOAuthEntry[]; /** The hub's effective featured subagent roster — what a client should delegate to. */ subagentModels: string[]; + /** + * At least one row was dropped to fit a cap above, so the arrays are a prefix rather than the + * whole truth. Silent truncation is what this flag exists to prevent: a client that lists 200 + * of 240 providers and says nothing has told the reader the other 40 do not exist, which is + * the same class of confident-and-wrong report #4236 is about. + */ + truncated: boolean; claudeCode: { enabled: boolean }; } @@ -101,6 +122,11 @@ export function parseHubStateBody(value: unknown): HubStateDTO | null { if (!claudeCode || typeof claudeCode !== "object" || Array.isArray(claudeCode)) return null; const enabled = (claudeCode as Record).enabled; if (typeof enabled !== "boolean") return null; + // Absent means false: a hub that predates the flag truncated nothing this client can detect, + // and refusing its document would turn a new honesty field into a compatibility break. A + // present non-boolean is still refused, like every other field here. + if (raw.truncated !== undefined && typeof raw.truncated !== "boolean") return null; + const truncated = raw.truncated === true; const providers: HubStateProvider[] = []; for (const row of raw.providers) { @@ -150,6 +176,7 @@ export function parseHubStateBody(value: unknown): HubStateDTO | null { providers, oauth, subagentModels, + truncated, claudeCode: { enabled }, }; } diff --git a/src/server/hub-state.ts b/src/server/hub-state.ts index 8ff30f568d..abf343b768 100644 --- a/src/server/hub-state.ts +++ b/src/server/hub-state.ts @@ -37,11 +37,15 @@ export interface HubStateLoginRow { * sees exactly what the hub itself would offer. */ export function hubSubagentRoster(config: Pick): string[] { + return uncappedSubagentRoster(config).slice(0, MAX_HUB_STATE_SUBAGENT_MODELS); +} + +/** The same roster before the cap, so `truncated` can be computed instead of guessed. */ +function uncappedSubagentRoster(config: Pick): string[] { const roster = config.subagentModels === undefined ? DEFAULT_SUBAGENT_MODELS : config.subagentModels; return roster .filter((entry): entry is string => typeof entry === "string" && entry.trim() !== "") - .map(entry => entry.trim()) - .slice(0, MAX_HUB_STATE_SUBAGENT_MODELS); + .map(entry => entry.trim()); } export function buildHubState( @@ -49,7 +53,13 @@ export function buildHubState( logins: readonly HubStateLoginRow[], hubVersion: string, ): HubStateDTO { - const providers: HubStateProvider[] = Object.entries(config.providers ?? {}) + // A disabled provider is dropped, not exported with a flag. `/v1/catalog` and `/v1/models` + // both filter it out (`src/router.ts`, `src/codex/catalog/*`), so exporting it here was the + // one thing this route told a data-key holder that no other data-plane route does — and a + // client has no use for it either: it cannot be routed to, so absence IS the truthful report, + // and `authMode` already explains a present-but-credential-less row without it. + const enabledProviders = Object.entries(config.providers ?? {}).filter(([, provider]) => provider.disabled !== true); + const providers: HubStateProvider[] = enabledProviders .slice(0, MAX_HUB_STATE_PROVIDERS) .map(([name, provider]) => ({ name, @@ -59,12 +69,20 @@ export function buildHubState( : null, // Presence only. Identical to the projection GET /api/providers already ships. hasCredential: Boolean(provider.apiKey), - disabled: provider.disabled === true, + // Always false here. The field stays in the contract for an older hub's documents; see + // `HubStateProvider` in src/remote/hub-state.ts. + disabled: false, })); // Field-by-field, never a spread: oauthLoginSummary also carries the operator's email. const oauth: HubStateOAuthEntry[] = logins .slice(0, MAX_HUB_STATE_OAUTH_PROVIDERS) .map(entry => ({ provider: entry.provider, loggedIn: entry.loggedIn === true })); + const rosterBeforeCap = uncappedSubagentRoster(config).length; + // Said out loud rather than silently: the caps are a prefix, and a client presenting a prefix + // as the whole list is the same confident-and-wrong report this route exists to prevent. + const truncated = enabledProviders.length > MAX_HUB_STATE_PROVIDERS + || logins.length > MAX_HUB_STATE_OAUTH_PROVIDERS + || rosterBeforeCap > MAX_HUB_STATE_SUBAGENT_MODELS; return { schemaVersion: HUB_STATE_SCHEMA_VERSION, runtimeRole: "hub", @@ -73,6 +91,7 @@ export function buildHubState( providers, oauth, subagentModels: hubSubagentRoster(config), + truncated, // Same predicate the launch path uses: absence means enabled. claudeCode: { enabled: config.claudeCode?.enabled !== false }, }; diff --git a/src/server/index.ts b/src/server/index.ts index b0f57eadbe..e1aa2be2bd 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -1411,6 +1411,13 @@ export function startServer(port?: number, deps: StartServerDeps = {}): Server = {}): HubStateDTO { ], oauth: [{ provider: "xai", loggedIn: true }, { provider: "anthropic", loggedIn: false }], subagentModels: ["xai/grok-4.6", "gpt-5.6-sol"], + truncated: false, claudeCode: { enabled: true }, ...overrides, }; @@ -141,6 +142,7 @@ describe("the hub banner and block", () => { providers: hubState().providers, oauth: hubState().oauth, subagentModels: hubState().subagentModels, + truncated: false, claudeCodeEnabled: true, }; @@ -178,6 +180,13 @@ describe("the hub banner and block", () => { expect(lines.join("\n")).toContain("Hub version: 2.51.0"); }); + test("a truncated hub state says so instead of presenting a prefix as the whole list", () => { + const lines = remoteHubStatusLines({ ...live, truncated: true }).join("\n"); + expect(lines).toContain("the hub truncated this state to fit its response caps"); + // And the honest case stays quiet: a note on every report would train the reader to skip it. + expect(remoteHubStatusLines(live).join("\n")).not.toContain("truncated"); + }); + test("no hub state means no hub block, rather than an empty one that reads as 'nothing configured'", () => { expect(remoteHubStatusLines({ ...live, stateSource: "unavailable", providers: [], oauth: [], subagentModels: [] })).toEqual([]); expect(remoteHubStatusLines(disconnectedRemoteHubStatus())).toEqual([]); @@ -296,6 +305,7 @@ describe("ocx status end to end on a connected client", () => { providers: [], oauth: [], subagentModels: [], + truncated: false, claudeCodeEnabled: null, }); const human = await runStatus(home, codexHome, false); diff --git a/tests/clients/client-hub-state.test.ts b/tests/clients/client-hub-state.test.ts index 716bff2c83..fd23059766 100644 --- a/tests/clients/client-hub-state.test.ts +++ b/tests/clients/client-hub-state.test.ts @@ -20,12 +20,13 @@ import { join } from "node:path"; import { fetchHubState, HubClientError } from "../../src/client/hub-client"; import { hubStateCachePath, + hubStateFailureReason, readCachedHubState, resolveHubState, writeCachedHubState, type HubStateOwner, } from "../../src/client/hub-state"; -import type { HubStateDTO } from "../../src/remote/hub-state"; +import { parseHubStateBody, type HubStateDTO } from "../../src/remote/hub-state"; import { removeTreeWithRetry } from "../helpers/remove-tree"; const previousHome = process.env.OPENCODEX_HOME; @@ -46,6 +47,7 @@ function hubState(overrides: Partial = {}): HubStateDTO { providers: [{ name: "xai", adapter: "openai-chat", authMode: "oauth", hasCredential: true, disabled: false }], oauth: [{ provider: "xai", loggedIn: true }], subagentModels: ["xai/grok-4.6"], + truncated: false, claudeCode: { enabled: true }, ...overrides, }; @@ -124,6 +126,55 @@ describe("fetchHubState", () => { }).catch((e: unknown) => e); expect((error as HubClientError).code).toBe("hub_state_schema_invalid"); }); + + test("the truncation flag crosses the wire, and an older hub's document still parses", async () => { + const flagged = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { + fetchImpl: jsonFetch(hubState({ truncated: true })), + }); + expect(flagged.truncated).toBe(true); + // A hub that predates the flag sends no `truncated` key. Refusing that document would turn + // an honesty field into a compatibility break; absent reads as "nothing was dropped". + const { truncated: _dropped, ...withoutFlag } = hubState(); + const older = await fetchHubState(OWNER.serverUrl, "ocx_data_x", { fetchImpl: jsonFetch(withoutFlag) }); + expect(older.truncated).toBe(false); + // A present non-boolean is still refused, like every other field in this contract. + expect(parseHubStateBody({ ...hubState(), truncated: "yes" })).toBeNull(); + }); +}); + +describe("hubStateFailureReason", () => { + // Every code `fetchHubState` throws needs a sentence: this string is printed verbatim in the + // `ocx status` banner, and `state unavailable (hub_state_http_507)` sends an operator hunting + // for a client bug when the hub has answered and said something. + test.each([ + ["hub_state_content_type_invalid", "the hub's state response was not JSON"], + ["hub_state_http_507", "the hub answered HTTP 507 to the state request"], + ["hub_state_http_502", "the hub answered HTTP 502 to the state request"], + ] as const)("%s renders as a sentence", (code, expected) => { + expect(hubStateFailureReason(new HubClientError(code, "raw"))).toBe(expected); + }); + + test("an unknown code still falls back to the code rather than inventing a status", () => { + expect(hubStateFailureReason(new HubClientError("hub_state_http_oops", "raw"))).toBe("hub_state_http_oops"); + expect(hubStateFailureReason(new HubClientError("something_else", "raw"))).toBe("something_else"); + }); + + test("the sentences reach the resolution, not just the helper", async () => { + const contentType = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: jsonFetch("captive portal", { contentType: "text/html" }), + }); + expect(contentType.stateSource).toBe("unavailable"); + expect(contentType.reason).toBe("the hub's state response was not JSON"); + const overSized = await resolveHubState({ + owner: OWNER, + token: "ocx_data_x", + fetchImpl: jsonFetch({ error: {} }, { status: 507 }), + }); + expect(overSized.stateSource).toBe("unavailable"); + expect(overSized.reason).toBe("the hub answered HTTP 507 to the state request"); + }); }); describe("resolveHubState", () => { diff --git a/tests/server/v1-hub-state.test.ts b/tests/server/v1-hub-state.test.ts index 7bfed259dd..2b74deee15 100644 --- a/tests/server/v1-hub-state.test.ts +++ b/tests/server/v1-hub-state.test.ts @@ -130,7 +130,11 @@ describe("GET /v1/hub-state", () => { hasCredential: true, disabled: false, }); - expect(state!.providers.find(p => p.name === "quiet")?.disabled).toBe(true); + // A disabled provider is not exported at all: `/v1/catalog` and `/v1/models` filter it + // out, so naming it here would be the only place a data key learns it exists. + expect(state!.providers.map(p => p.name)).not.toContain("quiet"); + expect(text).not.toContain("quiet"); + expect(state!.truncated).toBe(false); expect(state!.oauth.find(entry => entry.provider === "xai")?.loggedIn).toBe(true); expect(state!.subagentModels).toEqual(["xai/grok-4.6", "gpt-5.6-sol"]); expect(state!.claudeCode.enabled).toBe(true); @@ -210,6 +214,29 @@ describe("GET /v1/hub-state", () => { } }); + test("hitting a cap is reported as truncated rather than silently clipped", async () => { + // 201 providers against a 200 cap. A body that simply stopped at 200 would tell a client the + // other provider does not exist, which is the same confident-and-wrong report #4236 is about. + const providers: Record = {}; + for (let i = 0; i < 201; i += 1) { + providers[`p${i}`] = { adapter: "openai-chat", baseUrl: "https://example.test/v1", models: ["m"] }; + } + saveConfig(hubConfig({ providers: providers as OcxConfig["providers"], defaultProvider: "p0" })); + const server = startServer(0); + try { + const res = await fetch(new URL("/v1/hub-state", server.url), { + headers: { "x-opencodex-api-key": DATA_KEY }, + }); + expect(res.status).toBe(200); + const state = parseHubStateBody(await res.json()); + expect(state).not.toBeNull(); + expect(state!.providers).toHaveLength(200); + expect(state!.truncated).toBe(true); + } finally { + await server.stop(true); + } + }); + test("POST is not a hub-state verb", async () => { saveConfig(hubConfig()); const server = startServer(0); From d4680d945f892d76d3d7b1853a2f10a111b0faba Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:53:18 +0900 Subject: [PATCH 08/10] fix(cli,client): observe a client's connection, and drop its hub-state cache on disconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_remoteHub.connected` was hardcoded `true` for any config carrying a `client` block, so a machine whose data key was revoked at the hub, rotated away, or whose token file was gone still printed `connected: true` in `ocx config show`. Configuration is not evidence that a connection works, which is the defect this unit exists to fix, in miniature. `remoteHubConfigNote` now reads `collectClientConnectionStatus()` and requires both halves — a settled connection record and the token file whose fingerprint matches it — and names what is wrong when either fails. The status is passed as a thunk so a standalone or hub install returns before the probe, and `./connect` is imported lazily so `ocx config get/set` does not drag the client lifecycle in. `disconnectClient` also unlinks `/hub-state.json`. It is owner-stamped, so a reader would reject it, but a disconnected machine should not keep a file naming the former hub's providers and logins. Best effort, after the connection is cleared: the disconnect has already succeeded by then and a stubborn cache file must not fail it. Co-Authored-By: Claude Fable 5.1 --- src/cli/config-command.ts | 37 +++++++++-- src/client/connect.ts | 19 ++++++ tests/cli/cli-config-show-client.test.ts | 82 ++++++++++++++++++++---- tests/clients/client-connect.test.ts | 18 +++++- 4 files changed, 137 insertions(+), 19 deletions(-) diff --git a/src/cli/config-command.ts b/src/cli/config-command.ts index f3d662330a..f0d3c1cbae 100644 --- a/src/cli/config-command.ts +++ b/src/cli/config-command.ts @@ -4,6 +4,7 @@ import { getConfigPath, mutatePersistedConfig, readConfigDiagnostics, sanitizeMo import { VISION_REASONING_EFFORTS, isVisionReasoningEffort } from "../reasoning-effort"; import type { OcxConfig } from "../types"; import { normalizeVisionReasoningForModel } from "../vision/reasoning"; +import type { ClientConnectionStatus } from "./connect"; import { CliUsageError, printData, rejectArgs, runCliAction, takeFlag } from "./runtime-api"; const USAGE = `Usage: @@ -33,18 +34,37 @@ const BLOCKED_SEGMENTS = new Set(["__proto__", "prototype", "constructor"]); * an agent read a client's `config.json`, saw an empty `providers` map and no grok, and concluded * the hub could not serve grok. Naming the situation in the config output costs one key. * + * `connected` is OBSERVED, never assumed. It was briefly hardcoded `true` for any config with a + * `client` block, which is the same defect in miniature: the presence of configuration is not + * evidence that the connection works, and a machine whose data-plane token was revoked, rotated + * away or deleted would have been labelled `connected: true` while it could not reach the hub at + * all. `collectClientConnectionStatus` is the one reader that knows — it compares the token file's + * fingerprint against the connection record — so the caller passes its answer in and this stays + * pure and testable. + * * Synthetic and NOT persisted, for two reasons. `clientConnectionSchema` is `.strict()`, so a * `client.note` field would not validate; and persisted prose drifts from the behaviour it * describes. The leading underscore marks it as an annotation rather than a setting, and * `config export` emits the real config untouched so round-trips still validate. */ -export function remoteHubConfigNote(config: OcxConfig): { connected: boolean; origin: string; note: string } | null { +export function remoteHubConfigNote( + config: OcxConfig, + readConnection: () => Pick, +): { connected: boolean; origin: string; note: string } | null { if (config.runtimeRole !== "client" || !config.client) return null; - return { - connected: true, - origin: config.client.serverUrl, - note: "provider credentials and model availability live on the hub; run ocx status", - }; + // A thunk, so a standalone or hub install pays nothing: the guard above returns first and the + // connection probe (three file reads) never runs. + const connection = readConnection(); + // Both halves are required: a settled connection record AND the token it recorded. Either one + // alone describes a machine that cannot read its hub, and `ocx status` is still the command + // that has the facts — so the note points there in every case, connected or not. + const connected = connection.state === "connected" && connection.token === "owned"; + const note = connection.state !== "connected" + ? `this machine is configured as a client but its connection is ${connection.state}${connection.reason ? ` (${connection.reason})` : ""}; run ocx connect status` + : connection.token !== "owned" + ? `this machine is configured as a client but its hub data-plane token is ${connection.token}; run ocx connect status` + : "provider credentials and model availability live on the hub; run ocx status"; + return { connected, origin: config.client.serverUrl, note }; } function redact(value: unknown, key = ""): unknown { @@ -148,7 +168,10 @@ export async function handleConfigCommand(argv: string[]): Promise { rejectArgs(args, USAGE); const diagnostics = readConfigDiagnostics(); const redacted = redact(diagnostics.config); - const note = remoteHubConfigNote(diagnostics.config); + // Imported here rather than at module scope: `./connect` pulls the whole client lifecycle + // in, and `ocx config get/set` has no use for it. + const { collectClientConnectionStatus } = await import("./connect"); + const note = remoteHubConfigNote(diagnostics.config, () => collectClientConnectionStatus()); // First key, not last: it has to be read before the empty `providers` map that misled a // reader into concluding nothing was configured anywhere. const config = note && redacted && typeof redacted === "object" && !Array.isArray(redacted) diff --git a/src/client/connect.ts b/src/client/connect.ts index 3569f83c83..d949038609 100644 --- a/src/client/connect.ts +++ b/src/client/connect.ts @@ -68,6 +68,7 @@ import { assertNoClientDisconnectPending, assertClientConnectionUnchanged, sameClientConnectionOwner, } from "./state"; import { assertClientCatalogCompatible, type CatalogCompatibilityDeps } from "./catalog-compatibility"; +import { hubStateCachePath } from "./hub-state"; class RotationRecoveryRequiredError extends Error { constructor(message: string, options?: ErrorOptions) { @@ -891,6 +892,7 @@ export async function disconnectClient( if (!disconnectAtLeast(receipt, "clearing_connection")) advance("clearing_connection"); if (clearClientConnection(receipt.owner) === "conflict") throw new Error("client_disconnect_owner_changed"); if (!disconnectAtLeast(receipt, "connection_cleared")) advance("connection_cleared"); + removeHubStateCache(); requireDesktopResult(finishRemoteDesktopCleanup(held, receipt.owner)); if (receipt.phase !== "complete") advance("complete"); return { @@ -902,6 +904,23 @@ export async function disconnectClient( }), deps.lifecycleLockDeps); } +/** + * Drop the cached hub-state document (#4236). + * + * It is derived data from a connection that no longer exists, and it is owner-stamped, so a + * reader would reject it anyway — but leaving it behind means `/hub-state.json` + * keeps naming the previous hub's providers and logins on a machine that is no longer connected + * to anything, which is exactly the wrong artifact to leave where someone might read it. + * + * Best effort and unconditional on the phase: the disconnect has already succeeded by this point, + * and a cache file that cannot be removed must not fail it or block a retry. + */ +function removeHubStateCache(): void { + try { + unlinkSync(hubStateCachePath()); + } catch { /* absent, or not ours to remove */ } +} + export async function revokeConnectedClientKey( credential: { kind: "admin"; value: Uint8Array }, deps: ClientConnectDeps = {}, diff --git a/tests/cli/cli-config-show-client.test.ts b/tests/cli/cli-config-show-client.test.ts index 8b7b0532f2..336c869141 100644 --- a/tests/cli/cli-config-show-client.test.ts +++ b/tests/cli/cli-config-show-client.test.ts @@ -42,8 +42,13 @@ function runCli(args: string[], home: string) { /** 12 KB of base64 stands in for the real thing; the assertion is about the shape, not the size. */ const PRIOR_CATALOG = "A".repeat(12_288); -function clientHome(): string { +/** The data-plane token this fixture's `tokenFingerprint` is computed from. */ +const FIXTURE_TOKEN = "fixture-token"; + +function clientHome(options: { token?: string | null } = {}): string { const home = mkdtempSync(join(tmpdir(), "ocx-config-client-")); + const token = options.token === undefined ? FIXTURE_TOKEN : options.token; + if (token !== null) writeFileSync(join(home, "service-api-token"), token, { mode: 0o600 }); writeFileSync(join(home, "config.json"), JSON.stringify({ port: 10100, providers: {}, @@ -55,7 +60,7 @@ function clientHome(): string { selectedClients: ["codex", "claude"], tokenEnv: "OPENCODEX_API_AUTH_TOKEN", apiKeyId: "client-one", - tokenFingerprint: createHash("sha256").update("fixture-token").digest("hex"), + tokenFingerprint: createHash("sha256").update(FIXTURE_TOKEN).digest("hex"), protocolVersion: 1, connectedAt: "2026-09-01T00:00:00.000Z", priorCatalog: PRIOR_CATALOG, @@ -70,25 +75,64 @@ function standaloneHome(): string { return home; } +/** The shape `collectClientConnectionStatus()` returns, narrowed to what the note reads. */ +type NoteConnection = Parameters[1] extends () => infer T ? T : never; + +function connection(overrides: Partial = {}): NoteConnection { + return { state: "connected", token: "owned", ...overrides } as NoteConnection; +} + +const CLIENT_CONFIG = { + runtimeRole: "client", + client: { serverUrl: "https://hub.example.test:8443" }, +} as OcxConfig; + describe("remoteHubConfigNote", () => { - test("only a client with a connection block gets a note", () => { - expect(remoteHubConfigNote({ runtimeRole: "client" } as OcxConfig)).toBeNull(); - expect(remoteHubConfigNote({ runtimeRole: "standalone" } as OcxConfig)).toBeNull(); - expect(remoteHubConfigNote({ runtimeRole: "hub" } as OcxConfig)).toBeNull(); - expect(remoteHubConfigNote({} as OcxConfig)).toBeNull(); + test("only a client with a connection block gets a note, and nothing is probed otherwise", () => { + // The thunk throws: a standalone or hub install must not pay for the connection probe, and + // the guard has to return before it. + const refuse = (): NoteConnection => { throw new Error("connection must not be probed"); }; + expect(remoteHubConfigNote({ runtimeRole: "client" } as OcxConfig, refuse)).toBeNull(); + expect(remoteHubConfigNote({ runtimeRole: "standalone" } as OcxConfig, refuse)).toBeNull(); + expect(remoteHubConfigNote({ runtimeRole: "hub" } as OcxConfig, refuse)).toBeNull(); + expect(remoteHubConfigNote({} as OcxConfig, refuse)).toBeNull(); }); test("the note names the hub and points at the command that has the facts", () => { - const note = remoteHubConfigNote({ - runtimeRole: "client", - client: { serverUrl: "https://hub.example.test:8443" }, - } as OcxConfig); + const note = remoteHubConfigNote(CLIENT_CONFIG, () => connection()); expect(note).toEqual({ connected: true, origin: "https://hub.example.test:8443", note: "provider credentials and model availability live on the hub; run ocx status", }); }); + + test("connected is observed, not assumed: a revoked or rotated-away token reads false", () => { + // `connected: true` was hardcoded for any config carrying a `client` block. That is the same + // defect in miniature — configuration is not evidence the connection works — and this is the + // case that proves it: the key was revoked or rotated at the hub, the token file this machine + // holds is no longer the one the connection recorded, and nothing here can reach the hub. + for (const token of ["missing", "changed", "unsafe"] as const) { + const note = remoteHubConfigNote(CLIENT_CONFIG, () => connection({ token })); + expect({ token, connected: note?.connected }).toEqual({ token, connected: false }); + expect(note?.note).toContain(`hub data-plane token is ${token}`); + // Still the hub's origin, and still a pointer at the command that can say more. + expect(note?.origin).toBe("https://hub.example.test:8443"); + expect(note?.note).toContain("ocx connect status"); + } + }); + + test("a mismatched or invalid connection record is named rather than called connected", () => { + const mismatched = remoteHubConfigNote(CLIENT_CONFIG, () => connection({ + state: "mismatched", reason: "config.json.client is present without runtimeRole=client", + })); + expect(mismatched?.connected).toBe(false); + expect(mismatched?.note).toContain("its connection is mismatched"); + expect(mismatched?.note).toContain("config.json.client is present without runtimeRole=client"); + const disconnected = remoteHubConfigNote(CLIENT_CONFIG, () => connection({ state: "disconnected", token: "missing" })); + expect(disconnected?.connected).toBe(false); + expect(disconnected?.note).toContain("its connection is disconnected"); + }); }); describe("ocx config show on a client", () => { @@ -146,6 +190,22 @@ describe("ocx config show on a client", () => { } }); + test("a client holding no data-plane token is not reported as connected", () => { + // End to end, because the hardcoded `true` lived at the call site's expense: `ocx config + // show` is what an agent reads, and this is the machine that cannot reach its hub at all. + const home = clientHome({ token: null }); + try { + const result = runCli(["config", "show"], home); + expect(result.status).toBe(0); + const parsed = JSON.parse(result.stdout); + expect(parsed._remoteHub.connected).toBe(false); + expect(parsed._remoteHub.origin).toBe("https://hub.example.test:8443"); + expect(parsed._remoteHub.note).toContain("hub data-plane token is missing"); + } finally { + removeTreeWithRetry(home); + } + }); + test("a standalone machine's output is unannotated", () => { const home = standaloneHome(); try { diff --git a/tests/clients/client-connect.test.ts b/tests/clients/client-connect.test.ts index 3835f463bc..8b724bc438 100644 --- a/tests/clients/client-connect.test.ts +++ b/tests/clients/client-connect.test.ts @@ -344,6 +344,7 @@ function runTransactionScenario( const { connectClient, disconnectClient } = require("./src/client/connect"); const { readClientConnectionState } = require("./src/client/state"); const { serviceApiTokenFilePath } = require("./src/lib/service-secrets"); + const { hubStateCachePath, writeCachedHubState } = require("./src/client/hub-state"); const { DEFAULT_CATALOG_PATH } = require("./src/codex/paths"); const stage = ${JSON.stringify(stage)}; const { setPersistedConfigMutationBeforeCommitForTests } = require("./src/config"); @@ -388,6 +389,15 @@ function runTransactionScenario( }, lifecycleLockDeps: { lockPath: process.env.OPENCODEX_HOME + "/lifecycle.sqlite" } }); } catch (cause) { error = cause instanceof Error ? cause.message : String(cause); } const beforeDisconnect = readClientConnectionState(); + // The hub-state cache is derived from THIS connection; disconnect has to take it with it. + let hubStateCacheBefore = false; + if (beforeDisconnect.kind === "connected") { + writeCachedHubState(beforeDisconnect.value, { + schemaVersion: 1, runtimeRole: "hub", hubVersion: "9.9.9", origin: null, + providers: [], oauth: [], subagentModels: [], truncated: false, claudeCode: { enabled: true }, + }, "2026-08-28T00:00:00.000Z"); + hubStateCacheBefore = existsSync(hubStateCachePath()); + } const artifacts = { token: existsSync(serviceApiTokenFilePath()), catalog: existsSync(DEFAULT_CATALOG_PATH), @@ -396,7 +406,8 @@ function runTransactionScenario( let disconnected = null; if ((stage === "success" || stage === "prior-catalog") && connected) disconnected = await disconnectClient({}, { lifecycleLockDeps: { lockPath: process.env.OPENCODEX_HOME + "/lifecycle.sqlite" } }); const catalogAfter = existsSync(DEFAULT_CATALOG_PATH) ? readFileSync(DEFAULT_CATALOG_PATH, "utf8") : null; - console.log(JSON.stringify({ connected, error, beforeDisconnect, artifacts, disconnected, catalogAfter, after: readClientConnectionState(), calls, commitFaultTriggered })); + const hubStateCacheAfter = existsSync(hubStateCachePath()); + console.log(JSON.stringify({ connected, error, beforeDisconnect, artifacts, disconnected, catalogAfter, hubStateCacheBefore, hubStateCacheAfter, after: readClientConnectionState(), calls, commitFaultTriggered })); })(); `; const cleanup = () => { @@ -521,6 +532,11 @@ describe("connect transaction and offline disconnect", () => { expect(run.parsed.beforeDisconnect).toMatchObject({ kind: "connected", value: { apiKeyId: "issued-id" } }); expect(run.parsed.artifacts).toEqual({ token: true, catalog: true, credentialZeroed: true }); expect(run.parsed.disconnected).toMatchObject({ apiKeyId: "issued-id", tokenRemoved: true, catalogRemoved: true }); + // The cached hub state goes with the connection (#4236). It is owner-stamped, so a reader + // would reject it anyway — but leaving it behind means `hub-state.json` keeps naming the + // former hub's providers and logins on a machine connected to nothing. + expect(run.parsed.hubStateCacheBefore).toBe(true); + expect(run.parsed.hubStateCacheAfter).toBe(false); expect(run.parsed.after).toEqual({ kind: "disconnected" }); expect(run.parsed.calls.filter((call: any) => call.method === "DELETE")).toEqual([]); } finally { run.cleanup(); } From fe65afe78c087271abc3773eaa98a7846248bd76 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:53:22 +0900 Subject: [PATCH 09/10] test(server): pin that /v1/hub-state stays off the unauthenticated loopback listener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decision not to add the route to `loopbackRouteAllowed` had no test behind it. This one starts a real hub with the loopback listener bound, asks it for `GET /v1/hub-state` with no credential, and asserts a 404 whose code is `not_found` — the listener's refusal, not the route's own `hub_state_not_a_hub`, which would have meant the request reached the handler. The same run then reads the route successfully on the public listener with a data key, so a route that disappeared entirely cannot make the first assertion pass vacuously. Co-Authored-By: Claude Fable 5.1 --- .../loopback-listener-integration.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/server/loopback-listener-integration.test.ts b/tests/server/loopback-listener-integration.test.ts index 7411e1559d..3b616d822e 100644 --- a/tests/server/loopback-listener-integration.test.ts +++ b/tests/server/loopback-listener-integration.test.ts @@ -323,6 +323,33 @@ describe("unauthenticated loopback listener", () => { } }); + test("a hub's /v1/hub-state stays off this listener (#4236)", async () => { + // The route is reachable with a data key on the PUBLIC listener, by design. It must not be + // reachable with no credential at all: it names the hub's providers and which of them are + // logged in, and the unauthenticated listener exists for the inference wires a directly + // spawned `codex app-server` speaks — not for discovery. A hub reads its own config. + const loopbackPort = await freePort(); + saveConfig({ ...baseConfig(loopbackPort), runtimeRole: "hub" } as OcxConfig); + const server = await startLoopbackTestServer(loopbackPort); + try { + const viaLoopback = await fetch(`http://127.0.0.1:${loopbackPort}/v1/hub-state`); + expect(viaLoopback.status).toBe(404); + // The LISTENER's 404, not the route's. `hub_state_not_a_hub` would prove the request + // reached the handler and was merely turned away for the role; `not_found` proves the + // allowlist refused it first — and this host IS a hub, so the role gate would have passed. + expect(await viaLoopback.json()).toMatchObject({ error: { code: "not_found" } }); + // And the route genuinely exists on this build and on this host, so the 404 above is the + // allowlist rather than a missing route passing vacuously. + const viaPublic = await fetch(`http://127.0.0.1:${server.port}/v1/hub-state`, { + headers: { "x-opencodex-api-key": "public-secret" }, + }); + expect(viaPublic.status).toBe(200); + expect(await viaPublic.json()).toMatchObject({ runtimeRole: "hub" }); + } finally { + await server.stop(true); + } + }); + test("admits POST /v1/alpha/search so native web search reaches the relay (#3192)", async () => { const loopbackPort = await freePort(); saveConfig(baseConfig(loopbackPort)); From af7c114a9948f6684b4722a81d9247661c29d66b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Fri, 11 Sep 2026 12:53:28 +0900 Subject: [PATCH 10/10] docs(devlog,structure): record the PR6 review round and name hub-state ownership Seven findings, all accepted, with the corrected disclosure delta for `/v1/hub-state` written down in the place the wrong one was: "only the two booleans" became the exact list, and the note that a disabled provider is not exported at all. `structure/01_runtime.md` now names `src/remote/hub-state.ts` and `src/client/hub-state.ts` under remote-hub ownership, including the rule that a failed read reports "unavailable" rather than degrading to local state. Co-Authored-By: Claude Fable 5.1 --- .../060_client_hub_state.md | 136 +++++++++++++++++- structure/01_runtime.md | 2 +- 2 files changed, 131 insertions(+), 7 deletions(-) diff --git a/devlog/_plan/260911_hub_single_port/060_client_hub_state.md b/devlog/_plan/260911_hub_single_port/060_client_hub_state.md index 65c737f556..6c4bf43553 100644 --- a/devlog/_plan/260911_hub_single_port/060_client_hub_state.md +++ b/devlog/_plan/260911_hub_single_port/060_client_hub_state.md @@ -50,9 +50,14 @@ Body: "providers": [{ "name": "", "adapter": "", "authMode": "key|forward|oauth|local|null", "hasCredential": false, "disabled": false }], "oauth": [{ "provider": "", "loggedIn": false }], - "subagentModels": [], "claudeCode": { "enabled": true } } + "subagentModels": [], "truncated": false, "claudeCode": { "enabled": true } } ``` +A provider the operator marked `disabled` is **not exported at all** (review round, below), so +`disabled` is always `false` from a hub of this version; the field stays in the contract because an +older hub does send `true` and a client reading one must still label the row. `truncated` says out +loud that a cap was hit, so a prefix is never presented as the whole list. + `hasCredential` is the `!!p.apiKey` presence projection `GET /api/providers` already ships. `loggedIn` is `oauthLoginSummary`'s boolean with the **email and account id dropped, not masked**. `buildHubState` constructs every row field by field and never spreads a provider or a login @@ -95,7 +100,7 @@ oversized cache files are refused rather than followed. `CliStatusJson` gains `runtimeRole` and an always-present `remoteHub` block — `{ connected, origin, stateSource, reason?, fetchedAt?, ageSeconds?, hubVersion, providers, oauth, -subagentModels, claudeCodeEnabled }`. `schemaVersion` stays 1 (additive, same rule as +subagentModels, truncated, claudeCodeEnabled }`. `schemaVersion` stays 1 (additive, same rule as `versionSkew`), and `connection` is untouched: it describes the **link**, `remoteHub` describes what is on the other end of it. @@ -152,7 +157,9 @@ no hub round trip, so an offline hub cannot stand between an operator and a loca `_remoteHub` is the **first** key on a client: `{ connected, origin, note: "provider credentials and model availability live on the hub; run ocx status" }`. It must be read before the empty `providers` map, not after it. `client.priorCatalog` -prints as ``, mirroring `sanitizeModelCostsForDisplay`. +prints as ``, mirroring `sanitizeModelCostsForDisplay`. `connected` is observed +from `collectClientConnectionStatus()`, not assumed from the presence of a `client` block (review +round, finding 3), and when it is false the note names what is wrong instead. Both are display-only. `config export` emits the real config untouched, so round trips still validate; a persisted `client.note` was rejected because `clientConnectionSchema` is `.strict()` @@ -165,9 +172,12 @@ and persisted prose drifts. forwards whatever hub key the browser supplies — so it is not a CLI channel. Ingwannu's review note forbids adding `/api/*` to the unauthenticated listener or copying an admin credential into exported client configuration, and this does neither. -- **Booleans only, forever.** Provider *names* already leak through `/v1/catalog` slugs, so the - delta this route adds is `hasCredential` and `loggedIn`. Emails, account ids, quotas and usage - must never be added: a data key opens this. +- **Booleans only, forever.** Emails, account ids, quotas and usage must never be added: a data + key opens this. The disclosure delta over `/v1/catalog` and `/v1/models`, stated exactly (the + first draft said "only two booleans", which was wrong — see the review round): `hasCredential`, + `loggedIn`, `authMode`, the featured roster, and the NAME and adapter of an **enabled** provider + those routes omit for want of a usable credential. That last one is the point of the route, and + it is the whole widening. A `disabled` provider is not exported at all. - **`authMode` is included** even though it was not in the original sketch. Without it `hasCredential: false` on an OAuth provider reads as "not configured" — the precise inference that went wrong. It is shape, not secret. @@ -245,6 +255,120 @@ disconnect` and `launchctl` were **not** run, and the real `~/.opencodex`, `~/.c `OPENCODEX_HOME` to a `mkdtemp` directory; `tests/preload.ts` arms `OCX_TEST_HOME_GUARD=1` for every invocation including a bare `bun test `. +## Review round (PR #4255) + +Seven findings, all accepted. The two that mattered are the same mistake the PR itself is about, +committed by the PR: a boundary comment that claimed less disclosure than the code performed, and a +`connected: true` inferred from configuration rather than observed. The rest are honesty gaps — +a reason string that printed a raw error code, a silent truncation, a cache outliving its +connection, an untested 404, and two docs claims. + +### 1. The projection exported every provider row, and three comments said otherwise (should-fix) + +`buildHubState` mapped **all** of `config.providers`, including rows with `disabled: true`. +`/v1/catalog` and `/v1/models` both filter a disabled provider out (`src/router.ts:490`, +`src/codex/catalog/provider-fetch.ts:512`), so this route was the only data-plane surface that +named one — while `src/remote/hub-state.ts`, the route comment in `src/server/index.ts` and this +devlog all asserted the delta over `/v1/catalog` was "only the two booleans". A wrong boundary +claim is worse than no claim: it is what a future reviewer checks the code against. + +Fixed on both sides. `buildHubState` drops a disabled provider entirely — a client cannot route to +it, so absence is the truthful report, and `authMode` already explains a present-but-keyless row +without it. The three comments and the devlog now state the delta exactly: `hasCredential`, +`loggedIn`, `authMode`, the featured roster, and the **name and adapter of an enabled provider the +catalog omits for want of a usable credential**. That last item is the point of the route and the +whole widening. + +`HubStateProvider.disabled` stays in the contract, always `false` from a hub of this version. An +older hub does send `true`, and a client reading one must still be able to label the row rather +than present it as routable; dropping the field would have made a new client quietly promote an +old hub's disabled providers. + +### 2. `hub_state_http_` and the content-type code printed as bare codes (should-fix) + +`fetchHubState` throws `hub_state_content_type_invalid` and `hub_state_http_` +(`src/client/hub-client.ts:497,502`); `hubStateFailureReason` had a case for neither, so the +`ocx status` banner could read `state unavailable (hub_state_http_507)`. That sends an operator +hunting for a client bug when the hub has in fact answered and said something — 507 is the hub's +own `hub_state_too_large`. + +Both render as sentences now: "the hub's state response was not JSON" (captive portal, TLS +terminator, error page) and "the hub answered HTTP N to the state request". The prefix branch +validates the suffix is numeric, so a non-numeric code still falls back to the code rather than +printing `HTTP oops`. + +### 3. `_remoteHub.connected` was hardcoded `true` (should-fix) + +Any config with `runtimeRole: "client"` and a `client` block got `connected: true` — including a +machine whose data key was revoked at the hub, rotated away, or whose token file was deleted. The +presence of configuration is not evidence the connection works, which is #4236 in miniature. + +`remoteHubConfigNote` now takes the connection status and requires both halves — +`state === "connected"` **and** `token === "owned"`, the comparison of the token file's +fingerprint against the connection record that only `collectClientConnectionStatus()` performs. +When either fails the note says which (`…its hub data-plane token is missing; run ocx connect +status`) instead of claiming a working link. The parameter is a thunk, so the guard returns first +on a standalone or hub install and the probe never runs; the call site imports `./connect` +dynamically so `ocx config get/set` does not drag the client lifecycle in. + +### 4. `.slice(0, MAX_HUB_STATE_PROVIDERS)` truncated silently (nit) + +A hub with 240 providers served 200 and said nothing, so a client would have told its reader the +other 40 do not exist. `truncated: boolean` is now part of the contract, set when the provider, +oauth or roster cap is hit, and `remoteHubStatusLines` appends "(the hub truncated this state to +fit its response caps; some rows are not listed)". The parser treats an absent `truncated` as +`false` (an older hub sends no such key) but still refuses a present non-boolean. + +### 5. The hub-state cache outlived the connection (nit) + +`disconnectClient` removed the token, the catalog and the connection record but left +`/hub-state.json` naming the former hub's providers and logins. It is owner-stamped +so a reader would reject it, but it is the wrong artifact to leave where someone might read it. +Unlinked after `connection_cleared`, best effort: the disconnect has already succeeded by then and +a stubborn cache file must not fail it or block a retry. + +### 6. The loopback 404 was asserted nowhere, and the PR body named the wrong file (nit) + +The decision "no `loopbackRouteAllowed` entry" had no test. Now +`tests/server/loopback-listener-integration.test.ts` starts a real hub with the unauthenticated +loopback listener, asks it for `GET /v1/hub-state`, and asserts a 404 whose code is `not_found` — +the **listener's** refusal, not the route's `hub_state_not_a_hub`, which would have proved the +request reached the handler. The same run then reads the route successfully on the public listener +with a data key, so the 404 cannot pass vacuously through a missing route. + +The PR body claimed `v1-hub-state.test.ts` pins the standalone 404. It does pin a standalone 404 of +its own, but the admission-matrix proof lives in `tests/server/api-key-attribution.test.ts`; +the body now says so. + +### 7. `structure/01_runtime.md` did not mention hub-state (nit) + +"Remote Hub hardening ownership" named `src/remote/protocol.ts`, `src/client/hub-client.ts` and +`src/client/hub-relay.ts`. It now also names `src/remote/hub-state.ts` (contract, caps, shared +parser) and `src/client/hub-state.ts` (resolution, owner-stamped 0600 cache, and the rule that a +failed read reports "unavailable" rather than degrading to local state). + +### Verification (review round, this machine) + +``` +bun run typecheck # clean +bun run privacy:scan # Privacy scan passed +bun test tests/server/v1-hub-state.test.ts # 9 pass (was 8) +bun test tests/clients/client-hub-state.test.ts # 26 pass (was 20) +bun test tests/cli/cli-config-show-client.test.ts # 9 pass (was 6) +bun test tests/cli/cli-status-hub-state.test.ts # 13 pass (was 12) +bun test tests/cli/cli-status-json.test.ts # 54 pass +bun test tests/server/api-key-attribution.test.ts # 25 pass +bun test tests/server/loopback-listener-admission.test.ts # 31 pass +bun test tests/server/loopback-listener-integration.test.ts # 36 pass (was 35) +bun test tests/clients/client-connect.test.ts # 49 pass (cache-removal assertion) +bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts # 17 pass +``` + +No new test files, so `layout.json` and `tests/fixtures/test-layout-expected.json` are unchanged. +No repository-wide suite (operator instruction); no `ocx service …`, `ocx start/stop/ensure/sync/ +connect/disconnect/status` or `launchctl` was run, and every test kept `OPENCODEX_HOME` in a +`mkdtemp` directory. + ## Left undone - **ko docs.** The paragraph landed in `docs-site/src/content/docs/guides/remote-hub.md` (en) diff --git a/structure/01_runtime.md b/structure/01_runtime.md index 32cf8036c8..36c9a282be 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -202,4 +202,4 @@ not an authentication or entitlement decision. ## Remote Hub hardening ownership -`src/remote/protocol.ts` owns pure interval/feature negotiation. `src/client/hub-client.ts` owns bounded, schema-validated remote catalog consumption and key-id probes. `src/client/hub-relay.ts` is a fixed-authority management relay with URL, header, body, redirect, and stream bounds. The public data listener remains the direct client→hub path; the loopback management ingress never serves data-plane routes. +`src/remote/protocol.ts` owns pure interval/feature negotiation. `src/remote/hub-state.ts` owns the `GET|HEAD /v1/hub-state` contract, its caps, and the parser both sides share. `src/client/hub-client.ts` owns bounded, schema-validated remote catalog consumption, hub-state reads, and key-id probes; `src/client/hub-state.ts` owns the resolution and the owner-stamped 0600 cache, and a failed read reports "unavailable" rather than degrading to the client's own local provider and login state. `src/client/hub-relay.ts` is a fixed-authority management relay with URL, header, body, redirect, and stream bounds. The public data listener remains the direct client→hub path; the loopback management ingress never serves data-plane routes.