test(app): assert reopened rows leave the list; supersede SPEC-29 Decision 6

CodeRabbit review on PR #157.

Both reopen tests asserted only that the request went out: their fakes returned
the same closed list forever, so the tests passed even if the row never left the
view — which is precisely the user-visible failure worth guarding. Each fake now
behaves like the server and drops a reopened session from `session.listClosed`,
and both tests assert the row is gone (mobile ClosedScreen and the desktop
closed sidebar).

SPEC-29's Decision 6 still specified `session.archive`, the `archived` flag and a
bare `adapter.kill()` while the amendment at the top of the spec described the
close/reopen contract — one spec, two contradictory lifecycles. Decision 6 is now
marked superseded and restates the shipped contract: graceful agent-side release
then a verified reap, queued input dropped, teardown serialized against input,
`closed` persisted, `session.listClosed`/`session.reopen`, transparent reopen on
a message, `sub` never respawning, and the idle-sweep guards that keep every
auto-close reversible.

Author: leduckhc

app/test/desktop/chat/closed_sidebar_view_test.dart

@@ -19,11 +19,25 @@ class _ClosedConn extends ConnectionController {
   _ClosedConn(this.closed) : super(const _NoStore());
   final List<Map<String, dynamic>> closed;
   final sent = <Map<String, dynamic>>[];
+
+  /// Ids this fake has reopened — they must stop being reported as closed, as
+  /// the real server does, or a reopen test passes even when the row never left.
+  final reopened = <String>{};
+
   @override
   Future<Map<String, dynamic>> request(MsgType t, Map<String, dynamic> body) {
     sent.add(body);
+    if (body['kind'] == 'session.reopen') {
+      reopened.add(body['sessionId'] as String);
+      return Future.value(const {});
+    }
     if (body['kind'] == 'session.listClosed') {
-      return Future.value({'sessions': closed});
+      return Future.value({
+        'sessions': [
+          for (final s in closed)
+            if (!reopened.contains(s['id'])) s,
+        ],
+      });
     }
     return Future.value(const {});
   }
@@ -168,6 +182,13 @@ void main() {
       orElse: () => const {},
     );
     expect(u['sessionId'], 's1');
+    // What the user actually sees: the row must leave the list, not merely the
+    // request go out.
+    expect(
+      find.text('Adapter resume'),
+      findsNothing,
+      reason: 'the reopened row must leave the closed list',
+    );
   });
 
   testWidgets('Reopen reloads the list without surfacing an error', (

app/test/ui/home/closed_screen_test.dart

@@ -36,7 +36,13 @@ class _FakeStore extends StoreController {
     // FutureBuilder as a future error rather than before it can subscribe.
     await Future<void>.delayed(Duration.zero);
     if (throws) throw StateError('offline');
-    return closed;
+    // Behave like the server: a reopened session is no longer closed, so it must
+    // leave this list. Returning `closed` verbatim let the reopen test pass even
+    // if the row never disappeared.
+    return [
+      for (final session in closed)
+        if (!restored.contains(session.id)) session,
+    ];
   }
 
   @override
@@ -128,6 +134,11 @@ void main() {
     await tester.pumpAndSettle();
 
     expect(store.restored, ['a']);
+    expect(
+      find.text('sess-a'),
+      findsNothing,
+      reason: 'the reopened row must leave the list',
+    );
     // Reloaded so the restored row leaves the closed list.
     expect(store.loadCount, 2);
   });

docs/specs/2026-07-26-SPEC-29-session-lifecycle-resume-list-delete.md

@@ -140,21 +140,38 @@ compatible**.
 
 ## Decisions (frozen)
 
-> **Decision 6 UPDATED (2026-07-26) — archive, don't hard-delete.** The primary
-> destructive action is **archive** (recoverable), not delete. `session.archive`
-> sets a persisted `archived` flag on the makit session, stops the live agent
-> (`adapter.kill()`), and swaps in the `DetachedAdapter`. It is **makit-side
-> only** — it does **not** call any back-end native archive (codex
-> `thread/archive` is deliberately avoided so the underlying thread stays
-> directly resumable; the transcript + event log + resume handle are **kept**,
-> never destroyed). Archived sessions are **excluded from the active session
-> list** (`listSessions()` → `sessions.snapshot`) but survive a restart
-> (rehydration keeps them archived) and are restored with `session.unarchive`
-> (clears the flag; the session stays cold until the next subscribe re-attaches
-> it). No rows are deleted from the event log. A truly permanent hard-delete
-> (SQL delete + agent `thread/delete`/`session/delete`) is deferred to a
-> separate explicit
-> "delete permanently" action and is NOT wired in this pass.
+> **Decision 6 SUPERSEDED (2026-08-11) — close (release the agent), don't
+> archive.** The 2026-07-26 revision of this decision specified `session.archive`
+> / `session.unarchive`, a persisted `archived` flag, and "stops the live agent
+> (`adapter.kill()`)". All of it is replaced; see the AMENDMENT at the top of this
+> spec for why (one unverified SIGTERM left agents resident for days).
+>
+> The primary non-destructive action is **close** (recoverable) — not delete, and
+> no longer "archive". `session.close` releases the agent in two required steps:
+> the back end's own primitive (ACP `session/close` gated on
+> `sessionCapabilities.close`; codex `thread/unsubscribe`), then a **verified**
+> process reap (`SIGTERM` → `SIGKILL` after a grace period). Both are required
+> because makit runs one agent process per session, so the agent-side release
+> alone reclaims no memory. The manager bounds and swallows the graceful step,
+> drops pending mid-turn input ("stop means stop", SPEC-35), and serializes
+> teardown against input, then swaps in the `DetachedAdapter` and sets the
+> persisted **`closed`** flag.
+>
+> It stays **makit-side only** for the *record*: no back-end native archive is
+> called (codex `thread/archive` is deliberately avoided so the underlying
+> session/thread stays directly resumable and listable; the transcript + event log
+> + resume handle are **kept**, never destroyed). Closed sessions are **excluded
+> from the active session list** (`listSessions()` → `sessions.snapshot`), survive
+> a restart (rehydration keeps them closed), are enumerated by
+> `session.listClosed`, and come back via `session.reopen` — or transparently, by
+> sending a message (`ensureLiveForInput`). Reading a closed transcript (`sub`)
+> deliberately does **not** respawn an agent. Sessions idle beyond
+> `MAKIT_IDLE_CLOSE_MIN` (default 60, `0` disables) close automatically; that
+> sweep never touches a session that is busy, awaiting the user, a draft, already
+> cold, or not resumable, so every auto-close is reversible. A truly permanent
+> hard-delete (SQL delete + agent `thread/delete`/`session/delete`) remains
+> deferred to a separate explicit "delete permanently" action and is NOT wired in
+> this pass.
 
 1. **Capability negotiation lives on the adapter, not on `agent === "pi"`.**
    The `AgentAdapter` seam gains a `capabilities()` (or `readonly capabilities`)