fix(setup): preserve client-owned MCP policy on refresh

src/application/SetupClientConfiguration.ts

@@ -8,7 +8,6 @@ import {
 import { err, ok, type Result } from "../domain/result.js";
 import { ConfigurationError } from "../domain/configurationErrors.js";
 import {
-  clientRegistrationEntry,
   clientConfigurationValuesEqual,
   clientServerListedDisabled,
   clientServerPath,
@@ -33,14 +32,11 @@ import {
   type ProviderRequestBudgetResolver,
 } from "./ClientRegistrationEnvironment.js";
 
+import { refreshClientRegistration } from "./ClientRegistration.js";
+
 import { PRODUCT_IDENTITY } from "../identity.js";
 import { snapshotEnvironment } from "../process/snapshotEnvironment.js";
 import { npxRegistrationCommand } from "./ClientRegistrationIdentity.js";
-import {
-  MCP_STARTUP_POLICY,
-  recommendedMcpToolTimeoutMs,
-  recommendedMcpToolTimeoutSeconds,
-} from "../mcpStartupPolicy.js";
 import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js";
 import { packageReadTargetChanged } from "./MsixPackageStorage.js";
 import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js";
@@ -100,14 +96,20 @@ export const configureClientConfiguration = async (
   );
   if (transactionPath === undefined)
     return { status: "failed", reason: "path" };
-  return withClientConfigurationTransaction(transactionPath, () =>
-    configureClientDocument(
-      { client, transactionPath },
-      environment,
-      command,
-      resolveProviderRequestBudget,
-    ),
-  );
+  try {
+    return await withClientConfigurationTransaction(transactionPath, () =>
+      configureClientDocument(
+        { client, transactionPath },
+        environment,
+        command,
+        resolveProviderRequestBudget,
+      ),
+    );
+  } catch (cause: unknown) {
+    if (cause instanceof ConfigurationError)
+      return { status: "failed", reason: "readback" };
+    throw cause;
+  }
 };
 
 const configureClientDocument = async (
@@ -452,12 +454,21 @@ export const inspectClientConfiguration = async (
   command: readonly string[],
   resolveProviderRequestBudget?: ProviderRequestBudgetResolver,
 ): Promise<ClientConfigurationInspection> => {
-  const inspection = await inspectClientDocument(
-    client,
-    providerEnvironment,
-    command,
-    resolveProviderRequestBudget,
-  );
+  let inspection: ClientConfigurationInspection;
+  try {
+    inspection = await inspectClientDocument(
+      client,
+      providerEnvironment,
+      command,
+      resolveProviderRequestBudget,
+    );
+  } catch (cause: unknown) {
+    if (!(cause instanceof ConfigurationError)) throw cause;
+    return {
+      status: "invalid",
+      remediation: `${cause.message}: ${cause.settings.map(({ setting, constraint }) => `${setting}: ${constraint}`).join("; ")}`,
+    };
+  }
   if (inspection.status !== "create" && inspection.status !== "update")
     return inspection;
   // A change the running client would overwrite is not a persisted registration.
@@ -785,59 +796,14 @@ const clientConfigurationDesired = async (
           REA_MCP_INPUT_SCHEMA_PROFILE: "compact",
         }
       : environment.value;
-  const registration = clientRegistrationEntry(
+  const registration = refreshClientRegistration(
+    client,
     format,
     launcher,
     registrationEnvironment,
+    parsed?.servers[PRODUCT_IDENTITY.mcpServerKey],
+    validated.value.timeoutInputs,
   );
-  const previous = parsed?.servers[PRODUCT_IDENTITY.mcpServerKey];
-  if (
-    client.name === "kimi" &&
-    typeof previous === "object" &&
-    previous !== null &&
-    "enabledTools" in previous
-  )
-    registration.enabledTools = previous.enabledTools;
-  return {
-    ...registration,
-    ...(client.name === "codex" || client.name === "grok_build"
-      ? {
-          startup_timeout_sec:
-            client.name === "codex"
-              ? (positiveTimeout(previous, "startup_timeout_sec") ??
-                MCP_STARTUP_POLICY.codexStartupTimeoutSeconds)
-              : MCP_STARTUP_POLICY.codexStartupTimeoutSeconds,
-        }
-      : {}),
-    ...(client.name === "codex"
-      ? {
-          tool_timeout_sec:
-            positiveTimeout(previous, "tool_timeout_sec") ??
-            recommendedMcpToolTimeoutSeconds(validated.value.timeoutInputs),
-        }
-      : {}),
-    ...(client.name === "kimi"
-      ? {
-          startupTimeoutMs:
-            positiveTimeout(previous, "startupTimeoutMs") ??
-            MCP_STARTUP_POLICY.kimiStartupTimeoutMs,
-          toolTimeoutMs:
-            positiveTimeout(previous, "toolTimeoutMs") ??
-            recommendedMcpToolTimeoutMs(validated.value.timeoutInputs),
-        }
-      : {}),
-  };
-};
-
-const positiveTimeout = (
-  registration: unknown,
-  key: string,
-): number | bigint | undefined => {
-  if (typeof registration !== "object" || registration === null) return;
-  const value: unknown = Reflect.get(registration, key);
-  // TOML keeps integers beyond the safe Number range exact as bigint.
-  if (typeof value === "bigint") return value > 0n ? value : undefined;
-  return typeof value === "number" && Number.isFinite(value) && value > 0
-    ? value
-    : undefined;
+  if (!registration.ok) throw registration.error;
+  return registration.value;
 };