fix(service): preserve connected client key on uninstall

docs-site/src/content/docs/guides/remote-hub.md

@@ -60,6 +60,12 @@ The hub automatically issues a per-client key. The client writes it to the exist
 `service-api-token` file, never `config.json`. While connected, usage comes from the hub usage store
 filtered to that client's stable `apiKeyId`. After disconnect, usage comes from the local store.
 OpenCodex does not mirror usage between the two stores.
+`ocx service uninstall` removes the local service but preserves an existing key when the client is
+connected, its connection metadata is invalid or mismatched, or a pending connection marker matches
+the current key. A valid marker for an older key does not retain an unrelated service key.
+If a marker is unsafe, malformed, or unreadable, token cleanup cannot be verified;
+the command warns instead of claiming the key was kept. Use `ocx disconnect` to remove a connected
+client's local key and state.
 
 If a client saved a remote `http://` Hub URL before the secure transport rule, its Hub
 operations now return `insecure_http_refused`. Run `ocx disconnect` locally, then reconnect

docs-site/src/content/docs/ko/guides/remote-hub.md

@@ -39,6 +39,7 @@ ocx sync
 이 줄을 직접 만들 필요는 없습니다. 허브에서 `ocx hub invite`를 실행하면 코드를 발급하고, 두 Origin이 모두 채워진 명령을 그대로 출력합니다. [다른 컴퓨터 초대하기](#다른-컴퓨터-초대하기)를 보세요.
 
 허브가 발급한 클라이언트별 키는 권한이 제한된 `service-api-token` 파일에 저장됩니다. `config.json`에는 저장되지 않습니다. 연결 중 사용량은 허브 기록에서 해당 `apiKeyId`만 조회하고, 연결을 끊은 뒤에는 로컬 기록을 봅니다. 두 기록은 서로 복제되지 않습니다.
+`ocx service uninstall`은 로컬 서비스를 제거하지만 클라이언트가 연결되어 있거나, 연결 중 표시의 지문이 현재 키와 일치하거나, 연결 정보가 잘못되었거나 일치하지 않으면 기존 키를 보존합니다. 이전 키의 유효한 표시는 다른 서비스 키를 보존하지 않습니다. 표시 파일이 안전하지 않거나 손상되었거나 읽을 수 없어 키 정리를 확인할 수 없으면 보존했다고 단정하지 않고 경고합니다. 연결된 클라이언트의 로컬 키와 상태를 제거하려면 `ocx disconnect`를 사용하세요.
 
 ### 연결된 클라이언트의 상태 표시
 

src/client/connect.ts

@@ -37,6 +37,7 @@ import {
   replaceServiceApiTokenFile,
   restoreTokenBackup,
   serviceApiTokenBackupPath,
+  serviceApiTokenFingerprint,
   writeTokenBackup,
   writeServiceApiTokenFile,
 } from "../lib/service-secrets";
@@ -65,6 +66,7 @@ import {
   clearClientConnection,
   commitClientConnection,
   readClientConnectionState,
+  markClientConnectPending, clearClientConnectPending, pendingClientConnectMayOwnToken,
   assertNoClientDisconnectPending, assertClientConnectionUnchanged, sameClientConnectionOwner,
 } from "./state";
 import { assertClientCatalogCompatible, type CatalogCompatibilityDeps } from "./catalog-compatibility";
@@ -492,6 +494,7 @@ function assertConnectingState(expectedTokenFingerprint?: string): void {
   }
 }
 
