Skip to content

feat(runtimed): daemon-owned notebook loading infrastructure (#598)#601

Merged
rgbkrk merged 9 commits intomainfrom
feat/daemon-owned-notebook-loading
Mar 8, 2026
Merged

feat(runtimed): daemon-owned notebook loading infrastructure (#598)#601
rgbkrk merged 9 commits intomainfrom
feat/daemon-owned-notebook-loading

Conversation

@rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Mar 8, 2026

Summary

Implement the daemon-side infrastructure for daemon-owned notebook loading (issue #598). This PR completes Phases 1-3 and 5, providing all the building blocks for the Tauri integration in a follow-up PR.

Phase 1: Protocol Extension

  • Add OpenNotebook { path } and CreateNotebook { runtime, working_dir } handshake variants
  • Add NotebookConnectionInfo response with notebook_id, cell_count, needs_trust_approval, and error fields
  • Protocol version included in response (no separate ProtocolCapabilities needed for new handshakes)

Phase 2: Daemon-Side Loading Functions

  • load_notebook_from_disk() - parses .ipynb and populates NotebookDoc with cells and metadata
  • create_empty_notebook() - creates new notebook with one code cell and runtime-specific metadata
  • build_new_notebook_metadata() - generates metadata for Python (uv/conda) or Deno runtimes
  • Reuses existing parse_cells_from_ipynb() for cell parsing

Phase 3: Connection Handlers

  • handle_open_notebook: Canonicalizes path → derives notebook_id → creates room → loads from disk (if first connection) → checks trust → returns NotebookConnectionInfo → continues with sync
  • handle_create_notebook: Generates UUID notebook_id → creates room → populates empty notebook → returns NotebookConnectionInfo → continues with sync
  • Race condition prevention: check-and-load under single write lock
  • Room cleanup on failure to prevent stale trust state

Phase 5: NotebookSyncClient Extension

  • connect_open_split() - opens existing notebook via daemon, returns NotebookConnectionInfo
  • connect_create_split() - creates new notebook via daemon, returns NotebookConnectionInfo
  • init_open_notebook() / init_create_notebook() / do_initial_sync() - internal init helpers
  • Protocol version validation on NotebookConnectionInfo

What's Next

Phase 4-7 (Tauri integration) in a follow-up PR:

  • Update window creation to use connect_open_split/connect_create_split
  • Remove NotebookState-based loading
  • Update session restore
  • End-to-end integration tests

Test Plan

  • cargo fmt
  • cargo clippy --all-targets -- -D warnings (clean)
  • cargo test -p runtimed (233 tests pass, +6 new unit tests)
  • Manual QA - no divergence noted

Closes #598 (daemon-side infrastructure complete; Tauri integration to follow)

rgbkrk added 2 commits March 7, 2026 17:58
)

Phase 1 of daemon-owned notebook loading:

- Add Handshake::OpenNotebook { path } variant
- Add Handshake::CreateNotebook { runtime, working_dir } variant
- Add NotebookConnectionInfo response struct with:
  - notebook_id (derived by daemon)
  - cell_count (for progress indication)
  - needs_trust_approval (for untrusted deps)
  - error (for failures)
- Add placeholder handlers in daemon.rs (return "not implemented")
- Add serialization tests

The daemon will derive notebook_id from path (for existing files) or
generate env_id (for new notebooks), eliminating duplicate derivation
logic in Tauri.
Phase 2 of daemon-owned notebook loading:

- Add `load_notebook_from_disk()` - loads .ipynb file and populates
  NotebookDoc with cells and metadata
- Add `create_empty_notebook()` - creates new notebook with one code
  cell and runtime-specific metadata
- Add `build_new_notebook_metadata()` - builds NotebookMetadataSnapshot
  for new notebooks (Python/Deno, UV/Conda)
- Add `parse_metadata_from_ipynb()` - extracts metadata using existing
  `NotebookMetadataSnapshot::from_metadata_value()`

Reuses existing `parse_cells_from_ipynb()` for cell parsing.
These functions will be called by the OpenNotebook/CreateNotebook
handshake handlers.
@rgbkrk rgbkrk marked this pull request as draft March 8, 2026 02:41
rgbkrk added 6 commits March 7, 2026 19:18
Implement the actual daemon-side handlers for the new handshake types:

- handle_open_notebook: Canonicalizes path to derive notebook_id, creates
  room, loads from disk if first connection, checks trust, sends
  NotebookConnectionInfo, then continues with sync

- handle_create_notebook: Generates UUID as notebook_id, creates room,
  populates empty notebook with one code cell, sends NotebookConnectionInfo

Multi-window support: Second window joining same notebook skips disk load
and joins existing room via Automerge sync.

Also updated create_empty_notebook to accept optional env_id parameter
to avoid double UUID generation.
Initialize conda notebooks with ["conda-forge"] instead of empty channels
to match the launch logic normalization. Prevents false channel-drift
detection that would trigger unnecessary restart-required behavior.
…ng (#598)

Add new connection methods for the OpenNotebook and CreateNotebook handshakes:

- connect_open_split: Opens existing notebook file via daemon, returns
  NotebookConnectionInfo with notebook_id and trust status

- connect_create_split: Creates new notebook via daemon, returns
  NotebookConnectionInfo with generated notebook_id

Also adds:
- init_open_notebook: Generic init for OpenNotebook handshake
- init_create_notebook: Generic init for CreateNotebook handshake
- do_initial_sync: Extracted common Automerge sync logic

Both Unix and Windows platforms are supported.
Fixes two issues found in review:

1. Race condition: The cell_count check is now done atomically under
   the write lock. Previously, two concurrent opens could both see
   cell_count == 0 (checked with read lock), then both load, duplicating
   cells.

2. Stale room on failure: On load failure, the room is now removed from
   the rooms map before returning. Previously, the room would persist
   with potentially incorrect trust state (computed at room creation),
   which could bypass trust checks on later retries.
The new OpenNotebook/CreateNotebook handshakes return NotebookConnectionInfo
which includes the protocol version. Sending ProtocolCapabilities again
after that would break the client which expects AutomergeSync frames next.

Added skip_capabilities parameter to handle_notebook_sync_connection:
- false for legacy NotebookSync handshake (sends ProtocolCapabilities)
- true for OpenNotebook/CreateNotebook (protocol already in response)
1. CreateNotebook: Clean up room on failure for consistency with
   OpenNotebook (prevents stale UUID-keyed rooms from leaking)

2. Client: Add protocol version validation for NotebookConnectionInfo
   in init_open_notebook and init_create_notebook (defensive check)

3. Tests: Add unit tests for build_new_notebook_metadata (3 branches:
   deno, python-uv, python-conda) and create_empty_notebook (python,
   deno, custom env_id)

The sync loop duplication between init and do_initial_sync is noted
for follow-up refactoring but not addressed here.
@rgbkrk rgbkrk marked this pull request as ready for review March 8, 2026 03:43
@rgbkrk rgbkrk changed the title feat(runtimed): daemon-owned notebook loading phase 1-2 (#598) feat(runtimed): daemon-owned notebook loading infrastructure (#598) Mar 8, 2026
@rgbkrk rgbkrk merged commit 8e5399c into main Mar 8, 2026
23 of 24 checks passed
@rgbkrk rgbkrk deleted the feat/daemon-owned-notebook-loading branch March 8, 2026 04:14
rgbkrk pushed a commit that referenced this pull request Mar 8, 2026
…601)

Update both audit documents to cover new protocol surfaces introduced
by the daemon-owned notebook loading infrastructure (OpenNotebook and
CreateNotebook handshakes) and dead code removal from the local-first
migration.

New findings:
- OpenNotebook accepts arbitrary file paths for reading (medium)
- Error paths drain unbounded reader data (medium)
- do_initial_sync uses 100ms timeout for convergence (medium)
- CreateNotebook runtime field not validated (low)
- load_notebook_from_disk has no file size limit (low)

Positive changes noted:
- Trust verification fallback branches removed (#595)
- Daemon-side doc population reduces CRDT merge complexity
- Atomic check-and-load prevents duplicate cell population
- Client-side protocol version validation in new handshakes

https://claude.ai/code/session_01ToG68d36uTowAeraL5CF7R
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.

refactor: daemon-owned notebook loading — eliminate NotebookState

1 participant