Skip to content

fix(relay): handle Codex Responses terminal events - #685

Open
jjcc123312 wants to merge 6 commits into
mainfrom
codex/fix-codex-responses-terminal
Open

fix(relay): handle Codex Responses terminal events#685
jjcc123312 wants to merge 6 commits into
mainfrom
codex/fix-codex-responses-terminal

Conversation

@jjcc123312

@jjcc123312 jjcc123312 commented Aug 11, 2026

Copy link
Copy Markdown

Problem / background

Codex channels can terminate native streaming /v1/responses requests with response.done or response.failed. The shared Responses stream handler only recognized response.completed and response.incomplete as terminal events.

This caused successful Codex requests ending with response.done to settle with zero token usage, while response.failed could be treated as a successful HTTP 200 stream.

Evidence

Production request 202608110724223116292048268d9d6GBb22Z6z completed with HTTP 200 and EOF but recorded zero prompt and completion tokens. A production sample showed the same pattern across multiple Codex channels, so the issue was not isolated to one channel configuration.

Root cause

OaiResponsesStreamHandler forwarded all SSE events but extracted terminal usage only from response.completed and response.incomplete. Codex-specific terminal event names were therefore ignored.

Scope / design

The behavior change is gated by ChannelTypeCodex:

  • Extract usage and image-generation metadata from Codex response.done.
  • Convert Codex response.failed into a relay error.
  • If failure arrives before any bytes are committed, suppress the failed SSE event and allow channel retry.
  • If output is already committed, forward the failed event and mark the error skip-retry to prevent duplicated streams.
  • Preserve existing behavior for all non-Codex channel types.

Impact and risks

This changes native Responses streaming, retry behavior, and billing settlement for Codex channels only. The shared handler is used by other providers, so regression coverage explicitly verifies non-Codex isolation.

Production is multi-node, but the change is request-local and introduces no process-local coordination or persistent state.

Validation

  • go test ./relay/channel/openai ./relay/channel/codex ./relay -run "TestOaiResponsesStreamHandler|TestRelayChatOverCodex|TestResponses" -count=1
  • go vet ./relay/channel/openai ./relay/channel/codex
  • git diff --check origin/main...HEAD
  • Targeted terminal-event tests repeated 100 times during independent review.
  • GitNexus compare result: 2 files, 1 changed runtime symbol, low risk, no additional indexed execution flows.

The broader go test ./relay/... suite has unrelated pre-existing failures in BlockRun initialization, Claude file-content assertions, and Codex image settings. Race testing also exposes an existing race in relay/helper/stream_scanner.go; neither is introduced by this change.

Deployment recommendation

  • Router deploy: required — the diff affects /v1/responses relay streaming, retry decisions, and usage settlement.
  • Other deploy targets: newapi-console, newapi-web, Terraform, and Cloudflare are not required.
  • Minimum release validation: smoke-test one successful Codex native Responses stream ending in response.done and one response.failed path before production rollout.

Review follow-up

Code review identified two retry-state issues in the initial implementation:

  • A pre-commit response.failed attempt had already installed SSE/Codex response headers, so an exhausted retry could return a JSON error labeled text/event-stream, and another provider retry could inherit stale headers.
  • The suppressed failure had already recorded first-response timing and stopped the first-response watchdog, so the next attempt could lose FRT protection and report misleading TTFT metrics.

Follow-up commit 702918e4f fixes both while remaining Codex-only:

  • Snapshot and restore the pre-attempt response headers and event_stream_headers_set state before returning a retryable failure.
  • Reset first-response observation state only for an uncommitted Codex response.failed, re-arming the next attempt's watchdog.
  • Preserve the existing post-commit behavior: forward the failed event and skip retry.
  • Add regression coverage for JSON Content-Type restoration, stale Codex header removal, preservation of pre-existing headers, first-response state reset, and a successful second attempt.

The new failure/retry tests and RelayInfo reset test passed 100 consecutive runs. Independent follow-up review approved the amended diff with no remaining findings.

@KingCesc

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 commit 3ed7b3f3 · 共 2 条

