From 18ab2ab5950c941bb4f9269c71f3ab7f76811837 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Thu, 17 Sep 2026 08:26:29 +0200 Subject: [PATCH] docs(sessions): correct what a failed rebuild is actually likely to be MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the feature against a real server for the first time, on an isolated instance, and two claims in the code turned out to be wrong. A rebuild that fails after the session is registered was documented as commonly caused by a CLI binary missing from a freshly booted machine's PATH. It is not: the resolver finds its binary by absolute path, so PATH never enters into it, and a server started without claude on PATH restored every session normally. Nor does an un-enterable workspace fail — tmux falls back to another directory and the pane comes up there. Neither obvious cause throws, so the discard path is defended rather than expected, and the comments now say that instead of naming a cause that cannot happen. The four review rounds that shaped this path all reasoned about a trigger none of them could test. The path itself is still worth having, since a mux failure would reach it, but its comments should not claim a likelihood the machine disagrees with. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) --- src/web/routes/reboot-restore-routes.ts | 10 ++++++++-- test/routes/reboot-restore-rebuild-failure.test.ts | 13 +++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/web/routes/reboot-restore-routes.ts b/src/web/routes/reboot-restore-routes.ts index 3516ff19..3ff27b4f 100644 --- a/src/web/routes/reboot-restore-routes.ts +++ b/src/web/routes/reboot-restore-routes.ts @@ -220,8 +220,14 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes // One entry that will not start must not stop the rest of the pass, and // must not leave a registered session with no pane behind it: by this // point the session is in `ctx.sessions`, holds a tab-layout slot and has - // listeners, and the commonest cause is a CLI binary that is not on the - // PATH of a freshly booted machine. + // listeners. + // + // Reaching this is rarer than it looks, measured against a real server: + // the CLI resolver finds its binary by absolute path rather than through + // PATH, and tmux falls back to another directory rather than failing when + // it cannot enter the workspace, so neither of the two obvious "freshly + // booted machine" failures throws. What is left is the mux layer itself + // failing, which is why this path is defended rather than expected. console.error(`[reboot-restore] failed to rebuild ${entry.sessionId}:`, err); // Not cleanupSession(): that is the user-initiated delete, and it would // count this session's historical tokens into the lifetime totals, demote diff --git a/test/routes/reboot-restore-rebuild-failure.test.ts b/test/routes/reboot-restore-rebuild-failure.test.ts index 7a69e30d..98cf7a17 100644 --- a/test/routes/reboot-restore-rebuild-failure.test.ts +++ b/test/routes/reboot-restore-rebuild-failure.test.ts @@ -4,10 +4,15 @@ * The other route test file deliberately uses workspaces that do not exist, so it * never reaches `new Session()`. This one mocks the `Session` module so the route * runs its whole construction path — `addSession`, `setupSessionListeners`, - * `reapplyPersistedSessionState`, `startInteractive` — and then throws where a - * real one would when the CLI binary is missing from a freshly booted machine's - * PATH. Without the mock there is no way to exercise that path, which is how the - * original version of this route shipped a session leak the tests could not see. + * `reapplyPersistedSessionState`, `startInteractive` — and then throws. + * + * The mock is the only way in. Driven against a real server, `startInteractive()` + * does not throw for either obvious cause: the CLI resolver finds its binary by + * absolute path rather than through PATH, and tmux falls back to another + * directory rather than failing when it cannot enter the workspace. A mux-layer + * failure is what is left, and it cannot be provoked from a test. Without the + * mock this path would go unexercised, which is how the original version of this + * route shipped a session leak the tests could not see. * * It also covers the session caps, because those too are only reachable once the * route is actually willing to build something.