fix: tighten codex app-server lifecycle

extensions/codex/provider.test.ts

@@ -1,7 +1,10 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { buildCodexProvider, buildCodexProviderCatalog } from "./provider.js";
 import { CodexAppServerClient } from "./src/app-server/client.js";
-import { resetSharedCodexAppServerClientForTests } from "./src/app-server/shared-client.js";
+import {
+  getSharedCodexAppServerClient,
+  resetSharedCodexAppServerClientForTests,
+} from "./src/app-server/shared-client.js";
 
 afterEach(() => {
   resetSharedCodexAppServerClientForTests();
@@ -37,7 +40,7 @@ describe("codex provider", () => {
     });
 
     expect(listModels).toHaveBeenCalledWith(
-      expect.objectContaining({ limit: 100, timeoutMs: 1234 }),
+      expect.objectContaining({ limit: 100, timeoutMs: 1234, sharedClient: false }),
     );
     expect(result.provider).toMatchObject({
       auth: "token",
@@ -103,6 +106,32 @@ describe("codex provider", () => {
     expect(client.close).toHaveBeenCalledTimes(1);
   });
 
+  it("does not close an active shared app-server client during live discovery", async () => {
+    const activeClient = {
+      initialize: vi.fn(async () => undefined),
+      request: vi.fn(async () => ({ data: [] })),
+      addCloseHandler: vi.fn(() => () => undefined),
+      close: vi.fn(),
+    } as unknown as CodexAppServerClient;
+    const discoveryClient = {
+      initialize: vi.fn(async () => undefined),
+      request: vi.fn(async () => ({ data: [] })),
+      addCloseHandler: vi.fn(() => () => undefined),
+      close: vi.fn(),
+    } as unknown as CodexAppServerClient;
+    vi.spyOn(CodexAppServerClient, "start")
+      .mockReturnValueOnce(activeClient)
+      .mockReturnValueOnce(discoveryClient);
+
+    await getSharedCodexAppServerClient({ timeoutMs: 1000 });
+    await buildCodexProviderCatalog({
+      env: { OPENCLAW_CODEX_DISCOVERY_LIVE: "1" },
+    });
+
+    expect(activeClient.close).not.toHaveBeenCalled();
+    expect(discoveryClient.close).toHaveBeenCalledTimes(1);
+  });
+
   it("resolves arbitrary Codex app-server model ids through the codex provider", () => {
     const provider = buildCodexProvider();
 

extensions/codex/provider.ts

@@ -15,7 +15,6 @@ import {
   readCodexPluginConfig,
   resolveCodexAppServerRuntimeOptions,
 } from "./src/app-server/config.js";
-import { clearSharedCodexAppServerClient } from "./src/app-server/shared-client.js";
 
 const PROVIDER_ID = "codex";
 const CODEX_BASE_URL = "https://chatgpt.com/backend-api";
@@ -28,6 +27,7 @@ type CodexModelLister = (options: {
   timeoutMs: number;
   limit?: number;
   startOptions?: CodexAppServerStartOptions;
+  sharedClient?: boolean;
 }) => Promise<CodexAppServerModelListResult>;
 
 type BuildCodexProviderOptions = {
@@ -102,15 +102,11 @@ export async function buildCodexProviderCatalog(
   const timeoutMs = normalizeTimeoutMs(config.discovery?.timeoutMs);
   let discovered: CodexAppServerModel[] = [];
   if (config.discovery?.enabled !== false && !shouldSkipLiveDiscovery(options.env)) {
-    try {
-      discovered = await listModelsBestEffort({
-        listModels: options.listModels ?? listCodexAppServerModels,
-        timeoutMs,
-        startOptions: appServer.start,
-      });
-    } finally {
-      clearSharedCodexAppServerClient();
-    }
+    discovered = await listModelsBestEffort({
+      listModels: options.listModels ?? listCodexAppServerModels,
+      timeoutMs,
+      startOptions: appServer.start,
+    });
   }
   const models = (discovered.length > 0 ? discovered : FALLBACK_CODEX_MODELS).map(
     codexModelToDefinition,
@@ -180,6 +176,7 @@ async function listModelsBestEffort(params: {
       timeoutMs: params.timeoutMs,
       limit: 100,
       startOptions: params.startOptions,
+      sharedClient: false,
     });
     return result.models.filter((model) => !model.hidden);
   } catch {

extensions/codex/src/app-server/models.ts

@@ -1,7 +1,9 @@
 import type { CodexAppServerStartOptions } from "./config.js";
 import { type JsonObject, type JsonValue } from "./protocol.js";
-import { getSharedCodexAppServerClient } from "./shared-client.js";
-import { withTimeout } from "./timeout.js";
+import {
+  createIsolatedCodexAppServerClient,
+  getSharedCodexAppServerClient,
+} from "./shared-client.js";
 
 export type CodexAppServerModel = {
   id: string;
@@ -26,32 +28,39 @@ export type CodexAppServerListModelsOptions = {
   includeHidden?: boolean;
   timeoutMs?: number;
   startOptions?: CodexAppServerStartOptions;
+  sharedClient?: boolean;
 };
 
 export async function listCodexAppServerModels(
   options: CodexAppServerListModelsOptions = {},
 ): Promise<CodexAppServerModelListResult> {
   const timeoutMs = options.timeoutMs ?? 2500;
-  return await withTimeout(
-    (async () => {
-      const client = await getSharedCodexAppServerClient({
+  const useSharedClient = options.sharedClient !== false;
+  const client = useSharedClient
+    ? await getSharedCodexAppServerClient({
+        startOptions: options.startOptions,
+        timeoutMs,
+      })
+    : await createIsolatedCodexAppServerClient({
         startOptions: options.startOptions,
         timeoutMs,
       });
-      const response = await client.request<JsonObject>(
-        "model/list",
-        {
-          limit: options.limit ?? null,
-          cursor: options.cursor ?? null,
-          includeHidden: options.includeHidden ?? null,
-        },
-        { timeoutMs },
-      );
-      return readModelListResult(response);
-    })(),
-    timeoutMs,
-    "codex app-server model/list timed out",
-  );
+  try {
+    const response = await client.request<JsonObject>(
+      "model/list",
+      {
+        limit: options.limit ?? null,
+        cursor: options.cursor ?? null,
+        includeHidden: options.includeHidden ?? null,
+      },
+      { timeoutMs },
+    );
+    return readModelListResult(response);
+  } finally {
+    if (!useSharedClient) {
+      client.close();
+    }
+  }
 }
 
 function readModelListResult(value: JsonValue | undefined): CodexAppServerModelListResult {

extensions/codex/src/app-server/run-attempt.ts

@@ -227,14 +227,10 @@ export async function runCodexAppServerAttempt(
   );
 
   const abortListener = () => {
-    void client
-      .request("turn/interrupt", {
-        threadId: thread.threadId,
-        turnId: activeTurnId,
-      })
-      .catch((error: unknown) => {
-        embeddedAgentLog.debug("codex app-server turn interrupt failed during abort", { error });
-      });
+    interruptCodexTurnBestEffort(client, {
+      threadId: thread.threadId,
+      turnId: activeTurnId,
+    });
     resolveCompletion?.();
   };
   runAbortController.signal.addEventListener("abort", abortListener, { once: true });
@@ -268,6 +264,20 @@ export async function runCodexAppServerAttempt(
   }
 }
 
+function interruptCodexTurnBestEffort(
+  client: CodexAppServerClient,
+  params: {
+    threadId: string;
+    turnId: string;
+  },
+): void {
+  void Promise.resolve()
+    .then(() => client.request("turn/interrupt", params))
+    .catch((error: unknown) => {
+      embeddedAgentLog.debug("codex app-server turn interrupt failed during abort", { error });
+    });
+}
+
 type DynamicToolBuildParams = {
   params: EmbeddedRunAttemptParams;
   resolvedWorkspace: string;

extensions/codex/src/app-server/shared-client.ts

@@ -59,6 +59,23 @@ export async function getSharedCodexAppServerClient(options?: {
   }
 }
 
+export async function createIsolatedCodexAppServerClient(options?: {
+  startOptions?: CodexAppServerStartOptions;
+  timeoutMs?: number;
+}): Promise<CodexAppServerClient> {
+  const startOptions = options?.startOptions ?? resolveCodexAppServerRuntimeOptions().start;
+  const client = CodexAppServerClient.start(startOptions);
+  const initialize = client.initialize();
+  try {
+    await withTimeout(initialize, options?.timeoutMs ?? 0, "codex app-server initialize timed out");
+    return client;
+  } catch (error) {
+    client.close();
+    await initialize.catch(() => undefined);
+    throw error;
+  }
+}
+
 export function resetSharedCodexAppServerClientForTests(): void {
   const state = getSharedCodexAppServerClientState();
   state.client = undefined;

src/agents/model-fallback-observation.ts

@@ -57,6 +57,9 @@ export function logModelFallbackDecision(params: {
   const reasonText = params.reason ?? "unknown";
   const observedError = buildErrorObservationFields(params.error);
   const detailText = observedError.providerErrorMessagePreview ?? observedError.errorPreview;
+  const providerErrorTypeSuffix = observedError.providerErrorType
+    ? ` providerErrorType=${sanitizeForLog(observedError.providerErrorType)}`
+    : "";
   const detailSuffix = detailText ? ` detail=${sanitizeForLog(detailText)}` : "";
   decisionLog.warn("model fallback decision", {
     event: "model_fallback_decision",
@@ -90,6 +93,6 @@ export function logModelFallbackDecision(params: {
     })),
     consoleMessage:
       `model fallback decision: decision=${params.decision} requested=${sanitizeForLog(params.requestedProvider)}/${sanitizeForLog(params.requestedModel)} ` +
-      `candidate=${sanitizeForLog(params.candidate.provider)}/${sanitizeForLog(params.candidate.model)} reason=${reasonText} next=${nextText}${detailSuffix}`,
+      `candidate=${sanitizeForLog(params.candidate.provider)}/${sanitizeForLog(params.candidate.model)} reason=${reasonText}${providerErrorTypeSuffix} next=${nextText}${detailSuffix}`,
   });
 }

src/agents/skills.loadworkspaceskillentries.test.ts

@@ -1,7 +1,9 @@
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { resetLogger, setLoggerOverride } from "../logging/logger.js";
+import { loggingState } from "../logging/state.js";
 import { withPathResolutionEnv } from "../test-utils/env.js";
 import { writeSkill } from "./skills.e2e-test-helpers.js";
 import { loadWorkspaceSkillEntries } from "./skills.js";
@@ -21,6 +23,9 @@ function withWorkspaceHome<T>(workspaceDir: string, cb: () => T): T {
 }
 
 afterEach(async () => {
+  setLoggerOverride(null);
+  loggingState.rawConsole = null;
+  resetLogger();
   await Promise.all(
     tempDirs.splice(0, tempDirs.length).map((dir) => fs.rm(dir, { recursive: true, force: true })),
   );
@@ -282,7 +287,16 @@ describe("loadWorkspaceSkillEntries", () => {
         description: "Outside",
       });
       await fs.mkdir(path.join(workspaceDir, "skills"), { recursive: true });
-      await fs.symlink(escapedSkillDir, path.join(workspaceDir, "skills", "escaped-skill"), "dir");
+      const requestedPath = path.join(workspaceDir, "skills", "escaped-skill");
+      await fs.symlink(escapedSkillDir, requestedPath, "dir");
+      setLoggerOverride({ level: "silent", consoleLevel: "warn" });
+      const warn = vi.fn();
+      loggingState.rawConsole = {
+        log: vi.fn(),
+        info: vi.fn(),
+        warn,
+        error: vi.fn(),
+      };
 
       const entries = loadWorkspaceSkillEntries(workspaceDir, {
         managedSkillsDir: path.join(workspaceDir, ".managed"),
@@ -290,6 +304,15 @@ describe("loadWorkspaceSkillEntries", () => {
       });
 
       expect(entries.map((entry) => entry.skill.name)).not.toContain("outside-skill");
+      const [line] = warn.mock.calls[0] ?? [];
+      const warningLine = String(line);
+      expect(warningLine).toContain(
+        "Skipping skill path that resolves outside its configured root:",
+      );
+      expect(warningLine).toContain("source=openclaw-workspace");
+      expect(warningLine).toContain(`root=${path.join(workspaceDir, "skills")}`);
+      expect(warningLine).toContain(`requested=${requestedPath}`);
+      expect(warningLine).toContain("resolved=");
     },
   );
 

src/agents/skills/workspace.ts

@@ -165,14 +165,24 @@ function tryRealpath(filePath: string): string | null {
 function warnEscapedSkillPath(params: {
   source: string;
   rootDir: string;
+  rootRealPath: string;
   candidatePath: string;
   candidateRealPath: string;
 }) {
+  const rootResolved =
+    path.resolve(params.rootDir) === params.rootRealPath
+      ? ""
+      : ` rootResolved=${params.rootRealPath}`;
   skillsLogger.warn("Skipping skill path that resolves outside its configured root.", {
     source: params.source,
     rootDir: params.rootDir,
+    rootRealPath: params.rootRealPath,
     path: params.candidatePath,
     realPath: params.candidateRealPath,
+    consoleMessage:
+      `Skipping skill path that resolves outside its configured root: ` +
+      `source=${params.source} root=${params.rootDir}${rootResolved} ` +
+      `requested=${params.candidatePath} resolved=${params.candidateRealPath}`,
   });
 }
 
@@ -192,6 +202,7 @@ function resolveContainedSkillPath(params: {
   warnEscapedSkillPath({
     source: params.source,
     rootDir: params.rootDir,
+    rootRealPath: params.rootRealPath,
     candidatePath: path.resolve(params.candidatePath),
     candidateRealPath,
   });