Add rename and delete for modifiable library nodes

Search-term nodes created via % are now modifiable: e renames the
selected node (prefilled overlay; the new title re-runs the search,
colliding titles merge) and d deletes it, both gated on new additive
LibraryNodeChild.is_editable/is_deletable flags and marked [ed] in the
library list. Two new rpcs follow the create contract: RenameLibraryNode
returns the renamed node (the TUI navigates into it), DeleteLibraryNode
returns the refreshed parent listing. Queued tracks from a renamed or
deleted term keep playing; verified end-to-end against the live API.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Test User 2026-07-20 23:34:47 +02:00
parent d741523e53
commit 333c6e040a
16 changed files with 1178 additions and 38 deletions

View File

@ -0,0 +1,201 @@
# Editable and deletable library nodes
## Context and problem statement
The search feature (`architecture/search.md`) introduced *creatable* nodes:
`%` under `/tidal/search` turns a typed term into a tree node holding search
results. Those nodes are currently immutable — a typo'd term can only be
abandoned, and stale terms accumulate for the lifetime of the server process.
This feature makes such nodes *modifiable*: in the library pane, `e` renames
the selected node (for a search term: re-runs the search under the new term)
and `d` deletes it. Modifiability is a per-node capability advertised by the
provider, exactly like `is_creatable` — the TUI never hardcodes which paths
support what.
Run autonomously per standing user instruction; every decision below records
the options considered and the rationale.
## Assumptions
- "Nodes that got created via `%`" are today exactly the search-term nodes
under `/tidal/search`; the design must not special-case them (playlists are
the obvious future candidate), but they are the only provider implementation
in this iteration.
- Editing means **renaming** (the title is the only user-supplied property a
node has). For a search term, the title *is* the query, so a rename re-runs
the search.
- `e`/`d` act on the **selected item in the library list** (the same selection
model as queueing), not on the currently-open node.
- Proto changes must stay wire-compatible (additive fields/rpcs only), as with
the search feature.
## Decisions
### D1 — Two capability flags, on the child only
Options considered:
1. One `is_modifiable` flag implying both rename and delete.
2. Two flags `is_editable` / `is_deletable`.
3. Flags on both `LibraryNodeChild` and `LibraryNode` (mirroring
`is_creatable`).
**Decision: (2), child-only** — `LibraryNodeChild.is_editable = 5`,
`LibraryNodeChild.is_deletable = 6`. Future node kinds plausibly support only
one of the two (a favorites entry may be deletable but not renamable), and an
extra bool costs nothing on the wire. Unlike `is_creatable` (consumed for the
*open* node: pane hint + `%` target), edit/delete are only ever checked
against the selected **child**, so node-level copies would have no consumer —
they are left out until something reads them.
### D2 — RPC shapes
Options considered:
1. One generic `UpdateLibraryNode` with optional fields + a separate delete.
2. `RenameLibraryNode(path, new_title) → node` and
`DeleteLibraryNode(path) → parent node`.
**Decision: (2).** Rename is the only edit that exists; a generic update
message would be speculative surface area. Both responses carry the node the
TUI should display next, following `CreateLibraryNode`:
- **Rename returns the renamed node** and the TUI navigates into it — the
same "show me the result" behavior as `%` create. The path changes on
rename (term is percent-encoded into the path), so returning the node is
also what tells the client the new path.
- **Delete returns the refreshed parent node** — the user is looking at the
parent listing when they press `d`; returning it saves a follow-up
`GetLibraryNode` and can never serve a stale cached listing.
### D3 — Provider semantics (tidaldy)
- `rename_lib_node(path, new_title)`: `path` must parse to
`TidalPath::SearchTerm`, else `NotSupported`. Title is trimmed; empty →
`InvalidInput`. The old term is **replaced in place** (keeps its position in
the terms list). Renaming to an already-existing term merges: the old entry
is removed, the existing one wins — the list never holds duplicates.
Renaming a term the server doesn't know (stale client cache, restart)
registers the new term — same forgiveness as `get_lib_node` on unknown
terms. Returns `get_lib_node(new_path)`.
- `delete_lib_node(path)`: `path` must parse to `TidalPath::SearchTerm`, else
`NotSupported`. Removing an unknown term succeeds silently — delete is
idempotent. Returns the refreshed parent node (`get_lib_node(/tidal/search)`).
- Both reuse the `search_terms` `RwLock` discipline: poison-tolerant, never
held across an await.
- **Queued search tracks survive rename/delete**: a queue entry
`/tidal/search/<enc-term>/<track-id>` resolves URLs/metadata from the
embedded track id alone; term registration is irrelevant to playback.
### D4 — No delete confirmation (for now)
**Options**: confirm prompt (`y`/`n` mini-mode) vs immediate delete.
**Decision: immediate.** The only deletable nodes are search terms, which are
free to recreate (`%` + retype); a confirmation mode adds a third input state
for no protected value. **Open question**: when higher-value nodes (user
playlists) become deletable, a confirmation step must be revisited — noted
here so the decision is rediscovered.
### D5 — TUI input overlay grows a purpose
`InputState` today hardcodes creation (`parent_path` + buffer). It becomes
```rust
InputState { purpose: InputPurpose, buffer: String }
enum InputPurpose { Create { parent_path }, Rename { path } }
```
- `e` opens the overlay **prefilled with the current title** (append-only
editing as before: chars push, Backspace pops); Enter sends
`MessageFromUi::RenameNode`, Esc cancels. Overlay label: `rename: <buffer>▏`
vs `new node: <buffer>▏`.
- Prefilling means "rename" degrades gracefully to "retype" — no cursor
movement is introduced in this iteration (matches the existing overlay).
- Submitting an unchanged title is sent anyway; the provider treats it as a
no-op rename and returns the node (harmless refresh).
- `d` sends `MessageFromUi::DeleteNode { path }` directly (D4).
- Both keys are silently ignored when the selected item lacks the flag,
mirroring `%` on non-creatable nodes.
- List marker: editable/deletable children render a `[e]`, `[d]` or `[ed]`
suffix in `COLOR_SECONDARY`, alongside the existing `[%]` for creatable
ones. The bindings table gains `e`/`d` in the Library scope (plain `e` and
`d` are unbound there today; `d` only exists in Queue scope), so the help
modal picks them up automatically.
### D6 — Client cache handling
`RpcClient` keeps `library_node_cache`. On rename: evict the **old path** and
the **parent**, insert the returned node under its new path. On delete: evict
the deleted path and the parent, insert the returned parent. (Same reasoning
as create's eviction — a stale `/tidal/search` listing would resurrect the
old term in the UI.)
### D7 — Server plumbing
Two new `ProviderCommand`s (`RenameLibraryNode`, `DeleteLibraryNode`) with the
established bounded(1) reply rendezvous; orchestrator routes `/tidal`-prefixed
paths to the tidal client and answers anything else `NotSupported`. gRPC error
mapping is identical to create: `NotSupported``failed_precondition`,
`InvalidInput``invalid_argument`, rest → `internal`; no internals leak into
`Status` messages.
## Flows
```d2
shape: sequence_diagram
user: { shape: person }
tui: cbd-tui
server: crabidy-server
tidal: tidaldy
user -> tui: "e on selected [ed] node"
tui -> tui: open overlay prefilled with title
user -> tui: edit text, Enter
tui -> server: RenameLibraryNode(path, new_title)
server -> tidal: rename_lib_node
tidal -> tidal: replace term in list (merge on collision)
tidal -> server: node at new path (fresh search)
server -> tui: renamed node
tui -> tui: evict old path + parent, show renamed node
user -> tui: "d on selected [ed] node"
tui -> server: DeleteLibraryNode(path)
server -> tidal: delete_lib_node
tidal -> tidal: remove term (idempotent)
tidal -> server: refreshed parent node
server -> tui: parent node
tui -> tui: evict path + parent, show parent listing
```
Capability flags travel with every listing:
```d2
direction: right
tidaldy: {
search_arm: "get_lib_node(/tidal/search)"
}
proto: "LibraryNodeChild { is_editable=5, is_deletable=6 }"
tui: {
list: "library list: title [ed]"
keys: "e -> rename overlay\nd -> DeleteNode"
}
tidaldy.search_arm -> proto: term children flagged
proto -> tui.list: render marker
proto -> tui.keys: gate actions
```
## Boundaries and risks
- **Proto**: additive only — two rpcs, two child fields (5, 6). Old clients
ignore the flags and never call the rpcs; old servers reject unknown rpcs
with `unimplemented` (tonic default), which the TUI logs without crashing.
- **Rename-to-collision** merges terms; the response node is the *existing*
term's node. The user sees the results they asked for either way.
- **Concurrent clients**: two TUIs editing the same term list race benignly —
the list is a `RwLock`-guarded Vec, every operation is atomic under the
write lock, and stale views self-heal on the next listing fetch.
- **Not in scope**: editing anything but the title; deleting non-search
nodes; confirmation UX (D4); persistent search terms (still per-process,
as shipped by the search feature).

