fix: route codex responses over websocket and preserve tool warnings (#53702) (thanks @Nanako0129)

* fix: route codex responses over websocket and suppress gated core tool warnings * fix: rebase codex websocket patch onto main * fix: preserve explicit alsoAllow warnings (#53702) (thanks @Nanako0129) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
2026-03-27 09:21:35 +07:00 · 2026-03-26 14:58:17 +08:00
parent 00e932a83c
commit d72cc7a380
8 changed files with 153 additions and 13 deletions
--- a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts
+++ b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts
@@ -425,7 +425,7 @@ let runEmbeddedAttemptPromise:
  | Promise<typeof import("./attempt.js").runEmbeddedAttempt>
  | undefined;

-export async function loadRunEmbeddedAttempt() {
+async function loadRunEmbeddedAttempt() {
  runEmbeddedAttemptPromise ??= import("./attempt.js").then((mod) => mod.runEmbeddedAttempt);
  return await runEmbeddedAttemptPromise;
 }
--- a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.websocket.test.ts
+++ b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.websocket.test.ts
@@ -0,0 +1,43 @@
+import { describe, expect, it } from "vitest";
+import { shouldUseOpenAIWebSocketTransport } from "./attempt.thread-helpers.js";
+
+describe("openai websocket transport selection", () => {
+  it("accepts the direct OpenAI responses transport pair", () => {
+    expect(
+      shouldUseOpenAIWebSocketTransport({
+        provider: "openai",
+        modelApi: "openai-responses",
+      }),
+    ).toBe(true);
+  });
+
+  it("accepts the Codex responses transport pair", () => {
+    expect(
+      shouldUseOpenAIWebSocketTransport({
+        provider: "openai-codex",
+        modelApi: "openai-codex-responses",
+      }),
+    ).toBe(true);
+  });
+
+  it("rejects mismatched OpenAI websocket transport pairs", () => {
+    expect(
+      shouldUseOpenAIWebSocketTransport({
+        provider: "openai",
+        modelApi: "openai-codex-responses",
+      }),
+    ).toBe(false);
+    expect(
+      shouldUseOpenAIWebSocketTransport({
+        provider: "openai-codex",
+        modelApi: "openai-responses",
+      }),
+    ).toBe(false);
+    expect(
+      shouldUseOpenAIWebSocketTransport({
+        provider: "anthropic",
+        modelApi: "openai-responses",
+      }),
+    ).toBe(false);
+  });
+});
--- a/src/agents/pi-embedded-runner/run/attempt.thread-helpers.ts
+++ b/src/agents/pi-embedded-runner/run/attempt.thread-helpers.ts
@@ -31,6 +31,16 @@ export function resolveAttemptSpawnWorkspaceDir(params: {
    : undefined;
 }

+export function shouldUseOpenAIWebSocketTransport(params: {
+  provider: string;
+  modelApi?: string | null;
+}): boolean {
+  return (
+    (params.modelApi === "openai-responses" && params.provider === "openai") ||
+    (params.modelApi === "openai-codex-responses" && params.provider === "openai-codex")
+  );
+}
+
 export function shouldAppendAttemptCacheTtl(params: {
  timedOutDuringCompaction: boolean;
  compactionOccurredThisAttempt: boolean;
--- a/src/agents/pi-embedded-runner/run/attempt.ts
+++ b/src/agents/pi-embedded-runner/run/attempt.ts
@@ -148,6 +148,7 @@ import {
  appendAttemptCacheTtlIfNeeded,
  composeSystemPromptWithHookContext,
  resolveAttemptSpawnWorkspaceDir,
+  shouldUseOpenAIWebSocketTransport,
 } from "./attempt.thread-helpers.js";
 import { waitForCompactionRetryWithAggregateTimeout } from "./compaction-retry-aggregate-timeout.js";
 import {
@@ -2247,7 +2248,12 @@ export async function runEmbeddedAttempt(
        });
        activeSession.agent.streamFn = ollamaStreamFn;
        ensureCustomApiRegistered(params.model.api, ollamaStreamFn);
-      } else if (params.model.api === "openai-responses" && params.provider === "openai") {
+      } else if (
+        shouldUseOpenAIWebSocketTransport({
+          provider: params.provider,
+          modelApi: params.model.api,
+        })
+      ) {
        const wsApiKey = await params.authStorage.getApiKey(params.provider);
        if (wsApiKey) {
          activeSession.agent.streamFn = createOpenAIWebSocketStreamFn(wsApiKey, params.sessionId, {
--- a/src/agents/pi-tools.ts
+++ b/src/agents/pi-tools.ts
@@ -589,8 +589,10 @@ export function createOpenClawCodingTools(options?: {
      ...buildDefaultToolPolicyPipelineSteps({
        profilePolicy: profilePolicyWithAlsoAllow,
        profile,
+        profileAlsoAllow,
        providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
        providerProfile,
+        providerProfileAlsoAllow,
        globalPolicy,
        globalProviderPolicy,
        agentPolicy,
--- a/src/agents/tool-policy-pipeline.test.ts
+++ b/src/agents/tool-policy-pipeline.test.ts
@@ -52,7 +52,7 @@ describe("tool-policy-pipeline", () => {
    expect(warnings[0]).toContain("unknown entries (wat)");
  });

-  test("warns gated core tools as unavailable instead of plugin-only unknowns", () => {
+  test("suppresses built-in profile warnings for unavailable gated core tools", () => {
    const warnings: string[] = [];
    const tools = [{ name: "exec" }] as unknown as DummyTool[];
    applyToolPolicyPipeline({
@@ -66,6 +66,52 @@ describe("tool-policy-pipeline", () => {
          policy: { allow: ["apply_patch"] },
          label: "tools.profile (coding)",
          stripPluginOnlyAllowlist: true,
+          suppressUnavailableCoreToolWarning: true,
+        },
+      ],
+    });
+    expect(warnings).toEqual([]);
+  });
+
+  test("still warns for profile steps when explicit alsoAllow entries are present", () => {
+    const warnings: string[] = [];
+    const tools = [{ name: "exec" }] as unknown as DummyTool[];
+    applyToolPolicyPipeline({
+      // oxlint-disable-next-line typescript/no-explicit-any
+      tools: tools as any,
+      // oxlint-disable-next-line typescript/no-explicit-any
+      toolMeta: () => undefined,
+      warn: (msg) => warnings.push(msg),
+      steps: [
+        {
+          policy: { allow: ["apply_patch"] },
+          label: "tools.profile (coding)",
+          stripPluginOnlyAllowlist: true,
+          suppressUnavailableCoreToolWarning: false,
+        },
+      ],
+    });
+    expect(warnings.length).toBe(1);
+    expect(warnings[0]).toContain("unknown entries (apply_patch)");
+    expect(warnings[0]).toContain(
+      "shipped core tools but unavailable in the current runtime/provider/model/config",
+    );
+  });
+
+  test("still warns for explicit allowlists that mention unavailable gated core tools", () => {
+    const warnings: string[] = [];
+    const tools = [{ name: "exec" }] as unknown as DummyTool[];
+    applyToolPolicyPipeline({
+      // oxlint-disable-next-line typescript/no-explicit-any
+      tools: tools as any,
+      // oxlint-disable-next-line typescript/no-explicit-any
+      toolMeta: () => undefined,
+      warn: (msg) => warnings.push(msg),
+      steps: [
+        {
+          policy: { allow: ["apply_patch"] },
+          label: "tools.allow",
+          stripPluginOnlyAllowlist: true,
        },
      ],
    });
@@ -88,8 +134,8 @@ describe("tool-policy-pipeline", () => {
      warn: (msg: string) => warnings.push(msg),
      steps: [
        {
-          policy: { allow: ["apply_patch"] },
-          label: "tools.profile (coding)",
+          policy: { allow: ["wat"] },
+          label: "tools.allow",
          stripPluginOnlyAllowlist: true,
        },
      ],
--- a/src/agents/tool-policy-pipeline.ts
+++ b/src/agents/tool-policy-pipeline.ts
@@ -30,13 +30,16 @@ export type ToolPolicyPipelineStep = {
  policy: ToolPolicyLike | undefined;
  label: string;
  stripPluginOnlyAllowlist?: boolean;
+  suppressUnavailableCoreToolWarning?: boolean;
 };

 export function buildDefaultToolPolicyPipelineSteps(params: {
  profilePolicy?: ToolPolicyLike;
  profile?: string;
+  profileAlsoAllow?: string[];
  providerProfilePolicy?: ToolPolicyLike;
  providerProfile?: string;
+  providerProfileAlsoAllow?: string[];
  globalPolicy?: ToolPolicyLike;
  globalProviderPolicy?: ToolPolicyLike;
  agentPolicy?: ToolPolicyLike;
@@ -52,6 +55,8 @@ export function buildDefaultToolPolicyPipelineSteps(params: {
      policy: params.profilePolicy,
      label: profile ? `tools.profile (${profile})` : "tools.profile",
      stripPluginOnlyAllowlist: true,
+      suppressUnavailableCoreToolWarning:
+        !Array.isArray(params.profileAlsoAllow) || params.profileAlsoAllow.length === 0,
    },
    {
      policy: params.providerProfilePolicy,
@@ -59,6 +64,9 @@ export function buildDefaultToolPolicyPipelineSteps(params: {
        ? `tools.byProvider.profile (${providerProfile})`
        : "tools.byProvider.profile",
      stripPluginOnlyAllowlist: true,
+      suppressUnavailableCoreToolWarning:
+        !Array.isArray(params.providerProfileAlsoAllow) ||
+        params.providerProfileAlsoAllow.length === 0,
    },
    { policy: params.globalPolicy, label: "tools.allow", stripPluginOnlyAllowlist: true },
    {
@@ -113,14 +121,22 @@ export function applyToolPolicyPipeline(params: {
          isKnownCoreToolId(entry),
        );
        const otherEntries = resolved.unknownAllowlist.filter((entry) => !isKnownCoreToolId(entry));
-        const suffix = describeUnknownAllowlistSuffix({
-          strippedAllowlist: resolved.strippedAllowlist,
-          hasGatedCoreEntries: gatedCoreEntries.length > 0,
-          hasOtherEntries: otherEntries.length > 0,
-        });
-        const warning = `tools: ${step.label} allowlist contains unknown entries (${entries}). ${suffix}`;
-        if (rememberToolPolicyWarning(warning)) {
-          params.warn(warning);
+        if (
+          !shouldSuppressUnavailableCoreToolWarning({
+            suppressUnavailableCoreToolWarning: step.suppressUnavailableCoreToolWarning === true,
+            hasGatedCoreEntries: gatedCoreEntries.length > 0,
+            hasOtherEntries: otherEntries.length > 0,
+          })
+        ) {
+          const suffix = describeUnknownAllowlistSuffix({
+            strippedAllowlist: resolved.strippedAllowlist,
+            hasGatedCoreEntries: gatedCoreEntries.length > 0,
+            hasOtherEntries: otherEntries.length > 0,
+          });
+          const warning = `tools: ${step.label} allowlist contains unknown entries (${entries}). ${suffix}`;
+          if (rememberToolPolicyWarning(warning)) {
+            params.warn(warning);
+          }
        }
      }
      policy = resolved.policy;
@@ -132,6 +148,21 @@ export function applyToolPolicyPipeline(params: {
  return filtered;
 }

+function shouldSuppressUnavailableCoreToolWarning(params: {
+  suppressUnavailableCoreToolWarning: boolean;
+  hasGatedCoreEntries: boolean;
+  hasOtherEntries: boolean;
+}): boolean {
+  if (
+    !params.suppressUnavailableCoreToolWarning ||
+    !params.hasGatedCoreEntries ||
+    params.hasOtherEntries
+  ) {
+    return false;
+  }
+  return true;
+}
+
 function describeUnknownAllowlistSuffix(params: {
  strippedAllowlist: boolean;
  hasGatedCoreEntries: boolean;
--- a/src/gateway/tools-invoke-http.ts
+++ b/src/gateway/tools-invoke-http.ts
@@ -278,8 +278,10 @@ export async function handleToolsInvokeHttpRequest(
      ...buildDefaultToolPolicyPipelineSteps({
        profilePolicy: profilePolicyWithAlsoAllow,
        profile,
+        profileAlsoAllow,
        providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
        providerProfile,
+        providerProfileAlsoAllow,
        globalPolicy,
        globalProviderPolicy,
        agentPolicy,