relay/channel/openai/relay_responses.go

  • L135-137: [阻塞] 这里在 Codex 已经向客户端写出部分流内容后仍直接返回 terminalErr,上层 responses_handlernewAPIError != nil 时会立即返回,跳过后续 PostTextConsumeQuota 计费逻辑。这样上游已产生并下发的部分输出不会被结算,存在计费漏扣风险。建议对已写出响应的 response.failed 仅阻止重试并继续走本函数后面的 usage 估算/返回成功路径,或在上层显式支持“带 usage 的错误”计费。
if terminalErr != nil && !types.IsSkipRetryError(terminalErr) {
		return usage, terminalErr
	}
  • L206-208: [严重] 这里把所有 Codex response.failed 都包装成 500;当上游错误实际是参数错误、鉴权失败、策略拒绝等不可重试错误时,在响应尚未写出时会被重试策略当作 5xx 继续切换渠道重试,导致请求放大、额外上游消耗,并向客户端返回失真的状态码。建议根据 OpenAIError.Type/Code 映射 4xx/429 等真实语义,无法判断时再兜底 500;对明确不可重试的错误同时设置 SkipRetry
if openAIError := response.GetOpenAIError(); openAIError != nil && openAIError.Message != "" {
			statusCode := codexResponsesFailedStatusCode(openAIError)
			if !skipRetry && !operation_setting.ShouldRetryByStatusCode(statusCode) {
				options = append(options, types.ErrOptionWithSkipRetry())
			}
			return types.WithOpenAIError(*openAIError, statusCode, options...)
		}

@jjcc123312

Copy link
Copy Markdown
Author

已处理 原 review comment 中的两项问题:

已写出部分流内容后返回 terminalErr,会跳过 PostTextConsumeQuota,导致已交付输出未结算。

response.failed 全部包装为 500,会扭曲参数、鉴权、策略和额度错误的状态/重试语义。

修复提交:63f775a96 fix(relay): settle failed Codex partial responses

现象与根因

  1. Codex response.failed 在下游已提交后仍返回错误;ResponsesHelper 原先见到错误立即返回,controller 随后退款,导致部分输出免费。
  2. SSE 本身是 HTTP 200,终止事件的真实语义只在 response.error.code/type 中;统一改成 500 会让 controller 把确定性的客户端错误当成服务端错误处理。

修复范围

  • 仅限 ChannelTypeCodex 的 Responses 流。
  • 已提交且有正 usage(含缺失 terminal usage 时对文本、推理、函数/自定义/MCP/code-interpreter delta 的估算)才执行错误路径结算;错误仍原样返回,且不会额外记录 success relay sample。
  • 实际 usage 结算失败且 BillingSession 仍可退款时,保留预扣额度,避免 controller deferred refund 让已交付输出免费;若 funding 已提交而后续 token 调整报错,则保留实际结算额度,不用预扣额度覆盖。
  • response.error.code 优先于 type 映射 400/401/403/429/503;未知、server_errorvector_store_timeout 回退 500。
  • pre-commit 不在适配器内强制 SkipRetry,继续由 controller 的集中式、可配置状态码策略决定跨渠道重试;post-commit 始终禁止重试。
  • 同步更新设计文档,明确 Codex 部分失败的结算边界与多节点语义。

验证

  • 目标测试(包含成功、pre/post-commit failure、usage 捕获/估算、错误码映射、结算成功及两类结算失败)重复 100 次通过。
  • go vet ./relay/common ./relay/channel/openai ./relay/channel/codex ./relay ./service 通过。
  • go build ./relay/... ./service/... 通过。
  • git diff --check 通过。
  • GitNexus compare-main 变更检测:low risk,无意外 execution flow。
  • 独立 code-reviewer:APPROVE;architect:CLEAR。

备注:仓库根 go build ./... 仍因本 worktree 缺少预构建的 web/classic/dist embed 目录而无法执行,与本次 Go diff 无关;相关 Go 包已单独构建通过。

部署建议

  • Router deploy: required
  • Reason: 修改了 /v1/responses 流式终止、重试状态和计费结算路径。
  • Other deploy targets: 同一 Go release 的 newapi-console 需保持运行时一致;newapi-web、Terraform、Cloudflare、DB migration 不涉及。
  • Risk / validation: 建议 staging 验证 response.done、pre/post-commit response.failed、400/401/403/429/500 跨渠道行为及结算/退款闭环。

