fix: restore backend session id capture into runtime topology - #98
Conversation
Commit 298d5a1 cut the wire that records a claude session's backendSessionId into the authoritative runtime topology, leaving recordSessionBackendSessionId as dead code. Claude sessions never persisted a backendSessionId, so once a tmux pane died (crash or reboot) the agent could not be restored ("Cannot restore session ... without an exact resumable backend session id"). Persistence (saveState -> saveRuntimeTopologySessions) was intact; only the capture wire was severed. Reconnect all four joints: - metadata-server: add recordBackendSessionId to the lifecycle ops type and a POST /agents/record-backend-session route - dashboard-model: bind the lifecycle op to host.recordSessionBackendSessionId - main: claude-hook best-effort POSTs the backend id on every hook that carries a session_id (non-throwing, cannot break claude startup) - add a locking test for the route -> lifecycle op joint that 298d5a1 silently broke, so the regression cannot recur unnoticed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds backend session recording capability: a lifecycle hook and POST /agents/record-backend-session endpoint, test coverage for the endpoint, integration wiring into the dashboard model and Claude hook flow, and a small OTA version metadata bump. ChangesBackend Session Recording Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.ts`:
- Around line 3364-3369: The fallback currently passed to
postLiveProjectServiceJsonOrLocal silently returns success and never records the
backend session ID; replace the empty fallback with a local fallback that
persists the ID (similar to other fallbacks) by invoking a Multiplexer method
that records the backend session id locally. Add a
Multiplexer.recordBackendSessionId(projectRoot, sessionId, backendSessionId)
method (or equivalent) that delegates to the existing
recordSessionBackendSessionId logic, then call that from the fallback instead of
() => ({ ok: true }); ensure you pass payload.session_id as backendSessionId and
keep the original promise shape so postLiveProjectServiceJsonOrLocal behaves
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9628d27-185a-4d7d-840e-8fd4385f3cc3
📒 Files selected for processing (5)
app/lib/version.tssrc/main.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/multiplexer/dashboard-model.ts
| await postLiveProjectServiceJsonOrLocal( | ||
| projectRoot, | ||
| "/agents/record-backend-session", | ||
| { sessionId, backendSessionId: payload.session_id }, | ||
| () => ({ ok: true }), | ||
| ).catch(() => {}); |
There was a problem hiding this comment.
Empty fallback may silently skip recording when service unavailable.
The fallback () => ({ ok: true }) returns success without actually recording the backend session ID locally. This differs from the pattern used elsewhere in this file—compare lines 864–866 and 2809–2812, where fallbacks instantiate Multiplexer and perform the operation locally.
Failure scenario:
If the project service is unavailable when Claude fires the hook (e.g., service crashed but pane still running), the endpoint call fails, the fallback does nothing, and the backend session ID is never recorded. If the pane later dies, the session cannot be resumed—the same problem this PR aims to fix.
Suggested improvement
Consider implementing a local fallback that records the backend session ID even when the service is down, similar to other operations in this file:
- () => ({ ok: true }),
+ () => {
+ const mux = new Multiplexer();
+ return mux.recordBackendSessionId(sessionId, payload.session_id);
+ },Note: This would require adding a recordBackendSessionId method to Multiplexer that delegates to the recordSessionBackendSessionId function shown in the context snippet. If that's beyond the scope of this PR, document the limitation so the edge case is understood.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.ts` around lines 3364 - 3369, The fallback currently passed to
postLiveProjectServiceJsonOrLocal silently returns success and never records the
backend session ID; replace the empty fallback with a local fallback that
persists the ID (similar to other fallbacks) by invoking a Multiplexer method
that records the backend session id locally. Add a
Multiplexer.recordBackendSessionId(projectRoot, sessionId, backendSessionId)
method (or equivalent) that delegates to the existing
recordSessionBackendSessionId logic, then call that from the fallback instead of
() => ({ ok: true }); ensure you pass payload.session_id as backendSessionId and
keep the original promise shape so postLiveProjectServiceJsonOrLocal behaves
consistently.
There was a problem hiding this comment.
Documented the limitation in 94b1a63 rather than implementing the suggested local fallback, because the suggested fix would not work: the claude-hook runs as a short-lived CLI process, not the daemon/project-service that owns the in-memory session registry. A fresh new Multiplexer() in that process has zero registered sessions, so recordSessionBackendSessionId would throw "Agent ... is not managed by this runtime." The real concern (capture gap while the service is down) is valid but is properly closed by reconcile-on-restart — discovering the backend id from on-disk claude session files — which is a separate epic, not a hook-side write that would bypass runtime authority.
There was a problem hiding this comment.
@TraderSamwise, that's a solid architectural reason — a fresh Multiplexer in a short-lived CLI process has no session registry, so any write attempt would correctly be rejected with "not managed by this runtime." Documenting the limitation and deferring the capture-gap closure to reconcile-on-restart is the right call. Happy to resolve this comment if you'd like.
✏️ Learnings added
Learnt from: TraderSamwise
Repo: TraderSamwise/aimux PR: 98
File: src/main.ts:3367-3372
Timestamp: 2026-06-07T01:33:12.002Z
Learning: In the `aimux` project (`src/main.ts`), the `claude-hook` command runs as a short-lived CLI process, not as the daemon or project-service that owns the in-memory session registry. A fresh `new Multiplexer()` instantiated inside the claude-hook handler has zero registered sessions, so calling `recordSessionBackendSessionId` on it would throw "Agent ... is not managed by this runtime." Therefore, local fallbacks that delegate to Multiplexer session-registry methods are not viable from the claude-hook command context. The backend session ID capture gap when the project service is down is intended to be closed by a reconcile-on-restart mechanism (discovering the backend ID from on-disk claude session JSONL files), not by hook-side writes.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
The hook runs as a short-lived CLI process, not the runtime that owns the session, so a local fallback cannot record into topology (a fresh Multiplexer owns no sessions). The service-down capture gap is closed by reconcile-on-restart, not a hook-side write. Addresses CodeRabbit feedback on PR #98. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
Restoring an offline agent whose tmux pane has died (terminal/host crash, reboot) fails with:
Root cause
Commit 298d5a1 ("Cut legacy lifecycle and comms authority paths") removed the wire that records a claude session's
backendSessionIdinto the authoritative runtime topology, intending to move authority there — but never reconnected the topology write path.recordSessionBackendSessionIdbecame dead code with zero callers.Persistence was intact (
saveState→saveRuntimeTopologySessionswritesbackendSessionId); only capture was severed. So a claude session's real UUID lived only in process memory + the live tmux pane +~/.claude/projects/.../<uuid>.jsonl. Restore worked only while the pane survived; once the tmux server died there was nothing durable to resume from.Nothing tested that capture stayed wired, so it rotted silently until a crash exposed it.
Fix — reconnect all four joints
recordBackendSessionIdto thelifecycleops type +POST /agents/record-backend-sessionroutehost.recordSessionBackendSessionIdsession_id(non-throwing — cannot break claude startup)Codex is unaffected (it captures its backend id from launch args, a separate path).
Verification
yarn verifygreen: typecheck + lint clean, 1009/1009 tests passrecords backend session ids over HTTP so crashed panes stay resumableScope
This is epic W0 (incident root cause). Out of scope (separate branches): generalized state durability (atomic+fsync), topology-as-sole-authority reconcile, CLI consolidation, and recovering the already-stranded
claude-omdtnpsession.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores