fix(agent): preserve Codex message items for replay
This commit is contained in:
@@ -33,15 +33,18 @@ class TestChatCompletionsBasic:
|
||||
def test_convert_messages_strips_codex_fields(self, transport):
|
||||
msgs = [
|
||||
{"role": "assistant", "content": "ok", "codex_reasoning_items": [{"id": "rs_1"}],
|
||||
"codex_message_items": [{"id": "msg_1", "type": "message"}],
|
||||
"tool_calls": [{"id": "call_1", "call_id": "call_1", "response_item_id": "fc_1",
|
||||
"type": "function", "function": {"name": "t", "arguments": "{}"}}]},
|
||||
]
|
||||
result = transport.convert_messages(msgs)
|
||||
assert "codex_reasoning_items" not in result[0]
|
||||
assert "codex_message_items" not in result[0]
|
||||
assert "call_id" not in result[0]["tool_calls"][0]
|
||||
assert "response_item_id" not in result[0]["tool_calls"][0]
|
||||
# Original list untouched (deepcopy-on-demand)
|
||||
assert "codex_reasoning_items" in msgs[0]
|
||||
assert "codex_message_items" in msgs[0]
|
||||
|
||||
|
||||
class TestChatCompletionsBuildKwargs:
|
||||
|
||||
@@ -194,6 +194,36 @@ class TestCodexNormalizeResponse:
|
||||
assert nr.content == "Hello world"
|
||||
assert nr.finish_reason == "stop"
|
||||
|
||||
def test_message_items_preserved_in_provider_data(self, transport):
|
||||
"""Codex assistant message item ids/phases must survive transport normalization."""
|
||||
r = SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(
|
||||
type="message",
|
||||
role="assistant",
|
||||
id="msg_abc",
|
||||
phase="final_answer",
|
||||
content=[SimpleNamespace(type="output_text", text="Hello world")],
|
||||
status="completed",
|
||||
),
|
||||
],
|
||||
status="completed",
|
||||
incomplete_details=None,
|
||||
usage=SimpleNamespace(input_tokens=10, output_tokens=5,
|
||||
input_tokens_details=None, output_tokens_details=None),
|
||||
)
|
||||
nr = transport.normalize_response(r)
|
||||
assert nr.codex_message_items == [
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "completed",
|
||||
"content": [{"type": "output_text", "text": "Hello world"}],
|
||||
"id": "msg_abc",
|
||||
"phase": "final_answer",
|
||||
}
|
||||
]
|
||||
|
||||
def test_tool_call_response(self, transport):
|
||||
"""Normalize a Codex response with tool calls."""
|
||||
r = SimpleNamespace(
|
||||
|
||||
@@ -60,6 +60,13 @@ class TestTransportRegistry:
|
||||
assert t is not None
|
||||
assert t.api_mode == "anthropic_messages"
|
||||
|
||||
def test_discovers_missing_transport_when_registry_partially_populated(self):
|
||||
"""Importing one transport directly must not hide other valid api_modes."""
|
||||
import agent.transports.chat_completions # noqa: F401
|
||||
t = get_transport("codex_responses")
|
||||
assert t is not None
|
||||
assert t.api_mode == "codex_responses"
|
||||
|
||||
def test_register_and_get(self):
|
||||
class DummyTransport(ProviderTransport):
|
||||
@property
|
||||
|
||||
@@ -270,3 +270,15 @@ class TestNormalizedResponseBackwardCompat:
|
||||
def test_codex_reasoning_items_none_when_absent(self):
|
||||
nr = NormalizedResponse(content="hi", tool_calls=None, finish_reason="stop")
|
||||
assert nr.codex_reasoning_items is None
|
||||
|
||||
def test_codex_message_items_from_provider_data(self):
|
||||
items = [{"id": "msg_1", "type": "message"}]
|
||||
nr = NormalizedResponse(
|
||||
content="hi", tool_calls=None, finish_reason="stop",
|
||||
provider_data={"codex_message_items": items},
|
||||
)
|
||||
assert nr.codex_message_items == items
|
||||
|
||||
def test_codex_message_items_none_when_absent(self):
|
||||
nr = NormalizedResponse(content="hi", tool_calls=None, finish_reason="stop")
|
||||
assert nr.codex_message_items is None
|
||||
|
||||
@@ -716,6 +716,103 @@ class TestNormalizeCodexResponse:
|
||||
assert len(msg.tool_calls) == 1
|
||||
assert msg.tool_calls[0].function.name == "web_search"
|
||||
|
||||
def test_message_items_captured_with_id_and_phase(self, monkeypatch):
|
||||
"""Exact message items (with id/phase) must be captured for cache replay."""
|
||||
agent = self._make_codex_agent(monkeypatch)
|
||||
response = SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(
|
||||
type="message", status="completed", id="msg_abc",
|
||||
phase="commentary",
|
||||
content=[SimpleNamespace(type="output_text", text="Thinking...")],
|
||||
),
|
||||
SimpleNamespace(
|
||||
type="message", status="completed", id="msg_def",
|
||||
phase="final_answer",
|
||||
content=[SimpleNamespace(type="output_text", text="Done!")],
|
||||
),
|
||||
],
|
||||
status="completed",
|
||||
)
|
||||
msg, reason = _normalize_codex_response(response)
|
||||
assert msg.codex_message_items is not None
|
||||
assert len(msg.codex_message_items) == 2
|
||||
assert msg.codex_message_items[0]["id"] == "msg_abc"
|
||||
assert msg.codex_message_items[0]["phase"] == "commentary"
|
||||
assert msg.codex_message_items[0]["content"][0]["text"] == "Thinking..."
|
||||
assert msg.codex_message_items[1]["id"] == "msg_def"
|
||||
assert msg.codex_message_items[1]["phase"] == "final_answer"
|
||||
assert msg.codex_message_items[1]["content"][0]["text"] == "Done!"
|
||||
|
||||
def test_message_items_none_when_no_messages(self, monkeypatch):
|
||||
"""Only reasoning + tool calls should yield None codex_message_items."""
|
||||
agent = self._make_codex_agent(monkeypatch)
|
||||
response = SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(type="function_call", status="completed",
|
||||
call_id="call_1", name="web_search", arguments='{}', id="fc_1"),
|
||||
],
|
||||
status="completed",
|
||||
)
|
||||
msg, reason = _normalize_codex_response(response)
|
||||
assert msg.codex_message_items is None
|
||||
|
||||
|
||||
class TestChatMessagesToResponsesInputMessageItems:
|
||||
"""Verify codex_message_items are replayed verbatim instead of reconstructed."""
|
||||
|
||||
def test_replays_exact_message_items(self, monkeypatch):
|
||||
agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses",
|
||||
base_url="https://chatgpt.com/backend-api/codex")
|
||||
messages = [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "Hello world",
|
||||
"codex_message_items": [
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "completed",
|
||||
"id": "msg_123",
|
||||
"phase": "final_answer",
|
||||
"content": [{"type": "output_text", "text": "Hello world"}],
|
||||
},
|
||||
],
|
||||
},
|
||||
{"role": "user", "content": "follow up"},
|
||||
]
|
||||
items = _chat_messages_to_responses_input(messages)
|
||||
msg_items = [i for i in items if i.get("type") == "message"]
|
||||
assert len(msg_items) == 1
|
||||
assert msg_items[0]["id"] == "msg_123"
|
||||
assert msg_items[0]["phase"] == "final_answer"
|
||||
assert msg_items[0]["content"][0]["text"] == "Hello world"
|
||||
|
||||
def test_fallback_to_plain_when_no_message_items(self, monkeypatch):
|
||||
agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses",
|
||||
base_url="https://chatgpt.com/backend-api/codex")
|
||||
messages = [{"role": "assistant", "content": "Hello world"}]
|
||||
items = _chat_messages_to_responses_input(messages)
|
||||
assert items == [{"role": "assistant", "content": "Hello world"}]
|
||||
|
||||
def test_skips_invalid_message_items(self, monkeypatch):
|
||||
agent = _make_agent(monkeypatch, "openai-codex", api_mode="codex_responses",
|
||||
base_url="https://chatgpt.com/backend-api/codex")
|
||||
messages = [
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "fallback text",
|
||||
"codex_message_items": [
|
||||
{"type": "function_call", "role": "assistant"}, # wrong type
|
||||
{"type": "message", "role": "user"}, # wrong role
|
||||
{"type": "message", "role": "assistant", "content": "not a list"},
|
||||
],
|
||||
},
|
||||
]
|
||||
items = _chat_messages_to_responses_input(messages)
|
||||
# All invalid — falls back to plain text reconstruction
|
||||
assert items == [{"role": "assistant", "content": "fallback text"}]
|
||||
|
||||
|
||||
# ── Chat completions response handling (OpenRouter/Nous) ─────────────────────
|
||||
|
||||
|
||||
@@ -943,6 +943,33 @@ def test_normalize_codex_response_marks_commentary_only_message_as_incomplete(mo
|
||||
assert "inspect the repository" in (assistant_message.content or "")
|
||||
|
||||
|
||||
def test_normalize_codex_response_preserves_message_status_for_replay(monkeypatch):
|
||||
"""Incomplete Codex output messages must not be replayed as completed."""
|
||||
agent = _build_agent(monkeypatch)
|
||||
from agent.codex_responses_adapter import _normalize_codex_response
|
||||
|
||||
response = SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(
|
||||
type="message",
|
||||
id="msg_partial",
|
||||
phase="commentary",
|
||||
status="in_progress",
|
||||
content=[SimpleNamespace(type="output_text", text="Still working...")],
|
||||
)
|
||||
],
|
||||
usage=SimpleNamespace(input_tokens=4, output_tokens=2, total_tokens=6),
|
||||
status="in_progress",
|
||||
model="gpt-5-codex",
|
||||
)
|
||||
|
||||
assistant_message, finish_reason = _normalize_codex_response(response)
|
||||
|
||||
assert finish_reason == "incomplete"
|
||||
assert assistant_message.codex_message_items[0]["id"] == "msg_partial"
|
||||
assert assistant_message.codex_message_items[0]["status"] == "in_progress"
|
||||
|
||||
|
||||
def test_normalize_codex_response_detects_leaked_tool_call_text(monkeypatch):
|
||||
"""Harmony-style `to=functions.foo` leaked into assistant content with no
|
||||
structured function_call items must be treated as incomplete so the
|
||||
@@ -1403,6 +1430,44 @@ def test_chat_messages_to_responses_input_reasoning_only_has_following_item(monk
|
||||
assert following.get("role") == "assistant"
|
||||
|
||||
|
||||
def test_codex_message_item_status_survives_conversion_and_preflight(monkeypatch):
|
||||
"""Stored Codex assistant message statuses must survive replay normalization."""
|
||||
agent = _build_agent(monkeypatch)
|
||||
from agent.codex_responses_adapter import (
|
||||
_chat_messages_to_responses_input,
|
||||
_preflight_codex_input_items,
|
||||
)
|
||||
|
||||
items = _chat_messages_to_responses_input([
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "partial",
|
||||
"codex_message_items": [
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "incomplete",
|
||||
"id": "msg_incomplete",
|
||||
"phase": "commentary",
|
||||
"content": [{"type": "output_text", "text": "partial"}],
|
||||
}
|
||||
],
|
||||
}
|
||||
])
|
||||
replay_item = next(item for item in items if item.get("type") == "message")
|
||||
assert replay_item["status"] == "incomplete"
|
||||
|
||||
normalized = _preflight_codex_input_items([
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "in_progress",
|
||||
"content": [{"type": "output_text", "text": "working"}],
|
||||
}
|
||||
])
|
||||
assert normalized[0]["status"] == "in_progress"
|
||||
|
||||
|
||||
def test_duplicate_detection_distinguishes_different_codex_reasoning(monkeypatch):
|
||||
"""Two consecutive reasoning-only responses with different encrypted content
|
||||
must NOT be treated as duplicates."""
|
||||
@@ -1453,6 +1518,58 @@ def test_duplicate_detection_distinguishes_different_codex_reasoning(monkeypatch
|
||||
assert "enc_second" in encrypted_contents
|
||||
|
||||
|
||||
def test_duplicate_detection_distinguishes_different_codex_message_items(monkeypatch):
|
||||
"""Incomplete turns with new message ids/phases/statuses must not be collapsed."""
|
||||
agent = _build_agent(monkeypatch)
|
||||
responses = [
|
||||
SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(
|
||||
type="message",
|
||||
id="msg_first",
|
||||
phase="commentary",
|
||||
status="in_progress",
|
||||
content=[SimpleNamespace(type="output_text", text="Still working...")],
|
||||
)
|
||||
],
|
||||
usage=SimpleNamespace(input_tokens=50, output_tokens=10, total_tokens=60),
|
||||
status="in_progress",
|
||||
model="gpt-5-codex",
|
||||
),
|
||||
SimpleNamespace(
|
||||
output=[
|
||||
SimpleNamespace(
|
||||
type="message",
|
||||
id="msg_second",
|
||||
phase="commentary",
|
||||
status="in_progress",
|
||||
content=[SimpleNamespace(type="output_text", text="Still working...")],
|
||||
)
|
||||
],
|
||||
usage=SimpleNamespace(input_tokens=50, output_tokens=10, total_tokens=60),
|
||||
status="in_progress",
|
||||
model="gpt-5-codex",
|
||||
),
|
||||
_codex_message_response("Final answer after progress updates."),
|
||||
]
|
||||
monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0))
|
||||
|
||||
result = agent.run_conversation("keep going")
|
||||
|
||||
assert result["completed"] is True
|
||||
interim_msgs = [
|
||||
msg for msg in result["messages"]
|
||||
if msg.get("role") == "assistant"
|
||||
and msg.get("finish_reason") == "incomplete"
|
||||
]
|
||||
assert len(interim_msgs) == 2
|
||||
assert [msg["codex_message_items"][0]["id"] for msg in interim_msgs] == [
|
||||
"msg_first",
|
||||
"msg_second",
|
||||
]
|
||||
assert all(msg["codex_message_items"][0]["status"] == "in_progress" for msg in interim_msgs)
|
||||
|
||||
|
||||
def test_chat_messages_to_responses_input_deduplicates_reasoning_ids(monkeypatch):
|
||||
"""Duplicate reasoning item IDs across multi-turn incomplete responses
|
||||
must be deduplicated so the Responses API doesn't reject with HTTP 400."""
|
||||
|
||||
@@ -308,6 +308,33 @@ class TestMessageStorage:
|
||||
assert "reasoning_content" in conv[0]
|
||||
assert conv[0]["reasoning_content"] == ""
|
||||
|
||||
def test_codex_message_items_persisted_and_restored(self, db):
|
||||
"""codex_message_items must round-trip through JSON serialization."""
|
||||
db.create_session(session_id="s1", source="cli")
|
||||
items = [
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "completed",
|
||||
"id": "msg_123",
|
||||
"phase": "commentary",
|
||||
"content": [{"type": "output_text", "text": "Thinking..."}],
|
||||
},
|
||||
{
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"status": "completed",
|
||||
"id": "msg_456",
|
||||
"phase": "final_answer",
|
||||
"content": [{"type": "output_text", "text": "Done!"}],
|
||||
},
|
||||
]
|
||||
db.append_message("s1", role="assistant", content="Done!", codex_message_items=items)
|
||||
|
||||
conv = db.get_messages_as_conversation("s1")
|
||||
assert len(conv) == 1
|
||||
assert conv[0].get("codex_message_items") == items
|
||||
|
||||
def test_reasoning_not_set_for_non_assistant(self, db):
|
||||
"""reasoning is never leaked onto user or tool messages."""
|
||||
db.create_session(session_id="s1", source="telegram")
|
||||
@@ -1173,7 +1200,7 @@ class TestSchemaInit:
|
||||
def test_schema_version(self, db):
|
||||
cursor = db._conn.execute("SELECT version FROM schema_version")
|
||||
version = cursor.fetchone()[0]
|
||||
assert version == 8
|
||||
assert version == 9
|
||||
|
||||
def test_title_column_exists(self, db):
|
||||
"""Verify the title column was created in the sessions table."""
|
||||
@@ -1229,12 +1256,12 @@ class TestSchemaInit:
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
# Open with SessionDB — should migrate to v8
|
||||
# Open with SessionDB — should migrate to v9
|
||||
migrated_db = SessionDB(db_path=db_path)
|
||||
|
||||
# Verify migration
|
||||
cursor = migrated_db._conn.execute("SELECT version FROM schema_version")
|
||||
assert cursor.fetchone()[0] == 8
|
||||
assert cursor.fetchone()[0] == 9
|
||||
|
||||
# Verify title column exists and is NULL for existing sessions
|
||||
session = migrated_db.get_session("existing")
|
||||
|
||||
Reference in New Issue
Block a user