@KingCesc

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 增量 702918e4..63f775a9 · 共 2 条

relay/channel/openai/relay_responses.go

  • L129-135: [严重] 这里把工具参数、MCP 参数和 code interpreter 代码等 delta 都纳入 responseTextBuilder,失败且终止事件不带 usage 时会完整累积后再做 CountTextToken。这些字段可能远大于普通输出文本,且当前没有总长度上限;长流或大工具参数会放大单请求内存占用和 token 统计 CPU,影响服务稳定性。建议对回退统计的累积内容设置最大字节/字符上限,或改为增量计数并在超过上限后停止继续追加。
case "response.output_text.delta",
			"response.reasoning_summary_text.delta",
			"response.reasoning_text.delta",
			"response.function_call_arguments.delta",
			"response.custom_tool_call_input.delta",
			"response.mcp_call_arguments.delta",
			"response.code_interpreter_call_code.delta":
			if responseTextBuilder.Len() < maxResponsesFallbackUsageBytes {
				delta := streamResponse.Delta
				remaining := maxResponsesFallbackUsageBytes - responseTextBuilder.Len()
				if len(delta) > remaining {
					delta = delta[:remaining]
				}
				responseTextBuilder.WriteString(delta)
			}

service/text_quota.go

  • L401-404: [阻塞] 当 SettleBilling 失败且后续保留预扣也失败/不适用时,当前流程仍会继续更新用户/渠道用量并写入消费日志,导致记录的 summary.Quota 与实际已完成的扣费状态不一致;在 Codex 终止错误路径中还可能随后走失败退款,形成“日志显示已消费但余额未扣/已退”的计费数据错误。建议显式标记结算是否已生效,仅在结算成功或保留预扣成功后再更新用量和记录消费日志;未生效时直接返回原始结算错误。
settlementApplied := settleErr == nil
	// 在保留预扣成功的分支中同步设置 settlementApplied = true。
	if !settlementApplied {
		return settleErr
	}
	if summary.TotalTokens != 0 {
		model.UpdateUserUsedQuotaAndRequestCount(relayInfo.UserId, summary.Quota)
		model.UpdateChannelUsedQuota(relayInfo.ChannelId, summary.Quota)
	}

@jjcc123312

Copy link
Copy Markdown
Author

已处理 本轮 review comment 的两项问题。

工具参数、MCP 参数和 code interpreter delta 无上限累积,会放大单请求内存和 token 统计 CPU。

结算与预扣保留均未生效时仍写用户/渠道用量和消费日志,会造成账务记录与实际扣费不一致。

修复提交:9456ec8c6 fix(relay): bound Codex fallback accounting

修复

  1. fallback usage buffer 增加 1 MiB 累计硬上限:

    • 所有文本/推理/工具类 delta 共用同一上限;
    • 达到上限后不再追加;
    • terminal usage 正常返回时不依赖该 fallback;
    • 避免长工具参数造成无界内存与 tokenizer CPU。
  2. 显式跟踪 settlement 是否确实生效:

    • 实际结算成功:正常写用量/日志;
    • funding 已提交但后续 token 调整报错:视为已生效,保留实际结算额度;
    • 实际结算失败但成功保留预扣:按预扣额度写记录;
    • 实际结算与预扣保留均失败:立即返回,不更新用户/渠道用量,不写消费日志,controller 后续可按原失败路径退款。
  3. 未扩展高影响的 relaycommon.BillingSettler 公共接口;使用 service 内部、mutex 保护的只读结算状态能力,避免扩大共享接口影响面。

  4. 设计文档同步记录 1 MiB 资源上限和“结算生效后才记账”的约束。

验证

  • 新增 fallback buffer 累计上限回归测试。
  • 新增“实际结算与预扣保留均失败时不写 consume log”回归测试。
  • 相关 Codex/Responses/settlement 目标测试重复 100 次通过。
  • go vet ./relay/common ./relay/channel/openai ./relay/channel/codex ./relay ./service 通过。
  • go build ./relay/... ./service/... 通过。
  • git diff --check 通过。
  • GitNexus compare-main:low risk,无意外 execution flow。
  • 独立 code-reviewer:APPROVE;architect:CLEAR。

