refactor: use PI Codex Responses transport#79726
Conversation
|
Codex review: needs changes before merge. Summary Reproducibility: not applicable. for this refactor PR. The blocking regression is source-reproducible from the PR resolver path plus the current boundary-aware transport/request-policy contract. Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Gate PI-native Codex Responses to known native ChatGPT/Codex routes without custom request-policy requirements, and keep custom, proxied, TLS-overridden, or private-network-sensitive routes on OpenClaw's boundary-aware transport. Do we have a high-confidence way to reproduce the issue? Not applicable for this refactor PR. The blocking regression is source-reproducible from the PR resolver path plus the current boundary-aware transport/request-policy contract. Is this the best way to solve the issue? No, not as-is. The safer implementation is to use PI native streaming only when it preserves OpenClaw's request boundary, and otherwise fall back to the existing boundary-aware OpenAI Responses transport. Full review comments:
Overall correctness: patch is incorrect Security concerns:
Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against e8483ea610f2. |
1dad7e4 to
887d993
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 887d99300d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!isOpenAICodexResponsesModel(params.model)) { | ||
| return undefined; | ||
| } | ||
| if (!isDefaultPiStreamFnForModel(params.model, params.currentStreamFn)) { |
There was a problem hiding this comment.
Keep Codex responses on boundary-aware transport for custom routes
This new resolver selects the PI native Codex stream for every openai-codex/openai-codex-responses model as soon as the session stream is “default”, without checking endpoint/request-policy compatibility. That bypasses createBoundaryAwareStreamFnForModel (the OpenClaw path that enforces provider request policy, including guarded fetch behavior used for proxy/TLS/SSRF controls), so Codex models pointed at custom or private OpenAI-compatible base URLs can now run outside the expected transport policy. Please gate the native path by endpoint/policy compatibility (or fall back to boundary-aware transport when custom routing/policy is in play).
Useful? React with 👍 / 👎.
887d993 to
b86b196
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b86b196e8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!isOpenAICodexResponsesModel(params.model)) { | ||
| return undefined; | ||
| } | ||
| if (!isDefaultPiStreamFnForModel(params.model, params.currentStreamFn)) { | ||
| return undefined; | ||
| } | ||
| return piNativeCodexResponsesStreamFnForTest ?? params.currentStreamFn ?? streamSimple; |
There was a problem hiding this comment.
Preserve boundary-aware stream for custom Codex endpoints
resolvePiNativeCodexResponsesStreamFn now routes every default openai-codex/openai-codex-responses run to PI native streaming without checking endpoint/request-policy compatibility, so runs using custom baseUrl or provider request overrides can bypass OpenClaw’s boundary-aware transport (proxy/TLS/guarded-fetch policy path). Fresh evidence in this commit: the old endpoint-class guard was removed with shouldUseOpenAIWebSocketTransport in attempt.thread-helpers.ts, and no equivalent check was added before returning the PI native stream here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9b4a943c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| activeSession.agent.streamFn = resolveEmbeddedAgentStreamFn({ | ||
| currentStreamFn: defaultSessionStreamFn, | ||
| providerStreamFn, | ||
| shouldUseWebSocketTransport, | ||
| wsApiKey, | ||
| sessionId: params.sessionId, |
There was a problem hiding this comment.
Restore websocket routing for OpenAI Responses models
This call now resolves the embedded stream without any transport-routing inputs, so openai/openai-responses attempts can no longer take the WebSocket path and always fall back to the boundary-aware HTTP stream. Before this commit, shouldUseOpenAIWebSocketTransportForAttempt(...) handled transport intent (including explicit "websocket"), but after its removal the transport: "websocket" override is effectively ignored in this path and "auto" no longer does WebSocket-first for OpenAI Responses runs.
Useful? React with 👍 / 👎.
Routes explicit OpenAI Codex Responses runs through PI's native WebSocket-capable transport and removes the custom OpenClaw WebSocket implementation.
Routes explicit OpenAI Codex Responses runs through PI's native WebSocket-capable transport and removes the custom OpenClaw WebSocket implementation.
Routes explicit OpenAI Codex Responses runs through PI's native WebSocket-capable transport and removes the custom OpenClaw WebSocket implementation.
Routes explicit OpenAI Codex Responses runs through PI's native WebSocket-capable transport and removes the custom OpenClaw WebSocket implementation.
Summary
openai-codex/*Responses runs through PI native Codex Responses streaming instead of OpenClaw's custom Responses WebSocket stackopenai-ws-*transport, stale-terminal-event tests, WebSocket session cleanup hooks, andopenaiWsWarmupconfig surface0.73.1and refresh affected Codex prompt snapshotsReal behavior proof
Behavior or issue addressed: Explicit
openai-codex/*Responses runs should no longer use OpenClaw's deleted custom OpenAI Responses WebSocket transport. They should resolve to PI native Codex Responses streaming while preserving OpenClaw-owned auth injection, abort signal propagation, session id propagation, and prompt-cache boundary stripping.Real environment tested: Local OpenClaw PR checkout on
refactor/pi-native-codex-responseswith the updated dependency tree installed, running the productionsrc/agents/pi-embedded-runner/stream-resolution.tsmodule through Node/tsx. This is not a mocked unit test runner; the probe imports the same production resolver used by embedded agent runs and substitutes only the final PI stream call to avoid a live OpenAI request with secrets.Exact steps or command run after this patch:
node --import tsx --input-type=module -e '<stream-resolution proof probe>'from the PR checkout. The probe resolved model{ provider: "openai-codex", api: "openai-codex-responses", id: "gpt-5.5" }, invoked the returned stream with redacted auth, and printed the captured PI wrapper options/context.Evidence after fix: copied terminal output from the after-fix probe:
{ "head": "local", "strategy": "pi-native-codex-responses", "piAgentCore": "0.73.1", "customOpenClawWsPath": false, "wrappedOptions": { "sessionId": "live-proof-session", "signal": {}, "apiKey": "redacted-live-token" }, "strippedSystemPrompt": "alpha\nbeta" }Observed result after fix: The explicit OpenAI Codex Responses model resolves to
pi-native-codex-responses; OpenClaw injects the redacted API key, run abort signal, and session id into the PI stream wrapper; and the system prompt cache boundary is stripped before PI receives the prompt. The custom OpenClaw WebSocket path is absent.What was not tested: No live OpenAI network request was sent from the proof probe, to avoid exposing or consuming live credentials in the PR proof. The transport ownership and wrapper behavior are covered by the live production-module probe plus targeted tests and Testbox
pnpm check:changed.Verification
pnpm test src/agents/pi-embedded-runner/stream-resolution.test.ts src/agents/pi-embedded-runner-extraparams.test.ts src/agents/pi-embedded-runner-extraparams-resolve.test.ts src/agents/transport-params-runtime-contract.test.ts src/agents/schema-normalization-runtime-contract.test.ts extensions/openai/openai-provider.test.ts src/agents/pi-embedded-runner/run/attempt.test.ts src/agents/pi-embedded-runner/compact.hooks.test.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts test/scripts/prompt-snapshots.test.tspnpm test src/agents/transport-params-runtime-contract.test.tspnpm exec oxfmt --check --threads=1 CHANGELOG.md docs/concepts/model-providers.md docs/providers/openai.md src/agents/pi-embedded-runner/stream-resolution.tsgit diff --check origin/main...HEADrg -n "openai-ws-|createOpenAIWebSocketStreamFn|releaseWsSession|hasWsSession|shouldUseOpenAIWebSocketTransport|openaiWsWarmup|WebSocket warm-up" src/agents extensions/openai docs src/proxy-capture test/helpers -g '*.ts' -g '*.md' -g '*.json'returned no matchesnode --import tsx --input-type=module -e '<stream-resolution proof probe>'tbx_01kr5xx5k7tvsj5n2tggj8q2s1:pnpm check:changedpassed after the PI/Codex test and lint fixesRefs #78055.
Supersedes #78142, #78146, #78147.