diff --git a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java index 38b9e54..e5d6d50 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java @@ -29,8 +29,7 @@ import java.util.function.LongSupplier; * teardown on top. * *

The state machine is intentionally one-shot / no-reuse: every acquired worker is fresh, - * and a finished or released worker is torn down, never pooled. {@link #recycle} is a convenience - * for {@code release + acquire} with a new distinct pane id. + * and a finished or released worker is torn down, never pooled or reused. * *

The manager implements {@link TurnListener} so the injector's turn boundaries drive * {@code READY → BUSY → DONE} (or {@code FAILED}). It exposes a {@link MemberPresence} view via @@ -180,7 +179,7 @@ public final class SessionManager implements TurnListener { *

CB-544: these are two concerns that used to be fused. Stopping the pane is correct on every * teardown — the worker process must end. Removing the worktree is a destructive act that is only * correct for a deliberately-finished teardown (an explicit stop of a completed session, the - * reaper releasing a genuinely idle one, a context-capped or recycled session). A shutdown drain + * reaper releasing a genuinely idle one, or a context-capped session). A shutdown drain * must stop panes but preserve worktrees: a worker's uncommitted work exists in exactly one * place — its worktree — so deleting it while the daemon simply goes down is silent data loss, * with no copy and no error. Do NOT fuse these back together; the cost of an orphaned worktree @@ -248,7 +247,7 @@ public final class SessionManager implements TurnListener { /** * Register a callback invoked with a session's {@code terminalId} whenever it is released * (CB-516). Every teardown path funnels through {@link #release}, so one hook covers the REST - * and MCP stop tools, the idle-TTL reaper, {@code recycle}, and shutdown drain alike. + * and MCP stop tools, the idle-TTL reaper, and shutdown drain alike. * *

Added rather than injected because {@code MessageService} — one intended listener — is * constructed after this manager (it needs the injector and rendezvous, which need the session @@ -370,19 +369,6 @@ public final class SessionManager implements TurnListener { return launcher.defaultProfile(); } - /** - * Release the old session and acquire a fresh one with the same profile and working directory. - * The new session is guaranteed to have a pane id distinct from the old one (no-reuse invariant). - */ - public MemberSession recycle(String paneId) { - MemberSession old = registry.get(paneId); - if (old == null) { - throw new IllegalArgumentException("no session for paneId " + paneId); - } - release(paneId); - return acquire(old.profile(), old.cwd(), old.cwd(), old.ownerTerminal()); - } - /** The session for {@code paneId}, if it is still registered and not released. */ public Optional get(String paneId) { return Optional.ofNullable(registry.get(paneId)); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java index 5dd2c8e..6488d7e 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java @@ -157,35 +157,6 @@ class SessionManagerTest { assertTrue(sessions.roster().contains(updated), "FAILED is still in acquired-minus-released roster"); } - @Test - void recycleProducesNewPaneIdAndOldOneIsGone() { - FakeHerdr herdr = new FakeHerdr(); - SessionManager sessions = sessionManager(herdr); - MemberSession oldSession = sessions.acquire("ltms-local", null, "/caller", "term_primary"); - String oldPane = oldSession.paneId(); - String oldTerminal = oldSession.terminalId(); - - MemberSession fresh = sessions.recycle(oldPane); - - assertNotEquals(oldPane, fresh.paneId(), "recycle yields a new pane id"); - assertNotEquals(oldTerminal, fresh.terminalId(), "recycle yields a new terminal id"); - assertEquals(oldSession.profile(), fresh.profile(), "profile is preserved"); - assertEquals(oldSession.cwd(), fresh.cwd(), "cwd is preserved"); - assertEquals(oldSession.ownerTerminal(), fresh.ownerTerminal(), "owner is preserved"); - - assertTrue(sessions.get(oldPane).isEmpty(), "old pane is deregistered"); - assertEquals(1, sessions.roster().size(), "only the fresh session remains"); - assertEquals(fresh.paneId(), sessions.roster().getFirst().paneId()); - - // The old session was the first spawn → pane w9:pRoot_1 (CB-519: the registry key is the - // uuid id, so teardown is asserted on the real pane coordinate). - long paneCloseCount = herdr.calls.stream() - .filter(c -> "pane.close".equals(c.method())) - .filter(c -> "w9:pRoot_1".equals(((Map) c.params()).get("pane_id"))) - .count(); - assertEquals(1, paneCloseCount, "the old worker was torn down"); - } - @Test void rosterReflectsAcquiredMinusReleased() { FakeHerdr herdr = new FakeHerdr(); diff --git a/docs/CB-301-Session-Manager.md b/docs/CB-301-Session-Manager.md index 4979d31..07013af 100644 --- a/docs/CB-301-Session-Manager.md +++ b/docs/CB-301-Session-Manager.md @@ -35,7 +35,7 @@ build on. - **No checkpoint content.** Writing `STATE.md` + commit on teardown is CB-302; CB-301 only exposes the release hook it will attach to. -"Recycle" under no-reuse is simply **release + fresh acquire** — a helper, not a pool operation. +Under no-reuse, a released session is terminal. A new `acquire` always creates a fresh session. ## Design @@ -46,11 +46,6 @@ ownership on top. **Package:** new `dev.ltms.bridged.session` — keeps the registry/lifecycle concern separate from the `worker` spawn mechanics. Holds `SessionManager` + `WorkerSession`. -**`recycle` is IN SCOPE for CB-301** (decided): implement `recycle(paneId, …)` = `release` the old -session then `acquire` a fresh one, asserting a new distinct paneId (the no-reuse invariant). It is -a thin convenience over the two primitives, shipped now so the no-reuse teardown+respawn path is -covered by a test from day one. - ### `WorkerSession` (record or small mutable holder) | Field | Source | Notes | @@ -88,7 +83,6 @@ SPAWNING|READY|BUSY|DONE --vanished/drop--> FAILED final class SessionManager { WorkerSession acquire(String profile, String requestedCwd, String callerCwd, String ownerTerminal); void release(String paneId); // deterministic teardown + deregister - WorkerSession recycle(String paneId, ...); // release + acquire (no-reuse convenience) Optional get(String paneId); List roster(); // bridge-owned view (CB-304 consumes this) // lifecycle hooks (package-private): onReady/onDelivered/onComplete/onFailed(target) @@ -120,8 +114,7 @@ final class SessionManager { 3. `release` tears the worker down via `WorkerService.stop` and removes it from `roster()`; a second `release` on the same paneId is a harmless no-op. 4. `onTurnFailed` / drop moves the session to `FAILED` and it is absent from the live roster. -5. `recycle` produces a new paneId and the old one is gone (no-reuse invariant). -6. `roster()` reflects exactly the sessions acquired-minus-released, joined with live status. +5. `roster()` reflects exactly the sessions acquired-minus-released, joined with live status. ## Seams left open (deliberately)