From ba586ed4b5fc0cd56f9f7dd44e40b7fcfbcae162 Mon Sep 17 00:00:00 2001 From: Ben Awad Date: Sat, 19 Sep 2026 12:23:28 -0700 Subject: [PATCH] fix: let CodexAppServerSession resume a stored codex thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ``resume_thread_id`` to CodexAppServerSession: when set, the first ensure_started() issues ``thread/resume`` (with the same cwd / personality / developerInstructions / model params thread/start sends) instead of starting an empty thread, and verifies codex handed back the requested id. A refused or mismatched resume raises the typed CodexThreadResumeError once; the next ensure_started() falls through to ``thread/start`` on the same handshaken client (initialize now runs once per client, not once per attempt). Why: the thread id lived in memory only, so every new AIAgent for the same Hermes session — a later API-server request or the first turn after a restart — started a fresh codex thread and the model lost its own memory of the conversation (#100531). The runtime decides the fail-closed policy; this adapter only speaks the wire contract (verified against codex-cli 0.147.0: thread/resume{threadId,...} -> result.thread.id; unknown id -> -32600 "no rollout found"; killed writer -> -32600 "already has an active writer"). Salvaged from #103352 (thread/resume + id cross-fill + mismatch guard). --- agent/transports/codex_app_server_session.py | 60 +++++++++++++++---- .../emails/benawad@bens-mbp.attlocal.net | 2 + 2 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 contributors/emails/benawad@bens-mbp.attlocal.net diff --git a/agent/transports/codex_app_server_session.py b/agent/transports/codex_app_server_session.py index 8cb93c2afe..7a9a6ebf9d 100644 --- a/agent/transports/codex_app_server_session.py +++ b/agent/transports/codex_app_server_session.py @@ -190,6 +190,21 @@ class _ServerRequestRouting: auto_approve_apply_patch: bool = False +class CodexThreadResumeError(CodexAppServerError): + """``thread/resume`` did not hand back the stored thread (unknown/garbage id, rollout still locked by + a killed app-server, or codex answered with a different thread). The caller decides the policy.""" + + def __init__(self, thread_id: str, detail: str) -> None: + super().__init__(code=-32600, message=f"codex thread {thread_id[:8]} could not be resumed: {detail}") + self.thread_id = thread_id + + +def _extract_thread_id(result: dict) -> Optional[str]: + """Different codex versions serialize the id under thread.id / sessionId / threadId.""" + thread_obj = result.get("thread") or {} + return thread_obj.get("id") or thread_obj.get("sessionId") or result.get("sessionId") or result.get("threadId") + + class CodexAppServerSession: """One Codex thread per Hermes session, lifetime owned by AIAgent. Not thread-safe: one caller at a time.""" @@ -201,11 +216,14 @@ class CodexAppServerSession: request_routing: Optional[_ServerRequestRouting] = None, client_factory: Optional[Callable[..., CodexAppServerClient]] = None, model: Optional[str] = None, model_provider: Optional[str] = None, - developer_instructions: Optional[str] = None, + developer_instructions: Optional[str] = None, resume_thread_id: Optional[str] = None, ) -> None: self._cwd = cwd or os.getcwd() self._codex_bin = codex_bin self._codex_home = codex_home + # A codex thread id persisted by an earlier process for this Hermes session: the first + # ``ensure_started`` issues ``thread/resume`` for it instead of ``thread/start``. + self._resume_thread_id = resume_thread_id # ``thread/start.model`` / ``.modelProvider``: select a provider from codex's own # ``[model_providers.]`` table. Only the id travels; codex reads base_url/env_key itself. self._model = (model or "").strip() or None @@ -234,12 +252,14 @@ class CodexAppServerSession: self._closed = False def ensure_started(self) -> str: - """Spawn, handshake, and ``thread/start``; idempotent, returns the codex thread id.""" + """Spawn, handshake, and ``thread/start`` (or ``thread/resume`` for a stored id); idempotent, returns + the codex thread id. A failed resume raises :class:`CodexThreadResumeError` once; the next call + starts a fresh thread on the same handshaken client.""" if self._thread_id is not None: return self._thread_id if self._client is None: self._client = self._client_factory(codex_bin=self._codex_bin, codex_home=self._codex_home) - self._client.initialize(client_name="hermes", client_title="Hermes Agent", client_version=_get_hermes_version()) + self._client.initialize(client_name="hermes", client_title="Hermes Agent", client_version=_get_hermes_version()) # Permissions are NOT sent on thread/start: codex gates ``thread/start.permissions`` # behind experimentalApi + a matching ``[permissions]`` table in ~/.codex/config.toml. # Hermes supplies the agent identity through its own system prompt; ``personality: "none"`` strips @@ -251,18 +271,34 @@ class CodexAppServerSession: params["modelProvider"] = self._model_provider if self._model: params["model"] = self._model - result = self._client.request("thread/start", params, timeout=15) - # Different codex versions serialize the id under thread.id / sessionId / threadId. - thread_obj = result.get("thread") or {} - thread_id = thread_obj.get("id") or thread_obj.get("sessionId") or result.get("sessionId") or result.get("threadId") - if not thread_id: - raise CodexAppServerError( - code=-32603, message=f"codex thread/start returned no thread id (payload keys: {sorted(result.keys())})", - ) + if self._resume_thread_id: + wanted, self._resume_thread_id = self._resume_thread_id, None # one attempt per stored id + thread_id = self._resume_thread(wanted, params) + logger.info("codex app-server thread resumed: id=%s cwd=%s", thread_id[:8], self._cwd) + else: + result = self._client.request("thread/start", params, timeout=15) + thread_id = _extract_thread_id(result) + if not thread_id: + raise CodexAppServerError( + code=-32603, message=f"codex thread/start returned no thread id (payload keys: {sorted(result.keys())})", + ) + logger.info("codex app-server thread started: id=%s profile=%s cwd=%s", thread_id[:8], self._permission_profile, self._cwd) self._thread_id = thread_id - logger.info("codex app-server thread started: id=%s profile=%s cwd=%s", thread_id[:8], self._permission_profile, self._cwd) return thread_id + def _resume_thread(self, wanted: str, params: dict[str, Any]) -> str: + """``thread/resume`` for the stored id; the same thread/start params ride along so the resumed thread + carries the CURRENT prompt composition and provider (accepted by the resume schema, codex 0.147).""" + assert self._client is not None + try: + result = self._client.request("thread/resume", {"threadId": wanted, **params}, timeout=15) + except CodexAppServerError as exc: + raise CodexThreadResumeError(wanted, exc.message) from exc + thread_id = _extract_thread_id(result) + if thread_id != wanted: + raise CodexThreadResumeError(wanted, f"app-server answered with thread {str(thread_id)[:8]!r}") + return wanted + def close(self) -> None: if self._closed: return diff --git a/contributors/emails/benawad@bens-mbp.attlocal.net b/contributors/emails/benawad@bens-mbp.attlocal.net new file mode 100644 index 0000000000..301a088fd0 --- /dev/null +++ b/contributors/emails/benawad@bens-mbp.attlocal.net @@ -0,0 +1,2 @@ +benawad +# PR #103352 salvage (codex thread/resume)