Skip to content

refactor(storage, workspace, cli): Implement layered storage backend#543

Open
JeanMertz wants to merge 1 commit intomainfrom
prr110
Open

refactor(storage, workspace, cli): Implement layered storage backend#543
JeanMertz wants to merge 1 commit intomainfrom
prr110

Conversation

@JeanMertz
Copy link
Copy Markdown
Collaborator

Replace Workspace's Option<Storage> with four non-optional trait objects — PersistBackend, LoadBackend, LockBackend, and SessionBackend — eliminating pervasive Option branching throughout the workspace layer.

The root problem was that at least 15 methods on Workspace branched on self.storage.as_ref(), yielding three different failure modes for the same underlying condition: some silently no-oped, others returned errors, and others returned None. A disable_persistence boolean added a third dimension of optionality, and ConversationMut carried an Option<writer> that callers could not inspect.

The new jp_storage::backend module introduces:

  • FsStorageBackend — wraps Storage, implements all four traits, and retains filesystem-specific path accessors (storage_path, user_storage_path) outside the trait surface.
  • InMemoryStorageBackend — fully in-memory implementation for tests and future non-filesystem environments.
  • NullPersistBackend — silently discards all writes; used for --no-persist and error-path persistence suppression.
  • NoopLockGuard — a no-op ConversationLockGuard for contexts where real locking is not needed.

Workspace now holds persist, loader, locker, and sessions as always-present trait objects. Workspace::new initialises them with NullPersistBackend + InMemoryStorageBackend; callers opt into filesystem storage by calling with_backend(fs). The old builder chain Workspace::new(root).persisted_at(&storage)?.with_local_storage()? is replaced by constructing FsStorageBackend first, then passing it in. disable_persistence() retains its signature but now swaps the persist field for NullPersistBackend instead of flipping a boolean.

ConversationLock and ConversationMut lose their internal optionality: writer and lock_guard are always present. The if let Some(writer) branches in flush and Drop are gone — NullPersistBackend handles the no-write case through the trait.

jp_workspace::persist is deleted; the traits are now owned by jp_storage. Workspace::sanitize delegates to
LoadBackend::sanitize, which returns an empty report for in-memory backends. remove_ephemeral_conversations is reimplemented on Workspace using LoadBackend::load_expired_conversation_ids and PersistBackend::remove, preserving the fast-path metadata reader.

In jp_cli, Ctx gains an fs_backend: Option<Arc<FsStorageBackend>> field. Path queries and config loading that previously went through ctx.workspace.storage_path() now go through ctx.storage_path() or ctx.fs_backend. cleanup_stale_files and
build_partial_from_cfg_args receive the typed backend reference explicitly, keeping filesystem concerns out of the Workspace API.

The RFD 073 design document is added alongside the implementation.

@JeanMertz JeanMertz marked this pull request as draft April 12, 2026 05:31
@JeanMertz JeanMertz marked this pull request as ready for review April 14, 2026 13:34
Replace `Workspace`'s `Option<Storage>` with four non-optional trait
objects — `PersistBackend`, `LoadBackend`, `LockBackend`, and
`SessionBackend` — eliminating pervasive `Option` branching throughout
the workspace layer.

The root problem was that at least 15 methods on `Workspace` branched on
`self.storage.as_ref()`, yielding three different failure modes for the
same underlying condition: some silently no-oped, others returned
errors, and others returned `None`. A `disable_persistence` boolean
added a third dimension of optionality, and `ConversationMut` carried an
`Option<writer>` that callers could not inspect.

The new `jp_storage::backend` module introduces:

- `FsStorageBackend` — wraps `Storage`, implements all four traits, and
  retains filesystem-specific path accessors (`storage_path`,
  `user_storage_path`) outside the trait surface.
- `InMemoryStorageBackend` — fully in-memory implementation for tests
  and future non-filesystem environments.
- `NullPersistBackend` — silently discards all writes; used for
  `--no-persist` and error-path persistence suppression.
- `NoopLockGuard` — a no-op `ConversationLockGuard` for contexts where
  real locking is not needed.

`Workspace` now holds `persist`, `loader`, `locker`, and `sessions` as
always-present trait objects. `Workspace::new` initialises them with
`NullPersistBackend` + `InMemoryStorageBackend`; callers opt into
filesystem storage by calling `with_backend(fs)`. The old builder chain
`Workspace::new(root).persisted_at(&storage)?.with_local_storage()?` is
replaced by constructing `FsStorageBackend` first, then passing it in.
`disable_persistence()` retains its signature but now swaps the
`persist` field for `NullPersistBackend` instead of flipping a boolean.

`ConversationLock` and `ConversationMut` lose their internal
optionality: `writer` and `lock_guard` are always present. The `if let
Some(writer)` branches in `flush` and `Drop` are gone —
`NullPersistBackend` handles the no-write case through the trait.

`jp_workspace::persist` is deleted; the traits are now owned by
`jp_storage`. `Workspace::sanitize` delegates to
`LoadBackend::sanitize`, which returns an empty report for in-memory
backends. `remove_ephemeral_conversations` is reimplemented on
`Workspace` using `LoadBackend::load_expired_conversation_ids` and
`PersistBackend::remove`, preserving the fast-path metadata reader.

In `jp_cli`, `Ctx` gains an `fs_backend: Option<Arc<FsStorageBackend>>`
field. Path queries and config loading that previously went through
`ctx.workspace.storage_path()` now go through `ctx.storage_path()` or
`ctx.fs_backend`. `cleanup_stale_files` and
`build_partial_from_cfg_args` receive the typed backend reference
explicitly, keeping filesystem concerns out of the `Workspace` API.

The RFD 073 design document is added alongside the implementation.

Signed-off-by: Jean Mertz <git@jeanmertz.com>
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