Skip to content

Preserve Responses API phase param#43475

Merged
steipete merged 1 commit into
openclaw:mainfrom
by-openai:dev/by/preserve-phase
Mar 11, 2026
Merged

Preserve Responses API phase param#43475
steipete merged 1 commit into
openclaw:mainfrom
by-openai:dev/by/preserve-phase

Conversation

@by-openai

Copy link
Copy Markdown
Contributor

Summary

Describe the problem and fix in 2–5 bullets:

  • Problem: Message phase is not preserved across Responses API requests
  • Why it matters: This is important for model behavior on new OpenAI models.
  • What changed: Preserve phase in openclaw representation of Responses API items

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • API / contracts

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? Changes calls to Responses API
  • Command/tool execution surface changed? No
  • Data access scope changed? No

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Mar 11, 2026
@greptile-apps

greptile-apps Bot commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR preserves the Responses API phase parameter ("commentary" | "final_answer") across round-trips by extracting it from response output items into the internal AssistantMessage representation and re-injecting it when converting messages back to API input items.

Key changes:

  • openai-ws-connection.ts — Introduces the OpenAIResponsesAssistantPhase type and adds the optional phase field to both OutputItem (message variant) and InputItem (message variant).
  • openai-ws-stream.tsbuildAssistantMessageFromResponse collects the phase from output items and attaches it to the produced AssistantMessage via a spread; convertMessagesToInputItems reads that phase back and includes it on every generated assistant message input item.
  • openai-ws-stream.test.ts — Adds targeted tests for all four affected paths (plain assistant message, assistant message with tool call, buildAssistantMessageFromResponse, and the full WebSocket stream path).

Minor style observations:

  • The InputItem message type union allows phase on all roles (system, developer, user, assistant), while only assistant messages actually carry a phase from the API. Narrowing the type would tighten the contract.
  • buildAssistantMessageFromResponse takes the last phase encountered when multiple message output items exist in one response — a low-risk edge case given current API behaviour but worth documenting.

Confidence Score: 4/5

  • This PR is safe to merge; it is a targeted, well-tested bug fix with no security implications.
  • The change is narrow in scope, has good test coverage across all affected code paths, and introduces no new network calls beyond those already present. The two noted issues (over-broad InputItem type and last-phase-wins aggregation) are style/edge-case concerns rather than bugs that affect current behaviour.
  • No files require special attention.

Comments Outside Diff (2)

  1. src/agents/openai-ws-connection.ts, line 191-197 (link)

    phase permitted on all message roles

    The phase field is added to the union member that covers role: "system" | "developer" | "user" | "assistant", but phase is only meaningful for "assistant" role messages. Allowing it on system/developer/user messages could silently accept invalid data from callers.

    Consider narrowing the type so only the "assistant" role carries phase:

      | {
          type: "message";
          role: "system" | "developer" | "user";
          content: string | ContentPart[];
        }
      | {
          type: "message";
          role: "assistant";
          content: string | ContentPart[];
          phase?: OpenAIResponsesAssistantPhase;
        }

    This makes the type-level contract match the API semantics and lets TypeScript catch accidental misuse.

  2. src/agents/openai-ws-stream.ts, line 304-313 (link)

    Last phase wins when multiple message items have different phases

    assistantPhase is overwritten on every "message" output item in the loop, so if a response contains — for example — a "commentary" message followed by a "final_answer" message, only "final_answer" is attached to the produced AssistantMessage, even though the aggregated content array holds text from both phases.

    In practice the OpenAI Responses API appears to only emit a single phase per output turn, so this is low-risk today. However, it could become a silent correctness bug if that assumption changes. Consider either:

    • Taking the first non-undefined phase (fail-safe), or
    • Asserting/logging a warning when two different phases are encountered in the same response.

Last reviewed commit: dacc064

@steipete steipete merged commit cced1e0 into openclaw:main Mar 11, 2026
18 of 31 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Landed via temp rebase onto main.

  • Gate: pnpm build && pnpm check && pnpm test && pnpm check:docs
  • Source head commit: dacc064
  • Land commit: 55a7eab
  • Merge commit: cced1e0

Thanks @by-openai!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants