Skip to content

Commit f62d169

Browse files
committed
fix: preserve request option values on invalid updates
1 parent bf882b7 commit f62d169

2 files changed

Lines changed: 123 additions & 12 deletions

File tree

lib/codex_pooler/gateway/payloads/request_options.ex

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -624,8 +624,12 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptions do
624624
defp maybe_put_websocket_owner_update(updates, key, value, normalizer \\ & &1)
625625
defp maybe_put_websocket_owner_update(updates, _key, nil, _normalizer), do: updates
626626

627-
defp maybe_put_websocket_owner_update(updates, key, value, normalizer),
628-
do: Map.put(updates, key, normalizer.(value))
627+
defp maybe_put_websocket_owner_update(updates, key, value, normalizer) do
628+
case normalizer.(value) do
629+
nil -> updates
630+
value -> Map.put(updates, key, value)
631+
end
632+
end
629633

630634
defp websocket_owner_forwarder_opts(values) when is_list(values), do: values
631635
defp websocket_owner_forwarder_opts(_value), do: []
@@ -680,6 +684,13 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptions do
680684

681685
defp forwarded_headers(_headers), do: []
682686

687+
defp forwarded_headers_update(headers) do
688+
case forwarded_headers(headers) do
689+
[] -> nil
690+
headers -> headers
691+
end
692+
end
693+
683694
defp safe_client_request_id(value) when is_binary(value) do
684695
value
685696
|> String.trim()
@@ -772,9 +783,9 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptions do
772783

773784
defp normalize_continuity_updates(updates) do
774785
updates
775-
|> normalize_update(:bridge_owner_lease_ttl_seconds, &optional_positive_integer/1)
776-
|> normalize_update(:reconnect_window_seconds, &optional_non_negative_integer/1)
777-
|> normalize_update(:session_header_source, &normalized_session_header_source/1)
786+
|> normalize_optional_update(:bridge_owner_lease_ttl_seconds, &optional_positive_integer/1)
787+
|> normalize_optional_update(:reconnect_window_seconds, &optional_non_negative_integer/1)
788+
|> normalize_optional_update(:session_header_source, &normalized_session_header_source/1)
778789
end
779790

780791
defp normalize_transport_updates(updates, %WebsocketOwnerContext{} = current_owner) do
@@ -792,7 +803,11 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptions do
792803
])
793804

794805
transport_updates =
795-
normalize_update(transport_updates, :forwarded_metadata_headers, &forwarded_headers/1)
806+
normalize_optional_update(
807+
transport_updates,
808+
:forwarded_metadata_headers,
809+
&forwarded_headers_update/1
810+
)
796811

797812
if map_size(owner_updates) == 0 do
798813
transport_updates
@@ -807,22 +822,25 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptions do
807822

808823
defp normalize_file_bridge_updates(updates) do
809824
updates
810-
|> normalize_update(:forwarded_headers, &forwarded_headers/1)
811-
|> normalize_update(:finalize_retry_timeout_ms, &optional_non_negative_integer/1)
812-
|> normalize_update(:finalize_retry_interval_ms, &optional_non_negative_integer/1)
825+
|> normalize_optional_update(:forwarded_headers, &forwarded_headers_update/1)
826+
|> normalize_optional_update(:finalize_retry_timeout_ms, &optional_non_negative_integer/1)
827+
|> normalize_optional_update(:finalize_retry_interval_ms, &optional_non_negative_integer/1)
813828
end
814829

815830
defp normalize_runtime_updates(updates) do
816-
normalize_update(
831+
normalize_optional_update(
817832
updates,
818833
:payload_compression,
819834
&RequestCompressionMetadata.runtime_metadata/1
820835
)
821836
end
822837

823-
defp normalize_update(updates, key, normalizer) do
838+
defp normalize_optional_update(updates, key, normalizer) do
824839
if Map.has_key?(updates, key) do
825-
Map.update!(updates, key, normalizer)
840+
case normalizer.(Map.fetch!(updates, key)) do
841+
nil -> Map.delete(updates, key)
842+
value -> Map.put(updates, key, value)
843+
end
826844
else
827845
updates
828846
end

test/codex_pooler/gateway/payloads/request_options_test.exs

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,99 @@ defmodule CodexPooler.Gateway.Payloads.RequestOptionsTest do
10061006
assert options.file_bridge.pool_upstream_assignment_id == "assignment-id"
10071007
end
10081008

1009+
test "optional normalizers ignore invalid updates instead of clearing existing values" do
1010+
options =
1011+
RequestOptions.build(
1012+
%{
1013+
session_header_source: "session-id",
1014+
forwarded_headers: [{"x-codex-client", "fixture"}],
1015+
finalize_retry_timeout_ms: 500,
1016+
payload_compression: %{"attempted" => true, "status" => "compressed"},
1017+
websocket_owner_forwarding_enabled?: true,
1018+
websocket_owner_downstream: %{pid: self()},
1019+
websocket_owner_downstream_epoch: 3
1020+
},
1021+
"/backend-api/codex/responses",
1022+
%{"model" => "example-model"}
1023+
)
1024+
1025+
updated =
1026+
options
1027+
|> RequestOptions.put_continuity(
1028+
session_header_source: "x-unsafe-header",
1029+
reconnect_window_seconds: -1
1030+
)
1031+
|> RequestOptions.put_transport(
1032+
forwarded_metadata_headers: [{"x-codex-client", :invalid}],
1033+
websocket_owner_downstream_epoch: "stale"
1034+
)
1035+
|> RequestOptions.put_file_bridge(
1036+
forwarded_headers: [{"x-codex-client", :invalid}],
1037+
finalize_retry_timeout_ms: -1
1038+
)
1039+
|> RequestOptions.put_runtime_context(
1040+
payload_compression: %{"enabled" => true, "attempted" => false, "status" => "disabled"}
1041+
)
1042+
1043+
assert updated.continuity.session_header_source == "session-id"
1044+
assert updated.continuity.reconnect_window_seconds == nil
1045+
assert updated.transport.forwarded_metadata_headers == [{"x-codex-client", "fixture"}]
1046+
assert updated.transport.websocket_owner.downstream_epoch == 3
1047+
assert updated.file_bridge.forwarded_headers == [{"x-codex-client", "fixture"}]
1048+
assert updated.file_bridge.finalize_retry_timeout_ms == 500
1049+
1050+
assert updated.runtime.payload_compression == %{
1051+
"attempted" => true,
1052+
"status" => "compressed"
1053+
}
1054+
end
1055+
1056+
test "optional normalizers still accept explicit valid replacements" do
1057+
options =
1058+
RequestOptions.build(
1059+
%{
1060+
session_header_source: "session-id",
1061+
forwarded_headers: [{"x-codex-client", "fixture"}],
1062+
finalize_retry_timeout_ms: 500,
1063+
payload_compression: %{"attempted" => true, "status" => "compressed"},
1064+
websocket_owner_forwarding_enabled?: true,
1065+
websocket_owner_downstream_epoch: 3
1066+
},
1067+
"/backend-api/codex/responses",
1068+
%{"model" => "example-model"}
1069+
)
1070+
1071+
updated =
1072+
options
1073+
|> RequestOptions.put_continuity(session_header_source: "x-session-id")
1074+
|> RequestOptions.put_transport(
1075+
forwarded_metadata_headers: [{"x-openai-client-user-agent", "synthetic"}],
1076+
websocket_owner_downstream_epoch: 4
1077+
)
1078+
|> RequestOptions.put_file_bridge(
1079+
forwarded_headers: [{"user-agent", "codex_cli_rs/0.0.0"}],
1080+
finalize_retry_timeout_ms: 0
1081+
)
1082+
|> RequestOptions.put_runtime_context(
1083+
payload_compression: %{"attempted" => true, "status" => "no_change"}
1084+
)
1085+
1086+
assert updated.continuity.session_header_source == "x-session-id"
1087+
1088+
assert updated.transport.forwarded_metadata_headers == [
1089+
{"x-openai-client-user-agent", "synthetic"}
1090+
]
1091+
1092+
assert updated.transport.websocket_owner.downstream_epoch == 4
1093+
assert updated.file_bridge.forwarded_headers == [{"user-agent", "codex_cli_rs/0.0.0"}]
1094+
assert updated.file_bridge.finalize_retry_timeout_ms == 0
1095+
1096+
assert updated.runtime.payload_compression == %{
1097+
"attempted" => true,
1098+
"status" => "no_change"
1099+
}
1100+
end
1101+
10091102
test "reject unknown section fields" do
10101103
options =
10111104
RequestOptions.build(%{}, "/backend-api/codex/responses", %{"model" => "example-model"})

0 commit comments

Comments
 (0)