+/** Enroll a client key, keeping its pending ownership visible until commit or rollback. */
 export async function connectClient(
   options: ConnectOptions,
   deps: ClientConnectDeps = {},
@@ -501,6 +504,7 @@ export async function connectClient(
   let issued: IssuedClientKey | null = null;
   let cleanupCredential: { kind: "admin"; value: Uint8Array } | { kind: "gui-session"; value: ConnectGuiSession } | null = null;
   let tokenFingerprint: string | null = null;
+  let pendingConnectFingerprint: string | null = null;
   let priorCatalog: CatalogSnapshot | null = null;
   let writtenCatalogFingerprint: string | null = null;
   let injectionCommitted = false;
@@ -536,6 +540,9 @@ export async function connectClient(
 
     const initialFiles = withClientLifecycleSync(() => withConfigMutationLockSync(() => {
       assertConnectingState();
+      const fingerprint = serviceApiTokenFingerprint(issued!.key);
+      markClientConnectPending(fingerprint);
+      pendingConnectFingerprint = fingerprint;
       return { prior: catalogSnapshot(), persisted: writeServiceApiTokenFile(issued!.key) };
     }), deps.lifecycleLockDeps);
     priorCatalog = initialFiles.prior;
@@ -602,6 +609,7 @@ export async function connectClient(
     };
     withClientLifecycleSync(() => withConfigMutationLockSync(() => {
       assertConnectingState(persisted.fingerprint);
+      clearClientConnectPending(persisted.fingerprint);
       commitClientConnection(connection);
       committed = true;
     }), deps.lifecycleLockDeps);
@@ -618,9 +626,11 @@ export async function connectClient(
         if (priorCatalog && writtenCatalogFingerprint && !restoreCatalogSnapshot(priorCatalog, writtenCatalogFingerprint)) {
           rollbackFailures.push("catalog rollback did not match the written artifact");
         }
-        if (tokenFingerprint) {
-          const removed = removeServiceApiTokenFileIfOwned(tokenFingerprint);
+        if (pendingConnectFingerprint) {
+          const removed = removeServiceApiTokenFileIfOwned(pendingConnectFingerprint);
           if (removed === "changed") rollbackFailures.push("service token changed during rollback");
+          // Final commit may fail after this attempt already cleared its marker under the same lock.
+          else if (pendingClientConnectMayOwnToken()) clearClientConnectPending(pendingConnectFingerprint);
         }
       }), deps.lifecycleLockDeps);
     } catch { rollbackFailures.push("client cleanup ownership unavailable"); }

src/client/state.ts

@@ -1,5 +1,7 @@
-import { readFileSync } from "node:fs";
+import { lstatSync, readFileSync, unlinkSync } from "node:fs";
+import { join } from "node:path";
 import {
+  getConfigDir,
   getConfigPath,
   deleteConfigTopLevelKey,
   getDefaultConfig,
@@ -8,6 +10,7 @@ import {
   saveConfig,
   withConfigMutationLockSync,
 } from "../config";
+import { atomicWriteFileNoFollowUnclaimed } from "../config/atomic-write";
 import type { OcxClientConnectionConfig } from "../types";
 import { inspectRemoteDesktopStore, readDesktopDisconnectReceipt } from "../claude/desktop-remote-store";
 import { withClientLifecycleSync, type ClientLifecycleLockDeps } from "./lifecycle-lock";
@@ -23,6 +26,41 @@ export type ClientConnectionState =
   | { kind: "invalid"; reason: string }
   | { kind: "mismatched"; reason: string };
 
+const pendingConnectPath = (): string => join(getConfigDir(), "client-connect-pending");
+
+/** Validate pending ownership; an optional fingerprint restricts it to that exact key. */
+export function pendingClientConnectMayOwnToken(fingerprint?: string): boolean {
+  const path = pendingConnectPath();
+  let stat;
+  try { stat = lstatSync(path); }
+  catch (error) {
+    if ((error as NodeJS.ErrnoException).code === "ENOENT") return false;
+    throw error;
+  }
+  if (!stat.isFile() || stat.nlink !== 1 || stat.size !== 65) {
+    throw new Error("pending client connection owner is unsafe");
+  }
+  const marker = readFileSync(path, "utf8");
+  if (!/^[a-f0-9]{64}\n$/.test(marker)) throw new Error("pending client connection owner is malformed");
+  return fingerprint === undefined || marker === `${fingerprint}\n`;
+}
+
+/** Publish only the token fingerprint, before the key file, under the client lifecycle lock. */
+export function markClientConnectPending(fingerprint: string): void {
+  if (!/^[a-f0-9]{64}$/.test(fingerprint)) throw new Error("invalid pending client fingerprint");
+  atomicWriteFileNoFollowUnclaimed(pendingConnectPath(), `${fingerprint}\n`);
+}
+
+/** Clear only the marker for this connect attempt while the client lifecycle lock is held. */
+export function clearClientConnectPending(fingerprint: string): void {
+  const path = pendingConnectPath();
+  const stat = lstatSync(path);
+  if (!stat.isFile() || stat.nlink !== 1 || stat.size !== 65 || readFileSync(path, "utf8") !== `${fingerprint}\n`) {
+    throw new Error("pending client connection owner changed");
+  }
+  unlinkSync(path);
+}
+
 export type ClientRotationRecoveryGate =
   | { kind: "clean" }
   | { kind: "orphan-cleaned" }

src/service/cli.ts

@@ -1,9 +1,12 @@
-import { existsSync, unlinkSync } from "node:fs";
+import { existsSync, lstatSync, unlinkSync } from "node:fs";
 import { join } from "node:path";
 import { restoreNativeCodexAsync } from "../codex/inject";
 import { describeRetainedCodexProviderTable } from "../codex/inject/restore";
 import { stripGrokConfig } from "../grok/inject";
-import { serviceApiTokenFilePath } from "../lib/service-secrets";
+import { withConfigMutationLockSync } from "../config/mutation-lock";
+import { withClientLifecycleSync, type ClientLifecycleLockDeps } from "../client/lifecycle-lock";
+import { pendingClientConnectMayOwnToken, readClientConnectionState } from "../client/state";
+import { readServiceApiTokenState, serviceApiTokenFilePath } from "../lib/service-secrets";
 import { statusWinswRaw, type WinswStatus } from "../lib/winsw";
 import { withWindowsServiceMutationLock } from "../lib/windows-service-mutation-lock";
 import { maybeShowStarPrompt } from "../cli/star-prompt";
@@ -182,6 +185,32 @@ export function parseServiceArgs(args: string[]): ParsedServiceArgs {
   return { sub: normalizeServiceSubcommand(sub), backend, invalid };
 }
 
+/** Remove the service credential only when no client connection can own it. */
+export function removeServiceTokenAfterUninstall(
+  lockDeps: ClientLifecycleLockDeps = {},
+): "removed" | "absent" | "retained" | "unverified" {
+  try {
+    return withClientLifecycleSync(() => withConfigMutationLockSync(() => {
+      const path = serviceApiTokenFilePath();
+      try { lstatSync(path); }
+      catch (error) {
+        if ((error as NodeJS.ErrnoException).code === "ENOENT") return "absent";
+        throw error;
+      }
+      if (readClientConnectionState().kind !== "disconnected") return "retained";
+      const token = readServiceApiTokenState();
+      if (token.kind !== "present") return token.kind === "absent" ? "absent" : "unverified";
+      if (pendingClientConnectMayOwnToken(token.fingerprint)) return "retained";
+      unlinkSync(path);
+      return "removed";
+    }), lockDeps);
+  } catch {
+    // Lock, state-read and unlink failures all leave cleanup unverified, not successful.
+    return "unverified";
+  }
+}
+
+/** Execute a service verb while preserving client-owned credentials during uninstall. */
 export async function serviceCommand(...args: (string | undefined)[]): Promise<void> {
   const filteredArgs = args.filter((a): a is string => Boolean(a));
   const execute = async (): Promise<void> => {
@@ -424,7 +453,9 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise<v
         }
       }
       removeServiceInstallState();
-      try { if (existsSync(serviceApiTokenFilePath())) unlinkSync(serviceApiTokenFilePath()); } catch { /* best-effort */ }
+      const tokenCleanup = removeServiceTokenAfterUninstall();
+      if (tokenCleanup === "retained") console.warn("⚠️  Service token kept because client state may own it.");
+      else if (tokenCleanup === "unverified") console.warn("⚠️  Service token cleanup could not be verified; inspect client state before deleting it.");
       console.log("✅ service uninstalled.");
       break;
     default:

structure/clients/claude-desktop.md

@@ -161,6 +161,10 @@ the previous selection only while the managed profile is still selected. A later
 selection is not changed. A newly created profile with user additions is retained in readable
 standard mode instead of deleting those additions.
 
+During initial enrollment, `src/client/state.ts` records a pending key fingerprint before the token
+is published. Service uninstall retains only the matching key; an unsafe or unreadable marker leaves cleanup unverified. Connect clears its marker on commit or rollback; the marker
+does not claim any Desktop restoration ownership.
+
 A proven legacy current-hub/recognized-key profile without an original baseline can be adopted
 by apply, rotation/recovery or direct disconnect without a new flag or prerequisite reapply.
 Its explicit standard-fallback outcome is distinct from original restoration: only owned gateway

structure/runtime.md

@@ -187,7 +187,7 @@ described in [OpenAI quota ownership](providers/openai-tiers.md#public-provider-
 `runtime-port.json` through `src/config/process-state.ts`, syncs Codex config/catalog, then serves
 until shutdown. Normal shutdown restores native Codex. Service mode sets
 `OCX_SERVICE=1`, so managed restarts do not repeatedly restore/reinject; explicit service stop and
-uninstall still restore.
+uninstall still restore. `src/service/cli.ts` removes the service token on uninstall only when persisted client state is disconnected and no pending connect marker owns the newly issued key. `src/client/connect.ts` publishes that fingerprint marker before the key, then clears it with the connection commit or rollback under the client lifecycle and config mutation locks. Connected, invalid, or mismatched client state retains an existing token. A valid pending marker retains only its matching fingerprint; an older marker does not own a replacement service key. An absent token is reported as absent; unsafe, malformed, or unreadable markers and lock, state-read, or deletion failures leave cleanup unverified.
 The package-tree integrity fence for live package replacement follows the
 [update transaction contract](ops/docs-and-release.md#package-tree-integrity-fence).
 

tests/clients/client-connect.test.ts

@@ -392,8 +392,9 @@ describe("remote hub client boundary", () => {
 /** A catalog the user already had before ever connecting. */
 const PRIOR_CATALOG_BYTES = '{"models":[{"slug":"local/only-model"}]}';
 
+/** Exercise enrollment and rollback in a fresh process with isolated client homes. */
 function runTransactionScenario(
-  stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator",
+  stage: "success" | "catalog" | "preflight" | "commit" | "prior-catalog" | "coordinator" | "uninstall-during-catalog",
   options: { script?: string; timeoutMs?: number } = {},
 ) {
   const opencodexHome = mkdtempSync(join(tmpdir(), "ocx-client-connect-home-"));
@@ -430,6 +431,7 @@ function runTransactionScenario(
     const stage = ${JSON.stringify(stage)};
     markTransaction("module_ready");
     let commitFaultTriggered = false;
+    let uninstallDuringCatalog = null;
     const catalog = '{"models":[]}';
     const etag = '"sha256-' + createHash("sha256").update(catalog).digest("base64url") + '"';
     const calls = [];
@@ -447,6 +449,13 @@ function runTransactionScenario(
       if (url.endsWith("/api/keys") && init.method === "DELETE") return Response.json({ success: true });
       if (url.endsWith("/v1/catalog")) {
         if (stage === "catalog") return Response.json({ error: "down" }, { status: 503 });
+        if (stage === "uninstall-during-catalog") {
+          const { removeServiceTokenAfterUninstall } = require("./src/service/cli");
+          uninstallDuringCatalog = {
+            cleanup: removeServiceTokenAfterUninstall({ lockPath: process.env.OPENCODEX_HOME + "/lifecycle.sqlite" }),
+            tokenExists: existsSync(serviceApiTokenFilePath()),
+          };
+        }
         return new Response(catalog, { headers: { ETag: etag, "Content-Type": "application/json" } });
       }
       throw new Error("unexpected request " + url);
@@ -500,7 +509,7 @@ function runTransactionScenario(
       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;
       const hubStateCacheAfter = existsSync(hubStateCachePath());
-      writeSync(1, JSON.stringify({ connected, error, coordinatorUnavailable, beforeDisconnect, artifacts, disconnected, catalogAfter, hubStateCacheBefore, hubStateCacheAfter, after: readClientConnectionState(), calls, commitFaultTriggered }) + "\\n");
+      writeSync(1, JSON.stringify({ connected, error, coordinatorUnavailable, beforeDisconnect, artifacts, disconnected, catalogAfter, hubStateCacheBefore, hubStateCacheAfter, after: readClientConnectionState(), calls, commitFaultTriggered, uninstallDuringCatalog, pendingAtResult: existsSync(process.env.OPENCODEX_HOME + "/client-connect-pending") }) + "\\n");
       markTransaction("result_published");
     })();
   `;
@@ -669,7 +678,19 @@ describe("connect transaction and offline disconnect", () => {
       expect(run.parsed.after).toEqual({ kind: "disconnected" });
       expect(run.parsed.calls.filter((call: any) => call.method === "DELETE")).toEqual([]);
     } finally { run.cleanup(); }
-  });
+  }, SPAWN_BUDGET_MS);
+
+  test("service uninstall during catalog download retains the pending client key", () => {
+    const run = runTransactionScenario("uninstall-during-catalog");
+    try {
+      expect(run.status).toBe(0);
+      expect(run.parsed.uninstallDuringCatalog).toEqual({ cleanup: "retained", tokenExists: true });
+      expect(run.parsed.error).toBeNull();
+      expect(run.parsed.connected.apiKeyId).toBe("issued-id");
+      expect(run.parsed.beforeDisconnect.kind).toBe("connected");
+      expect(run.parsed.pendingAtResult).toBe(false);
+    } finally { run.cleanup(); }
+  }, SPAWN_BUDGET_MS);
 
   test("disconnect puts back the catalog the user had before connecting", () => {
     // Connect overwrites whatever catalog is already on disk. Disconnect used to delete the
@@ -684,7 +705,7 @@ describe("connect transaction and offline disconnect", () => {
       expect(run.parsed.catalogAfter).toBe(PRIOR_CATALOG_BYTES);
       expect(run.parsed.after).toEqual({ kind: "disconnected" });
     } finally { run.cleanup(); }
-  });
+  }, SPAWN_BUDGET_MS);
 
   test("disconnect removes the catalog when the user had none", () => {
     // The other half of the same contract: `priorCatalog: ""` records "there genuinely was
@@ -694,7 +715,7 @@ describe("connect transaction and offline disconnect", () => {
       expect(run.parsed.disconnected).toMatchObject({ catalogRemoved: true, catalogRestored: false });
       expect(run.parsed.catalogAfter).toBeNull();
     } finally { run.cleanup(); }
-  });
+  }, SPAWN_BUDGET_MS);
 
   for (const stage of ["catalog", "preflight", "commit"] as const) {
     test(`rolls back local artifacts when ${stage} fails before final commit`, () => {
@@ -704,11 +725,13 @@ describe("connect transaction and offline disconnect", () => {
         expect(run.parsed.connected).toBeNull();
         expect(run.parsed.beforeDisconnect).toEqual({ kind: "disconnected" });
         expect(run.parsed.artifacts.token).toBe(false);
+        expect(run.parsed.pendingAtResult).toBe(false);
         expect(run.parsed.artifacts.catalog).toBe(false);
         expect(run.parsed.artifacts.credentialZeroed).toBe(true);
         expect(run.parsed.calls.some((call: any) => call.method === "DELETE")).toBe(true);
         if (stage === "commit") {
           expect(run.parsed.commitFaultTriggered).toBe(true);
+          expect(run.parsed.error).not.toContain("client cleanup ownership unavailable");
           expect(run.parsed.calls.some((call: any) => call.method === "POST" && call.url.endsWith("/api/keys"))).toBe(true);
         }
         expect(run.configBytes).not.toContain("issued-id");