fix(sessions): make session teardown atomic so tmux and the registry cannot diverge

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - force fast-uri 3.1.5 in aidlc-portfolio examples (#551) (#552)
+- make session teardown atomic so tmux and the terminal registry can no longer diverge (#498). `delete_session` previously trusted a pre-loop liveness reading and an unverified `kill_session` result, so a session could survive while its registry rows were deleted (orphaned tmux session) or vice versa (ghost rows that later misattributed a reused session name). Now: session creation and session teardown are mutually exclusive per session *name* (a concurrent launch can no longer interleave with a teardown of the same name), `kill_session` returns True only once the session is confirmed gone, and registry rows are deleted only *after* that confirmation. **Error-contract change:** a teardown whose tmux kill cannot be confirmed now raises (surfaced as HTTP 500 on `DELETE /sessions/{name}`) instead of reporting success — the registry rows are left intact and the operation is safe to re-run, which reconciles the survivor. Backend authors: `TerminalBackend.kill_session` must not return True for a merely-dispatched kill
 
 
 ### Other

docs/terminal-lifecycle.md

@@ -13,14 +13,20 @@ exhaust system resources. CAO provides automatic and manual cleanup paths.
 | Handoff completes successfully (auto-delete) | Yes |
 | `delete_terminal` MCP tool | Yes |
 | `DELETE /terminals/{id}` API | Yes |
-| `cao shutdown --session <name>` | No |
-| `cao shutdown --all` | No |
+| `cao shutdown --session <name>` | Yes |
+| `cao shutdown --all` | Yes |
 | Process crash | No |
 
-Snapshots are only saved when a terminal is deleted individually via
-`terminal_service.delete_terminal`. Session-level shutdown (`delete_session`)
-kills windows directly and does not snapshot. If you want scrollback preserved,
-delete terminals individually before shutting down the session.
+Individual deletion snapshots via `terminal_service.delete_terminal`.
+Session-level shutdown (`delete_session`, which both `cao shutdown` modes reach
+over `DELETE /sessions/{name}`) snapshots too: capturing each terminal's
+scrollback is an explicit step of the teardown, and it deliberately runs
+*before* the session kill, since scrollback only exists while the pane does. A
+crash bypasses both paths, so nothing is captured.
+
+Capture is best-effort everywhere, not just at session level: a snapshot whose
+write fails is logged and teardown continues regardless of which path took it.
+So "Yes" above means the path attempts a snapshot, not that one is guaranteed.
 
 ## Snapshot files
 

src/cli_agent_orchestrator/backends/base.py

@@ -72,6 +72,32 @@ def session_exists(self, session_name: str) -> bool:
         """
         ...
 
+    def session_exists_strict(self, session_name: str) -> bool:
+        """Check if a session exists, RAISING when the lookup cannot be answered.
+
+        Semantics differ from ``session_exists`` only in the error case:
+        ``session_exists`` collapses a lookup error into False ("assume
+        absent"), whereas this must distinguish a confirmed absence (return
+        False) from an inability to tell (raise). Teardown confirmation MUST
+        use this so a transient backend error is never misread as "session
+        gone" (#498).
+
+        Implementations must query in a way that keeps the two apart. That is a
+        real constraint, not a formality: a client library that swallows its own
+        transport errors and reports an empty result set makes a lookup failure
+        LOOK like an absence, and a strict check layered over it fails OPEN no
+        matter what it does with its own exceptions. ``TmuxBackend`` therefore
+        issues its own ``list-sessions`` and classifies the exit status
+        (``clients/tmux.py``) rather than using libtmux's session collection.
+
+        The default implementation delegates to ``session_exists`` for backends
+        that cannot yet make the distinction; those backends therefore retain the
+        old lenient, fail-OPEN behavior, and the teardown guarantee is only as
+        strong as this method. HerdrBackend is in that position — tracked as a
+        follow-up.
+        """
+        return self.session_exists(session_name)
+
     @abstractmethod
     def list_sessions(self) -> List[Dict[str, str]]:
         """List all sessions managed by this backend.
@@ -85,11 +111,20 @@ def list_sessions(self) -> List[Dict[str, str]]:
     def kill_session(self, session_name: str) -> bool:
         """Kill/destroy a session.
 
+        Implementations MUST NOT return True on a merely-dispatched kill: session
+        teardown treats True as proof the session is gone and only then drops the
+        matching registry rows, so an optimistic True is what lets tmux and the
+        registry diverge (#498). Confirm the session is actually gone — poll if
+        the kill is asynchronous — before returning True.
+
         Args:
             session_name: Session to kill
 
         Returns:
-            True if session was killed, False if not found
+            True once the session is confirmed gone; False if it was not found
+            OR the kill could not be confirmed within the backend's bound. A
+            caller that must tell those two apart re-checks existence itself via
+            ``session_exists_strict``.
         """
         ...
 

src/cli_agent_orchestrator/backends/tmux_backend.py

@@ -45,6 +45,9 @@ def create_session(
     def session_exists(self, session_name: str) -> bool:
         return self._client.session_exists(session_name)
 
+    def session_exists_strict(self, session_name: str) -> bool:
+        return self._client.session_exists_strict(session_name)
+
     def list_sessions(self) -> List[Dict[str, str]]:
         return self._client.list_sessions()
 

src/cli_agent_orchestrator/clients/database.py

@@ -1046,6 +1046,27 @@ def delete_terminals_by_session(tmux_session: str) -> int:
         return deleted
 
 
+def delete_terminals_by_ids(terminal_ids: List[str]) -> int:
+    """Delete specific terminal rows by id. Returns the number deleted.
+
+    Unlike ``delete_terminals_by_session`` (which deletes EVERY row for a
+    session name), this deletes only the given ids. Session teardown uses it to
+    scope its reconciliation sweep to the incarnation it started tearing down,
+    so a concurrent same-name recreate — whose rows carry freshly generated ids
+    — is never swept (#498).
+    """
+    if not terminal_ids:
+        return 0
+    with SessionLocal() as db:
+        deleted = (
+            db.query(TerminalModel)
+            .filter(TerminalModel.id.in_(terminal_ids))
+            .delete(synchronize_session=False)
+        )
+        db.commit()
+        return deleted
+
+
 def create_inbox_message(sender_id: str, receiver_id: str, message: str) -> InboxMessage:
     """Create inbox message with status=MessageStatus.PENDING.