Skip to content

Cut offline lifecycle writes to topology - #353

Merged
TraderSamwise merged 3 commits into
masterfrom
chore/core-sidecar-next-78
Jul 7, 2026
Merged

Cut offline lifecycle writes to topology#353
TraderSamwise merged 3 commits into
masterfrom
chore/core-sidecar-next-78

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Write stop/runtime-exit/resume backend recovery transitions to runtime topology instead of mutating offline projection first
  • Reload offline projections from topology after resurrection and runtime exits
  • Add regression coverage that stale offline projection cannot suppress current topology writes

Verification

  • yarn vitest src/multiplexer/session-runtime-core.test.ts src/multiplexer/runtime-state.test.ts src/multiplexer/persistence-methods.test.ts src/multiplexer/runtime-lifecycle-methods.test.ts
  • yarn typecheck
  • yarn lint && yarn vitest && yarn build

Summary by CodeRabbit

  • Bug Fixes
    • Offline sessions now persist more reliably after session exit, resume, and resurrection.
    • Restored backend session IDs are now correctly reflected in offline session state.
    • Dashboard and status updates refresh automatically after offline session changes.
    • Session recovery no longer depends on stale in-memory offline lists, reducing mismatches between what you see and what’s saved.

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jul 7, 2026 4:11am

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e0d97d87-04a6-460b-9346-5960dd50c931

📥 Commits

Reviewing files that changed from the base of the PR and between 19bea9d and 0bbad64.

📒 Files selected for processing (5)
  • docs/north-star-completion-tracker.md
  • src/multiplexer/runtime-state.test.ts
  • src/multiplexer/runtime-state.ts
  • src/multiplexer/session-runtime-core.test.ts
  • src/multiplexer/session-runtime-core.ts
📝 Walkthrough

Walkthrough

Offline session state persistence is refactored across session-runtime-core.ts, runtime-state.ts, and persistence-methods.ts to write through upsertTopologySession and refresh via loadOfflineTopologySessions/topology queries instead of directly mutating host.offlineSessions. Corresponding tests are updated to assert against topology state.

Changes

Offline topology persistence

Layer / File(s) Summary
Session-exit offline persistence
src/multiplexer/session-runtime-core.ts, src/multiplexer/session-runtime-core.test.ts
Adds projectRootFor helper and upsertTopologySession import; exit handler upserts offline topology entries (preserving restoreBlockedReason logic) and calls optional host.loadOfflineTopologySessions?.() after saveState(); tests assert against listTopologySessionStates and add a stale-projection test.
Stop-to-offline and resume-offline updates
src/multiplexer/runtime-state.ts, src/multiplexer/runtime-state.test.ts
stopSessionToOffline and resumeOfflineSession persist offline state via upsertTopologySession with projectRootFor, and resumeOfflineSession reloads via loadOfflineTopologySessions instead of mutating host.offlineSessions directly; test asserts empty host.offlineSessions and topology row.
Graveyard resurrection refresh
src/multiplexer/persistence-methods.ts, src/multiplexer/persistence-methods.test.ts
resurrectGraveyardSession removes manual offlineSessions merge, instead reloading offline topology sessions and triggering snapshot invalidation, statusline write, metadata notify, and dashboard re-render; tests assert empty offlineSessions and one call to loadOfflineTopologySessions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SessionRuntimeCore
  participant RuntimeState
  participant PersistenceMethods
  participant TopologyStore
  participant Host

  SessionRuntimeCore->>TopologyStore: upsertTopologySession(offline entry, exit)
  SessionRuntimeCore->>Host: loadOfflineTopologySessions()

  RuntimeState->>TopologyStore: upsertTopologySession(offline entry, stop/resume)
  RuntimeState->>Host: loadOfflineTopologySessions()

  PersistenceMethods->>TopologyStore: resurrectTopologySession(sessionId)
  PersistenceMethods->>Host: loadOfflineTopologySessions()
  PersistenceMethods->>Host: invalidateDesktopStateSnapshot / writeStatuslineFile / notifyChange
