-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Context
After the local-first Automerge migration (#540) and the save-to-daemon delegation (#545, #567), save-as migrates the notebook to a new room based on the file path. The daemon writes the .ipynb, the Tauri relay disconnects from the old room and reconnects to a new room keyed by the new path. However, the kernel stays attached to the old room and gets orphaned.
Current behavior (post-migration)
- User has untitled notebook with running kernel (room = UUID-based)
- User does Cmd+S → save dialog → picks
~/projects/analysis.ipynb - Daemon writes file via
SaveNotebook { path: Some(...) } - Tauri relay disconnects from old room, reconnects to new path-based room
- Old room still has the kernel running — no peers, so it schedules eviction (30s timeout via
keep_alive_secs) - New room has no kernel — daemon auto-launches a fresh kernel (from prewarmed pool or cold start)
- User loses kernel state (variables, imports, etc.)
- After 30s, old room evicts and shuts down the orphaned kernel
What the user sees
- Kernel status briefly shows "Not Started" then flips to "Starting" → "Idle" as the new kernel launches
- All variables and imports from the previous session are gone
- If they had a long-running computation, it continues in the orphaned kernel until eviction (but output can't reach the frontend)
Architecture details (current)
Save-as flow (crates/notebook/src/lib.rs save_notebook_as):
- Sends
NotebookRequest::SaveNotebook { format_cells: true, path: Some(path) }to daemon - Daemon formats cells, writes
.ipynbto the new path - Tauri updates
context.path, window title, dirty flag - Clears sync handle (disconnects from old room)
- Calls
initialize_notebook_sync()with new path-derivednotebook_id - New room created on daemon, auto-launch triggers fresh kernel
Room eviction (crates/runtimed/src/notebook_sync_server.rs L794-854):
- When all peers disconnect, schedules eviction check after
keep_alive_secs(default 30s) - If no peers reconnect, shuts down kernel and removes room
Kernel state: Kernels are tied to rooms via NotebookRoom.kernel. There's no mechanism to migrate a RoomKernel from one room to another.
Recommended approach: Option C (auto-relaunch) with dismissable warning
The cleanest approach for the common case (saving an untitled notebook for the first time):
- Before save-as: Check if a kernel is running in the current room
- If yes: Show a brief toast: "Kernel will restart in the new location" (dismissable, not blocking)
- During save-as: Explicitly shut down the old kernel before disconnecting (don't wait for 30s eviction)
- After reconnect: The daemon auto-launches a fresh kernel with the correct working directory (derived from the new file path)
Why not kernel migration (Option B)?
Kernel migration (move kernel from old room to new room) is technically possible but has issues:
- The kernel's cwd was set at launch time (from the old room's working_dir or the prewarmed pool). Moving it to a path-based room doesn't change the kernel's cwd — relative paths in the notebook would break.
- The kernel's connection file, ZMQ sockets, and PID are tied to the old room's
RoomKernelstruct. Moving them requires careful lifecycle management. - The daemon's
auto_launch_kernellogic in the new room would need to detect "kernel already present" and skip launching — adding complexity.
Implementation sketch
In save_notebook_as (Tauri side):
// Before disconnecting from old room, shut down the kernel explicitly
if let Some(handle) = sync_handle {
let _ = handle.send_request(NotebookRequest::ShutdownKernel {}).await;
}This is one line. The kernel shuts down immediately instead of lingering for 30s. The new room's auto-launch gives the user a fresh kernel with the correct cwd.
Toast notification (frontend):
The save callback in useAutomergeNotebook could return metadata about what happened (e.g., { saved: true, kernelRestarted: true }). App.tsx shows a brief toast: "Saved to analysis.ipynb — kernel restarted."
Or simpler: useDaemonKernel already tracks kernelStatus. When it transitions from "idle" → "not started" → "starting" → "idle" during save-as, the toolbar status indicator shows this naturally. The user sees the kernel restart in real time. A toast might be unnecessary.
Edge cases
- No kernel running: Save-as proceeds without any kernel lifecycle concern. No toast needed.
- Long-running computation: The explicit shutdown interrupts it. This is the same behavior as "Restart Kernel" — the user should save/checkpoint before save-as if they have important state. A warning dialog ("You have a running kernel. Save As will restart it. Continue?") could be added for this case.
- Save to same directory: Even if the new path is in the same directory, the room ID changes (UUID → path-based), so the kernel still restarts. This is correct — the kernel cwd should match the file location.
- Save-as failure: If the daemon save fails, we don't disconnect from the old room, so the kernel stays. No orphan.
Related
- Local-First Automerge Migration Plan #540 — Local-first Automerge migration (completed)
- feat(runtimed): Phase 1.4 - delegate save-to-disk to daemon #545 — Save-to-disk delegated to daemon
- refactor: eliminate NotebookState from save, clone, and reconnect #567 — Eliminate NotebookState from save/clone/reconnect
- PR fix(notebook): migrate room ID when saving to a new path #387 — Original save-as room migration