Skip to content

Commit d72cc7a

Browse files
Nanako0129obviyus
Nanako0129
and
obviyus
authored
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>
1 parent 00e932a commit d72cc7a

8 files changed

Lines changed: 153 additions & 13 deletions

‎src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ let runEmbeddedAttemptPromise:
425425
| Promise<typeof import("./attempt.js").runEmbeddedAttempt>
426426
| undefined;
427427

428-
export async function loadRunEmbeddedAttempt() {
428+
async function loadRunEmbeddedAttempt() {
429429
runEmbeddedAttemptPromise ??= import("./attempt.js").then((mod) => mod.runEmbeddedAttempt);
430430
return await runEmbeddedAttemptPromise;
431431
}

‎src/agents/pi-embedded-runner/run/attempt.spawn-workspace.websocket.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { describe, expect, it } from "vitest";
2+
import { shouldUseOpenAIWebSocketTransport } from "./attempt.thread-helpers.js";
3+
4+
describe("openai websocket transport selection", () => {
5+
it("accepts the direct OpenAI responses transport pair", () => {
6+
expect(
7+
shouldUseOpenAIWebSocketTransport({
8+
provider: "openai",
9+
modelApi: "openai-responses",
10+
}),
11+
).toBe(true);
12+
});
13+
14+
it("accepts the Codex responses transport pair", () => {
15+
expect(
16+
shouldUseOpenAIWebSocketTransport({
17+
provider: "openai-codex",
18+
modelApi: "openai-codex-responses",
19+
}),
20+
).toBe(true);
21+
});
22+
23+
it("rejects mismatched OpenAI websocket transport pairs", () => {
24+
expect(
25+
shouldUseOpenAIWebSocketTransport({
26+
provider: "openai",
27+
modelApi: "openai-codex-responses",
28+
}),
29+
).toBe(false);
30+
expect(
31+
shouldUseOpenAIWebSocketTransport({
32+
provider: "openai-codex",
33+
modelApi: "openai-responses",
34+
}),
35+
).toBe(false);
36+
expect(
37+
shouldUseOpenAIWebSocketTransport({
38+
provider: "anthropic",
39+
modelApi: "openai-responses",
40+
}),
41+
).toBe(false);
42+
});
43+
});

‎src/agents/pi-embedded-runner/run/attempt.thread-helpers.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ export function resolveAttemptSpawnWorkspaceDir(params: {
3131
: undefined;
3232
}
3333

34+
export function shouldUseOpenAIWebSocketTransport(params: {
35+
provider: string;
36+
modelApi?: string | null;
37+
}): boolean {
38+
return (
39+
(params.modelApi === "openai-responses" && params.provider === "openai") ||
40+
(params.modelApi === "openai-codex-responses" && params.provider === "openai-codex")
41+
);
42+
}
43+
3444
export function shouldAppendAttemptCacheTtl(params: {
3545
timedOutDuringCompaction: boolean;
3646
compactionOccurredThisAttempt: boolean;

‎src/agents/pi-embedded-runner/run/attempt.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ import {
148148
appendAttemptCacheTtlIfNeeded,
149149
composeSystemPromptWithHookContext,
150150
resolveAttemptSpawnWorkspaceDir,
151+
shouldUseOpenAIWebSocketTransport,
151152
} from "./attempt.thread-helpers.js";
152153
import { waitForCompactionRetryWithAggregateTimeout } from "./compaction-retry-aggregate-timeout.js";
153154
import {
@@ -2247,7 +2248,12 @@ export async function runEmbeddedAttempt(
22472248
});
22482249
activeSession.agent.streamFn = ollamaStreamFn;
22492250
ensureCustomApiRegistered(params.model.api, ollamaStreamFn);
2250-
} else if (params.model.api === "openai-responses" && params.provider === "openai") {
2251+
} else if (
2252+
shouldUseOpenAIWebSocketTransport({
2253+
provider: params.provider,
2254+
modelApi: params.model.api,
2255+
})
2256+
) {
22512257
const wsApiKey = await params.authStorage.getApiKey(params.provider);
22522258
if (wsApiKey) {
22532259
activeSession.agent.streamFn = createOpenAIWebSocketStreamFn(wsApiKey, params.sessionId, {

‎src/agents/pi-tools.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,10 @@ export function createOpenClawCodingTools(options?: {
589589
...buildDefaultToolPolicyPipelineSteps({
590590
profilePolicy: profilePolicyWithAlsoAllow,
591591
profile,
592+
profileAlsoAllow,
592593
providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
593594
providerProfile,
595+
providerProfileAlsoAllow,
594596
globalPolicy,
595597
globalProviderPolicy,
596598
agentPolicy,

‎src/agents/tool-policy-pipeline.test.ts‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe("tool-policy-pipeline", () => {
5252
expect(warnings[0]).toContain("unknown entries (wat)");
5353
});
5454

55-
test("warns gated core tools as unavailable instead of plugin-only unknowns", () => {
55+
test("suppresses built-in profile warnings for unavailable gated core tools", () => {
5656
const warnings: string[] = [];
5757
const tools = [{ name: "exec" }] as unknown as DummyTool[];
5858
applyToolPolicyPipeline({
@@ -66,6 +66,52 @@ describe("tool-policy-pipeline", () => {
6666
policy: { allow: ["apply_patch"] },
6767
label: "tools.profile (coding)",
6868
stripPluginOnlyAllowlist: true,
69+
suppressUnavailableCoreToolWarning: true,
70+
},
71+
],
72+
});
73+
expect(warnings).toEqual([]);
74+
});
75+
76+
test("still warns for profile steps when explicit alsoAllow entries are present", () => {
77+
const warnings: string[] = [];
78+
const tools = [{ name: "exec" }] as unknown as DummyTool[];
79+
applyToolPolicyPipeline({
80+
// oxlint-disable-next-line typescript/no-explicit-any
81+
tools: tools as any,
82+
// oxlint-disable-next-line typescript/no-explicit-any
83+
toolMeta: () => undefined,
84+
warn: (msg) => warnings.push(msg),
85+
steps: [
86+
{
87+
policy: { allow: ["apply_patch"] },
88+
label: "tools.profile (coding)",
89+
stripPluginOnlyAllowlist: true,
90+
suppressUnavailableCoreToolWarning: false,
91+
},
92+
],
93+
});
94+
expect(warnings.length).toBe(1);
95+
expect(warnings[0]).toContain("unknown entries (apply_patch)");
96+
expect(warnings[0]).toContain(
97+
"shipped core tools but unavailable in the current runtime/provider/model/config",
98+
);
99+
});
100+
101+
test("still warns for explicit allowlists that mention unavailable gated core tools", () => {
102+
const warnings: string[] = [];
103+
const tools = [{ name: "exec" }] as unknown as DummyTool[];
104+
applyToolPolicyPipeline({
105+
// oxlint-disable-next-line typescript/no-explicit-any
106+
tools: tools as any,
107+
// oxlint-disable-next-line typescript/no-explicit-any
108+
toolMeta: () => undefined,
109+
warn: (msg) => warnings.push(msg),
110+
steps: [
111+
{
112+
policy: { allow: ["apply_patch"] },
113+
label: "tools.allow",
114+
stripPluginOnlyAllowlist: true,
69115
},
70116
],
71117
});
@@ -88,8 +134,8 @@ describe("tool-policy-pipeline", () => {
88134
warn: (msg: string) => warnings.push(msg),
89135
steps: [
90136
{
91-
policy: { allow: ["apply_patch"] },
92-
label: "tools.profile (coding)",
137+
policy: { allow: ["wat"] },
138+
label: "tools.allow",
93139
stripPluginOnlyAllowlist: true,
94140
},
95141
],

‎src/agents/tool-policy-pipeline.ts‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,16 @@ export type ToolPolicyPipelineStep = {
3030
policy: ToolPolicyLike | undefined;
3131
label: string;
3232
stripPluginOnlyAllowlist?: boolean;
33+
suppressUnavailableCoreToolWarning?: boolean;
3334
};
3435

3536
export function buildDefaultToolPolicyPipelineSteps(params: {
3637
profilePolicy?: ToolPolicyLike;
3738
profile?: string;
39+
profileAlsoAllow?: string[];
3840
providerProfilePolicy?: ToolPolicyLike;
3941
providerProfile?: string;
42+
providerProfileAlsoAllow?: string[];
4043
globalPolicy?: ToolPolicyLike;
4144
globalProviderPolicy?: ToolPolicyLike;
4245
agentPolicy?: ToolPolicyLike;
@@ -52,13 +55,18 @@ export function buildDefaultToolPolicyPipelineSteps(params: {
5255
policy: params.profilePolicy,
5356
label: profile ? `tools.profile (${profile})` : "tools.profile",
5457
stripPluginOnlyAllowlist: true,
58+
suppressUnavailableCoreToolWarning:
59+
!Array.isArray(params.profileAlsoAllow) || params.profileAlsoAllow.length === 0,
5560
},
5661
{
5762
policy: params.providerProfilePolicy,
5863
label: providerProfile
5964
? `tools.byProvider.profile (${providerProfile})`
6065
: "tools.byProvider.profile",
6166
stripPluginOnlyAllowlist: true,
67+
suppressUnavailableCoreToolWarning:
68+
!Array.isArray(params.providerProfileAlsoAllow) ||
69+
params.providerProfileAlsoAllow.length === 0,
6270
},
6371
{ policy: params.globalPolicy, label: "tools.allow", stripPluginOnlyAllowlist: true },
6472
{
@@ -113,14 +121,22 @@ export function applyToolPolicyPipeline(params: {
113121
isKnownCoreToolId(entry),
114122
);
115123
const otherEntries = resolved.unknownAllowlist.filter((entry) => !isKnownCoreToolId(entry));
116-
const suffix = describeUnknownAllowlistSuffix({
117-
strippedAllowlist: resolved.strippedAllowlist,
118-
hasGatedCoreEntries: gatedCoreEntries.length > 0,
119-
hasOtherEntries: otherEntries.length > 0,
120-
});
121-
const warning = `tools: ${step.label} allowlist contains unknown entries (${entries}). ${suffix}`;
122-
if (rememberToolPolicyWarning(warning)) {
123-
params.warn(warning);
124+
if (
125+
!shouldSuppressUnavailableCoreToolWarning({
126+
suppressUnavailableCoreToolWarning: step.suppressUnavailableCoreToolWarning === true,
127+
hasGatedCoreEntries: gatedCoreEntries.length > 0,
128+
hasOtherEntries: otherEntries.length > 0,
129+
})
130+
) {
131+
const suffix = describeUnknownAllowlistSuffix({
132+
strippedAllowlist: resolved.strippedAllowlist,
133+
hasGatedCoreEntries: gatedCoreEntries.length > 0,
134+
hasOtherEntries: otherEntries.length > 0,
135+
});
136+
const warning = `tools: ${step.label} allowlist contains unknown entries (${entries}). ${suffix}`;
137+
if (rememberToolPolicyWarning(warning)) {
138+
params.warn(warning);
139+
}
124140
}
125141
}
126142
policy = resolved.policy;
@@ -132,6 +148,21 @@ export function applyToolPolicyPipeline(params: {
132148
return filtered;
133149
}
134150

151+
function shouldSuppressUnavailableCoreToolWarning(params: {
152+
suppressUnavailableCoreToolWarning: boolean;
153+
hasGatedCoreEntries: boolean;
154+
hasOtherEntries: boolean;
155+
}): boolean {
156+
if (
157+
!params.suppressUnavailableCoreToolWarning ||
158+
!params.hasGatedCoreEntries ||
159+
params.hasOtherEntries
160+
) {
161+
return false;
162+
}
163+
return true;
164+
}
165+
135166
function describeUnknownAllowlistSuffix(params: {
136167
strippedAllowlist: boolean;
137168
hasGatedCoreEntries: boolean;

‎src/gateway/tools-invoke-http.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,10 @@ export async function handleToolsInvokeHttpRequest(
278278
...buildDefaultToolPolicyPipelineSteps({
279279
profilePolicy: profilePolicyWithAlsoAllow,
280280
profile,
281+
profileAlsoAllow,
281282
providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
282283
providerProfile,
284+
providerProfileAlsoAllow,
283285
globalPolicy,
284286
globalProviderPolicy,
285287
agentPolicy,

0 commit comments

Comments
 (0)