View File

@ -60,6 +60,14 @@ pub enum Action {
/// /tidal/search). While the overlay is open, keys bypass this table
/// entirely (see `App::handle_input_key`).
LibraryCreateNode,
/// Open the input overlay prefilled with the selected item's title to
/// rename it. No-op unless the selection `is_editable` (e.g. a search
/// term, whose rename re-runs the search).
LibraryEditNode,
/// Delete the selected item. No-op unless the selection `is_deletable`.
/// Deliberately unconfirmed: today's deletable nodes (search terms) are
/// free to recreate (architecture/node-editing.md, D4).
LibraryDeleteNode,
// Queue pane
QueueInsertHere,
QueueFirst,
@ -251,6 +259,20 @@ pub const BINDINGS: &[Binding] = &[
action: Action::LibraryCreateNode,
description: "Create node here (e.g. search term)",
},
Binding {
scope: Scope::Library,
mods: KeyModifiers::NONE,
code: KeyCode::Char('e'),
action: Action::LibraryEditNode,
description: "Rename selected node (e.g. search term)",
},
Binding {
scope: Scope::Library,
mods: KeyModifiers::NONE,
code: KeyCode::Char('d'),
action: Action::LibraryDeleteNode,
description: "Delete selected node",
},
Binding {
scope: Scope::Library,
mods: KeyModifiers::NONE,
@ -483,20 +505,20 @@ mod tests {
),
None
);
// 'd' removes a track in the queue but is unbound in the library.
// 'c' clears the queue there but is unbound in the library.
assert_eq!(
lookup(
UiFocus::Queue,
false,
key(KeyCode::Char('d'), KeyModifiers::NONE)
key(KeyCode::Char('c'), KeyModifiers::NONE)
),
Some(Action::QueueRemoveTrack)
Some(Action::QueueClearKeepCurrent)
);
assert_eq!(
lookup(
UiFocus::Library,
false,
key(KeyCode::Char('d'), KeyModifiers::NONE)
key(KeyCode::Char('c'), KeyModifiers::NONE)
),
None
);
@ -600,6 +622,44 @@ mod tests {
);
}
#[test]
fn edit_and_delete_bind_in_the_library_scope_only() {
assert_eq!(
lookup(
UiFocus::Library,
false,
key(KeyCode::Char('e'), KeyModifiers::NONE)
),
Some(Action::LibraryEditNode)
);
// Plain 'd' renames per scope: library delete vs queue remove-track;
// Ctrl+d stays the jump (see control_must_match_exactly).
assert_eq!(
lookup(
UiFocus::Library,
false,
key(KeyCode::Char('d'), KeyModifiers::NONE)
),
Some(Action::LibraryDeleteNode)
);
assert_eq!(
lookup(
UiFocus::Queue,
false,
key(KeyCode::Char('d'), KeyModifiers::NONE)
),
Some(Action::QueueRemoveTrack)
);
assert_eq!(
lookup(
UiFocus::Queue,
false,
key(KeyCode::Char('e'), KeyModifiers::NONE)
),
None
);
}
#[test]
fn open_help_swallows_everything_but_close() {
for focus in [UiFocus::Library, UiFocus::Queue] {

View File

@ -53,6 +53,19 @@ impl Library {
pub fn is_creatable(&self) -> bool {
self.is_creatable
}
/// Path and current title of the selected item, if it may be renamed
/// (`e`). `None` when nothing is selected or the item is not editable.
pub fn selected_editable(&self) -> Option<(String, String)> {
let item = self.list.get(self.list_state.selected()?)?;
item.is_editable
.then(|| (item.path.clone(), item.title.clone()))
}
/// Path of the selected item, if it may be deleted (`d`). `None` when
/// nothing is selected or the item is not deletable.
pub fn selected_deletable(&self) -> Option<String> {
let item = self.list.get(self.list_state.selected()?)?;
item.is_deletable.then(|| item.path.clone())
}
pub fn get_selected(&self) -> Option<Vec<String>> {
if self.list.iter().any(|i| i.marked) {
return Some(
@ -163,6 +176,8 @@ impl Library {
marked: false,
is_queable: true,
is_creatable: false,
is_editable: false,
is_deletable: false,
})
.chain(node.children.iter().map(|c| UiItem {
path: c.path.clone(),
@ -171,6 +186,8 @@ impl Library {
marked: false,
is_queable: c.is_queable,
is_creatable: c.is_creatable,
is_editable: c.is_editable,
is_deletable: c.is_deletable,
}))
.collect();
@ -190,11 +207,22 @@ impl Library {
if i.is_creatable {
text.push_str(" [%]");
}
// Modifiable items advertise their keys: [e], [d] or [ed].
if i.is_editable || i.is_deletable {
text.push_str(" [");
if i.is_editable {
text.push('e');
}
if i.is_deletable {
text.push('d');
}
text.push(']');
}
let style = if i.marked {
Style::default()
.fg(COLOR_GREEN)
.add_modifier(Modifier::BOLD)
} else if i.is_creatable {
} else if i.is_creatable || i.is_editable || i.is_deletable {
Style::default().fg(COLOR_SECONDARY)
} else {
Style::default()

View File

@ -44,6 +44,10 @@ struct UiItem {
is_queable: bool,
/// Children may be created under this item — rendered with a `%` marker.
is_creatable: bool,
/// This item may be renamed (`e`) — part of the `[ed]` marker.
is_editable: bool,
/// This item may be deleted (`d`) — part of the `[ed]` marker.
is_deletable: bool,
}
pub const COLOR_PRIMARY: Color = Color::Rgb(129, 161, 193);
@ -71,6 +75,17 @@ pub enum MessageFromUi {
parent_path: String,
title: String,
},
/// Rename an editable node; the orchestrator navigates the library into
/// the renamed node (its path changes with the title).
RenameNode {
path: String,
new_title: String,
},
/// Delete a deletable node; the orchestrator shows the refreshed parent
/// listing the server returns.
DeleteNode {
path: String,
},
AppendTracks(Vec<String>),
QueueTracks(Vec<String>),
InsertTracks(Vec<String>, usize),
@ -99,13 +114,23 @@ pub enum DispatchResult {
Quit,
}
/// State of the one-line text input overlay (currently only node creation).
/// Why the input overlay is open — decides the submit message and the
/// overlay label.
pub enum InputPurpose {
/// `%`: create a child under the creatable node at `parent_path`.
Create { parent_path: String },
/// `e`: rename the node at `path`. The buffer starts prefilled with the
/// current title, so "rename" degrades to "retype" (append-only editing,
/// no cursor movement).
Rename { path: String },
}
/// State of the one-line text input overlay (node creation and rename).
///
/// While this is `Some` on [`App`], key events bypass `bindings::lookup`
/// entirely and go to [`App::handle_input_key`].
pub struct InputState {
/// Creatable node the new child will be created under.
pub parent_path: String,
pub purpose: InputPurpose,
/// Text typed so far. Append-only editing: chars push, Backspace pops.
pub buffer: String,
}
@ -142,10 +167,10 @@ impl App {
/// Handles one key while the input overlay is open (`input.is_some()`).
///
/// `Esc` cancels, `Enter` submits a non-empty trimmed buffer as
/// `MessageFromUi::CreateNode` (empty submits just close), `Backspace`
/// pops, printable chars append. Everything else is ignored — the
/// bindings table is never consulted while the overlay is open.
/// `Esc` cancels, `Enter` submits a non-empty trimmed buffer as the
/// message matching [`InputPurpose`] (empty submits just close),
/// `Backspace` pops, printable chars append. Everything else is ignored —
/// the bindings table is never consulted while the overlay is open.
pub fn handle_input_key(&mut self, key: crossterm::event::KeyEvent) {
use crossterm::event::KeyCode;
let Some(input) = self.input.as_mut() else {
@ -158,11 +183,21 @@ impl App {
KeyCode::Enter => {
let title = input.buffer.trim().to_string();
if !title.is_empty() {
match &input.purpose {
InputPurpose::Create { parent_path } => {
let _ = self.tx.send(MessageFromUi::CreateNode {
parent_path: input.parent_path.clone(),
parent_path: parent_path.clone(),
title,
});
}
InputPurpose::Rename { path } => {
let _ = self.tx.send(MessageFromUi::RenameNode {
path: path.clone(),
new_title: title,
});
}
}
}
self.input = None;
}
KeyCode::Backspace => {
@ -225,11 +260,30 @@ impl App {
// creatable; silently ignored otherwise.
if self.library.is_creatable() {
self.input = Some(InputState {
purpose: InputPurpose::Create {
parent_path: self.library.path().to_string(),
},
buffer: String::new(),
});
}
}
Action::LibraryEditNode => {
// Opens the overlay prefilled with the current title when the
// selected item is editable; silently ignored otherwise.
if let Some((path, title)) = self.library.selected_editable() {
self.input = Some(InputState {
purpose: InputPurpose::Rename { path },
buffer: title,
});
}
}
Action::LibraryDeleteNode => {
// Unconfirmed by design: today's deletable nodes are search
// terms, free to recreate (architecture/node-editing.md, D4).
if let Some(path) = self.library.selected_deletable() {
let _ = self.tx.send(MessageFromUi::DeleteNode { path });
}
}
Action::QueueInsertHere => {
if let Some(selected) = self.queue.selected() {
self.library.queue_insert(selected);
@ -283,15 +337,19 @@ impl App {
self.queue.render(f, right_side[0], queue_focused);
self.now_playing.render(f, right_side[1]);
// The node-creation input: one line inside the bottom of the
// The node-creation/rename input: one line inside the bottom of the
// library pane, drawn over the list while open.
if let Some(input) = &self.input {
let area = main[0];
if area.height >= 3 && area.width >= 4 {
let label = match &input.purpose {
InputPurpose::Create { .. } => "new node",
InputPurpose::Rename { .. } => "rename",
};
let line = Rect::new(area.x + 1, area.y + area.height - 2, area.width - 2, 1);
f.render_widget(Clear, line);
f.render_widget(
Paragraph::new(format!("new node: {}", input.buffer))
Paragraph::new(format!("{label}: {}", input.buffer))
.style(Style::default().fg(COLOR_SECONDARY)),
line,
);
@ -415,7 +473,9 @@ mod tests {
app.library.update(creatable_node("/tidal/search"));
let _ = app.dispatch(Action::LibraryCreateNode);
let input = app.input.as_ref().expect("input overlay open");
assert_eq!(input.parent_path, "/tidal/search");
assert!(
matches!(&input.purpose, InputPurpose::Create { parent_path } if parent_path == "/tidal/search")
);
assert_eq!(input.buffer, "");
}
@ -497,6 +557,126 @@ mod tests {
assert!(rx.try_recv().is_err(), "whitespace-only must not create");
}
/// A /tidal/search listing with one term child, modifiable or not.
fn search_listing(modifiable: bool) -> LibraryNode {
use crabidy_core::proto::crabidy::LibraryNodeChild;
LibraryNode {
children: vec![LibraryNodeChild {
is_editable: modifiable,
is_deletable: modifiable,
..LibraryNodeChild::new("/tidal/search/abba".to_string(), "abba".to_string(), false)
}],
..creatable_node("/tidal/search")
}
}
#[test]
fn edit_opens_the_overlay_prefilled_only_on_editable_selections() {
let (mut app, _rx) = app();
// Nothing loaded: 'e' must be a no-op.
assert_eq!(
app.dispatch(Action::LibraryEditNode),
DispatchResult::Continue
);
assert!(app.input.is_none());
app.library.update(search_listing(false));
let _ = app.dispatch(Action::LibraryEditNode);
assert!(app.input.is_none(), "non-editable selection: no overlay");
app.library.update(search_listing(true));
let _ = app.dispatch(Action::LibraryEditNode);
let input = app.input.as_ref().expect("rename overlay open");
assert!(
matches!(&input.purpose, InputPurpose::Rename { path } if path == "/tidal/search/abba")
);
assert_eq!(input.buffer, "abba", "prefilled with the current title");
}
#[test]
fn rename_submit_sends_the_trimmed_new_title() {
let (mut app, rx) = app();
app.library.update(search_listing(true));
let _ = app.dispatch(Action::LibraryEditNode);
type_str(&mut app, " tribute ");
app.handle_input_key(key(crossterm::event::KeyCode::Enter));
assert!(app.input.is_none());
match rx.try_recv() {
Ok(MessageFromUi::RenameNode { path, new_title }) => {
assert_eq!(path, "/tidal/search/abba");
assert_eq!(new_title, "abba tribute");
}
other => panic!("expected RenameNode, got {:?}", other.is_ok()),
}
}
#[test]
fn rename_cancels_and_emptied_buffers_send_nothing() {
let (mut app, rx) = app();
app.library.update(search_listing(true));
let _ = app.dispatch(Action::LibraryEditNode);
app.handle_input_key(key(crossterm::event::KeyCode::Esc));
assert!(app.input.is_none());
assert!(rx.try_recv().is_err(), "Esc must not rename");
let _ = app.dispatch(Action::LibraryEditNode);
for _ in 0.."abba".len() {
app.handle_input_key(key(crossterm::event::KeyCode::Backspace));
}
app.handle_input_key(key(crossterm::event::KeyCode::Enter));
assert!(app.input.is_none());
assert!(rx.try_recv().is_err(), "emptied buffer must not rename");
}
#[test]
fn delete_sends_only_for_deletable_selections() {
let (mut app, rx) = app();
let _ = app.dispatch(Action::LibraryDeleteNode);
assert!(rx.try_recv().is_err(), "no selection: no delete");
app.library.update(search_listing(false));
let _ = app.dispatch(Action::LibraryDeleteNode);
assert!(rx.try_recv().is_err(), "non-deletable selection: no delete");
app.library.update(search_listing(true));
let _ = app.dispatch(Action::LibraryDeleteNode);
match rx.try_recv() {
Ok(MessageFromUi::DeleteNode { path }) => {
assert_eq!(path, "/tidal/search/abba");
}
other => panic!("expected DeleteNode, got {:?}", other.is_ok()),
}
}
#[test]
fn modifiable_items_are_marked_and_the_rename_overlay_is_labelled() {
let (mut app, _rx) = app();
app.library.update(search_listing(true));
let draw = |app: &mut App| {
let backend = ratatui::backend::TestBackend::new(80, 24);
let mut terminal = ratatui::Terminal::new(backend).expect("test terminal");
terminal.draw(|f| app.render(f)).expect("draw app");
let buf = terminal.backend().buffer();
let mut text = String::new();
for y in 0..buf.area.height {
for x in 0..buf.area.width {
text.push_str(buf[(x, y)].symbol());
}
text.push('\n');
}
text
};
let text = draw(&mut app);
assert!(text.contains("abba [ed]"), "modifiable marker: {text}");
let _ = app.dispatch(Action::LibraryEditNode);
let text = draw(&mut app);
assert!(text.contains("rename: abba"), "rename overlay label");
}
#[test]
fn queue_clear_actions_carry_the_keep_current_flag() {
let (mut app, rx) = app();

View File

@ -64,6 +64,8 @@ impl Queue {
marked: false,
is_queable: false,
is_creatable: false,
is_editable: false,
is_deletable: false,
})
.collect();

View File

@ -127,6 +127,30 @@ async fn poll(
}
}
},
MessageFromUi::RenameNode { path, new_title } => {
// Navigates the library into the renamed node on success;
// on failure the library stays where it is.
match rpc_client.rename_library_node(&path, &new_title).await {
Ok(node) => {
let _ = tx.send(MessageToUi::ReplaceLibraryNode(node.clone()));
}
Err(err) => {
error!(path, new_title, "failed to rename node: {err}");
}
}
},
MessageFromUi::DeleteNode { path } => {
// Shows the refreshed parent listing on success; on
// failure the library stays where it is.
match rpc_client.delete_library_node(&path).await {
Ok(parent) => {
let _ = tx.send(MessageToUi::ReplaceLibraryNode(parent.clone()));
}
Err(err) => {
error!(path, "failed to delete node: {err}");
}
}
},
MessageFromUi::AppendTracks(uuids) => {
rpc_client.append_tracks(uuids).await?
}

View File

@ -1,10 +1,10 @@
use crabidy_core::proto::crabidy::{
crabidy_service_client::CrabidyServiceClient, AppendRequest, ChangeVolumeRequest,
ClearQueueRequest, CreateLibraryNodeRequest, GetLibraryNodeRequest, GetUpdateStreamRequest,
GetUpdateStreamResponse, InitRequest, InitResponse, InsertRequest, LibraryNode, NextRequest,
PrevRequest, QueueRequest, RemoveRequest, ReplaceRequest, RestartTrackRequest,
SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest, ToggleRepeatRequest,
ToggleShuffleRequest,
ClearQueueRequest, CreateLibraryNodeRequest, DeleteLibraryNodeRequest, GetLibraryNodeRequest,
GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest, InitResponse, InsertRequest,
LibraryNode, NextRequest, PrevRequest, QueueRequest, RemoveRequest, RenameLibraryNodeRequest,
ReplaceRequest, RestartTrackRequest, SetCurrentRequest, ToggleMuteRequest, TogglePlayRequest,
ToggleRepeatRequest, ToggleShuffleRequest,
};
use std::{collections::HashMap, error::Error, fmt, time::Duration};
@ -123,6 +123,58 @@ impl RpcClient {
Ok(&self.library_node_cache[&path])
}
/// Renames an editable node and returns it at its (changed) path.
///
/// Cache contract: the old path's entry and the parent's entry are
/// evicted (the parent's child listing changed; the old path is dead),
/// and the renamed node is inserted under its new path.
pub async fn rename_library_node(
&mut self,
path: &str,
new_title: &str,
) -> Result<&LibraryNode, Box<dyn Error>> {
let request = Request::new(RenameLibraryNodeRequest {
path: path.to_string(),
new_title: new_title.to_string(),
});
let response = self.client.rename_library_node(request).await?;
let Some(node) = response.into_inner().node else {
return Err(Box::new(RpcClientError::NotFound));
};
// The old path is dead and the parent's child listing changed;
// cached copies would resurrect the old term.
self.library_node_cache.remove(path);
if let Some(parent) = crabidy_core::parent_path(path) {
self.library_node_cache.remove(parent);
}
let new_path = node.path.clone();
self.library_node_cache.insert(new_path.clone(), node);
Ok(&self.library_node_cache[&new_path])
}
/// Deletes a node and returns the refreshed parent listing.
///
/// Cache contract: the deleted path's entry and the parent's stale entry
/// are evicted, and the returned parent node is inserted fresh.
pub async fn delete_library_node(
&mut self,
path: &str,
) -> Result<&LibraryNode, Box<dyn Error>> {
let request = Request::new(DeleteLibraryNodeRequest {
path: path.to_string(),
});
let response = self.client.delete_library_node(request).await?;
let Some(parent) = response.into_inner().parent else {
return Err(Box::new(RpcClientError::NotFound));
};
// Drop the deleted node and the stale parent listing; the response
// carries the fresh parent to cache instead.
self.library_node_cache.remove(path);
let parent_path = parent.path.clone();
self.library_node_cache.insert(parent_path.clone(), parent);
Ok(&self.library_node_cache[&parent_path])
}
pub async fn append_tracks(&mut self, paths: Vec<String>) -> Result<(), Box<dyn Error>> {
let append_request = Request::new(AppendRequest { paths });
self.client.append(append_request).await?;

View File

@ -12,6 +12,14 @@ service CrabidyService {
// a search term and the created node holds its results. Idempotent: an
// existing title returns the existing node.
rpc CreateLibraryNode(CreateLibraryNodeRequest) returns (CreateLibraryNodeResponse);
// Renames a node whose listing entry sets is_editable. For a search term
// the title is the query, so a rename re-runs the search; the node's path
// changes with the title. Renaming onto an existing sibling title merges
// with it. Returns the renamed node at its new path.
rpc RenameLibraryNode(RenameLibraryNodeRequest) returns (RenameLibraryNodeResponse);
// Deletes a node whose listing entry sets is_deletable. Idempotent:
// deleting an already-gone node succeeds. Returns the refreshed parent.
rpc DeleteLibraryNode(DeleteLibraryNodeRequest) returns (DeleteLibraryNodeResponse);
// Queue
rpc Queue(QueueRequest) returns (QueueResponse);
@ -67,6 +75,27 @@ message CreateLibraryNodeResponse {
LibraryNode node = 1;
}
message RenameLibraryNodeRequest {
// Path of the node to rename.
string path = 1;
// New human-entered title; the provider derives the new path segment.
string new_title = 2;
}
message RenameLibraryNodeResponse {
// The renamed node, at its (possibly changed) path.
LibraryNode node = 1;
}
message DeleteLibraryNodeRequest {
// Path of the node to delete.
string path = 1;
}
message DeleteLibraryNodeResponse {
// The parent node with the deleted child gone what a client should
// display after the delete, without a follow-up GetLibraryNode.
LibraryNode parent = 1;
}
// Queue
message QueueRequest {
repeated string paths = 1;
@ -160,6 +189,10 @@ message LibraryNodeChild {
bool is_queable = 3;
// Children may be created under this node (see CreateLibraryNode).
bool is_creatable = 4;
// This node may be renamed (see RenameLibraryNode).
bool is_editable = 5;
// This node may be deleted (see DeleteLibraryNode).
bool is_deletable = 6;
}
message QueueModifiers {

View File

@ -41,6 +41,25 @@ pub trait ProviderClient: std::fmt::Debug + Send + Sync {
parent_path: &str,
title: &str,
) -> Result<LibraryNode, ProviderError>;
/// Renames a node advertised as editable (`LibraryNodeChild.is_editable`).
///
/// For a search term the title is the query, so a rename re-runs the
/// search; the node's path changes with the title. Renaming onto an
/// existing sibling title merges with it (that node is returned).
/// Errors: [`ProviderError::NotSupported`] when the path is not
/// editable, [`ProviderError::InvalidInput`] when the new title is empty
/// or whitespace-only.
async fn rename_lib_node(
&self,
path: &str,
new_title: &str,
) -> Result<LibraryNode, ProviderError>;
/// Deletes a node advertised as deletable (`LibraryNodeChild.is_deletable`).
///
/// Idempotent: deleting an already-gone node succeeds. Returns the
/// refreshed parent node (what a client should display next). Errors:
/// [`ProviderError::NotSupported`] when the path is not deletable.
async fn delete_lib_node(&self, path: &str) -> Result<LibraryNode, ProviderError>;
}
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
@ -137,14 +156,17 @@ impl LibraryNode {
}
impl LibraryNodeChild {
/// A regular, non-creatable child. Creatable children (e.g. the search
/// node) set `is_creatable` explicitly via struct update.
/// A regular, non-creatable, immutable child. Creatable/editable/
/// deletable children (e.g. the search node and its terms) set the
/// capability flags explicitly via struct update.
pub fn new(path: String, title: String, is_queable: bool) -> Self {
Self {
path,
title,
is_queable,
is_creatable: false,
is_editable: false,
is_deletable: false,
}
}
}
@ -248,4 +270,14 @@ mod tests {
let _ = decode_segment(bad);
}
}
#[test]
fn child_new_defaults_all_capability_flags_off() {
// Wire contract: plain children are immutable; providers opt into
// capabilities explicitly via struct update.
let child = LibraryNodeChild::new("/tidal/x".to_string(), "x".to_string(), true);
assert!(!child.is_creatable);
assert!(!child.is_editable);
assert!(!child.is_deletable);
}
}

View File

@ -163,6 +163,19 @@ pub enum ProviderCommand {
title: String,
result_tx: flume::Sender<Result<LibraryNode, ProviderError>>,
},
/// Renames an editable node (see `ProviderClient::rename_lib_node`);
/// replies with the renamed node at its new path.
RenameLibraryNode {
path: String,
new_title: String,
result_tx: flume::Sender<Result<LibraryNode, ProviderError>>,
},
/// Deletes a deletable node (see `ProviderClient::delete_lib_node`);
/// replies with the refreshed parent node.
DeleteLibraryNode {
path: String,
result_tx: flume::Sender<Result<LibraryNode, ProviderError>>,
},
}
impl ProviderCommand {
@ -172,6 +185,8 @@ impl ProviderCommand {
Self::GetTrackUrls { .. } => "get_track_urls",
Self::ResolveTracks { .. } => "resolve_tracks",
Self::CreateLibraryNode { .. } => "create_library_node",
Self::RenameLibraryNode { .. } => "rename_library_node",
Self::DeleteLibraryNode { .. } => "delete_library_node",
}
}
}

View File

@ -56,6 +56,22 @@ impl ProviderOrchestrator {
error!("failed to send create_library_node result: {err}");
}
}
ProviderCommand::RenameLibraryNode {
path,
new_title,
result_tx,
} => {
let result = self.rename_lib_node(&path, &new_title).await;
if let Err(err) = result_tx.send_async(result).await {
error!("failed to send rename_library_node result: {err}");
}
}
ProviderCommand::DeleteLibraryNode { path, result_tx } => {
let result = self.delete_lib_node(&path).await;
if let Err(err) = result_tx.send_async(result).await {
error!("failed to send delete_library_node result: {err}");
}
}
}
}
@ -194,4 +210,30 @@ impl ProviderClient for ProviderOrchestrator {
warn!(parent_path, "no provider supports creating nodes here");
Err(ProviderError::NotSupported)
}
/// Routes to the provider that owns the path. The synthetic root's own
/// children are fixed and never editable.
#[instrument(skip(self))]
async fn rename_lib_node(
&self,
path: &str,
new_title: &str,
) -> Result<LibraryNode, ProviderError> {
if path == tidaldy::PROVIDER_ROOT || path.starts_with("/tidal/") {
return self.tidal_client.rename_lib_node(path, new_title).await;
}
warn!(path, "no provider supports renaming this node");
Err(ProviderError::NotSupported)
}
/// Routes to the provider that owns the path. The synthetic root's own
/// children are fixed and never deletable.
#[instrument(skip(self))]
async fn delete_lib_node(&self, path: &str) -> Result<LibraryNode, ProviderError> {
if path == tidaldy::PROVIDER_ROOT || path.starts_with("/tidal/") {
return self.tidal_client.delete_lib_node(path).await;
}
warn!(path, "no provider supports deleting this node");
Err(ProviderError::NotSupported)
}
}

View File

@ -2,14 +2,16 @@ use crate::{PlaybackCommand, PlaybackMessage, ProviderCommand, ProviderMessage};
use crabidy_core::proto::crabidy::{
crabidy_service_server::CrabidyService, get_update_stream_response::Update as StreamUpdate,
AppendRequest, AppendResponse, ChangeVolumeRequest, ChangeVolumeResponse, ClearQueueRequest,
ClearQueueResponse, CreateLibraryNodeRequest, CreateLibraryNodeResponse, GetLibraryNodeRequest,
ClearQueueResponse, CreateLibraryNodeRequest, CreateLibraryNodeResponse,
DeleteLibraryNodeRequest, DeleteLibraryNodeResponse, GetLibraryNodeRequest,
GetLibraryNodeResponse, GetUpdateStreamRequest, GetUpdateStreamResponse, InitRequest,
InitResponse, InsertRequest, InsertResponse, NextRequest, NextResponse, PrevRequest,
PrevResponse, QueueRequest, QueueResponse, RemoveRequest, RemoveResponse, ReplaceRequest,
ReplaceResponse, RestartTrackRequest, RestartTrackResponse, SaveQueueRequest,
SaveQueueResponse, SetCurrentRequest, SetCurrentResponse, StopRequest, StopResponse,
ToggleMuteRequest, ToggleMuteResponse, TogglePlayRequest, TogglePlayResponse,
ToggleRepeatRequest, ToggleRepeatResponse, ToggleShuffleRequest, ToggleShuffleResponse,
PrevResponse, QueueRequest, QueueResponse, RemoveRequest, RemoveResponse,
RenameLibraryNodeRequest, RenameLibraryNodeResponse, ReplaceRequest, ReplaceResponse,
RestartTrackRequest, RestartTrackResponse, SaveQueueRequest, SaveQueueResponse,
SetCurrentRequest, SetCurrentResponse, StopRequest, StopResponse, ToggleMuteRequest,
ToggleMuteResponse, TogglePlayRequest, TogglePlayResponse, ToggleRepeatRequest,
ToggleRepeatResponse, ToggleShuffleRequest, ToggleShuffleResponse,
};
use crabidy_core::ProviderError;
use std::pin::Pin;
@ -146,6 +148,91 @@ impl CrabidyService for RpcService {
}
}
/// Renames an editable node via the provider loop. Same error mapping as
/// `create_library_node`: `NotSupported` → `failed_precondition`,
/// `InvalidInput` → `invalid_argument`, everything else `internal`.
#[instrument(skip(self, request), fields(path, new_title))]
async fn rename_library_node(
&self,
request: Request<RenameLibraryNodeRequest>,
) -> Result<Response<RenameLibraryNodeResponse>, Status> {
let RenameLibraryNodeRequest { path, new_title } = request.into_inner();
tracing::Span::current().record("path", path.as_str());
tracing::Span::current().record("new_title", new_title.as_str());
debug!("received rename_library_node request");
let (result_tx, result_rx) = flume::bounded(1);
self.provider_tx
.send_async(ProviderMessage::new(ProviderCommand::RenameLibraryNode {
path,
new_title,
result_tx,
}))
.await
.map_err(|err| {
error!("provider channel closed: {err}");
Status::internal("provider unavailable")
})?;
let result = result_rx.recv_async().await.map_err(|err| {
error!("no reply from provider: {err}");
Status::internal("provider did not reply")
})?;
match result {
Ok(node) => Ok(Response::new(RenameLibraryNodeResponse {
node: Some(node),
})),
Err(ProviderError::NotSupported) => {
Err(Status::failed_precondition("this node cannot be renamed"))
}
Err(ProviderError::InvalidInput) => Err(Status::invalid_argument("invalid node title")),
Err(err) => {
error!("rename_library_node failed: {err}");
Err(Status::internal(err.to_string()))
}
}
}
/// Deletes a deletable node via the provider loop and returns the
/// refreshed parent. Same error mapping as `create_library_node`.
#[instrument(skip(self, request), fields(path))]
async fn delete_library_node(
&self,
request: Request<DeleteLibraryNodeRequest>,
) -> Result<Response<DeleteLibraryNodeResponse>, Status> {
let DeleteLibraryNodeRequest { path } = request.into_inner();
tracing::Span::current().record("path", path.as_str());
debug!("received delete_library_node request");
let (result_tx, result_rx) = flume::bounded(1);
self.provider_tx
.send_async(ProviderMessage::new(ProviderCommand::DeleteLibraryNode {
path,
result_tx,
}))
.await
.map_err(|err| {
error!("provider channel closed: {err}");
Status::internal("provider unavailable")
})?;
let result = result_rx.recv_async().await.map_err(|err| {
error!("no reply from provider: {err}");
Status::internal("provider did not reply")
})?;
match result {
Ok(parent) => Ok(Response::new(DeleteLibraryNodeResponse {
parent: Some(parent),
})),
Err(ProviderError::NotSupported) => {
Err(Status::failed_precondition("this node cannot be deleted"))
}
// No provider raises this for delete today; mapped anyway so the
// contract stays uniform across the node-mutation rpcs.
Err(ProviderError::InvalidInput) => Err(Status::invalid_argument("invalid node path")),
Err(err) => {
error!("delete_library_node failed: {err}");
Err(Status::internal(err.to_string()))
}
}
}
#[instrument(skip(self, request), fields(paths))]
async fn queue(
&self,

83
plan/node-editing.md Normal file
View File

@ -0,0 +1,83 @@
# Plan — editable and deletable nodes
Ordered tasks for the `implement` stage. Inputs: `architecture/node-editing.md`,
stubs across the workspace, gates in `quality/node-editing.md`. Tests:
`devenv shell -- cargo test --workspace` (10 failing at plan time = the
target: 5 in tidaldy, 5 in cbd-tui). Use the session-local `CARGO_TARGET_DIR`
if `target/` contains artifacts owned by the repo owner.
## 1. Term-list mutations (tidaldy)
- [x] Implement `rename_search_term(old, new)`: under one write lock, if
`new` already exists remove the `old` slot (merge); else if `old`
exists replace it in place; else push `new` (stale-client
forgiveness). Implement `remove_search_term(term)`: `retain`
everything but `term`. Both poison-tolerant like
`register_search_term`, no await under the lock. **Verify**:
`rename_replaces_in_place_and_merges_duplicates` passes; gate "lock
discipline".
## 2. Provider rename/delete (tidaldy)
- [x] `rename_lib_node`: trim new title → `InvalidInput` if empty;
`parse_path(path)?` must be `TidalPath::SearchTerm(old)` else
`NotSupported`; decode old term, `rename_search_term`, return
`get_lib_node(join_path(parent, encode_segment(new)))`. **Verify**:
`rename_rejects_empty_titles_and_foreign_paths` passes (validation
before any network call).
- [x] `delete_lib_node`: `parse_path(path)?` must be `SearchTerm` else
`NotSupported`; decode + `remove_search_term` (idempotent); return
`get_lib_node(/tidal/search)`. **Verify**: `delete_rejects_foreign_paths`
and `delete_removes_terms_idempotently_and_returns_the_parent` pass.
- [x] Flag term children in the `Search` arm of `get_lib_node`:
`is_editable: true, is_deletable: true` via struct update. **Verify**:
`search_node_lists_created_terms` passes (extended assertion).
## 3. Server handlers (crabidy-server)
- [x] Replace the `todo!()` bodies of `rpc.rs
rename_library_node`/`delete_library_node`: record span fields,
bounded(1) rendezvous with the matching `ProviderCommand`, map errors
exactly like `create_library_node` (`NotSupported` →
`failed_precondition` with an operation-specific message,
`InvalidInput``invalid_argument`, rest → `internal`). **Verify**:
gates "error mapping" + "bounded rendezvous"; `cargo check`.
(Orchestrator routing and the command-loop arms were finalized at
stub time — re-read them against the gate rather than re-doing them.)
## 4. TUI library accessors + marker (cbd-tui)
- [x] Implement `Library::selected_editable` / `selected_deletable` from
`list_state.selected()` + the `UiItem` flags. **Verify**:
`edit_opens_the_overlay_prefilled_only_on_editable_selections`,
`delete_sends_only_for_deletable_selections`,
`rename_submit_sends_the_trimmed_new_title`,
`rename_cancels_and_emptied_buffers_send_nothing` pass (dispatch and
overlay logic already landed with the stubs).
- [x] Render the `[e]`/`[d]`/`[ed]` suffix for modifiable items in
`Library::render`, `COLOR_SECONDARY` like `[%]`. **Verify**:
`modifiable_items_are_marked_and_the_rename_overlay_is_labelled`
passes; gate "marked in UI".
## 5. TUI ↔ server wiring (cbd-tui)
- [x] `RpcClient::rename_library_node`: send the rpc, evict the old path and
the parent (`crabidy_core::parent_path`), insert + return the renamed
node. `delete_library_node`: send the rpc, evict the deleted path and
the parent, insert + return the returned parent node. **Verify**: gate
"cache eviction" (read; the cache is private, as with create).
- [x] `main.rs poll`: replace the `todo!()` arms — `RenameNode` calls
`rename_library_node`, `DeleteNode` calls `delete_library_node`; on
success `ReplaceLibraryNode(node)`, on failure `error!` and leave the
UI unchanged. **Verify**: gate "failures never panic".
## 6. End-to-end + gates sweep
- [x] Exercise the full path against the live API if the local tidal config
is available (create a term, rename it, delete it, confirm the parent
listing and that a queued search track still resolves a URL); note the
outcome in the summary. If unavailable, rely on the offline tests and
say so.
- [x] Sweep: no `todo!()` left (workspace grep), fmt + clippy + tests green,
every `quality/node-editing.md` box checked, docs updated where
behavior shifted. Append the outcome + deviations to `plan/summary.md`.

View File

@ -1,5 +1,44 @@
# Implementation summaries
## node-editing (2026-07-20)
Built per `plan/node-editing.md`: search-term nodes (created via `%`) are now
modifiable — `e` opens the input overlay prefilled with the current title and
renames (re-running the search; merge on title collision), `d` deletes
without confirmation (documented decision, architecture/node-editing.md D4).
Capabilities travel as `LibraryNodeChild.is_editable`/`is_deletable` (fields
5/6, child-only — no consumer for node-level copies), surfaced as a `[ed]`
marker; two new rpcs `RenameLibraryNode` (returns the renamed node, TUI
navigates into it) and `DeleteLibraryNode` (returns the refreshed parent).
All 59 workspace tests green; every gate in `quality/node-editing.md`
checked. Verified against the live Tidal API with a temporary ignored probe
(removed after passing): create `beatles` → rename to `rolling stones`
(in-place, 20 tracks / 40 children) → a track queued under the old term
still resolved a stream URL → delete emptied the listing.
The whole feature ran autonomously per standing instruction; decisions are
recorded in `architecture/node-editing.md` (options + rationale per topic).
### Deviations from plan / architecture (node-editing)
- **Self-rename bug caught by the gate tests**: the first
`rename_search_term` implementation deleted a term renamed to itself (the
merge branch removed the "old" slot). Fixed with an explicit `old != new`
guard; the architecture text ("merge on collision") now implicitly means
*distinct* titles.
- **`pane_bindings_only_match_their_own_pane` (help-modal suite) updated**:
it asserted plain `d` is unbound in the library — now it is
`LibraryDeleteNode` by design; the test's queue-only example key moved to
`c`.
- **`delete_library_node` also maps `InvalidInput``invalid_argument`**
although no provider raises it for delete today — keeps the error contract
uniform across the three node-mutation rpcs.
- **End-to-end check ran at the provider layer** (as with search): no
interactive terminal/audio device in this environment; the gRPC handler
and TUI layers above it are covered by unit tests and review.
- **Environment note**: builds/tests again ran with a session-local
`CARGO_TARGET_DIR` (owner-built artifacts in `target/`); no repo change.
## search (2026-07-20)
Built per `plan/search.md`: `%` inside `/tidal/search` opens a one-line input;

91
quality/node-editing.md Normal file
View File

@ -0,0 +1,91 @@
# Quality gates — editable and deletable nodes
Checklist for the `implement` stage. Automatic tests live in
`crabidy-core/src/lib.rs`, `tidaldy/src/lib.rs`, and
`cbd-tui/src/app/{bindings,mod}.rs` test modules; run with
`devenv shell -- cargo test --workspace` (network-dependent tidaldy tests
stay `#[ignore]`). 10 tests fail at gate-writing time — they define the
target.
## Contract & wire
- [x] Proto changes are additive only: existing field numbers untouched;
`LibraryNodeChild.is_editable = 5`, `is_deletable = 6`;
`RenameLibraryNode` and `DeleteLibraryNode` rpcs present. Old clients
keep working against the new server.
- [x] `rpc.rs` (server) maps errors per the documented contract for **both**
new rpcs: `NotSupported``failed_precondition`, `InvalidInput`
`invalid_argument`, everything else → `internal`. No `color-eyre`
report or debug formatting of internals leaks into `Status` messages.
- [x] `ProviderOrchestrator::{rename,delete}_lib_node` route `/tidal`-prefixed
paths to the Tidal client and answer anything else (including the
synthetic root `/`) with `NotSupported` — same prefix discipline as
`create_lib_node`.
- [x] Both new commands follow the existing bounded(1)-reply rendezvous
pattern; no new unbounded channels anywhere in the feature.
## Provider semantics (tidaldy)
- [x] `rename_lib_node` trims the new title; empty/whitespace →
`InvalidInput`; any path that is not a `SearchTerm` (including
`/tidal/search` itself) → `NotSupported`; validation happens before any
network call.
- [x] Rename replaces the term **in place** (keeps its list position);
renaming onto an existing term merges (the old slot is removed, no
duplicates ever); renaming an unknown term registers the new one
(stale-client forgiveness). Returns `get_lib_node(new_path)` — the
node at its new percent-encoded path.
- [x] `delete_lib_node` accepts only `SearchTerm` paths (`NotSupported`
otherwise), removes the term idempotently (unknown term → success),
and returns the refreshed `/tidal/search` parent node.
- [x] `rename_search_term` / `remove_search_term` follow the established
lock discipline: poison-tolerant, the `search_terms` lock is never
held across an `await`.
- [x] Term children returned by the `Search` arm set
`is_editable: true, is_deletable: true`; nothing else in the provider
sets either flag.
- [x] Queued search tracks keep playing after a rename/delete of their term:
`get_urls_for_track` / `get_metadata_for_track` resolve from the track
id embedded in the path, independent of term registration.
## No panics on user input (hard rule)
- [x] All `todo!()` stubs from api-design are gone (grep the workspace).
- [x] Rename/delete failures (network, auth, malformed paths from stale
clients) surface as `ProviderError`/`Status`, never a panic; the TUI's
rename and delete paths handle an error reply without crashing the
orchestrate task (log + stay put).
## TUI behavior
- [x] `e` opens the overlay only when the **selected** item `is_editable`,
prefilled with the current title; `d` sends `DeleteNode` only when the
selected item `is_deletable`; both are silent no-ops otherwise
(including when nothing is selected or the list is empty).
- [x] The overlay carries its purpose: submit sends `CreateNode` for
`InputPurpose::Create` and `RenameNode` for `InputPurpose::Rename`;
the rendered label distinguishes them (`new node:` vs `rename:`).
While the overlay is open the bindings table stays unreachable
(existing input-mode bypass, unchanged).
- [x] Delete is deliberately unconfirmed (architecture/node-editing.md D4);
the open question about confirmation for higher-value nodes is
preserved in the architecture doc, not silently dropped.
- [x] Modifiable children are visibly marked in the library list (`[e]`,
`[d]` or `[ed]` suffix, `COLOR_SECONDARY`), coexisting with the `[%]`
marker for creatable ones.
- [x] `RpcClient::rename_library_node` evicts the old path **and** the
parent entry, then caches the node under its new path;
`delete_library_node` evicts the deleted path and the parent, then
caches the returned parent. No stale `/tidal/search` listing can
resurrect an old term.
- [x] On successful rename the library navigates into the renamed node; on
successful delete it shows the refreshed parent; on failure the
library stays where it was.
## Code quality
- [x] Public items added in all five crates have doc comments matching final
behavior; the `e`/`d` bindings appear in the help modal automatically.
- [x] No new dependencies.
- [x] `devenv shell -- cargo fmt --check`, `cargo clippy --workspace`
(no new warnings), `cargo test --workspace` all pass.

View File

@ -260,11 +260,17 @@ impl crabidy_core::ProviderClient for Client {
.search_terms_snapshot()
.iter()
.map(|term| {
crabidy_core::proto::crabidy::LibraryNodeChild::new(
// Term nodes are the modifiable nodes: renamable
// (`e`) and deletable (`d`).
crabidy_core::proto::crabidy::LibraryNodeChild {
is_editable: true,
is_deletable: true,
..crabidy_core::proto::crabidy::LibraryNodeChild::new(
crabidy_core::join_path(path, &crabidy_core::encode_segment(term)),
term.clone(),
false,
)
}
})
.collect(),
is_queable: false,
@ -306,6 +312,48 @@ impl crabidy_core::ProviderClient for Client {
let term_path = crabidy_core::join_path(parent_path, &crabidy_core::encode_segment(term));
self.get_lib_node(&term_path).await
}
#[instrument(skip(self))]
async fn rename_lib_node(
&self,
path: &str,
new_title: &str,
) -> Result<crabidy_core::proto::crabidy::LibraryNode, crabidy_core::ProviderError> {
let new_term = new_title.trim();
if new_term.is_empty() {
return Err(crabidy_core::ProviderError::InvalidInput);
}
// Only search terms are editable.
let TidalPath::SearchTerm(encoded_old) = parse_path(path)? else {
warn!(path, "renaming not supported here");
return Err(crabidy_core::ProviderError::NotSupported);
};
let old_term = crabidy_core::decode_segment(encoded_old);
// In-place replace keeps the list position; renaming onto an
// existing term merges with it.
self.rename_search_term(&old_term, new_term);
// A SearchTerm path always has the search node as its parent.
let parent = crabidy_core::parent_path(path).unwrap_or(PROVIDER_ROOT);
let new_path = crabidy_core::join_path(parent, &crabidy_core::encode_segment(new_term));
self.get_lib_node(&new_path).await
}
#[instrument(skip(self))]
async fn delete_lib_node(
&self,
path: &str,
) -> Result<crabidy_core::proto::crabidy::LibraryNode, crabidy_core::ProviderError> {
// Only search terms are deletable.
let TidalPath::SearchTerm(encoded) = parse_path(path)? else {
warn!(path, "deleting not supported here");
return Err(crabidy_core::ProviderError::NotSupported);
};
// Idempotent: removing an already-gone term is a success.
self.remove_search_term(&crabidy_core::decode_segment(encoded));
// A SearchTerm path always has the search node as its parent.
let parent = crabidy_core::parent_path(path).unwrap_or(PROVIDER_ROOT);
self.get_lib_node(parent).await
}
}
/// The root of this provider in the global library tree.
@ -645,6 +693,39 @@ impl Client {
}
}
/// Renames `old` to `new` in place, keeping its position. If `new`
/// already exists elsewhere the entries merge (the `old` slot is
/// removed) — the list never holds duplicates. An unknown `old` behaves
/// like registration of `new` (stale-client forgiveness). The lock is
/// released before any await point.
fn rename_search_term(&self, old: &str, new: &str) {
let mut terms = match self.search_terms.write() {
Ok(terms) => terms,
Err(poisoned) => poisoned.into_inner(),
};
if terms.iter().any(|t| t == new) {
// Merge — but a self-rename must not delete the very term it
// is supposed to keep.
if old != new {
terms.retain(|t| t != old);
}
} else if let Some(slot) = terms.iter_mut().find(|t| *t == old) {
*slot = new.to_string();
} else {
terms.push(new.to_string());
}
}
/// Removes a term. Unknown terms are a no-op — delete is idempotent.
/// The lock is released before any await point.
fn remove_search_term(&self, term: &str) {
let mut terms = match self.search_terms.write() {
Ok(terms) => terms,
Err(poisoned) => poisoned.into_inner(),
};
terms.retain(|t| t != term);
}
/// Builds the node for one search term: its `tracks` are the track
/// results (queueable individually, paths under this node), its children
/// are artist and album results pointing at **canonical**
@ -1171,6 +1252,96 @@ mod tests {
assert_eq!(titles, vec!["AC/DC", "abba"]);
assert_eq!(node.children[0].path, "/tidal/search/AC%2FDC");
assert!(node.children.iter().all(|c| !c.is_queable));
// Term nodes are the modifiable nodes: renamable and deletable.
assert!(node
.children
.iter()
.all(|c| c.is_editable && c.is_deletable));
}
#[tokio::test]
async fn rename_rejects_empty_titles_and_foreign_paths() {
use crabidy_core::ProviderError;
let client = offline_client();
// Validation happens before any network call, so this works offline.
assert_eq!(
client.rename_lib_node("/tidal/search/abba", " ").await,
Err(ProviderError::InvalidInput)
);
assert_eq!(
client
.rename_lib_node("/tidal/playlists/xyz", "queen")
.await,
Err(ProviderError::NotSupported)
);
// The search node itself is creatable, not editable.
assert_eq!(
client.rename_lib_node("/tidal/search", "queen").await,
Err(ProviderError::NotSupported)
);
assert_eq!(
client.rename_lib_node("/nope", "queen").await,
Err(ProviderError::MalformedPath)
);
}
#[tokio::test]
async fn delete_rejects_foreign_paths() {
use crabidy_core::ProviderError;
let client = offline_client();
assert_eq!(
client.delete_lib_node("/tidal/playlists/xyz").await,
Err(ProviderError::NotSupported)
);
// The search node itself is not deletable, only its terms are.
assert_eq!(
client.delete_lib_node("/tidal/search").await,
Err(ProviderError::NotSupported)
);
assert_eq!(
client.delete_lib_node("/nope").await,
Err(ProviderError::MalformedPath)
);
}
#[tokio::test]
async fn delete_removes_terms_idempotently_and_returns_the_parent() {
let client = offline_client();
client.register_search_term("abba");
client.register_search_term("queen");
let parent = client
.delete_lib_node("/tidal/search/abba")
.await
.expect("refreshed parent");
assert_eq!(parent.path, "/tidal/search");
let titles: Vec<_> = parent.children.iter().map(|c| c.title.as_str()).collect();
assert_eq!(titles, vec!["queen"]);
// Deleting an already-gone term succeeds — delete is idempotent.
let parent = client
.delete_lib_node("/tidal/search/abba")
.await
.expect("idempotent delete");
assert_eq!(parent.children.len(), 1);
}
#[test]
fn rename_replaces_in_place_and_merges_duplicates() {
let client = offline_client();
client.register_search_term("abba");
client.register_search_term("queen");
client.register_search_term("kiss");
// In-place: the renamed term keeps its list position.
client.rename_search_term("queen", "wham");
assert_eq!(client.search_terms_snapshot(), vec!["abba", "wham", "kiss"]);
// Merge: renaming onto an existing term drops the old slot.
client.rename_search_term("abba", "kiss");
assert_eq!(client.search_terms_snapshot(), vec!["wham", "kiss"]);
// An unknown old term registers the new one (stale-client forgiveness).
client.rename_search_term("ghost", "toto");
assert_eq!(client.search_terms_snapshot(), vec!["wham", "kiss", "toto"]);
// Renaming a term to itself changes nothing.
client.rename_search_term("kiss", "kiss");
assert_eq!(client.search_terms_snapshot(), vec!["wham", "kiss", "toto"]);
}
#[test]