Loading

Possibly related PRs

  • TraderSamwise/aimux#1: Both PRs modify offline/graveyard persistence logic including resurrectGraveyardSession and offline state transitions.
  • TraderSamwise/aimux#31: Both PRs move graveyard/offline lifecycle flows to topology-backed persistence via upsert/reload instead of direct host.offlineSessions mutation.
  • TraderSamwise/aimux#100: Both PRs modify resumeOfflineSession in src/multiplexer/runtime-state.ts for offline session resumption logic.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: shifting offline lifecycle writes from in-memory projection handling to topology-backed persistence.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/core-sidecar-next-78

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/multiplexer/session-runtime-core.test.ts (1)

487-487: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Select the expected offline row by id.

listTopologySessionStates({ statuses: ["offline"] })[0] makes these assertions depend on topology row order. Use .find((s) => s.id === expectedId) so the tests validate the intended session when additional offline rows exist.

Also applies to: 527-530, 601-601

🤖 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/multiplexer/session-runtime-core.test.ts` at line 487, The
offline-session assertions in session-runtime-core.test.ts rely on array index
order from listTopologySessionStates({ statuses: ["offline"] }), which can break
when more offline rows exist. Update the affected tests to select the expected
session by id using listTopologySessionStates(...) and .find((s) => s.id ===
expectedId) before calling toMatchObject, so the assertions target the intended
row regardless of ordering.
🤖 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/multiplexer/runtime-state.ts`:
- Line 533: The topology/session writes in runtime-state should not rely on
projectRootFor(host) when host.projectRoot is missing, because it can fall back
to process.cwd() and attach the row to the wrong rig. Update the lifecycle
write(s) around upsertTopologySession in runtime-state.ts to use getRepoRoot()
as the fallback, matching the existing default behavior in
upsertTopologySession, and apply the same change to the other affected writes in
this area.

In `@src/multiplexer/session-runtime-core.ts`:
- Around line 27-29: The fallback in projectRootFor is using process.cwd(),
which can override the initialized repo root and write topology data under the
wrong rig. Update projectRootFor so it falls back to the repo root used by
initialization (the same default path as upsertTopologySession/getRepoRoot), and
keep the existing host.projectRoot trimming logic intact.

---

Nitpick comments:
In `@src/multiplexer/session-runtime-core.test.ts`:
- Line 487: The offline-session assertions in session-runtime-core.test.ts rely
on array index order from listTopologySessionStates({ statuses: ["offline"] }),
which can break when more offline rows exist. Update the affected tests to
select the expected session by id using listTopologySessionStates(...) and
.find((s) => s.id === expectedId) before calling toMatchObject, so the
assertions target the intended row regardless of ordering.
🪄 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: 87e60a9a-1234-4861-959c-4c32a6fd3a77

📥 Commits

Reviewing files that changed from the base of the PR and between 509b5bc and 19bea9d.

📒 Files selected for processing (6)
  • src/multiplexer/persistence-methods.test.ts
  • src/multiplexer/persistence-methods.ts
  • src/multiplexer/runtime-state.test.ts
  • src/multiplexer/runtime-state.ts
  • src/multiplexer/session-runtime-core.test.ts
  • src/multiplexer/session-runtime-core.ts
💤 Files with no reviewable changes (1)
  • src/multiplexer/persistence-methods.ts

Comment thread src/multiplexer/runtime-state.ts
Comment thread src/multiplexer/session-runtime-core.ts
@TraderSamwise

Copy link
Copy Markdown
Owner Author

Sub-agent review finding fixed in 0bbad64: preserved runtime exits now reload the offline projection from topology before saveState(), and explicit stops prune stale projection entries before saveState(), with regression coverage using real saveState reconciliation.

@TraderSamwise
TraderSamwise merged commit 5929d08 into master Jul 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant