fix: refuse ambiguous Codex session-id capture
When more than one rollout matches the project cwd within the capture window, return None and warn instead of guessing the earliest. The worst case is now a kept placeholder id, never a wrong one. Part of the P2 capture-race fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,9 +1,12 @@
|
|||||||
import json
|
import json
|
||||||
|
import logging
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from hqt.harnesses.base import HarnessConfigurator, SpawnConfig
|
from hqt.harnesses.base import HarnessConfigurator, SpawnConfig
|
||||||
|
|
||||||
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class CodexConfigurator(HarnessConfigurator):
|
class CodexConfigurator(HarnessConfigurator):
|
||||||
name = "codex"
|
name = "codex"
|
||||||
@@ -73,5 +76,15 @@ class CodexConfigurator(HarnessConfigurator):
|
|||||||
candidates.append((started_at, payload["id"]))
|
candidates.append((started_at, payload["id"]))
|
||||||
except (json.JSONDecodeError, KeyError, OSError, TypeError):
|
except (json.JSONDecodeError, KeyError, OSError, TypeError):
|
||||||
continue
|
continue
|
||||||
candidates.sort(key=lambda t: t[0])
|
if len(candidates) > 1:
|
||||||
|
# Ambiguous: more than one Codex rollout matches this cwd + time
|
||||||
|
# window (e.g. a second Codex started in the same project). Guessing
|
||||||
|
# risks storing the wrong conversation id, so keep the placeholder.
|
||||||
|
log.warning(
|
||||||
|
"capture_session_id: %d rollouts match cwd=%s since=%s; refusing to guess",
|
||||||
|
len(candidates),
|
||||||
|
project_path,
|
||||||
|
since,
|
||||||
|
)
|
||||||
|
return None
|
||||||
return candidates[0][1] if candidates else None
|
return candidates[0][1] if candidates else None
|
||||||
|
|||||||
+9
-42
@@ -1,7 +1,6 @@
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import time
|
import time
|
||||||
from datetime import UTC, datetime
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@@ -184,57 +183,25 @@ def test_codex_capture_accepts_file_after_since(tmp_path):
|
|||||||
assert result == "new-id"
|
assert result == "new-id"
|
||||||
|
|
||||||
|
|
||||||
def test_codex_capture_first_post_since_cwd_match_wins(tmp_path):
|
def test_codex_capture_ambiguous_returns_none(tmp_path):
|
||||||
"""Among multiple post-since rollouts, the one started nearest since wins."""
|
"""Two rollouts match cwd + since window -> refuse to guess, return None."""
|
||||||
sessions_dir = tmp_path / ".codex" / "sessions" / "dir"
|
sessions_dir = tmp_path / ".codex" / "sessions" / "dir"
|
||||||
sessions_dir.mkdir(parents=True)
|
sessions_dir.mkdir(parents=True)
|
||||||
|
|
||||||
rollout_older = sessions_dir / "rollout-older.jsonl"
|
rollout_a = sessions_dir / "rollout-a.jsonl"
|
||||||
rollout_newer = sessions_dir / "rollout-newer.jsonl"
|
rollout_b = sessions_dir / "rollout-b.jsonl"
|
||||||
_write_rollout(rollout_older, "older-id", "/projects/foo")
|
_write_rollout(rollout_a, "id-a", "/projects/foo")
|
||||||
_write_rollout(rollout_newer, "newer-id", "/projects/foo")
|
_write_rollout(rollout_b, "id-b", "/projects/foo")
|
||||||
|
|
||||||
now = time.time()
|
now = time.time()
|
||||||
since = now - 120
|
since = now - 120
|
||||||
os.utime(rollout_older, (now - 30, now - 30))
|
os.utime(rollout_a, (now - 30, now - 30))
|
||||||
os.utime(rollout_newer, (now - 10, now - 10))
|
os.utime(rollout_b, (now - 10, now - 10))
|
||||||
|
|
||||||
c = CodexConfigurator()
|
c = CodexConfigurator()
|
||||||
with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path):
|
with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path):
|
||||||
result = c.capture_session_id(Path("/projects/foo"), since)
|
result = c.capture_session_id(Path("/projects/foo"), since)
|
||||||
assert result == "older-id"
|
assert result is None
|
||||||
|
|
||||||
|
|
||||||
def test_codex_capture_prefers_started_nearest_since_over_newest_file(tmp_path):
|
|
||||||
"""Late capture should match the Codex session spawned at since, not a later same-cwd session."""
|
|
||||||
sessions_dir = tmp_path / ".codex" / "sessions" / "dir"
|
|
||||||
sessions_dir.mkdir(parents=True)
|
|
||||||
|
|
||||||
target = sessions_dir / "rollout-target.jsonl"
|
|
||||||
unrelated_later = sessions_dir / "rollout-later.jsonl"
|
|
||||||
|
|
||||||
since = datetime(2026, 6, 10, 21, 9, 15, tzinfo=UTC).timestamp()
|
|
||||||
_write_rollout(
|
|
||||||
target,
|
|
||||||
"target-id",
|
|
||||||
"/projects/foo",
|
|
||||||
timestamp="2026-06-10T21:09:16.000Z",
|
|
||||||
)
|
|
||||||
_write_rollout(
|
|
||||||
unrelated_later,
|
|
||||||
"later-id",
|
|
||||||
"/projects/foo",
|
|
||||||
timestamp="2026-06-10T21:45:45.000Z",
|
|
||||||
)
|
|
||||||
|
|
||||||
now = time.time()
|
|
||||||
os.utime(target, (now - 60, now - 60))
|
|
||||||
os.utime(unrelated_later, (now, now))
|
|
||||||
|
|
||||||
c = CodexConfigurator()
|
|
||||||
with patch("hqt.harnesses.configurators.codex.Path.home", return_value=tmp_path):
|
|
||||||
result = c.capture_session_id(Path("/projects/foo"), since)
|
|
||||||
assert result == "target-id"
|
|
||||||
|
|
||||||
|
|
||||||
def test_codex_capture_skips_non_matching_cwd_picks_older_matching(tmp_path):
|
def test_codex_capture_skips_non_matching_cwd_picks_older_matching(tmp_path):
|
||||||
|
|||||||
Reference in New Issue
Block a user