Skip to content

fix(responses): forward verified reasoning accounting header - #2035

Open
asuan-dev wants to merge 2 commits into
looplj:unstablefrom
asuan-dev:codex/fix-beta5-reasoning-header
Open

fix(responses): forward verified reasoning accounting header#2035
asuan-dev wants to merge 2 commits into
looplj:unstablefrom
asuan-dev:codex/fix-beta5-reasoning-header

Conversation

@asuan-dev

Copy link
Copy Markdown

Summary

  • Preserve X-Reasoning-Included: true from successful upstream HTTP/SSE responses.
  • Keep non-streaming metadata on the existing httpclient.Response.Headers.
  • Carry streaming metadata through a small httpclient stream wrapper, without changing the generic streams.Stream interface or pipeline/orchestrator result structs.
  • Preserve the header when the Codex executor aggregates a stream into a non-streaming response.
  • Forward no header for missing, false, or conflicting values, and never forward unrelated headers such as Set-Cookie.

Why

AxonHub terminates the upstream HTTP response and rebuilds the downstream response. The old path discarded the handshake header before it reached Codex, so Codex could re-estimate reasoning tokens that the server had already accounted for.

Scope

This change covers the HTTP/SSE response path used by the current Codex channel. Successful WebSocket handshake response headers are not changed here; the WebSocket path has a separate metadata/event contract.

Design

The allowlist and validation live in llm/httpclient. Non-streaming responses reuse the existing response header field. Streaming responses carry a cloned, filtered header alongside the stream and are re-wrapped after protocol transformations. The API boundary applies the same allowlist again as defense in depth.

Validation

  • cd llm && go test ./httpclient ./pipeline ./transformer/openai/codex -count=1
  • go test ./internal/server/api ./internal/server/orchestrator -count=1
  • git diff --check

@asuan-dev
asuan-dev marked this pull request as ready for review July 19, 2026 12:02
@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a header forwarding gap where X-Reasoning-Included: true from upstream HTTP/SSE responses was discarded by AxonHub before reaching Codex, causing Codex to re-estimate reasoning tokens that were already accounted for by the server.

  • New allowlist infrastructure (llm/httpclient/response_headers.go): Introduces MergeForwardResponseHeaders (only passes through X-Reasoning-Included: true, strips everything else including Set-Cookie), WithResponseHeaders (wraps a streams.Stream with metadata without modifying the generic interface), and GetResponseHeaders (type-asserts the private responseHeadersProvider interface to safely retrieve attached metadata).
  • Non-streaming path (pipeline/non_streaming.go): Captures allowed headers from httpResp.Headers after raw middlewares and merges them onto the final response, with a second MergeForwardResponseHeaders pass as defense in depth.
  • Streaming path (pipeline/stream.go): Extracts responseHeaders from the raw outbound stream before any transformation wrappers, then re-attaches them to inboundStream at return — so transformations cannot accidentally lose the metadata.
  • Codex executor (llm/transformer/openai/codex/outbound.go): Propagates the header when the executor aggregates a Codex stream into a non-streaming Response.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to forwarding a single allowlisted header and does not touch auth, data persistence, or any other security boundary.

The header-forwarding logic is strictly allowlisted; every code path (non-streaming, streaming, Codex aggregation, auto-aggregate) captures the header before any transformation wrappers and re-attaches it at the return site, preventing accidental loss. Headers are always cloned on attach and on read, ruling out cross-request mutation. The retry path correctly discards headers from failed attempts and uses only the successful attempt's headers. Tests cover all four paths plus the allowlist edge cases (false values, missing header, conflicting duplicates, Set-Cookie exclusion). No unrelated headers reach the downstream client.

No files require special attention.

Important Files Changed

