Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed

- tmux listing parse failures are retried once and reported as a distinct condition instead of surfacing as a bare `ValueError` that reads like "session not found" one layer up. libtmux 0.53.1+ zips `parse_output`'s fields with `strict=True`, so any short row (a pane or session vanishing mid-listing, or trailing fields tmux omits) raised `ValueError: zip() argument 2 is shorter than argument 1` — which propagated through `server.sessions`/`window.panes`, blocked launches outright, and left the pipe-liveness watchdog unable to tell a genuinely-gone session from a transient parse failure. Adds `TmuxLookupError` and routes the listing reads in `clients/tmux.py` through a single retry-and-classify wrapper; a failed `create_session` no longer leaves an orphaned tmux session that blocks relaunching the same name. Also caps `libtmux<0.53.1`, the last release that zips non-strict (caom-anv)
- Codex handoff extraction now skips native TUI activity cells without relying on an English verb allowlist, including when the model's reply starts with prose (#545)

## [2.4.1] - 2026-08-04

Expand Down Expand Up @@ -129,7 +131,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- workflow: the background drive's FAILED backstop no longer overwrites an already-settled run (#505). It fired unconditionally on any exception, so a drive that raised *after* the engine journaled COMPLETED/CANCELLED — during post-settlement bookkeeping — rewrote that terminal state to FAILED, making the durable record misreport the run's outcome. The write is now a conditional `UPDATE ... WHERE state = 'running'` in the journal DAL (atomic, so no concurrent settle can interleave), covering both the `Exception` and `CancelledError` arms; a run that raises *before* settling still lands FAILED as before, so no run is left orphaned in `running`
- workflow: `cao workflow events` closes its streamed SSE response on every exit path (#505). The follower breaks out of its loop on a terminal frame and abandons the generator on each reconnect, so without an explicit close the socket survived until garbage collection and a long follow with repeated reconnects accumulated live file descriptors. The equivalent MCP tool was already hardened
- workflow: a caller-supplied `run_id` that loses a concurrent-submit race now returns `409` instead of `500` (#505). The uniqueness pre-check and the durable insert are not one atomic operation, so both submits can pass the pre-check; the loser's `IntegrityError` is now mapped to the same `409` the serialized case reports

- profile store writes are now atomic and inter-process safe. Both store writes were previously bare `write_text` calls, so a concurrent `cao profile` write and a server-side write could interleave or leave a partial file. Adds `locked_atomic_write` to `utils/atomic_file.py` as the blind-write sibling of `locked_atomic_rewrite` (#492): it shares the same lock, temp file, fsync, mode preservation and `os.replace`, but skips the read, so a corrupt or non-UTF-8 file in the agent store can still be replaced by the install that would have repaired it instead of failing with `UnicodeDecodeError` (#543)
- self-healing pipe-pane liveness watchdog for silently-stalled FIFO forwarding (fixes #388) (#397), including detection of a stall that settles into a new static frame before the next poll and of a pipe that never delivers a single byte from terminal creation (cold start, harness-control#93) — see `CAO_PIPE_LIVENESS_COLD_START_GRACE_S` / `CAO_PIPE_LIVENESS_MAX_COLD_START_ATTEMPTS` in `docs/configuration.md`
- web: attach web terminals through the configured backend so herdr-backed terminals no longer fail to attach (#417)
Expand Down
67 changes: 61 additions & 6 deletions src/cli_agent_orchestrator/providers/codex.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,63 @@ def _find_assistant_marker(text: str) -> Optional[re.Match[str]]:
return None


def _find_response_marker(text: str) -> Optional[re.Match[str]]:
"""Find the first model-reply marker after a structural activity prelude.

Native Codex activity cells have a ``•`` summary followed by a ``└`` tree
continuation. Require at least two complete cells before advancing the
response boundary: a single tree-formatted group may be a legitimate
answer, while two consecutive cells are strong evidence of TUI activity.
Compact bullet groups remain ambiguous and are preserved. This trades a
rare false positive for avoiding silent truncation of ordinary replies and
deliberately avoids matching English verbs such as ``Read`` or ``Called``.
"""

def line_end(start: int) -> int:
newline = text.find("\n", start)
return len(text) if newline == -1 else newline

matches = []
for match in re.finditer(ASSISTANT_PREFIX_PATTERN, text, re.IGNORECASE | re.MULTILINE):
if not re.match(MCP_TOOL_CALL_PATTERN, text[match.start() : line_end(match.start())]):
matches.append(match)

if not matches:
return None

complete_cells = []
prose_start = None
for index, match in enumerate(matches):
next_start = matches[index + 1].start() if index + 1 < len(matches) else len(text)
cell_tail = text[line_end(match.start()) : next_start]
continuation = re.search(r"^[^\S\n]*└[^\n]*(?:\n|$)", cell_tail, re.MULTILINE)
contains_mcp_call = re.search(MCP_TOOL_CALL_PATTERN, cell_tail, re.MULTILINE)
if continuation and not contains_mcp_call:
complete_cells.append(index)
if index == len(matches) - 1:
remaining = cell_tail[continuation.end() :]
separator = re.search(r"^[^\S\n]*\n", remaining, re.MULTILINE)
following = re.search(r"\S", remaining[separator.end() :]) if separator else None
if separator and following:
candidate = (
line_end(match.start())
+ continuation.end()
+ separator.end()
+ following.start()
)
if text[candidate] != "›":
prose_start = candidate

if len(complete_cells) >= 2:
last_cell = complete_cells[-1]
if last_cell + 1 < len(matches):
return matches[last_cell + 1]
if prose_start is not None:
return re.compile("").match(text, prose_start)

return matches[0]


class ProviderError(Exception):
"""Exception raised for provider-specific errors."""

Expand Down Expand Up @@ -956,12 +1013,10 @@ def extract_last_message_from_script(self, script_output: str) -> str:
if user_matches:
last_user = user_matches[-1]

# Find the first assistant response marker (• or assistant:) after
# the user message, skipping "• Called <server>.<tool>(...)" MCP
# tool call markers — those are followed by tool output, not the
# model's reply. Anchoring on a tool call marker would pull tool
# output (e.g. skill body text) into the extracted response.
asst_after_user = _find_assistant_marker(clean_output[last_user.start() :])
# Extraction uses a stricter anchor than status detection: skip MCP
# calls and at least two complete native activity cells before the
# model's actual reply, while preserving ambiguous compact groups.
asst_after_user = _find_response_marker(clean_output[last_user.start() :])

if asst_after_user:
response_start = last_user.start() + asst_after_user.start()
Expand Down
256 changes: 256 additions & 0 deletions test/providers/test_codex_provider_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from cli_agent_orchestrator.providers.codex import (
CodexProvider,
ProviderError,
_find_response_marker,
_has_startup_idle_composer,
_toml_override,
_toml_scalar,
Expand Down Expand Up @@ -1775,6 +1776,261 @@ def test_extract_does_not_filter_called_as_english_word(self):

assert "Called attention to the import bug" in message

def test_extract_preserves_ambiguous_compact_bullet_group(self):
"""Compact bullet groups are indistinguishable from a legitimate answer."""
output = (
"› fix the failing test\n"
"\n"
"• Explored src/providers\n"
"• Ran pytest -q\n"
"\n"
"• The bug is in the poll loop.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "Explored src/providers" in message
assert "Ran pytest -q" in message
assert "The bug is in the poll loop" in message

def test_response_marker_returns_none_without_assistant_output(self):
"""A user prompt without a response has no response marker."""
assert _find_response_marker("› still waiting") is None

def test_response_marker_handles_final_line_without_newline(self):
"""A marker on the final line is detected without a trailing newline."""
marker = _find_response_marker("• Complete")

assert marker is not None
assert marker.group() == "•"

def test_extract_preserves_single_tree_formatted_bullet(self):
"""One tree-formatted bullet can be a legitimate answer, so retain it."""
output = (
"› inspect the provider\n"
"• Explored src/providers\n"
" └ Read codex.py\n"
"\n"
"• The extraction starts at the wrong marker.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "Explored src/providers" in message
assert "Read codex.py" in message
assert "The extraction starts at the wrong marker" in message

def test_extract_skips_multiple_blank_separated_activity_cells(self):
"""The response starts after the last complete native activity cell."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"\n"
"• Ran pytest -q\n"
" └ 170 passed\n"
"\n"
"• The bug is fixed.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "• The bug is fixed."

def test_extract_skips_activity_cells_before_prose_reply(self):
"""A prose reply starts after the last complete native activity cell."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"\n"
"• Ran pytest -q\n"
" └ 170 passed\n"
"\n"
"The bug is fixed.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "The bug is fixed."

def test_extract_skips_multiple_tree_rows_before_prose_reply(self):
"""All tree rows in the final activity cell stay before the reply."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"\n"
"• Ran pytest -q\n"
" └ pytest -q\n"
" └ 170 passed\n"
"\n"
"All green.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "All green."

def test_extract_skips_indented_tree_output_before_prose_reply(self):
"""Indented output belonging to the final tree row is not returned."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"\n"
"• Ran pytest -q\n"
" └ 170 passed\n"
" 3 skipped\n"
"\n"
"All green.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "All green."

def test_extract_skips_three_activity_cells_before_prose_reply(self):
"""All complete activity cells are removed before a prose reply."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"\n"
"• Edited\n"
" └ Updated codex.py\n"
"\n"
"• Ran pytest -q\n"
" └ 170 passed\n"
"\n"
"The bug is fixed.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "The bug is fixed."

def test_extract_skips_interleaved_commentary_and_activity(self):
"""Commentary between complete activity cells stays before the reply boundary."""
output = (
"› inspect the provider\n"
"• Explored\n"
" └ Read codex.py\n"
"I will verify the focused behavior next.\n"
"\n"
"• Ran pytest -q\n"
" └ 170 passed\n"
"\n"
"• The bug is fixed.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert message == "• The bug is fixed."

def test_extract_preserves_two_consecutive_legitimate_answer_bullets(self):
"""An ambiguous compact answer is preserved rather than truncated."""
output = (
"› summarize the fix\n"
"• Fixed parser\n"
"• Added regression tests\n"
"\n"
"• Verification: all tests pass\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "Fixed parser" in message
assert "Added regression tests" in message
assert "Verification: all tests pass" in message

def test_extract_preserves_tree_formatted_legitimate_answer(self):
"""One tree-formatted answer followed by another bullet is not activity."""
output = (
"› summarize the fix\n"
"• Files changed\n"
" └ src/provider.py\n"
"\n"
"• Tests pass\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "Files changed" in message
assert "src/provider.py" in message
assert "Tests pass" in message

def test_extract_does_not_count_mcp_tree_output_as_activity_cells(self):
"""MCP output must not complete neighboring model-reply bullets."""
output = (
"› summarize the work\n"
"• First finding\n"
'• Called tools.inspect({"path":"src"})\n'
" └ inspection result\n"
"\n"
"• Second finding\n"
'• Called tools.verify({"path":"test"})\n'
" └ verification result\n"
"\n"
"• Conclusion\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "First finding" in message
assert "Second finding" in message
assert "Conclusion" in message

def test_extract_preserves_blank_separated_reply_bullets(self):
"""A single reply bullet before a blank line is not an activity prelude."""
output = (
"› summarize the fix\n"
"• The parser now uses structural layout.\n"
"\n"
"• English verbs remain valid answer text.\n"
"\n"
"› \n"
)

provider = CodexProvider("test1234", "test-session", "window-0")
message = provider.extract_last_message_from_script(output)

assert "parser now uses structural layout" in message
assert "English verbs remain valid" in message


class TestCodexV0111Extraction:
"""Extraction tests for Codex v0.111.0+ footer format."""
Expand Down
Loading