部署建议

  • Router deploy: required
  • Reason: 影响 /v1/responses fallback 资源控制和 relay 计费结算。
  • Other deploy targets: newapi-console 随同一 Go runtime 版本部署;newapi-web、Terraform、Cloudflare、DB migration 不涉及。
  • Risk / validation: staging 至少验证带 terminal usage、不带 usage、以及强制结算/保留失败三种 Codex 部分流场景。

@KingCesc

Copy link
Copy Markdown

🤖 OpenCodeReview · 评审 增量 63f775a9..9456ec8c · 共 1 条

service/text_quota.go

  • L406-408: [严重] 这里仅用 settlementApplied 判断是否继续记录消费,但 relayInfo.Billing == nil 的兼容路径里 SettleBilling 会调用 PostConsumeQuota,其资金/订阅扣减和 token 扣减是分步执行的:如果前一步已成功、后一步失败,当前逻辑会直接返回,导致已实际扣费的请求没有更新用量统计/消费日志,形成计费审计不一致。建议让 SettleBilling 返回“是否已提交扣费”的状态(或为兼容路径提供等价状态),只有确认完全未扣费时才提前返回;已发生部分提交时应继续记录日志并返回原错误。
if !recordRelaySample && !settlementApplied {
		// TODO: SettleBilling 需要返回是否已发生资金/订阅扣减;仅在确认完全未扣费时才提前返回。
		return settleErr
	}

@jjcc123312

Copy link
Copy Markdown
Author

Follow-up cleanup addressing the review discussion in comment 1 and comment 2.

Concern: the fix had grown beyond the size expected for a narrowly scoped Codex terminal-accounting correction.

I pushed commit 5c43b27 with a behavior-preserving consolidation:

  • Reduced the follow-up diff to 58 additions / 181 deletions (net -123 lines).
  • Removed the one-use failedCodexResponsesUsageToSettle helper and inlined its narrow predicate at the only call site.
  • Replaced duplicated fallback-usage tests with a table-driven test.
  • Consolidated repeated settlement-test setup into one fixture helper.
  • Kept the optional settlementApplied capability because it distinguishes committed funding from an uncommitted settlement failure without expanding the public BillingSettler interface.

Scope and behavior remain unchanged: Codex channels only; pre-commit failures stay retry-policy controlled; post-commit failures skip retry; partial usage is settled; failed settlement does not emit consume records; fallback buffering remains capped at 1 MiB; existing error mapping is preserved.

Validation:

  • Targeted regression suite across relay/channel/openai, relay/common, relay/channel/codex, relay, and service: PASS at count=100.
  • go vet on affected Go packages: PASS.
  • go build for relay/... and service/...: PASS.
  • git diff --check: PASS.
  • GitNexus compare against main: LOW risk, no additional affected execution flows.
  • Independent simplifier and verifier passes: no actionable findings; behavior-preserving verdict.

@jjcc123312

Copy link
Copy Markdown
Author

Resolved in c73a4f1, following up on the reported compatibility-path audit gap:

relayInfo.Billing == nil can commit the wallet/subscription mutation before the token mutation fails, so treating every returned error as “nothing settled” skips usage statistics and the consume log.

The fix keeps both public signatures unchanged (SettleBilling and PostConsumeQuota still return only error). Their service-local implementations now additionally report whether the funding-side mutation committed. PostTextConsumeQuotaOnError uses that status to continue audit recording only after a real commit, while still returning the original settlement error. A failure before any wallet/subscription mutation still exits without a consume record.

This is multi-node safe: the status is derived directly from the same DB-backed settlement operation; no process-local coordination or shared mutable cache is introduced.

Regression coverage now verifies:

  • the legacy wallet path reports committed funding when a later token debit fails;
  • a committed settlement followed by an error still records exactly one consume log;
  • a completely uncommitted failure still records none.

Validation:

  • Targeted Codex/relay/service regressions: PASS at -count=100.
  • go vet on affected packages: PASS.
  • go build ./relay/... ./service/...: PASS.
  • git diff --check: PASS.
  • GitNexus compare against main: LOW aggregate risk, no additional affected execution flows.

The broad go test ./service/... run also exposed four unrelated existing test-environment failures (SMTP pacing state and missing subscription/quota test tables); the changed billing tests and those four unrelated tests were isolated separately, and the billing regression remains green.

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