From 8d10e0aa6f380622f693d3d25063066fd4215cd2 Mon Sep 17 00:00:00 2001 From: Edwin Date: Fri, 10 Jul 2026 20:28:24 -0700 Subject: [PATCH] feat: unify fork harness selection --- crates/cli/src/app.rs | 80 ++++++++----------- crates/cli/src/keymap.rs | 46 +++++------ crates/cli/src/ui.rs | 6 +- specs/0031-sessions-fork-into-new-siblings.md | 14 ++-- ...rks-carry-lineage-branch-rail-and-merge.md | 30 +++---- 5 files changed, 74 insertions(+), 102 deletions(-) diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index 7e50beb8..ef4c493f 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -704,11 +704,10 @@ pub enum MinibufferIntent { session_id: String, }, NewSessionHarness, - /// Harness picker for forking the selected session into a new sibling - /// (`OpenForkCrossHarness`). Shares the harness-picker UI/completion with - /// `NewSessionHarness`; on submit, calls `client.fork_session`. The - /// explicit, cross-harness counterpart to `OpenFork`'s instant - /// same-harness path, which never opens a minibuffer. + /// Harness picker for forking the selected session into a new sibling. + /// Input starts with the source harness so Enter accepts the same-harness + /// default; editing or completion selects another harness. Submit calls + /// `client.fork_session` directly, with no separate initial-prompt stage. ForkSessionHarness { source_session_id: String, }, @@ -8499,10 +8498,6 @@ impl App { }); } OpenFork => { - // Primary path: instant same-harness fork, no minibuffer. - // Every fork is lineage-tracked (`forked_from` always set by - // `Client::fork_session`), so the branch rail and fork log - // apply the same way as the explicit cross-harness path. let Some(id) = self.selected_id() else { self.set_status("fork: no session selected".to_string()); return; @@ -8511,41 +8506,6 @@ impl App { self.set_status("fork: source disappeared".to_string()); return; }; - let (cols, rows) = self.active_pane_size(); - let opts = agentd_client::ForkOptions { - pty_size: Some(agentd_protocol::PtySize { - cols: cols.max(20), - rows: rows.max(5), - }), - ..Default::default() - }; - match self.client.fork_session(&id, &source.harness, opts).await { - Ok(new_id) => { - self.set_status(format!( - "forked {} → {}", - short_id(&id), - short_id(&new_id), - )); - self.refresh_sessions().await; - // Mirror the new-session path: pre-insert an empty PTY - // parser so the transcript bootstrap short-circuits and - // the live subscription isn't raced into a double banner. - if !self.histories.contains_key(&new_id) { - self.histories - .insert(new_id.clone(), crate::pty_render::ItemHistory::new()); - } - self.select_session(new_id); - self.sync_active_window_selection(); - self.focus = PaneFocus::View; - } - Err(e) => self.set_status(format!("fork failed: {e}")), - } - } - OpenForkCrossHarness => { - let Some(id) = self.selected_id() else { - self.set_status("fork: no session selected".to_string()); - return; - }; if self.harnesses.is_empty() { self.harnesses = self.client.harnesses().await.unwrap_or_default(); } @@ -8556,10 +8516,12 @@ impl App { .map(|h| h.name.as_str()) .collect(); let hint = names.join("|"); + let input = source.harness; + let cursor = input.chars().count(); self.minibuffer = Some(Minibuffer { prompt: format!("Fork → [{hint}] (Tab completes): "), - input: String::new(), - cursor: 0, + input, + cursor, intent: MinibufferIntent::ForkSessionHarness { source_session_id: id, }, @@ -24662,6 +24624,32 @@ mod tests { server.abort(); } + #[tokio::test] + async fn fork_picker_defaults_to_source_harness_without_prompt_stage() { + let (mut app, _dir, server) = captured_app().await; + app.harnesses = vec![agentd_protocol::HarnessInfo { + name: "shell".to_string(), + available: true, + detail: None, + binary: None, + description: None, + capabilities: Default::default(), + }]; + + app.run_action(KeyAction::OpenFork).await; + + let minibuffer = app.minibuffer.as_ref().expect("fork harness picker"); + assert!(matches!( + &minibuffer.intent, + MinibufferIntent::ForkSessionHarness { source_session_id } + if source_session_id == "s1" + )); + assert_eq!(minibuffer.input, "shell"); + assert_eq!(minibuffer.cursor, "shell".chars().count()); + assert!(minibuffer.prompt.starts_with("Fork → [shell]")); + server.abort(); + } + // --- Interactive tutorial (spec 0077) ----------------------------- #[tokio::test] diff --git a/crates/cli/src/keymap.rs b/crates/cli/src/keymap.rs index f47681c2..baf5094e 100644 --- a/crates/cli/src/keymap.rs +++ b/crates/cli/src/keymap.rs @@ -15,22 +15,11 @@ pub enum KeyAction { OpenNewSession, OpenDeleteConfirm, OpenRename, - /// Fork the selected session instantly, using its own harness — no - /// minibuffer prompt at all. The new sibling session is selected and - /// keyboard focus lands directly in its live input, like continuing any - /// normal session. Every fork carries lineage (`forked_from`), so the - /// branch rail and fork log apply uniformly regardless of which fork - /// path created it. Bound to `C-x f` (emacs) / `f` (vim) — distinct from - /// "new session" (`C-x C-f` / `n`) and from the explicit cross-harness - /// picker (`OpenForkCrossHarness`). + /// Open the harness picker for a fork, pre-filled with the selected + /// session's current harness. Enter accepts the same-harness default; + /// editing or completion selects a different harness. No separate initial + /// prompt is required. Bound to `C-x f` (emacs) / `f` (vim). OpenFork, - /// Fork the selected session into a different harness, via the harness - /// picker (`"Fork → [...] (Tab completes): "`). The explicit, - /// cross-harness counterpart to `OpenFork`'s instant same-harness path; - /// lands the same way once a harness is chosen — no forced prompt, - /// focus moves straight to the new session. Bound to `C-x F` (emacs) / - /// `O` (vim). - OpenForkCrossHarness, /// Toggle the sidebar lineage section's (spec 0081) keyboard focus /// from anywhere. Within the list pane, bare `Tab` also switches /// sessions⇄lineage (an `on_key` intercept, not a keymap binding, so @@ -331,12 +320,8 @@ fn emacs() -> Keymap { // Refresh moved to the command palette (M-x refresh) — it's rarely // needed since the daemon pushes state changes automatically. (Chord(vec![ctrl('x'), ch('r')]), OpenRename), - // `C-x f` forks the selected session instantly, same harness, no - // prompt (distinct from `C-x C-f`, which creates a fresh session). + // Unified fork picker; Enter accepts the source harness default. (Chord(vec![ctrl('x'), ch('f')]), OpenFork), - // `C-x F` is the explicit cross-harness fork (harness picker), - // mirroring the `C-x A` → ToggleAutomode shifted-letter pattern. - (Chord(vec![ctrl('x'), shift('F')]), OpenForkCrossHarness), (Chord(vec![ctrl('x'), ch('m')]), OpenMerge), // Toggle the sidebar lineage section's keyboard focus. Bare `Tab` // stays unbound in the keymap (the list pane's sessions⇄lineage @@ -416,11 +401,8 @@ fn vim() -> Keymap { (Chord(vec![ctrl('c')]), Interrupt), // `r` opens the rename minibuffer; refresh moved to M-x refresh. (Chord(vec![ch('r')]), OpenRename), - // `f` forks the selected session instantly, same harness, no - // prompt. `O` is repurposed as the explicit cross-harness picker - // (it used to be a redundant alias of bare `f`). + // Unified fork picker; uppercase `O` is intentionally unbound. (Chord(vec![ch('f')]), OpenFork), - (Chord(vec![shift('O')]), OpenForkCrossHarness), (Chord(vec![ch('m')]), OpenMerge), // Shared with the emacs profile — see its binding for the rationale. ( @@ -811,8 +793,11 @@ mod tests { assert_action(&km, vec![ch('g'), ch('d')], KeyAction::OpenDiff); assert_action(&km, vec![ch('o')], KeyAction::OpenNewSession); assert_action(&km, vec![ch('n')], KeyAction::OpenNewSession); - assert_action(&km, vec![shift('O')], KeyAction::OpenForkCrossHarness); assert_action(&km, vec![ch('f')], KeyAction::OpenFork); + assert!(matches!( + resolve(&km, vec![shift('O')]), + KeymapResult::Unhandled + )); assert_action(&km, vec![ch('m')], KeyAction::OpenMerge); assert_action(&km, vec![shift('J')], KeyAction::MoveSelectedDown); assert_action(&km, vec![shift('K')], KeyAction::MoveSelectedUp); @@ -828,6 +813,17 @@ mod tests { assert_action(&km, vec![ctrl('y')], KeyAction::ScrollUp); } + #[test] + fn shifted_fork_shortcut_is_removed() { + for profile in [Profile::Emacs, Profile::Vim] { + let km = default_for(profile); + assert!(matches!( + resolve(&km, vec![ctrl('x'), shift('F')]), + KeymapResult::Unhandled + )); + } + } + #[test] fn vim_c_w_window_chords_resolve_to_expected_actions() { let km = default_for(Profile::Vim); diff --git a/crates/cli/src/ui.rs b/crates/cli/src/ui.rs index 97a3cd6d..3bbdfb12 100644 --- a/crates/cli/src/ui.rs +++ b/crates/cli/src/ui.rs @@ -8128,8 +8128,7 @@ emacs keymap (default; CONSTRUCT_KEYMAP=vim for vim profile) C-x C-o focus session terminal / refocus Program C-x d show diff C-x r rename selected session (clears title on empty submit) - C-x f fork selected session instantly (same harness, no prompt) - C-x F fork selected session into a different harness (picker) + C-x f fork selected session (harness picker; same is default) C-x Tab / Tab focus lineage section (Tab: list pane only) C-x m merge the selected fork (take result, or discard) C-c C-c interrupt @@ -8201,8 +8200,7 @@ vim keymap (CONSTRUCT_KEYMAP=vim; unset for emacs profile) C-x C-o focus session terminal / refocus Program g d show diff r rename selected session (clears title on empty submit) - f fork selected session instantly (same harness, no prompt) - O fork selected session into a different harness (picker) + f fork selected session (harness picker; same is default) C-x Tab / Tab focus lineage section (Tab: list pane only) m merge the selected fork (take result, or discard) C-c interrupt diff --git a/specs/0031-sessions-fork-into-new-siblings.md b/specs/0031-sessions-fork-into-new-siblings.md index 44b876fe..f946b707 100644 --- a/specs/0031-sessions-fork-into-new-siblings.md +++ b/specs/0031-sessions-fork-into-new-siblings.md @@ -1,7 +1,7 @@ # 0031-sessions-fork-into-new-siblings Status: accepted -Date: 2026-06-17 +Date: 2026-07-10 Area: architecture Scope: Changing the harness backing an existing session, and carrying context across harnesses. @@ -23,13 +23,11 @@ harness-agnostic transcript**. A same-harness fork may use that harness's native fork/resume facility when available; this is a faithful continuation within one harness, never a cross-harness state translation. -Cross-harness forking as described in this spec — the harness picker, -transcript-seed context — is the **secondary, explicit** fork path (spec -0078): the user deliberately opts into a different harness. The **primary, -default** fork is same-harness, favors native context fidelity over the -transcript seed when the adapter supports it, and always records lineage -(`forked_from`) the same way a cross-harness fork does — see spec 0078 for -the unified fork/merge decision and its keybinding split. +The unified harness picker (spec 0078) defaults to the source harness. Accepting +that default favors native context fidelity when the adapter supports it; +editing the selection creates a cross-harness fork using portable transcript +seeding. Both choices record identical lineage, and neither adds a separate +initial-prompt step. ## Reason diff --git a/specs/0078-forks-carry-lineage-branch-rail-and-merge.md b/specs/0078-forks-carry-lineage-branch-rail-and-merge.md index 1b288188..cd736f02 100644 --- a/specs/0078-forks-carry-lineage-branch-rail-and-merge.md +++ b/specs/0078-forks-carry-lineage-branch-rail-and-merge.md @@ -1,11 +1,10 @@ # 0078-forks-carry-lineage-branch-rail-and-merge Status: accepted -Date: 2026-07-09 +Date: 2026-07-10 Area: ux -Scope: A single, unified fork action — instant same-harness by default, -explicit cross-harness on request — whose result is always lineage-tracked -and mergeable back into its parent. +Scope: A single harness-picker fork action whose same-harness default requires +no initial prompt and whose result is always lineage-tracked and mergeable. ## Decision @@ -15,17 +14,11 @@ normal top-level sibling session and always records durable lineage time — there is no separate gate that only tracks lineage for some forks and not others. -Forking has two entry points, distinguished only by how the target harness is -chosen, never by whether lineage is tracked: - -- **Primary (instant, same-harness).** No prompt of any kind. The session is - forked immediately using its own harness, the new session is selected, and - keyboard focus lands directly in its live input — continuing work reads the - same as jumping into any other session. -- **Secondary (explicit, cross-harness).** A harness picker lets the user - target a different harness than the source. Once a harness is chosen (or - the default accepted), it lands the same way as the primary path — no - forced prompt, focus moves straight to the new session. +Forking has one entry point. Its harness picker is pre-filled with the source +session's harness, so Enter accepts a same-harness fork while completion or an +edit selects another harness. Harness selection submits the fork immediately: +there is no second initial-prompt question, including for the same-harness +default. The new session is selected and focus moves to its live input. Merge records a durable result-or-discard outcome on the fork and then archives it. Taking a result injects a compact transcript rendering into the @@ -41,10 +34,9 @@ tracked) forced the user to predict, before acting, whether they'd want the branch rail and merge menu later — and picking wrong meant losing lineage retroactively. Tracking lineage unconditionally removes that up-front decision: every fork, however it was created, is available for the branch -rail, fork log, and merge later. The two-tier keybinding still exists because -same-harness vs. cross-harness is a real fork-time decision (native context -fidelity is only possible within one harness) — but it no longer decides -whether the daemon remembers where the session came from. +rail, fork log, and merge later. Harness choice remains explicit because +same-harness native context fidelity differs from portable cross-harness +transcript seeding, but it no longer requires separate keybindings or flows. ## Consequences