From 51b1fbd3841c6a04cf3407eaa8dcaad1ff1025a8 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 31 Aug 2026 14:04:00 +0900 Subject: [PATCH 1/2] fix(integrations): resolve both Aside paths on the direct writer path too The registry seam covered the coordinated writer and the state read, but applyIntegration, refreshIntegration and disableIntegration are public and callers may omit resolvedPaths. On that path preflight resolved configPath while the installation check resolved detectDir separately, so an Aside account switch landing between them produced a successful apply that verified one account's install and wrote the other account's catalog. Preflight now resolves the pair through resolveIntegrationPaths and hands detectDir down, so the directory whose existence authorizes a write is the directory the write lands in. The regression test asserts exactly that property rather than a proxy for it: it injects an account switch at the IO seam, then checks that the account whose catalog gained providers.opencodex is one whose directory was statted. Driven red against the split resolution before the fix. Also drops the statSync import left behind when the mtime cache went away. --- src/clients/config-export.ts | 2 +- src/integrations/writer.ts | 22 ++++++++++--- tests/aside-client.test.ts | 60 +++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/src/clients/config-export.ts b/src/clients/config-export.ts index 835437305c..1ed156c425 100644 --- a/src/clients/config-export.ts +++ b/src/clients/config-export.ts @@ -20,7 +20,7 @@ * targeting it is the caller's explicit act. */ import { homedir } from "node:os"; -import { existsSync, readFileSync, statSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import { isAbsolute, join, resolve } from "node:path"; import { shouldInjectApiAuthHeader } from "../codex/inject"; import { FORMAT_MEDIA_TYPE, serializeDocument, type ConfigFormat } from "../integrations/serialize"; diff --git a/src/integrations/writer.ts b/src/integrations/writer.ts index b2769eac13..831fa80c21 100644 --- a/src/integrations/writer.ts +++ b/src/integrations/writer.ts @@ -210,8 +210,20 @@ function preflight(input: IntegrationWriteInput) { * whole Integrations page because one client is misconfigured. */ let configPath: string; + let detectDir: string; try { - configPath = input.resolvedPaths?.configPath ?? spec.configPath(input.env, input.home); + /* + * Resolve the PAIR, never one half. + * + * The coordinated path hands us a frozen pair, but applyIntegration, + * refreshIntegration and disableIntegration are public and may be called + * without one. Resolving configPath here and detectDir separately later let + * an Aside account switch land between the two, so a direct apply could + * verify account 1 was installed and then write account 0's catalog. + */ + const resolved = input.resolvedPaths ?? resolveIntegrationPaths(clientId, input.env, input.home); + configPath = resolved.configPath; + detectDir = resolved.detectDir; } catch (error) { if (!(error instanceof ClientPathError)) throw error; return { failed: refuse(clientId, "unsafe", "unsafe", error.message) } as const; @@ -248,15 +260,17 @@ function preflight(input: IntegrationWriteInput) { const classified = classifyIntegration({ fileText: before, fileIsRegular: true, parsed, record, contribution, configPath, clientId, }); - return { failed: undefined, store, io, clientId, spec, exportSpec, configPath, before, parsed, contribution, record, classified } as const; + return { failed: undefined, store, io, clientId, spec, exportSpec, configPath, detectDir, before, parsed, contribution, record, classified } as const; } function applyOrRefreshIntegration(input: IntegrationWriteInput, allowAbsent: boolean): WriteOutcome { const pre = preflight(input); if (pre.failed) return pre.failed; - const { store, io, clientId, spec, exportSpec, configPath, before, parsed, contribution, record, classified } = pre; + const { store, io, clientId, spec, exportSpec, configPath, detectDir, before, parsed, contribution, record, classified } = pre; - if (io.statKind(input.resolvedPaths?.detectDir ?? spec.detectDir(input.env, input.home)) !== "dir") { + // The detect directory preflight already resolved, so it cannot name a + // different account than the config path this operation is about to write. + if (io.statKind(detectDir) !== "dir") { return refuse(clientId, "not_installed", "absent", `${clientId} is not installed`); } if (isLoopbackOnly(clientId) && !isLoopbackHostname(input.config.hostname)) { diff --git a/tests/aside-client.test.ts b/tests/aside-client.test.ts index 771106966a..b1433d2049 100644 --- a/tests/aside-client.test.ts +++ b/tests/aside-client.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -17,6 +17,9 @@ import { } from "../src/clients/config-export"; import { INTEGRATION_CLIENTS, resolveIntegrationPaths, unresolvedPathHintFor } from "../src/integrations/registry"; import { readIntegrationState } from "../src/integrations/state"; +import { createIntegrationStateStore } from "../src/integrations/store"; +import { defaultIntegrationIO } from "../src/integrations/config-io"; +import { applyIntegration } from "../src/integrations/writer"; import type { OcxConfig } from "../src/types"; const CONFIG = { @@ -174,6 +177,61 @@ describe("Aside client config", () => { expect(paths.detectDir).toBe(join("/home/u", ".prime", "agent")); }); + /* + * The direct writer path, which is the one that stayed broken after the seam + * landed. applyIntegration is public and callers may omit resolvedPaths, so + * preflight used to resolve configPath while the installation check resolved + * detectDir separately. An account switch landing between the two produced a + * successful apply that verified one account and wrote the other's catalog. + * + * The IO seam is where the switch is injected, because that is the moment + * between the two resolutions in the original ordering. + */ + test("a direct apply checks the install of the very account it writes", () => { + writeManifest(JSON.stringify({ currentAccountId: 0 })); + const manifest = join(home, ".aside", "accounts.json"); + writeFileSync(join(home, ".aside", "u", "0", "models.json"), "{}\n"); + mkdirSync(join(home, ".aside", "u", "1"), { recursive: true }); + + const store = createIntegrationStateStore(mkdtempSync(join(tmpdir(), "ocx-aside-store-"))); + const io = defaultIntegrationIO(store); + const statted: string[] = []; + const switching = { + ...io, + statKind: (path: string) => { + statted.push(path); + // Aside switches accounts exactly where the second resolution used to be. + writeFileSync(manifest, JSON.stringify({ currentAccountId: 1 })); + return io.statKind(path); + }, + }; + + const applied = applyIntegration({ + clientId: "aside", models: [], config: CONFIG, port: 10100, + env: {}, home, store, io: switching, + }); + expect(applied.ok).toBe(true); + + /* + * The property that was violated: the account directory whose existence + * authorized the write must be the account the write landed in. With the two + * paths resolved separately, the install check statted u/1 while the catalog + * was written to u/0 -- an apply authorized by an account it never touched. + */ + const accountDirs = statted.filter(path => /[\\/]u[\\/]\d+$/.test(path)); + expect(accountDirs.length).toBeGreaterThan(0); + const authorized = new Set(accountDirs); + + const owning = ([0, 1] as const).filter(account => { + const catalog = join(home, ".aside", "u", String(account), "models.json"); + if (!existsSync(catalog)) return false; + const parsed = JSON.parse(readFileSync(catalog, "utf8")) as { providers?: Record }; + return parsed.providers?.opencodex !== undefined; + }); + expect(owning).toHaveLength(1); + expect(authorized.has(join(home, ".aside", "u", String(owning[0])))).toBe(true); + }); + test("detects installation by the account directory, not the CLI directory", () => { // The CLI writes ~/.aside/cli for its own update check before any account // exists, so the outer directory is not an install signal. From 0d99ee67206047637040598cd458a6141e3dafb1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 31 Aug 2026 14:04:15 +0900 Subject: [PATCH 2/2] feat(gui): surface Aside on the Integrations page Registers Aside across the GUI so its card, tab and toggle render like any other file client: the CLIENTS tuple, FILE_INTEGRATION_CLIENTS, the tab hash, and the three exhaustive label maps. TABS and FILE_CLIENTS move to integrations/integration-tabs.ts. They are the only client lists in the GUI that neither the registry invariant compares nor the compiler forces, so a client added everywhere else still gets no tab and every gate stays green. They now live in a module a test can import, because exporting them from Integrations.tsx alongside the component breaks fast refresh. gui/tests/integrations-tab-coverage.test.ts derives its expectation from FILE_INTEGRATION_CLIENTS rather than restating a literal, so client thirteen cannot pass by accident. Nine locales gain the tab label, the client label, and the ownership semantics. "Aside" is a product name and stays English everywhere, so both labels join the intentional-English allowlists. The semantics string carries one Aside-specific clause: the running app rewrites models.json itself, so it has to be fully quit and reopened after applying, the same shape as the Claude Desktop restart copy. Docs pick up the Aside destination and guide rows, plus the ZCode guide row that was missing while the CLI reference already documented it. Verification: bun run typecheck and bun run lint:gui clean; gui focused tests (tab coverage, api, client-config panel, overview rows, surfaces, locale parity, fr localization) 55 pass / 0 fail. Full suite deferred to CI. --- .../src/content/docs/guides/integrations.md | 17 ++++- .../src/content/docs/reference/cli/agents.md | 5 +- gui/src/pages/Integrations.tsx | 50 +-------------- .../pages/integrations/integration-tabs.ts | 62 +++++++++++++++++++ gui/tests/integrations-tab-coverage.test.ts | 39 ++++++++++++ 5 files changed, 121 insertions(+), 52 deletions(-) create mode 100644 gui/src/pages/integrations/integration-tabs.ts create mode 100644 gui/tests/integrations-tab-coverage.test.ts diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index fc165907a1..8e5957b4c1 100644 --- a/docs-site/src/content/docs/guides/integrations.md +++ b/docs-site/src/content/docs/guides/integrations.md @@ -1,10 +1,10 @@ --- title: Integrations -description: Connect opencodex to OpenCode, Pi, OMP, Hermes, OpenClaw, Kimi Code, Gajae Code, DeepSeek Harness, MiniMax Code and Prime Agent from the dashboard — one switch per client, with a backup taken before every write. +description: Connect opencodex to OpenCode, Pi, OMP, Hermes, OpenClaw, Kimi Code, Gajae Code, DeepSeek Harness, MiniMax Code, ZCode, Prime Agent and Aside from the dashboard — one switch per client, with a backup taken before every write. --- The **Integrations** tab writes opencodex's provider block into a client's own config -file, and removes it again. Ten clients work this way, each with a switch: +file, and removes it again. Twelve clients work this way, each with a switch: | Client | Config file | Format | When the change takes effect | Credential | |---|---|---|---|---| @@ -18,6 +18,8 @@ file, and removes it again. Ten clients work this way, each with a switch: | DeepSeek Harness (DSH) | `$DSH_HOME/settings.yaml` (default `~/.dsh/settings.yaml`) | YAML | hot reload | non-secret loopback bearer placeholder | | MiniMax Code | `~/.minimax/config.yaml` | YAML | new sessions, or after opening the model picker | loopback placeholder | | Prime Agent | `~/.prime/agent/models.json` | JSON | new sessions | loopback placeholder | +| ZCode | `~/.zcode/v2/config.json` | JSON | on restart | loopback placeholder | +| Aside | `~/.aside/u//models.json` | JSON | after fully quitting and reopening Aside | loopback placeholder | The managed OpenCode integration owns two fragments: `provider.opencodex` (opencode V1) and `providers.opencodex` (opencode V2). Only the V2 block carries the per-model reasoning-effort @@ -46,6 +48,17 @@ disagree about which file is meant. Its managed block owns only stay untouched. Prime Agent reads `models.json` when a session starts, so start a new session after connecting it. +Aside is per-account: its state lives under `~/.aside/u//` and opencodex +writes the catalog of whichever account Aside's own `accounts.json` names as +current. If that manifest is missing or unreadable the integration refuses rather +than guessing an account, because a guess on a multi-account machine would write +into a different account's catalog. Its managed block owns only +`providers.opencodex`, so your other Aside providers stay untouched. + +One caveat specific to Aside: the running app rewrites `models.json` itself, so +fully quit and reopen Aside after applying, the same way Claude Desktop needs a +restart. Aside's block is loopback-only and never carries a real credential. + Paths honor each client's own environment override where it has one. For OMP, `OMP_PROFILE` wins over `PI_PROFILE` by presence, even when explicitly empty. A named profile uses `PI_CONFIG_DIR` as a directory name relative to the user's home and ignores `PI_CODING_AGENT_DIR`; without a named profile, diff --git a/docs-site/src/content/docs/reference/cli/agents.md b/docs-site/src/content/docs/reference/cli/agents.md index ea4e6a720b..e6470eae0e 100644 --- a/docs-site/src/content/docs/reference/cli/agents.md +++ b/docs-site/src/content/docs/reference/cli/agents.md @@ -207,7 +207,7 @@ Manage and apply the Grok Build model fence. ## Client config export -### `ocx export --client ` +### `ocx export --client ` Print a client config wired to the running proxy. The command serializes the `opencodex` provider block — base URL, model list, and the client's credential @@ -218,7 +218,7 @@ models Codex can currently see. | Flag | Action | | --- | --- | -| `--client ` | Required. Selects the client config dialect. | +| `--client ` | Required. Selects the client config dialect. | | `--json` | Print the generated document as JSON on stdout for scripts. This is JSON even when the selected client's native format is YAML, TOML, or JSON5. | | `--out ` | Write the client's native config format to ``. Refuses to replace an existing file. | | `--force` | Allow `--out` to replace an existing file. | @@ -247,6 +247,7 @@ client applies its own defaults for those). | `mcode` | `~/.minimax/config.yaml` (`MINIMAX_DATA_DIR`, then the legacy `MAVIS_DATA_DIR`, win when set; a relative value is refused) | `mcode-config.yaml` | none — loopback placeholder | | `zcode` | `~/.zcode/v2/config.json` (`ZCODE_DATA_DIR` wins when set; a relative value is refused) | `config.json` | none — loopback placeholder | | `prime` | `~/.prime/agent/models.json` (`PRIME_AGENT_CODING_AGENT_DIR` wins when set; a relative value is refused) | `prime-models.json` | none — loopback placeholder | +| `aside` | `~/.aside/u//models.json` for the account Aside's own `accounts.json` names as current; an unreadable manifest is refused rather than defaulting to an account | `aside-models.json` | none — loopback placeholder | The managed DSH export requires DSH 0.1.0-rc.6 or newer and owns only `llm-pi-ai.providers.opencodex`. DSH hot reloads that provider; the user's default model and diff --git a/gui/src/pages/Integrations.tsx b/gui/src/pages/Integrations.tsx index 70018037b9..f363173e18 100644 --- a/gui/src/pages/Integrations.tsx +++ b/gui/src/pages/Integrations.tsx @@ -1,6 +1,6 @@ import { useEffect, useRef, useState, type KeyboardEvent } from "react"; import { navigateHash, normalizeHashPath } from "../hash-routing"; -import { useT, type TKey } from "../i18n/shared"; +import { useT } from "../i18n/shared"; import ApiKeys from "./ApiKeys"; import Claude from "./Claude"; import Grok from "./Grok"; @@ -8,53 +8,7 @@ import IntegrationsOverview from "./integrations/IntegrationsOverview"; import FileIntegrationPage, { type FileIntegrationClientId, } from "./integrations/FileIntegrationPage"; - -type IntegrationTab = - | "overview" - | "keys" - | "codex" - | "claude" - | "grok" - | FileIntegrationClientId; - -interface TabDefinition { - id: IntegrationTab; - hash: string; - labelKey: TKey; -} - -const TABS: readonly TabDefinition[] = [ - { id: "overview", hash: "integrations", labelKey: "integrations.tab.overview" }, - { id: "keys", hash: "integrations/keys", labelKey: "integrations.tab.keys" }, - { id: "codex", hash: "integrations/codex", labelKey: "integrations.tab.codex" }, - { id: "claude", hash: "integrations/claude", labelKey: "integrations.tab.claude" }, - { id: "grok", hash: "integrations/grok", labelKey: "integrations.tab.grok" }, - { id: "opencode", hash: "integrations/opencode", labelKey: "integrations.tab.opencode" }, - { id: "pi", hash: "integrations/pi", labelKey: "integrations.tab.pi" }, - { id: "omp", hash: "integrations/omp", labelKey: "integrations.tab.omp" }, - { id: "hermes", hash: "integrations/hermes", labelKey: "integrations.tab.hermes" }, - { id: "openclaw", hash: "integrations/openclaw", labelKey: "integrations.tab.openclaw" }, - { id: "kimi", hash: "integrations/kimi", labelKey: "integrations.tab.kimi" }, - { id: "gajae", hash: "integrations/gajae", labelKey: "integrations.tab.gajae" }, - { id: "dsh", hash: "integrations/dsh", labelKey: "integrations.tab.dsh" }, - { id: "mcode", hash: "integrations/mcode", labelKey: "integrations.tab.mcode" }, - { id: "zcode", hash: "integrations/zcode", labelKey: "integrations.tab.zcode" }, - { id: "prime", hash: "integrations/prime", labelKey: "integrations.tab.prime" }, -] as const; - -const FILE_CLIENTS = new Set([ - "opencode", - "pi", - "omp", - "hermes", - "openclaw", - "kimi", - "gajae", - "dsh", - "mcode", - "zcode", - "prime", -]); +import { FILE_CLIENTS, TABS, type IntegrationTab } from "./integrations/integration-tabs"; function readIntegrationTab(hash = window.location.hash): IntegrationTab { const raw = normalizeHashPath(hash); diff --git a/gui/src/pages/integrations/integration-tabs.ts b/gui/src/pages/integrations/integration-tabs.ts new file mode 100644 index 0000000000..d9dc11346c --- /dev/null +++ b/gui/src/pages/integrations/integration-tabs.ts @@ -0,0 +1,62 @@ +/** + * The Integrations tab strip and the set of tabs backed by a file client. + * + * A separate module rather than exports on Integrations.tsx, because a file that + * exports both a component and constants breaks React fast refresh + * (react/only-export-components). These need to be importable: they are the only + * client lists in the GUI that neither tests/integrations-invariants.test.ts + * compares nor the compiler forces, so a client added everywhere else still gets + * no tab and nothing fails. gui/tests/integrations-tab-coverage.test.ts stands in + * that gap and reads them from here. + */ +import type { TKey } from "../../i18n/shared"; +import type { FileIntegrationClientId } from "./FileIntegrationPage"; + +export type IntegrationTab = + | "overview" + | "keys" + | "codex" + | "claude" + | "grok" + | FileIntegrationClientId; + +export interface TabDefinition { + id: IntegrationTab; + hash: string; + labelKey: TKey; +} + +export const TABS: readonly TabDefinition[] = [ + { id: "overview", hash: "integrations", labelKey: "integrations.tab.overview" }, + { id: "keys", hash: "integrations/keys", labelKey: "integrations.tab.keys" }, + { id: "codex", hash: "integrations/codex", labelKey: "integrations.tab.codex" }, + { id: "claude", hash: "integrations/claude", labelKey: "integrations.tab.claude" }, + { id: "grok", hash: "integrations/grok", labelKey: "integrations.tab.grok" }, + { id: "opencode", hash: "integrations/opencode", labelKey: "integrations.tab.opencode" }, + { id: "pi", hash: "integrations/pi", labelKey: "integrations.tab.pi" }, + { id: "omp", hash: "integrations/omp", labelKey: "integrations.tab.omp" }, + { id: "hermes", hash: "integrations/hermes", labelKey: "integrations.tab.hermes" }, + { id: "openclaw", hash: "integrations/openclaw", labelKey: "integrations.tab.openclaw" }, + { id: "kimi", hash: "integrations/kimi", labelKey: "integrations.tab.kimi" }, + { id: "gajae", hash: "integrations/gajae", labelKey: "integrations.tab.gajae" }, + { id: "dsh", hash: "integrations/dsh", labelKey: "integrations.tab.dsh" }, + { id: "mcode", hash: "integrations/mcode", labelKey: "integrations.tab.mcode" }, + { id: "zcode", hash: "integrations/zcode", labelKey: "integrations.tab.zcode" }, + { id: "prime", hash: "integrations/prime", labelKey: "integrations.tab.prime" }, + { id: "aside", hash: "integrations/aside", labelKey: "integrations.tab.aside" }, +] as const; + +export const FILE_CLIENTS = new Set([ + "opencode", + "pi", + "omp", + "hermes", + "openclaw", + "kimi", + "gajae", + "dsh", + "mcode", + "zcode", + "prime", + "aside", +]); diff --git a/gui/tests/integrations-tab-coverage.test.ts b/gui/tests/integrations-tab-coverage.test.ts new file mode 100644 index 0000000000..91f24af2b9 --- /dev/null +++ b/gui/tests/integrations-tab-coverage.test.ts @@ -0,0 +1,39 @@ +import { expect, test } from "bun:test"; +import { FILE_CLIENTS, TABS } from "../src/pages/integrations/integration-tabs"; +import { FILE_INTEGRATION_CLIENTS } from "../src/pages/integrations/integration-api"; +import { INTEGRATION_TAB_HASHES } from "../src/app-routing"; + +/* + * The gap this closes. + * + * tests/integrations-invariants.test.ts compares five client lists, and the + * per-page label maps are Record so the compiler + * forces those. TABS and FILE_CLIENTS are neither: they are a plain array and a + * plain Set, so a client added everywhere else still gets no tab and nothing + * fails. Aside was the twelfth client to walk this path, and the first with a + * test standing in it. + * + * The expectation is DERIVED rather than written out, so adding client thirteen + * cannot leave a stale literal here that passes by accident. + */ +test("every file client has a tab definition and is registered as a file client", () => { + const tabbed = new Set(TABS.map(tab => tab.id)); + const missingTab = FILE_INTEGRATION_CLIENTS.filter(id => !tabbed.has(id)); + expect(missingTab).toEqual([]); + + const missingFileClient = FILE_INTEGRATION_CLIENTS.filter(id => !FILE_CLIENTS.has(id)); + expect(missingFileClient).toEqual([]); +}); + +test("every tab hash is routable, so a tab can actually be reached", () => { + // App normalization strips an unregistered hash, which would render the + // overview instead of the tab and look like a missing client. + const routable = new Set(INTEGRATION_TAB_HASHES); + const unroutable = TABS.filter(tab => tab.hash !== "integrations" && !routable.has(tab.hash)); + expect(unroutable.map(tab => tab.hash)).toEqual([]); +}); + +test("FILE_CLIENTS carries no id the API does not know", () => { + const known = new Set(FILE_INTEGRATION_CLIENTS); + expect([...FILE_CLIENTS].filter(id => !known.has(id))).toEqual([]); +});