Filename Overview
llm/httpclient/response_headers.go New file containing the core allowlist logic; correctly clones headers on both attach and read, handles case-insensitive key matching, whitespace trimming, and conflicting duplicate values.
llm/pipeline/stream.go Extracts responseHeaders from the raw outbound stream before any middleware/transform wrappers, then re-attaches them to the final inbound stream via WithResponseHeaders — headers survive all intermediate wrapping.
llm/pipeline/non_streaming.go Captures filtered headers after raw-response middlewares in both notStream and autoAggregateStream, then merges them onto finalResp.Headers at the very end — correctly bridging non-streaming and stream-aggregated paths.
internal/server/api/chat.go writeForwardResponseHeaders is called before any write/flush, so X-Reasoning-Included is committed to the response headers at the correct time for both streaming and non-streaming paths.
llm/transformer/openai/codex/outbound.go MergeForwardResponseHeaders applied to the codexExecutor.Do aggregation path; GetResponseHeaders is called before the deferred stream.Close(), so headers are readable even after the stream is exhausted.
llm/httpclient/client.go Single-line change: DoStream now wraps the decoded stream with MergeForwardResponseHeaders(nil, rawResp.Header) via WithResponseHeaders, injecting allowed upstream headers at the HTTP boundary.
llm/pipeline/pipeline_retry_test.go Retry test correctly asserts that headers from a failed first attempt are not carried over — the second attempt's headers (false) drive an empty result on the final stream.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Upstream as Upstream LLM API
    participant HC as httpclient.DoStream
    participant Pipeline as pipeline.stream()
    participant Codex as codexExecutor.Do()
    participant API as api.writeForwardResponseHeaders
    participant Client as Downstream Client

    Upstream->>HC: HTTP response + X-Reasoning-Included: true
    HC->>HC: MergeForwardResponseHeaders(nil, rawResp.Header)
    HC->>HC: WithResponseHeaders(stream, filteredHeaders)
    HC-->>Pipeline: outboundStream (carries headers)

    Pipeline->>Pipeline: MergeForwardResponseHeaders(nil, GetResponseHeaders(outboundStream))
    note over Pipeline: responseHeaders captured before transformations
    Pipeline->>Pipeline: applyRawStreamMiddlewares → TransformStream → Inbound.TransformStream
    Pipeline->>Pipeline: WithResponseHeaders(inboundStream, responseHeaders)
    Pipeline-->>API: inboundStream (headers re-attached)

    alt Non-streaming (Codex aggregation)
        Pipeline->>Codex: executor.Do → inner.DoStream
        Codex->>Codex: consume stream, AggregateStreamChunks
        Codex->>Codex: MergeForwardResponseHeaders(baseHeaders, GetResponseHeaders(stream))
        Codex-->>Pipeline: "Response{Headers: filtered}"
        Pipeline->>Pipeline: MergeForwardResponseHeaders(nil, httpResp.Headers)
        Pipeline-->>API: "Response{Headers: filtered}"
        API->>API: MergeForwardResponseHeaders(c.Writer.Header(), response.Headers)
    else Streaming
        API->>API: MergeForwardResponseHeaders(c.Writer.Header(), GetResponseHeaders(stream))
    end

    API-->>Client: HTTP response with X-Reasoning-Included: true
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Upstream as Upstream LLM API
    participant HC as httpclient.DoStream
    participant Pipeline as pipeline.stream()
    participant Codex as codexExecutor.Do()
    participant API as api.writeForwardResponseHeaders
    participant Client as Downstream Client

    Upstream->>HC: HTTP response + X-Reasoning-Included: true
    HC->>HC: MergeForwardResponseHeaders(nil, rawResp.Header)
    HC->>HC: WithResponseHeaders(stream, filteredHeaders)
    HC-->>Pipeline: outboundStream (carries headers)

    Pipeline->>Pipeline: MergeForwardResponseHeaders(nil, GetResponseHeaders(outboundStream))
    note over Pipeline: responseHeaders captured before transformations
    Pipeline->>Pipeline: applyRawStreamMiddlewares → TransformStream → Inbound.TransformStream
    Pipeline->>Pipeline: WithResponseHeaders(inboundStream, responseHeaders)
    Pipeline-->>API: inboundStream (headers re-attached)

    alt Non-streaming (Codex aggregation)
        Pipeline->>Codex: executor.Do → inner.DoStream
        Codex->>Codex: consume stream, AggregateStreamChunks
        Codex->>Codex: MergeForwardResponseHeaders(baseHeaders, GetResponseHeaders(stream))
        Codex-->>Pipeline: "Response{Headers: filtered}"
        Pipeline->>Pipeline: MergeForwardResponseHeaders(nil, httpResp.Headers)
        Pipeline-->>API: "Response{Headers: filtered}"
        API->>API: MergeForwardResponseHeaders(c.Writer.Header(), response.Headers)
    else Streaming
        API->>API: MergeForwardResponseHeaders(c.Writer.Header(), GetResponseHeaders(stream))
    end

    API-->>Client: HTTP response with X-Reasoning-Included: true
Loading

Reviews (2): Last reviewed commit: "fix(api): tidy forwarded response header..." | Re-trigger Greptile

Comment thread internal/server/api/chat.go
Comment thread internal/server/api/chat.go Outdated
@looplj

looplj commented Jul 21, 2026

Copy link
Copy Markdown
Owner

需要帮忙确认下,本地是否验证过

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants