diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index 78e83640a67..a3726b418dd 100644 --- a/docs-site/src/content/docs/guides/integrations.md +++ b/docs-site/src/content/docs/guides/integrations.md @@ -83,6 +83,9 @@ The generated API key is a non-secret placeholder: Qoder integration requires lo access and refuses an authenticated non-loopback bind. When the dedicated unauthenticated loopback listener is enabled, exports point Qoder at that listener. `ocx sync` and catalog changes refresh an already enabled Qoder integration like the other managed clients. +If a catalog model ID contains `,` or `]` (for example `kimi/k3[1m]`), Qoder reports +that its configuration cannot be verified; other integration statuses remain available. +Managed preview and changes remain refused while the catalog contains that model ID. Generated catalogs include only enabled models from each provider selection. This applies to both downloads and managed integrations, including Pi and Aside. The management model list still shows diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 5352e76d754..6f4319f9ab6 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1200,7 +1200,7 @@ "management-inference-port.test.ts": "server", "management-google-tool-schema-policy.test.ts": "server", "management-integration-journal-delete.test.ts": "server", - "management-integration-routes.test.ts": "server", + "management-integration-routes.test.ts": "server", "management-qoder-selector-state.test.ts": "server", "management-metrics-export.test.ts": "server", "management-model-roster-gather-race.test.ts": "server", "management-model-roster.test.ts": "server", diff --git a/src/clients/config-export/qoder.ts b/src/clients/config-export/qoder.ts index 694c264a18d..b0472477736 100644 --- a/src/clients/config-export/qoder.ts +++ b/src/clients/config-export/qoder.ts @@ -1,7 +1,7 @@ import { homedir } from "node:os"; import { isAbsolute, join } from "node:path"; import { isLoopbackHostname, shouldInjectApiAuthHeader } from "../../codex/loopback-target"; -import { formatSelectorConjunction } from "../../integrations/merge"; +import { formatSelectorConjunction, UnrepresentableSelectorError } from "../../integrations/merge"; import { ClientPathError } from "../config-export"; import type { ExportContext, ManagedContribution, OpencodeLaunchEnv } from "./contracts"; import { LOOPBACK_API_KEY_PLACEHOLDER, OPENCODE_PROVIDER_ID } from "./constants"; @@ -85,7 +85,7 @@ export function buildQoderContribution(ctx: ExportContext): ManagedContribution { field: "provider", value: OPENCODE_PROVIDER_ID }, { field: "model", value: model.model }, ]); - if (selector === null) throw new Error("Qoder model ID cannot be represented by a managed array selector."); + if (selector === null) throw new UnrepresentableSelectorError("Qoder model ID cannot be represented by a managed array selector."); return { path: ["modelConfigs", "customModels", selector], value: model }; }), ], diff --git a/src/integrations/merge.ts b/src/integrations/merge.ts index 73eefe41468..4cb7bfab5b9 100644 --- a/src/integrations/merge.ts +++ b/src/integrations/merge.ts @@ -62,6 +62,14 @@ export class InvalidSelectorError extends Error { } } +/** A generated contribution cannot name its exact owned array element. */ +export class UnrepresentableSelectorError extends Error { + constructor(message: string) { + super(message); + this.name = "UnrepresentableSelectorError"; + } +} + function validConjunctionCriteria(criteria: readonly SelectorCriterion[]): boolean { return criteria.length >= 2 && criteria.every(criterion => ( SELECTOR_FIELD.test(criterion.field) diff --git a/src/integrations/state.ts b/src/integrations/state.ts index a8c0ef60596..8f1fa4da424 100644 --- a/src/integrations/state.ts +++ b/src/integrations/state.ts @@ -18,6 +18,7 @@ import { SNAPSHOT_RETENTION } from "./journal"; import { AmbiguousSelectorError, InvalidSelectorError, + UnrepresentableSelectorError, parseSegment, readPath, selectIndex, @@ -678,7 +679,18 @@ export function readIntegrationState(input: IntegrationStateInput): IntegrationS const parsed = input.clientId === "cline" ? parseClineDocument(loaded.before) : parseConfig(loaded.before, effective.format, EXPORT_CLIENTS[input.clientId].jsonc ? { jsonc: true } : undefined); - const contribution = buildIntegrationContribution(input, effective, parsed, record); + let contribution: ManagedContribution; + try { + contribution = buildIntegrationContribution(input, effective, parsed, record); + } catch (error) { + // One catalog row can make this client's ownership unaddressable. Keep its + // refusal visible without hiding every other client in a collection read. + if (!(error instanceof UnrepresentableSelectorError)) throw error; + return { clientId: input.clientId, state: "unsafe", installed, configPath, + reason: "unresolvable-path", + ...(record?.configPath === configPath ? { appliedAt: record.appliedAt, lastOpId: record.opId } : {}), + ...retention }; + } if (input.clientId === "droid" && input.models.length > 0 && contribution.fragments.length === 0 && (!record || record.configPath !== configPath)) { return { clientId: input.clientId, state: "unsafe", installed, configPath, diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index 87fc8f20a8f..ea023d8aa7b 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -125,6 +125,11 @@ client registry + export context -> ownership record + operation journal ``` +A generated contribution that raises `UnrepresentableSelectorError` produces an `unsafe` / +`unresolvable-path` status for that client. The shared reader catches only that typed refusal; +preview and mutation retain their contribution-construction refusal. Other clients in the +status collection remain visible, and unexpected construction errors still propagate. + Status and mutation must use the same classifier. A special case added only to a status endpoint would be misleading because refresh or disable could still reject the same file; a special case added only to a writer would let a mutation bypass the state users saw. diff --git a/structure/clients/qoder.md b/structure/clients/qoder.md index 7ae4ef16732..1a0e79016d4 100644 --- a/structure/clients/qoder.md +++ b/structure/clients/qoder.md @@ -12,6 +12,12 @@ relative override. The Qoder exporter in `model`, preserving same-named models belonging to other providers. Apply, refresh and disable use the existing recorded-fragment writer; restore uses its snapshots. Model IDs that cannot be represented by the shared selector grammar refuse managed contribution construction. +Status reads report only Qoder as `unsafe` with `unresolvable-path`, retaining its resolved +path, installation state, matching ownership metadata and retention information. Collection +and single-client reads remain available; preview and mutation still refuse those contributions. +Unexpected builder errors propagate rather than being classified as a selector refusal. + +> Decision record: [ADR-7512](../decisions/ADR-7512-qoder-selector-status.md) The default model is the first normalized model; an empty catalog omits the default. Input limits use authoritative context metadata or Qoder's requested 128000 fallback. Exports reject non-loopback placeholder use; the writer's existing admission gate protects diff --git a/structure/decisions/ADR-7512-qoder-selector-status.md b/structure/decisions/ADR-7512-qoder-selector-status.md new file mode 100644 index 00000000000..f18c509c460 --- /dev/null +++ b/structure/decisions/ADR-7512-qoder-selector-status.md @@ -0,0 +1,12 @@ +# ADR-7512 — decision recorded under "Qoder settings ownership" + +- Contract owner: [clients/qoder.md](../clients/qoder.md) + +## Decision record + +- 목적과 의도: Keep Integrations status available when Qoder cannot represent a catalog model in an exact managed array selector. +- 기존 구현 및 제약 조건: Qoder correctly refuses ambiguous selector construction, but its generic exception escapes the shared reader and fails the whole collection, even without a Qoder installation. +- 검토한 주요 대안: Drop unaddressable models, extend the persisted selector grammar, catch every error in the collection route, or classify a dedicated construction refusal in the shared reader. +- 선택한 방식: Raise a typed selector-construction error and catch only that error around contribution construction in the shared state reader, returning the existing unsafe/unresolvable-path status with resolved target metadata. +- 다른 대안 대신 이 방식을 선택한 이유: This preserves exact ownership and the existing fail-closed preview/write behavior without changing persisted selector interpretation, silently omitting catalog models, or hiding unrelated programming errors. +- 장점, 단점 및 영향: Collection and single-client status remain usable and unrelated clients are unaffected. Qoder management still refuses an unaddressable catalog; the existing generic status reason does not identify the particular model. No selector grammar or writer behavior changes. diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index bcda36ca8b1..76b383704be 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -858,6 +858,7 @@ "management-client-config-route.test.ts": "server", "management-google-tool-schema-policy.test.ts": "server", "management-inference-port.test.ts": "server", "management-integration-journal-delete.test.ts": "server", "management-integration-routes.test.ts": "server", + "management-qoder-selector-state.test.ts": "server", "management-metrics-export.test.ts": "server", "management-model-roster-gather-race.test.ts": "server", "management-model-roster.test.ts": "server", "model-export-new-model-policy.test.ts": "server", "management-origin-tls.test.ts": "server", "management-provider-compat-carry.test.ts": "server", diff --git a/tests/server/management-qoder-selector-state.test.ts b/tests/server/management-qoder-selector-state.test.ts new file mode 100644 index 00000000000..0f1b86c9ed1 --- /dev/null +++ b/tests/server/management-qoder-selector-state.test.ts @@ -0,0 +1,135 @@ +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; +import { mkdirSync, mkdtempSync, readFileSync, readdirSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { EXPORT_CLIENTS, qoderConfigPath } from "../../src/clients/config-export"; +import { INTEGRATION_CLIENT_IDS } from "../../src/integrations/registry"; +import { readIntegrationState } from "../../src/integrations/state"; +import { createIntegrationStateStore } from "../../src/integrations/store"; +import { handleManagementAPI } from "../../src/server/management-api"; +import { setIntegrationMutationFlightTestHooks, setIntegrationPathTestHooks } from "../../src/server/management/integration-routes"; +import { resetExportSnapshotForTests } from "../../src/server/management/model-rows"; +import type { OcxConfig } from "../../src/types"; +import { catalogConvergenceFactory } from "../helpers/catalog-convergence"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +let root: string; +let home: string; +let config: OcxConfig; +let store: ReturnType; +let server: ReturnType; +let persisted: number; + +function witness(): string { + return readdirSync(root, { recursive: true }).map(String).sort().map(path => { + const full = join(root, path); + return statSync(full).isDirectory() ? `${path}/` : `${path}:${readFileSync(full, "utf8")}`; + }).join("\n"); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "ocx-qoder-selector-state-")); + home = join(root, "home"); + mkdirSync(home); + store = createIntegrationStateStore(join(root, "integrations")); + persisted = 0; + config = { + port: 10100, hostname: "127.0.0.1", defaultProvider: "fixture", + providers: { fixture: { adapter: "openai-chat", baseUrl: "http://127.0.0.1/v1", liveModels: false, models: ["k3"] } }, + } as OcxConfig; + setIntegrationPathTestHooks({ home, env: {} }); + setIntegrationMutationFlightTestHooks({ store }); + resetExportSnapshotForTests(); + server = Bun.serve({ + hostname: "127.0.0.1", port: 0, + fetch: async req => await handleManagementAPI(req, new URL(req.url), config, { + saveConfigPreservingClaudeCode: () => { persisted += 1; }, + createManagementConvergeCodex: catalogConvergenceFactory(), + }) ?? new Response("not found", { status: 404 }), + }); +}); + +afterEach(async () => { + await server.stop(true); + setIntegrationPathTestHooks(null); + setIntegrationMutationFlightTestHooks(null); + resetExportSnapshotForTests(); + removeTreeWithRetry(root); +}); + +function api(path = "", body?: unknown) { + return fetch(new URL(`/api/client-integrations${path}`, server.url), body === undefined ? undefined : { + method: path === "/qoder" ? "PUT" : "POST", + headers: { "content-type": "application/json" }, body: JSON.stringify(body), + }); +} + +for (const model of ["k3[1m]", "k3,other"]) { + test(`an unrepresentable Qoder model (${model}) does not break collection or single status`, async () => { + const control = await (await api("/hermes")).json(); + config.providers.fixture!.models = ["k3", model]; + const before = witness(); + const response = await api(); + expect(response.status).toBe(200); + const { clients } = await response.json() as { clients: Array<{ clientId: string }> }; + expect(clients.map(client => client.clientId)).toEqual(INTEGRATION_CLIENT_IDS); + const qoder = clients.find(client => client.clientId === "qoder"); + expect(qoder).toMatchObject({ + state: "unsafe", reason: "unresolvable-path", installed: false, + configPath: qoderConfigPath({}, home), snapshotCount: 0, retentionDegraded: false, + }); + expect(clients.find(client => client.clientId === "hermes")).toEqual(control); + const single = await api("/qoder"); + expect(single.status).toBe(200); + expect(await single.json()).toEqual(qoder); + expect(witness()).toBe(before); + expect(persisted).toBe(0); + }); +} + +test("unsafe status does not authorize preview or mutation to write Qoder settings", async () => { + const path = qoderConfigPath({}, home); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, JSON.stringify({ theme: "user-owned", modelConfigs: { customModels: [{ provider: "other", model: "fixture/k3[1m]" }] } })); + config.providers.fixture!.models = ["k3", "k3[1m]"]; + const before = witness(); + expect((await api()).status).toBe(200); // Populates the authoritative preview roster. + expect(await (await api("/qoder")).json()).toMatchObject({ state: "unsafe", installed: true }); + const preview = await api("/preview", { clientId: "qoder", operation: "apply" }); + expect(preview.status).toBeGreaterThanOrEqual(400); + for (const enabled of [true, false]) { + expect((await api("/qoder", { enabled })).status).toBeGreaterThanOrEqual(400); + } + expect(witness()).toBe(before); + expect(persisted).toBe(0); +}); + +test("representable Qoder catalogs still support status, preview and apply", async () => { + mkdirSync(dirname(qoderConfigPath({}, home)), { recursive: true }); + expect((await api()).status).toBe(200); + expect(await (await api("/qoder")).json()).toMatchObject({ state: "absent", installed: true }); + const preview = await api("/preview", { clientId: "qoder", operation: "apply" }); + expect(preview.status).toBe(200); + expect(await preview.json()).toMatchObject({ canApply: true }); + expect((await api("/qoder", { enabled: true })).status).toBe(200); + const current = await (await api("/qoder")).json() as Record; + expect(current.state).toBe("current"); + const before = witness(); + config.providers.fixture!.models = ["k3[1m]"]; + const refused = await api("/qoder"); + expect(refused.status).toBe(200); + expect(await refused.json()).toMatchObject({ + ...current, state: "unsafe", reason: "unresolvable-path", + }); + expect(witness()).toBe(before); +}); + +test("unexpected contribution errors still propagate instead of becoming unsafe status", () => { + const error = new Error("unexpected builder failure"); + const builder = spyOn(EXPORT_CLIENTS.qoder, "buildContribution").mockImplementation(() => { throw error; }); + try { + expect(() => readIntegrationState({ clientId: "qoder", config, port: 10100, models: [], home, env: {}, store })).toThrow(error); + } finally { + builder.mockRestore(); + } +});