From 3254286503f4c06a05c6aac2feadd9b4821977bb Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 20 Sep 2026 21:12:05 +0900 Subject: [PATCH] fix(claude,server): keep Desktop applied-state markers honest across edits - Native launch fallback: strip CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST only when the admission credential is ours, so a user-owned gateway keeps its guard instead of losing it while the credential stays. - Desktop profile rebuilds: carry appliedFingerprint/appliedAt only when the desired profile is unchanged, so a saved edit no longer reports the old on-disk config as applied. The management PUT discards client-supplied markers and restores the server-owned ones only for an identical result. - ocx sync: persist the fingerprint of the exact Desktop config just written through the config-mutation lock, so the applied marker advances with the bytes instead of going stale. --- src/claude/desktop-profile.ts | 34 ++++++++++++------- src/cli/claude.ts | 4 ++- .../management/agent-settings-routes.ts | 13 +++++-- src/server/management/config-routes.ts | 26 ++++++++++++-- tests/claude-integration/claude-cli.test.ts | 4 ++- .../claude-management-api.test.ts | 32 +++++++++++++++++ tests/clients/desktop-profile.test.ts | 33 ++++++++++-------- .../clients/sync-client-integrations.test.ts | 11 +++--- 8 files changed, 117 insertions(+), 40 deletions(-) diff --git a/src/claude/desktop-profile.ts b/src/claude/desktop-profile.ts index e35b74cb464..4bcec6ea05c 100644 --- a/src/claude/desktop-profile.ts +++ b/src/claude/desktop-profile.ts @@ -77,15 +77,6 @@ function assertExactKeys(value: Record, keys: readonly string[] } } -/** - * Applied-state markers survive every profile rebuild. - * - * `parseDesktopProfile`, `reconcileDesktopProfile` and `moveDesktopRoute` each construct a - * fresh `{ version, assignments, defaults }`, and the management routes persist whatever they - * return. Without this carry-through, saving an assignment — or merely dragging a model to - * another family — would erase the fingerprint the apply route wrote, and the GUI would report - * "not applied" for a config that is applied on disk. - */ function appliedMarkers(source: { appliedFingerprint?: unknown; appliedAt?: unknown }): { appliedFingerprint?: string; appliedAt?: string; @@ -96,6 +87,19 @@ function appliedMarkers(source: { appliedFingerprint?: unknown; appliedAt?: unkn }; } +function sameProfileContent(left: DesktopProfile, right: DesktopProfile): boolean { + return JSON.stringify(left.defaults) === JSON.stringify(right.defaults) + && JSON.stringify(Object.entries(left.assignments).sort(([a], [b]) => a.localeCompare(b))) + === JSON.stringify(Object.entries(right.assignments).sort(([a], [b]) => a.localeCompare(b))); +} + +/** Retain applied-state bookkeeping only when the desired Desktop config is unchanged. */ +export function preserveDesktopAppliedState(source: DesktopProfile, rebuilt: DesktopProfile): DesktopProfile { + return sameProfileContent(source, rebuilt) + ? { ...rebuilt, ...appliedMarkers(source) } + : rebuilt; +} + function isFamily(value: unknown): value is DesktopFamily { return typeof value === "string" && (DESKTOP_FAMILIES as readonly string[]).includes(value); } @@ -244,7 +248,8 @@ export function reconcileDesktopProfile( const current = defaults[family]; defaults[family] = current && assignments[current]?.family === family ? current : (members[0] ?? null); } - return parseDesktopProfile({ version: 1, assignments, defaults, ...appliedMarkers(profile) }); + const rebuilt = parseDesktopProfile({ version: 1, assignments, defaults }); + return preserveDesktopAppliedState(profile, rebuilt); } export function moveDesktopRoute( @@ -268,7 +273,7 @@ export function moveDesktopRoute( const destinationMembers = Object.keys(assignments).filter(key => assignments[key]!.family === family).sort(); if (makeDefault || !defaults[family] || assignments[defaults[family]!]?.family !== family) defaults[family] = route; if (!defaults[family] && destinationMembers.length > 0) defaults[family] = destinationMembers[0]!; - return parseDesktopProfile({ version: 1, assignments, defaults, ...appliedMarkers(parsed) }); + return parseDesktopProfile({ version: 1, assignments, defaults }); } export function setDesktopFamilyDefault( @@ -280,7 +285,12 @@ export function setDesktopFamilyDefault( const members = Object.keys(parsed.assignments).filter(key => parsed.assignments[key]!.family === family); if (route === null && members.length > 0) throw new DesktopProfileError("cannot clear a non-empty family default", `profile.defaults.${family}`); if (route !== null && parsed.assignments[route]?.family !== family) throw new DesktopProfileError("route is not a member of this family", `profile.defaults.${family}`); - return parseDesktopProfile({ ...parsed, defaults: { ...parsed.defaults, [family]: route } }); + const rebuilt = parseDesktopProfile({ + version: 1, + assignments: parsed.assignments, + defaults: { ...parsed.defaults, [family]: route }, + }); + return preserveDesktopAppliedState(parsed, rebuilt); } export function renderDesktopProfile( diff --git a/src/cli/claude.ts b/src/cli/claude.ts index 3daec5744c9..846a8c921e6 100644 --- a/src/cli/claude.ts +++ b/src/cli/claude.ts @@ -572,7 +572,6 @@ export function claudeLaunchPreflight( */ const NATIVE_STRIPPED_LEVERS = [ "CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", - "CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST", "CLAUDE_CODE_MAX_CONTEXT_TOKENS", "CLAUDE_CODE_AUTO_COMPACT_WINDOW", "CLAUDE_CODE_ALWAYS_ENABLE_EFFORT", @@ -632,6 +631,9 @@ export function buildNativeClaudeEnv( } for (const name of NATIVE_STRIPPED_LEVERS) delete env[name]; + // An explicit caller-owned guard must follow a caller-owned gateway and credential; + // otherwise settings.env can replace the destination while retaining the credential. + if (hasOwnedAdmission) delete env.CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST; const providerNames = Object.keys(config.providers); for (const name of MODEL_ENV_SLOT_NAMES) { const value = env[name]; diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index 91976c144fd..fb61ac141fe 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -977,7 +977,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise let body: { profile?: unknown }; try { body = await readManagementJsonBody(req); } catch (error) { rethrowManagementBodyTooLarge(error); return jsonResponse({ error: "invalid JSON body" }, 400); } try { - const { parseDesktopProfile, reconcileDesktopProfile } = await import("../../claude/desktop-profile"); + const { parseDesktopProfile, preserveDesktopAppliedState, reconcileDesktopProfile } = await import("../../claude/desktop-profile"); const parsed = parseDesktopProfile(body.profile); const current = await buildClaudeDesktopState(config); const availableRoutes = new Set(current.models.filter(item => item.available).map(item => item.route)); @@ -1000,8 +1000,15 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise throw new Error(`현재 사용할 수 없는 모델은 기본값으로 지정할 수 없습니다: ${nextDefault}`); } } - const state = await buildClaudeDesktopState(config, parsed); - config.claudeCode = { ...(config.claudeCode ?? {}), desktopProfile: reconcileDesktopProfile(state.profile, state.models) }; + // Applied markers are server-owned bookkeeping. Discard client copies, then + // restore the trusted markers only if the desired profile stayed identical. + const editable = { version: 1 as const, assignments: parsed.assignments, defaults: parsed.defaults }; + const state = await buildClaudeDesktopState(config, editable); + const rebuilt = reconcileDesktopProfile(state.profile, state.models); + config.claudeCode = { + ...(config.claudeCode ?? {}), + desktopProfile: preserveDesktopAppliedState(current.profile, rebuilt), + }; saveConfigPreservingClaudeCode(config); const saved = await buildClaudeDesktopState(config); const runtimePort = Number(url.port) || config.port; diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 6db5ca3f35c..b45f8803a04 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -18,6 +18,7 @@ import { isValidProviderName, loadConfig, multiAgentGuidanceEnabled, + mutatePersistedConfig, providerBaseUrlConfigError, providerHeadersConfigError, saveConfigPreservingClaudeCode, @@ -228,9 +229,28 @@ export async function syncEnabledClientIntegrations( latest.claudeCode?.desktopProfile, nativeContextLimits(latest), ); - out.push(r.written - ? { client: "claude-desktop", ok: true, changed: true } - : { client: "claude-desktop", ok: false, reason: r.reason ?? "Claude Desktop write failed" }); + if (!r.written || !r.fingerprint) { + out.push({ client: "claude-desktop", ok: false, reason: r.reason ?? "Claude Desktop write failed" }); + } else { + const { emptyDesktopProfile } = await import("../../claude/desktop-profile"); + const marked = mutatePersistedConfig(persisted => { + const profile = persisted.claudeCode?.desktopProfile + ?? latest.claudeCode?.desktopProfile + ?? emptyDesktopProfile(); + persisted.claudeCode = { + ...(persisted.claudeCode ?? {}), + desktopProfile: { + ...profile, + appliedFingerprint: r.fingerprint, + appliedAt: new Date().toISOString(), + }, + }; + return { changed: true, value: true }; + }); + out.push(marked.status === "unavailable" + ? { client: "claude-desktop", ok: false, reason: `Claude Desktop applied marker was not saved (${marked.reason})` } + : { client: "claude-desktop", ok: true, changed: true }); + } } } catch (error) { out.push({ client: "claude-desktop", ok: false, reason: error instanceof Error ? error.message : String(error) }); diff --git a/tests/claude-integration/claude-cli.test.ts b/tests/claude-integration/claude-cli.test.ts index 900ebea069c..6e73cc8ed0f 100644 --- a/tests/claude-integration/claude-cli.test.ts +++ b/tests/claude-integration/claude-cli.test.ts @@ -126,17 +126,19 @@ describe("ocx claude native fallback", () => { expect(env.CLAUDE_CODE_AUTO_COMPACT_WINDOW).toBeUndefined(); }); - test("preserves an unrelated loopback gateway and its user credential", () => { + test("preserves an unrelated gateway, its user credential, and its host-managed guard", () => { for (const baseUrl of ["http://localhost:8080", "http://127.0.0.1:10100"]) { const env = buildNativeClaudeEnv(cfg({ port: 10100 }), { ANTHROPIC_BASE_URL: baseUrl, ANTHROPIC_API_KEY: "sk-ant-user-key", + CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST: "1", }, { preBunAnthropicSlots: ["ANTHROPIC_BASE_URL", "ANTHROPIC_API_KEY"], }); expect(env.ANTHROPIC_BASE_URL).toBe(baseUrl); expect(env.ANTHROPIC_API_KEY).toBe("sk-ant-user-key"); + expect(env.CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST).toBe("1"); } }); diff --git a/tests/claude-integration/claude-management-api.test.ts b/tests/claude-integration/claude-management-api.test.ts index 751b838ee99..8c4a7994b16 100644 --- a/tests/claude-integration/claude-management-api.test.ts +++ b/tests/claude-integration/claude-management-api.test.ts @@ -875,6 +875,38 @@ test("Claude Desktop PUT rejects invalid JSON profile without mutating saved con } }); +test("Claude Desktop PUT clears applied markers when routing changes", async () => { + const server = startServer(0); + try { + const apply = await fetch(new URL("/api/claude-desktop/apply", server.url), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ mode: "static" }), + }); + expect(apply.status).toBe(200); + expect(loadConfig().claudeCode?.desktopProfile?.appliedFingerprint).toBeString(); + + const state = await fetch(new URL("/api/claude-desktop", server.url)).then(r => r.json()) as Record; + const edited = structuredClone(state.profile); + edited.assignments["mock/test-model"].family = "sonnet"; + edited.defaults.opus = Object.keys(edited.assignments) + .filter(route => edited.assignments[route].family === "opus") + .sort()[0] ?? null; + edited.defaults.sonnet = "mock/test-model"; + + const put = await fetch(new URL("/api/claude-desktop", server.url), { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ profile: edited }), + }); + expect(put.status).toBe(200); + expect(loadConfig().claudeCode?.desktopProfile).not.toHaveProperty("appliedFingerprint"); + expect(loadConfig().claudeCode?.desktopProfile).not.toHaveProperty("appliedAt"); + } finally { + await server.stop(true); + } +}); + test("Claude Desktop PUT retains but cannot move an unavailable route", async () => { const seeded = loadConfig(); seeded.claudeCode = { diff --git a/tests/clients/desktop-profile.test.ts b/tests/clients/desktop-profile.test.ts index 7aa7cdda7e1..82725d65a70 100644 --- a/tests/clients/desktop-profile.test.ts +++ b/tests/clients/desktop-profile.test.ts @@ -120,10 +120,8 @@ describe("Claude Desktop profile", () => { }); // The apply route writes `appliedFingerprint`/`appliedAt` back onto the stored profile so the - // GUI can show applied-vs-saved state. Every rebuild in this module must accept AND carry them: - // rejecting them broke the Desktop tab outright after the first apply, and silently dropping - // them would make a saved edit — or a single drag between families — report "not applied" for a - // config that is applied on disk. + // GUI can show applied-vs-saved state. Parsing and no-op rebuilds retain them, while a change to + // the desired Desktop config must clear them so the old on-disk config is not reported as current. describe("applied-state markers", () => { const applied = { appliedFingerprint: "0123456789abcdef", @@ -140,23 +138,28 @@ describe("Claude Desktop profile", () => { expect(parsed.appliedAt).toBe(applied.appliedAt); }); - test("reconcileDesktopProfile keeps them across a catalog change", () => { + test("reconcileDesktopProfile clears them across a catalog change", () => { const next = reconcileDesktopProfile(seeded(), [...models, { route: "test/new-model", label: "New" }]); - expect(next.appliedFingerprint).toBe(applied.appliedFingerprint); - expect(next.appliedAt).toBe(applied.appliedAt); + expect(next).not.toHaveProperty("appliedFingerprint"); + expect(next).not.toHaveProperty("appliedAt"); }); - test("moveDesktopRoute keeps them — the drag-and-drop path", () => { - const moved = moveDesktopRoute(seeded(), "cursor/gpt-5.6-luna", "sonnet"); - expect(moved.appliedFingerprint).toBe(applied.appliedFingerprint); - expect(moved.appliedAt).toBe(applied.appliedAt); + test("reconcileDesktopProfile keeps them when profile content is unchanged", () => { + expect(reconcileDesktopProfile(seeded(), models)).toMatchObject(applied); }); - test("setDesktopFamilyDefault keeps them", () => { + test("moveDesktopRoute clears them — the drag-and-drop path", () => { const moved = moveDesktopRoute(seeded(), "cursor/gpt-5.6-luna", "sonnet"); - const next = setDesktopFamilyDefault(moved, "sonnet", "cursor/gpt-5.6-luna"); - expect(next.appliedFingerprint).toBe(applied.appliedFingerprint); - expect(next.appliedAt).toBe(applied.appliedAt); + expect(moved).not.toHaveProperty("appliedFingerprint"); + expect(moved).not.toHaveProperty("appliedAt"); + }); + + test("setDesktopFamilyDefault clears them only when the default changes", () => { + const changed = setDesktopFamilyDefault(seeded(), "opus", "native/gpt-5.6-sol"); + expect(changed).not.toHaveProperty("appliedFingerprint"); + expect(changed).not.toHaveProperty("appliedAt"); + expect(setDesktopFamilyDefault(seeded(), "opus", "anthropic/claude-fable-5")) + .toMatchObject(applied); }); test("a profile without the markers stays without them", () => { diff --git a/tests/clients/sync-client-integrations.test.ts b/tests/clients/sync-client-integrations.test.ts index 219a1480829..53f564f408c 100644 --- a/tests/clients/sync-client-integrations.test.ts +++ b/tests/clients/sync-client-integrations.test.ts @@ -69,11 +69,12 @@ describe("ocx sync fans out to enabled native clients and owned file integration expect(fn).toContain("refreshOwnedCatalogIntegrations"); // Native clients keep their catches; the owned catalog helper isolates file clients. expect(fn.match(/catch \(error\)/g)?.length).toBe(2); - // The Desktop write gets the native context limits, same as every other Desktop - // call site. 8b672205e threaded `nativeContextLimits` through those writers and - // left this assertion naming the retired `providerContextCap` spelling, so the - // source-shape check failed against the very change it is meant to pin. expect(fn).toContain("nativeContextLimits(latest)"); + // Cleanup accepts only the fingerprint of the exact credential-bearing profile we wrote. + // Sync must durably advance that ownership marker rather than leaving the old value behind. + expect(fn).toContain("mutatePersistedConfig(persisted =>"); + expect(fn).toContain("appliedFingerprint: r.fingerprint"); + expect(fn.indexOf("nativeContextLimits(latest)")).toBeLessThan(fn.indexOf("appliedFingerprint: r.fingerprint")); // A client that is off is omitted rather than reported: the caller has to be able to // tell "left alone" from "tried and failed", so there is no skipped state to emit. expect(fn).not.toContain('"skipped"'); @@ -144,7 +145,7 @@ describe("Desktop sync rechecks persisted state after discovery", () => { writes.push(args); return outcome === "refusal" ? { written: false, path: "fixture", reason: "desktop_remote_store_active" } - : { written: true, path: "fixture" }; + : { written: true, path: "fixture", fingerprint: "0123456789abcdef" }; }, }); try {