diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 19beded0..9f44d529 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Changed +- Public workspace, tab, and pane ids are now short stable handles such as `w1`, `w1:t1`, and `w1:p1`; closed tab and pane ids no longer retarget later resources. (#569) + ## [0.6.10] - 2026-06-11 This is a hotfix release for v0.6.9. See the v0.6.9 notes for the full feature release. @@ -652,7 +655,7 @@ This is a hotfix for v0.6.3. See the v0.6.3 notes for the full feature release. ### Added - Added a local Unix socket API for controlling running herdr sessions, including workspace and pane management, pane reads, text/key input, pane splitting, and output waits. - Added event subscriptions over the socket API for workspace and pane lifecycle events, pane output matches, and agent state changes. -- Added CLI wrappers on top of the socket API with `herdr workspace ...`, `herdr pane ...`, and `herdr wait ...`, using compact public ids like `1` and `1-2` for scripting and agent orchestration. +- Added CLI wrappers on top of the socket API with `herdr workspace ...`, `herdr pane ...`, and `herdr wait ...`, using compact public ids for scripting and agent orchestration. - Added a settings popup with mouse support for changing themes, sound alerts, and toast notifications from inside herdr. - Added 9 built-in themes: catppuccin, tokyo night, dracula, nord, gruvbox, one dark, solarized, kanagawa, and rosé pine. - Added interactive pane scrollbars, manual sidebar resizing, and upstream git ahead/behind indicators in the workspace sidebar. diff --git a/docs/next/website/src/content/docs/agents.mdx b/docs/next/website/src/content/docs/agents.mdx index ae45d26b..783ad68a 100644 --- a/docs/next/website/src/content/docs/agents.mdx +++ b/docs/next/website/src/content/docs/agents.mdx @@ -99,7 +99,7 @@ Each supported agent has its own integration name and behavior. See [Integration You can rename an agent target for display: ```bash -herdr agent rename 1-1 reviewer +herdr agent rename w1:p1 reviewer herdr agent rename reviewer --clear ``` @@ -110,7 +110,7 @@ Targets accept terminal IDs, unique agent names, detected or reported agent labe Integrations can report a visual status label without changing semantic state. ```bash -herdr pane report-agent 1-1 \ +herdr pane report-agent w1:p1 \ --source custom:indexer \ --agent docs-bot \ --state working \ @@ -132,7 +132,7 @@ herdr agent start reviewer --cwd ~/project --split right -- pi You can place that agent in a specific workspace or tab: ```bash -herdr agent start docs --workspace 1 --tab 1-1 -- claude +herdr agent start docs --workspace w1 --tab w1:t1 -- claude ``` Use `herdr pane ...` commands for ordinary terminals, servers, tests, shells, and low-level terminal input. For example, use `pane split` and `pane run` for `cargo test`, not `agent start`, unless that terminal is intentionally being treated as an agent target. diff --git a/docs/next/website/src/content/docs/integrations.mdx b/docs/next/website/src/content/docs/integrations.mdx index d4181e35..1506479f 100644 --- a/docs/next/website/src/content/docs/integrations.mdx +++ b/docs/next/website/src/content/docs/integrations.mdx @@ -231,7 +231,7 @@ Integrations can report a short visual label without changing the semantic state For example, an agent can remain semantically `working` while showing `indexing` in the UI. ```bash -herdr pane report-agent 1-1 \ +herdr pane report-agent w1:p1 \ --source custom:docs \ --agent docs-bot \ --state working \ @@ -264,7 +264,7 @@ herdr agent list Read a pane when you need to verify what Herdr can see: ```bash -herdr pane read 1-1 --source recent --lines 50 +herdr pane read w1:p1 --source recent --lines 50 ``` If integration state looks wrong, first confirm the agent is running inside Herdr and that the relevant hook or plugin was installed for the same user account. diff --git a/docs/next/website/src/content/docs/socket-api.mdx b/docs/next/website/src/content/docs/socket-api.mdx index a6b91461..058513f5 100644 --- a/docs/next/website/src/content/docs/socket-api.mdx +++ b/docs/next/website/src/content/docs/socket-api.mdx @@ -47,8 +47,8 @@ herdr tab create --label logs Split a pane and run a command: ```bash -herdr pane split 1-1 --direction right -herdr pane run 1-2 "npm test" +herdr pane split w1:p1 --direction right +herdr pane run w1:p2 "npm test" ``` Inspect and rearrange panes: @@ -59,19 +59,19 @@ herdr pane neighbor --direction right --current herdr pane resize --direction right --amount 0.1 --current herdr pane swap --direction right --current herdr pane zoom --on --current -herdr pane split 1-1 --direction right --ratio 0.333 +herdr pane split w1:p1 --direction right --ratio 0.333 ``` Wait for an agent: ```bash -herdr wait agent-status 1-1 --status done +herdr wait agent-status w1:p1 --status done ``` Read pane output: ```bash -herdr pane read 1-2 --source recent --lines 50 +herdr pane read w1:p2 --source recent --lines 50 ``` ## Raw methods @@ -92,16 +92,16 @@ Raw socket method names use dot notation: Some CLI commands are conveniences around these methods. For example, `herdr agent wait` resolves an agent target and then subscribes to pane agent state events. -Pane control methods use public pane ids such as `1-1`. Omit `pane_id` to use +Pane control methods use public pane ids such as `w1:p1`. Omit `pane_id` to use the server's active focused pane. ```json -{"id":"req_layout","method":"pane.layout","params":{"pane_id":"1-1"}} -{"id":"req_neighbor","method":"pane.neighbor","params":{"pane_id":"1-1","direction":"right"}} -{"id":"req_edges","method":"pane.edges","params":{"pane_id":"1-1"}} +{"id":"req_layout","method":"pane.layout","params":{"pane_id":"w1:p1"}} +{"id":"req_neighbor","method":"pane.neighbor","params":{"pane_id":"w1:p1","direction":"right"}} +{"id":"req_edges","method":"pane.edges","params":{"pane_id":"w1:p1"}} {"id":"req_focus","method":"pane.focus_direction","params":{"direction":"right"}} -{"id":"req_resize","method":"pane.resize","params":{"pane_id":"1-1","direction":"right","amount":0.1}} -{"id":"req_zoom","method":"pane.zoom","params":{"pane_id":"1-1","mode":"toggle"}} +{"id":"req_resize","method":"pane.resize","params":{"pane_id":"w1:p1","direction":"right","amount":0.1}} +{"id":"req_zoom","method":"pane.zoom","params":{"pane_id":"w1:p1","mode":"toggle"}} {"id":"req_split","method":"pane.split","params":{"direction":"right","ratio":0.333}} ``` @@ -113,8 +113,8 @@ can make the next decision without private layout state. `pane.swap` supports directional and explicit forms: ```json -{"id":"req_swap_dir","method":"pane.swap","params":{"pane_id":"1-1","direction":"right"}} -{"id":"req_swap_explicit","method":"pane.swap","params":{"source_pane_id":"1-1","target_pane_id":"1-2"}} +{"id":"req_swap_dir","method":"pane.swap","params":{"pane_id":"w1:p1","direction":"right"}} +{"id":"req_swap_explicit","method":"pane.swap","params":{"source_pane_id":"w1:p1","target_pane_id":"w1:p2"}} ``` Swap is same-tab only. It preserves split shape, split ratios, pane ids, and @@ -127,9 +127,9 @@ full-tab layout. `pane.zoom` toggles, enables, or disables zoom for the target pane's tab: ```json -{"id":"req_zoom_toggle","method":"pane.zoom","params":{"pane_id":"1-1"}} -{"id":"req_zoom_on","method":"pane.zoom","params":{"pane_id":"1-1","mode":"on"}} -{"id":"req_zoom_off","method":"pane.zoom","params":{"pane_id":"1-1","mode":"off"}} +{"id":"req_zoom_toggle","method":"pane.zoom","params":{"pane_id":"w1:p1"}} +{"id":"req_zoom_on","method":"pane.zoom","params":{"pane_id":"w1:p1","mode":"on"}} +{"id":"req_zoom_off","method":"pane.zoom","params":{"pane_id":"w1:p1","mode":"off"}} ``` Omitting `pane_id` targets the server's active focused pane. The response is @@ -165,13 +165,13 @@ Worktree methods manage Git checkouts as Herdr workspaces. `worktree.create` cre Create a worktree from a source workspace: ```json -{"id":"req_1","method":"worktree.create","params":{"workspace_id":"1","branch":"worktree/api","focus":false}} +{"id":"req_1","method":"worktree.create","params":{"workspace_id":"w1","branch":"worktree/api","focus":false}} ``` Open an existing checkout: ```json -{"id":"req_2","method":"worktree.open","params":{"workspace_id":"1","branch":"worktree/api","focus":true}} +{"id":"req_2","method":"worktree.open","params":{"workspace_id":"w1","branch":"worktree/api","focus":true}} ``` Remove a linked checkout: @@ -229,7 +229,7 @@ Integrations report agent state with `pane.report_agent`. "id": "req_1", "method": "pane.report_agent", "params": { - "pane_id": "1-1", + "pane_id": "w1:p1", "source": "custom:docs", "agent": "docs-bot", "state": "working", @@ -250,7 +250,7 @@ Session-only official integrations report native session references with `pane.r "id": "req_2", "method": "pane.report_agent_session", "params": { - "pane_id": "1-1", + "pane_id": "w1:p1", "source": "herdr:codex", "agent": "codex", "agent_session_id": "..." @@ -282,7 +282,7 @@ Use `pane.report_metadata` when a user hook wants to customize presentation with "id": "req_2", "method": "pane.report_metadata", "params": { - "pane_id": "1-1", + "pane_id": "w1:p1", "source": "user:claude-title", "agent": "claude", "title": "Refactor auth middleware", @@ -310,7 +310,7 @@ Subscribe to events when you need a long-lived stream: "method": "events.subscribe", "params": { "subscriptions": [ - { "type": "pane.agent_status_changed", "pane_id": "1-1", "agent_status": "blocked" } + { "type": "pane.agent_status_changed", "pane_id": "w1:p1", "agent_status": "blocked" } ] } } @@ -327,10 +327,10 @@ Use `events.wait` when you want one matching event and then a response. Use `pane.read` through the CLI unless you are writing a protocol client. ```bash -herdr pane read 1-1 --source visible --lines 80 -herdr pane read 1-1 --source recent --lines 120 -herdr pane read 1-1 --source recent-unwrapped --lines 120 -herdr pane read 1-1 --source detection +herdr pane read w1:p1 --source visible --lines 80 +herdr pane read w1:p1 --source recent --lines 120 +herdr pane read w1:p1 --source recent-unwrapped --lines 120 +herdr pane read w1:p1 --source detection ``` `recent-unwrapped` is useful for logs because it ignores soft wrapping. @@ -341,8 +341,8 @@ herdr pane read 1-1 --source detection Use waits to coordinate agents and scripts. ```bash -herdr wait agent-status 1-1 --status done -herdr wait agent-status 1-1 --status blocked +herdr wait agent-status w1:p1 --status done +herdr wait agent-status w1:p1 --status blocked ``` Agent waits observe semantic state, not arbitrary command completion. @@ -357,10 +357,10 @@ Successful responses look like this: "result": { "type": "pane_info", "pane": { - "pane_id": "1-1", + "pane_id": "w1:p1", "terminal_id": "term_abc123", - "workspace_id": "1", - "tab_id": "1-1", + "workspace_id": "w1", + "tab_id": "w1:t1", "focused": true, "agent_status": "working", "revision": 42 @@ -401,7 +401,7 @@ Fields such as `last_check_unix`, `last_result`, `active_version`, `cached_remot { "id": "req_2", "method": "agent.explain", - "params": { "target": "1-1" } + "params": { "target": "w1:p1" } } ``` diff --git a/src/api/schema.rs b/src/api/schema.rs index 0eb2d8e2..491693ad 100644 --- a/src/api/schema.rs +++ b/src/api/schema.rs @@ -1024,7 +1024,7 @@ mod tests { let request = Request { id: "req_hook".into(), method: Method::PaneReportAgent(PaneReportAgentParams { - pane_id: "1-1".into(), + pane_id: "w1:p1".into(), source: "herdr:pi".into(), agent: "pi".into(), state: PaneAgentState::Working, @@ -1046,7 +1046,7 @@ mod tests { let request = Request { id: "req_session".into(), method: Method::PaneReportAgentSession(PaneReportAgentSessionParams { - pane_id: "1-1".into(), + pane_id: "w1:p1".into(), source: "herdr:claude".into(), agent: "claude".into(), seq: Some(42), @@ -1065,7 +1065,7 @@ mod tests { let request = Request { id: "req_metadata".into(), method: Method::PaneReportMetadata(PaneReportMetadataParams { - pane_id: "1-1".into(), + pane_id: "w1:p1".into(), source: "user:claude-title".into(), agent: Some("claude".into()), applies_to_source: Some("herdr:claude".into()), @@ -1092,7 +1092,7 @@ mod tests { let request = Request { id: "req_clear".into(), method: Method::PaneClearAgentAuthority(PaneClearAgentAuthorityParams { - pane_id: "1-1".into(), + pane_id: "w1:p1".into(), source: Some("herdr:pi".into()), seq: Some(42), }), @@ -1108,7 +1108,7 @@ mod tests { let request = Request { id: "req_release".into(), method: Method::PaneReleaseAgent(PaneReleaseAgentParams { - pane_id: "1-1".into(), + pane_id: "w1:p1".into(), source: "herdr:pi".into(), agent: "pi".into(), seq: Some(42), @@ -1315,8 +1315,8 @@ mod tests { let event = EventEnvelope { event: EventKind::PaneOutputChanged, data: EventData::PaneOutputChanged { - pane_id: "p_1".into(), - workspace_id: "w_1".into(), + pane_id: "w1:p1".into(), + workspace_id: "w1".into(), revision: 42, }, }; @@ -1380,12 +1380,12 @@ mod tests { let event = SubscriptionEventEnvelope { event: SubscriptionEventKind::PaneOutputMatched, data: SubscriptionEventData::PaneOutputMatched(PaneOutputMatchedEvent { - pane_id: "p_1_1".into(), + pane_id: "w1:p1".into(), matched_line: "auth: received".into(), read: PaneReadResult { - pane_id: "p_1_1".into(), - workspace_id: "w_1".into(), - tab_id: "t_1_1".into(), + pane_id: "w1:p1".into(), + workspace_id: "w1".into(), + tab_id: "w1:t1".into(), source: ReadSource::Recent, format: ReadFormat::Text, text: "auth: received\n".into(), @@ -1437,13 +1437,13 @@ mod tests { id: "req_worktree".into(), result: ResponseResult::WorktreeCreated { workspace: WorkspaceInfo { - workspace_id: "w_1".into(), + workspace_id: "w1".into(), number: 2, label: "herdr".into(), focused: true, pane_count: 1, tab_count: 1, - active_tab_id: "w_1:1".into(), + active_tab_id: "w1:t1".into(), agent_status: AgentStatus::Unknown, worktree: Some(WorkspaceWorktreeInfo { repo_key: "/repo/herdr/.git".into(), @@ -1454,8 +1454,8 @@ mod tests { }), }, tab: TabInfo { - tab_id: "w_1:1".into(), - workspace_id: "w_1".into(), + tab_id: "w1:t1".into(), + workspace_id: "w1".into(), number: 1, label: "herdr".into(), focused: true, @@ -1463,10 +1463,10 @@ mod tests { agent_status: AgentStatus::Unknown, }, root_pane: PaneInfo { - pane_id: "w_1-1".into(), + pane_id: "w1:p1".into(), terminal_id: "term_1".into(), - workspace_id: "w_1".into(), - tab_id: "w_1:1".into(), + workspace_id: "w1".into(), + tab_id: "w1:t1".into(), focused: true, cwd: Some("/worktrees/herdr/worktree-api".into()), foreground_cwd: None, @@ -1487,7 +1487,7 @@ mod tests { is_detached: false, is_prunable: false, is_linked_worktree: true, - open_workspace_id: Some("w_1".into()), + open_workspace_id: Some("w1".into()), label: "herdr".into(), }, }, @@ -1505,8 +1505,8 @@ mod tests { id: "req_2".into(), result: ResponseResult::TabCreated { tab: TabInfo { - tab_id: "w_1:2".into(), - workspace_id: "w_1".into(), + tab_id: "w1:t2".into(), + workspace_id: "w1".into(), number: 2, label: "review".into(), focused: false, @@ -1514,10 +1514,10 @@ mod tests { agent_status: AgentStatus::Unknown, }, root_pane: PaneInfo { - pane_id: "w_1-3".into(), + pane_id: "w1:p3".into(), terminal_id: "term_example".into(), - workspace_id: "w_1".into(), - tab_id: "w_1:2".into(), + workspace_id: "w1".into(), + tab_id: "w1:t2".into(), focused: false, cwd: Some("/tmp/review".into()), foreground_cwd: None, diff --git a/src/api/subscriptions.rs b/src/api/subscriptions.rs index 9c92a41b..e8563468 100644 --- a/src/api/subscriptions.rs +++ b/src/api/subscriptions.rs @@ -219,7 +219,7 @@ impl ActiveSubscription { let initial_event = agent_status .is_some_and(|wanted| wanted == probe.agent_status) .then_some(PaneAgentStatusChangedEvent { - pane_id: probe.pane_id, + pane_id: probe.pane_id.clone(), workspace_id: probe.workspace_id, agent_status: probe.agent_status, agent: probe.agent, @@ -231,7 +231,7 @@ impl ActiveSubscription { Ok(Self::AgentStatusChanged(Box::new( ActiveAgentStatusChangedSubscription { - pane_id, + pane_id: probe.pane_id, status_filter: agent_status, last_status: Some(last_status), last_presentation: Some(last_presentation), @@ -295,7 +295,7 @@ impl ActiveOutputMatchedSubscription { Some(SubscriptionEventEnvelope { event: SubscriptionEventKind::PaneOutputMatched, data: SubscriptionEventData::PaneOutputMatched(PaneOutputMatchedEvent { - pane_id: self.pane_id.clone(), + pane_id: read.pane_id.clone(), matched_line, read, }), diff --git a/src/api/wait.rs b/src/api/wait.rs index cbb39b58..3ed08d70 100644 --- a/src/api/wait.rs +++ b/src/api/wait.rs @@ -95,7 +95,7 @@ pub(super) fn wait_for_output( serde_json::to_string(&SuccessResponse { id: request_id, result: ResponseResult::OutputMatched { - pane_id: params.pane_id, + pane_id: read.pane_id.clone(), revision, matched_line, read, diff --git a/src/app/actions.rs b/src/app/actions.rs index b218e53c..c2b35dbd 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -3162,6 +3162,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); diff --git a/src/app/agent_resume.rs b/src/app/agent_resume.rs index 894b0b37..cf691e33 100644 --- a/src/app/agent_resume.rs +++ b/src/app/agent_resume.rs @@ -223,6 +223,9 @@ impl App { ); return false; }; + let public_pane_id = self + .find_pane(pane_id) + .and_then(|(ws_idx, _)| self.public_pane_id(ws_idx, pane_id)); let runtime = match crate::terminal::TerminalRuntime::spawn( pane_id, @@ -235,6 +238,7 @@ impl App { self.event_tx.clone(), self.render_notify.clone(), self.render_dirty.clone(), + public_pane_id.as_deref(), ) { Ok(runtime) => runtime, Err(err) => { diff --git a/src/app/api.rs b/src/app/api.rs index ea4ea378..32192a07 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -327,6 +327,7 @@ impl App { let Some((ws_idx, pane_state)) = self.find_pane(pane_id) else { return false; }; + let public_pane_id = self.public_pane_id(ws_idx, pane_id); let terminal_id = pane_state.attached_terminal_id.clone(); let Some(terminal) = self.state.terminals.get(&terminal_id) else { return false; @@ -349,6 +350,7 @@ impl App { self.event_tx.clone(), self.render_notify.clone(), self.render_dirty.clone(), + public_pane_id.as_deref(), ) { Ok(runtime) => runtime, Err(err) => { @@ -1236,6 +1238,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); @@ -1327,6 +1330,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); diff --git a/src/app/api/panes.rs b/src/app/api/panes.rs index 5afac850..326fbd9f 100644 --- a/src/app/api/panes.rs +++ b/src/app/api/panes.rs @@ -599,6 +599,9 @@ impl App { let Some((ws_idx, pane_id)) = self.parse_pane_id(¶ms.pane_id) else { return pane_not_found(id, ¶ms.pane_id); }; + let Some(public_pane_id) = self.public_pane_id(ws_idx, pane_id) else { + return pane_not_found(id, ¶ms.pane_id); + }; let Some((pane, workspace_id)) = self.lookup_runtime(ws_idx, pane_id) else { return pane_not_found(id, ¶ms.pane_id); }; @@ -630,7 +633,7 @@ impl App { id, ResponseResult::PaneRead { read: PaneReadResult { - pane_id: params.pane_id, + pane_id: public_pane_id, workspace_id, tab_id: self.public_tab_id(ws_idx, tab_idx).unwrap(), source: params.source, @@ -887,6 +890,10 @@ impl App { let Some((ws_idx, pane_id)) = self.parse_pane_id(&target.pane_id) else { return pane_not_found(id, &target.pane_id); }; + let Some(public_pane_id) = self.public_pane_id(ws_idx, pane_id) else { + return pane_not_found(id, &target.pane_id); + }; + let workspace_id = self.public_workspace_id(ws_idx); if self.state.close_pane_would_close_workspace(ws_idx, pane_id) && self.state.confirm_implicit_worktree_group_close(ws_idx) { @@ -896,7 +903,6 @@ impl App { "closing this pane would close a worktree group", ); } - let workspace_id = self.state.workspaces[ws_idx].id.clone(); let terminal_id = self.state.terminal_id_for_pane(ws_idx, pane_id); let should_close_workspace = { let Some(ws) = self.state.workspaces.get_mut(ws_idx) else { @@ -911,7 +917,7 @@ impl App { self.emit_event(EventEnvelope { event: EventKind::PaneClosed, data: EventData::PaneClosed { - pane_id: target.pane_id.clone(), + pane_id: public_pane_id, workspace_id: workspace_id.clone(), }, }); @@ -926,7 +932,7 @@ impl App { self.emit_event(EventEnvelope { event: EventKind::PaneClosed, data: EventData::PaneClosed { - pane_id: target.pane_id, + pane_id: public_pane_id, workspace_id, }, }); diff --git a/src/app/api/tabs.rs b/src/app/api/tabs.rs index 4057eced..7c7ad9d7 100644 --- a/src/app/api/tabs.rs +++ b/src/app/api/tabs.rs @@ -98,9 +98,13 @@ impl App { ); if let Some(label) = label { let workspace_id = self.state.workspaces[ws_idx].id.clone(); - let tab_id = self - .public_tab_id(ws_idx, tab_idx) - .unwrap_or_else(|| format!("{}:{}", workspace_id, tab_idx + 1)); + let tab_id = self.public_tab_id(ws_idx, tab_idx).unwrap_or_else(|| { + format!( + "{}:t{}", + workspace_id, + crate::workspace::encode_public_number(tab_idx + 1) + ) + }); if let Some(tab) = self .state .workspaces @@ -186,6 +190,10 @@ impl App { let Some((ws_idx, tab_idx)) = self.parse_tab_id(&target.tab_id) else { return tab_not_found(id, &target.tab_id); }; + let Some(tab_id) = self.public_tab_id(ws_idx, tab_idx) else { + return tab_not_found(id, &target.tab_id); + }; + let workspace_id = self.public_workspace_id(ws_idx); let terminal_ids = self.state.terminal_ids_for_tab(ws_idx, tab_idx); let Some(ws) = self.state.workspaces.get_mut(ws_idx) else { return tab_not_found(id, &target.tab_id); @@ -210,8 +218,8 @@ impl App { self.emit_event(EventEnvelope { event: EventKind::TabClosed, data: EventData::TabClosed { - tab_id: target.tab_id, - workspace_id: self.public_workspace_id(ws_idx), + tab_id, + workspace_id, }, }); diff --git a/src/app/api/workspaces.rs b/src/app/api/workspaces.rs index 92b41486..e1966adf 100644 --- a/src/app/api/workspaces.rs +++ b/src/app/api/workspaces.rs @@ -146,14 +146,13 @@ impl App { if self.state.workspaces.get(index).is_none() { return workspace_not_found(id, &target.workspace_id); } + let workspace_id = self.public_workspace_id(index); self.state.selected = index; self.state.close_selected_workspace(); self.shutdown_detached_terminal_runtimes(); self.emit_event(EventEnvelope { event: EventKind::WorkspaceClosed, - data: EventData::WorkspaceClosed { - workspace_id: target.workspace_id, - }, + data: EventData::WorkspaceClosed { workspace_id }, }); encode_success(id, ResponseResult::Ok {}) diff --git a/src/app/creation.rs b/src/app/creation.rs index 3f643986..7be324e5 100644 --- a/src/app/creation.rs +++ b/src/app/creation.rs @@ -122,9 +122,13 @@ impl App { self.state.mode = Mode::Terminal; } let workspace_id = self.state.workspaces[ws_idx].id.clone(); - let tab_id = self - .public_tab_id(ws_idx, idx) - .unwrap_or_else(|| format!("{}:{}", workspace_id, idx + 1)); + let tab_id = self.public_tab_id(ws_idx, idx).unwrap_or_else(|| { + format!( + "{}:t{}", + workspace_id, + crate::workspace::encode_public_number(idx + 1) + ) + }); let root_pane = self.state.workspaces[ws_idx].tabs[idx].root_pane.raw(); crate::logging::tab_created(&workspace_id, &tab_id, root_pane); self.schedule_session_save(); diff --git a/src/app/ids.rs b/src/app/ids.rs index a18efd2d..0ce02c1f 100644 --- a/src/app/ids.rs +++ b/src/app/ids.rs @@ -18,8 +18,12 @@ impl App { pub(super) fn public_tab_id(&self, ws_idx: usize, tab_idx: usize) -> Option { let ws = self.state.workspaces.get(ws_idx)?; - ws.tabs.get(tab_idx)?; - Some(format!("{}:{}", ws.id, tab_idx + 1)) + let tab_number = ws.public_tab_number(tab_idx)?; + Some(format!( + "{}:t{}", + ws.id, + crate::workspace::encode_public_number(tab_number) + )) } pub(super) fn public_pane_id( @@ -29,7 +33,11 @@ impl App { ) -> Option { let ws = self.state.workspaces.get(ws_idx)?; let pane_number = ws.public_pane_number(pane_id)?; - Some(format!("{}-{pane_number}", ws.id)) + Some(format!( + "{}:p{}", + ws.id, + crate::workspace::encode_public_number(pane_number) + )) } pub(super) fn parse_workspace_id(&self, id: &str) -> Option { @@ -52,7 +60,18 @@ impl App { let (ws_raw, tab_raw) = id.rsplit_once(':')?; let ws_idx = self.parse_workspace_id(ws_raw)?; - let tab_idx = tab_raw.parse::().ok()?.checked_sub(1)?; + let tab_number = if let Some(encoded) = tab_raw.strip_prefix('t') { + crate::workspace::decode_public_number(encoded)? + } else { + tab_raw.parse::().ok()? + }; + let tab_idx = self + .state + .workspaces + .get(ws_idx)? + .tabs + .iter() + .position(|tab| tab.number == tab_number)?; self.state.workspaces.get(ws_idx)?.tabs.get(tab_idx)?; Some((ws_idx, tab_idx)) } @@ -81,6 +100,17 @@ impl App { return self.find_pane(pane_id).map(|(ws_idx, _)| (ws_idx, pane_id)); } + if let Some((ws_raw, pane_number_raw)) = id.rsplit_once(":p") { + let ws_idx = self.parse_workspace_id(ws_raw)?; + let pane_number = crate::workspace::decode_public_number(pane_number_raw)?; + let ws = self.state.workspaces.get(ws_idx)?; + let pane_id = ws + .public_pane_numbers + .iter() + .find_map(|(pane_id, number)| (*number == pane_number).then_some(*pane_id))?; + return Some((ws_idx, pane_id)); + } + let (ws_raw, pane_number_raw) = id.rsplit_once('-')?; let ws_idx = self.parse_workspace_id(ws_raw)?; let pane_number = pane_number_raw.parse::().ok()?; diff --git a/src/app/input/modal.rs b/src/app/input/modal.rs index 79d066f9..3d106765 100644 --- a/src/app/input/modal.rs +++ b/src/app/input/modal.rs @@ -355,7 +355,7 @@ fn next_new_tab_default_name(state: &AppState) -> String { state .active .and_then(|i| state.workspaces.get(i)) - .map(|ws| (ws.tabs.len() + 1).to_string()) + .map(|ws| ws.next_public_tab_number.to_string()) .unwrap_or_else(|| "1".to_string()) } @@ -1288,7 +1288,7 @@ mod tests { } #[test] - fn closing_first_auto_tab_resets_remaining_auto_tab_and_next_prompt() { + fn closing_first_auto_tab_keeps_remaining_auto_tab_number_and_next_prompt() { let mut state = state_with_workspaces(&["test"]); open_new_tab_dialog(&mut state); handle_rename_key( @@ -1303,11 +1303,11 @@ mod tests { state.workspaces[0].close_tab(0); state.workspaces[0].switch_tab(0); - assert_eq!(state.workspaces[0].tabs[0].display_name(), "1"); + assert_eq!(state.workspaces[0].tabs[0].display_name(), "2"); assert!(state.workspaces[0].tabs[0].custom_name.is_none()); open_new_tab_dialog(&mut state); - assert_eq!(state.name_input, "2"); + assert_eq!(state.name_input, "3"); } #[test] diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index 9d861f66..b27b5cb9 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -1943,8 +1943,8 @@ last_pane = "prefix+tab" let lines: Vec<&str> = content.lines().collect(); assert_eq!(lines.len(), 3); assert_eq!(lines[0], app.state.workspaces[0].id); - assert_eq!(lines[1], format!("{}:1", app.state.workspaces[0].id)); - assert_eq!(lines[2], format!("{}-1", app.state.workspaces[0].id)); + assert_eq!(lines[1], format!("{}:t1", app.state.workspaces[0].id)); + assert_eq!(lines[2], format!("{}:p1", app.state.workspaces[0].id)); assert_eq!(app.state.mode, Mode::Terminal); let _ = std::fs::remove_file(output_path); diff --git a/src/app/input/sidebar.rs b/src/app/input/sidebar.rs index 43093a82..c4bd52d9 100644 --- a/src/app/input/sidebar.rs +++ b/src/app/input/sidebar.rs @@ -1300,13 +1300,16 @@ mod tests { .iter() .map(|tab| tab.display_name()) .collect(); - assert_eq!(labels, vec!["foo", "2", "3"]); + assert_eq!(labels, vec!["foo", "3", "1"]); assert_eq!( app.state.workspaces[0].tabs[0].custom_name.as_deref(), Some("foo") ); assert!(app.state.workspaces[0].tabs[1].custom_name.is_none()); assert!(app.state.workspaces[0].tabs[2].custom_name.is_none()); + assert_eq!(app.state.workspaces[0].tabs[0].number, 2); + assert_eq!(app.state.workspaces[0].tabs[1].number, 3); + assert_eq!(app.state.workspaces[0].tabs[2].number, 1); assert_eq!(app.state.workspaces[0].tabs[2].root_pane, moved_root); assert_eq!(app.state.workspaces[0].active_tab, 2); } diff --git a/src/app/mod.rs b/src/app/mod.rs index 99f4e461..5d09800b 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -2799,14 +2799,14 @@ mod tests { id: "req_2".into(), method: crate::api::schema::Method::WorkspaceFocus( crate::api::schema::WorkspaceTarget { - workspace_id: "w_1".into(), + workspace_id: "w1".into(), }, ), }; let pane_rename = crate::api::schema::Request { id: "req_3".into(), method: crate::api::schema::Method::PaneRename(crate::api::schema::PaneRenameParams { - pane_id: "w_1-1".into(), + pane_id: "w1:p1".into(), label: Some("logs".into()), }), }; @@ -2825,7 +2825,7 @@ mod tests { let pane_swap = crate::api::schema::Request { id: "req_6".into(), method: crate::api::schema::Method::PaneSwap(crate::api::schema::PaneSwapParams { - pane_id: Some("w_1-1".into()), + pane_id: Some("w1:p1".into()), direction: Some(crate::api::schema::PaneDirection::Right), ..crate::api::schema::PaneSwapParams::default() }), @@ -2834,7 +2834,7 @@ mod tests { id: "req_7".into(), method: crate::api::schema::Method::PaneFocusDirection( crate::api::schema::PaneFocusDirectionParams { - pane_id: Some("w_1-1".into()), + pane_id: Some("w1:p1".into()), direction: crate::api::schema::PaneDirection::Right, }, ), @@ -2842,7 +2842,7 @@ mod tests { let pane_resize = crate::api::schema::Request { id: "req_8".into(), method: crate::api::schema::Method::PaneResize(crate::api::schema::PaneResizeParams { - pane_id: Some("w_1-1".into()), + pane_id: Some("w1:p1".into()), direction: crate::api::schema::PaneDirection::Right, amount: Some(0.05), }), diff --git a/src/integration/mod.rs b/src/integration/mod.rs index 67ca03b8..5d6b8e7e 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -360,9 +360,18 @@ pub(crate) struct HermesUninstallResult { pub updated_config: bool, } -pub(crate) fn apply_pane_env(cmd: &mut CommandBuilder, pane_id: PaneId) { +pub(crate) fn apply_pane_env( + cmd: &mut CommandBuilder, + pane_id: PaneId, + public_pane_id: Option<&str>, +) { cmd.env(crate::api::SOCKET_PATH_ENV_VAR, crate::api::socket_path()); - cmd.env(HERDR_PANE_ID_ENV_VAR, format!("p_{}", pane_id.raw())); + cmd.env( + HERDR_PANE_ID_ENV_VAR, + public_pane_id + .map(str::to_string) + .unwrap_or_else(|| format!("p_{}", pane_id.raw())), + ); } pub(crate) const INSTALL_WARNING_PREFIX: &str = "warning:"; diff --git a/src/pane.rs b/src/pane.rs index 64fbf32b..601334d9 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -1323,6 +1323,7 @@ impl PaneRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { Self::spawn_with_initial_history( pane_id, @@ -1336,9 +1337,12 @@ impl PaneRuntime { events, render_notify, render_dirty, + public_pane_id, ) } + // Runtime construction needs to thread PTY size, environment, theme, and render hooks together. + #[allow(clippy::too_many_arguments)] pub(crate) fn spawn_with_initial_history( pane_id: PaneId, rows: u16, @@ -1351,12 +1355,13 @@ impl PaneRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { let mut cmd = pane_shell_command_builder(shell_config)?; cmd.cwd(cwd); cmd.env(crate::HERDR_ENV_VAR, crate::HERDR_ENV_VALUE); apply_pane_terminal_env(&mut cmd); - crate::integration::apply_pane_env(&mut cmd, pane_id); + crate::integration::apply_pane_env(&mut cmd, pane_id, public_pane_id); Self::spawn_command_builder( pane_id, rows, @@ -1375,6 +1380,8 @@ impl PaneRuntime { ) } + // Runtime construction needs to thread PTY size, environment, theme, and render hooks together. + #[allow(clippy::too_many_arguments)] pub fn spawn_shell_command( pane_id: PaneId, rows: u16, @@ -1387,6 +1394,7 @@ impl PaneRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { let mut cmd = CommandBuilder::new("/bin/sh"); cmd.arg("-c"); @@ -1394,7 +1402,7 @@ impl PaneRuntime { cmd.cwd(cwd); cmd.env(crate::HERDR_ENV_VAR, crate::HERDR_ENV_VALUE); apply_pane_terminal_env(&mut cmd); - crate::integration::apply_pane_env(&mut cmd, pane_id); + crate::integration::apply_pane_env(&mut cmd, pane_id, public_pane_id); for (key, value) in extra_env { cmd.env(key, value); } @@ -1424,6 +1432,7 @@ impl PaneRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { let Some((program, args)) = argv.split_first() else { return Err(std::io::Error::new( @@ -1438,7 +1447,7 @@ impl PaneRuntime { cmd.cwd(cwd); cmd.env(crate::HERDR_ENV_VAR, crate::HERDR_ENV_VALUE); apply_pane_terminal_env(&mut cmd); - crate::integration::apply_pane_env(&mut cmd, pane_id); + crate::integration::apply_pane_env(&mut cmd, pane_id, public_pane_id); Self::spawn_command_builder( pane_id, rows, diff --git a/src/persist/restore.rs b/src/persist/restore.rs index d5661b30..ca1748e0 100644 --- a/src/persist/restore.rs +++ b/src/persist/restore.rs @@ -57,6 +57,7 @@ type RestoredTab = ( crate::workspace::Tab, Vec, HashMap, + HashMap, ); type RestoreFailures = (T, usize); @@ -154,6 +155,35 @@ fn collect_snapshot_ids_inner(node: &LayoutSnapshot, ids: &mut Vec) { } } +fn migrated_public_pane_numbers_by_old_raw( + snap: &WorkspaceSnapshot, + next_public_pane_number: &mut usize, +) -> HashMap { + let mut public_numbers = snap.public_pane_numbers.clone(); + for tab in &snap.tabs { + let mut pane_ids = Vec::new(); + collect_layout_snapshot_pane_ids(&tab.layout, &mut pane_ids); + for old_raw in pane_ids { + public_numbers.entry(old_raw).or_insert_with(|| { + let number = *next_public_pane_number; + *next_public_pane_number += 1; + number + }); + } + } + public_numbers +} + +fn collect_layout_snapshot_pane_ids(node: &LayoutSnapshot, ids: &mut Vec) { + match node { + LayoutSnapshot::Pane(id) => ids.push(*id), + LayoutSnapshot::Split { first, second, .. } => { + collect_layout_snapshot_pane_ids(first, ids); + collect_layout_snapshot_pane_ids(second, ids); + } + } +} + #[cfg(unix)] fn restore_with_imports_strict( snapshot: &SessionSnapshot, @@ -269,6 +299,7 @@ fn restore_with_imports_and_failures( workspaces.push(workspace); } } + crate::workspace::reserve_workspace_ids(&workspaces); ((workspaces, terminals, terminal_runtimes), failed_imports) } @@ -284,8 +315,42 @@ fn restore_workspace( let mut tabs = Vec::new(); let mut terminals = Vec::new(); let mut terminal_runtimes = HashMap::new(); + let workspace_id = snap + .id + .clone() + .unwrap_or_else(crate::workspace::generate_workspace_id); + let mut next_public_pane_number = snap + .public_pane_numbers + .values() + .copied() + .max() + .and_then(|max| max.checked_add(1)) + .unwrap_or(1) + .max(snap.next_public_pane_number); + let public_pane_numbers_by_old_raw = + migrated_public_pane_numbers_by_old_raw(snap, &mut next_public_pane_number); + let public_pane_ids_by_old_raw: HashMap = public_pane_numbers_by_old_raw + .iter() + .map(|(old_raw, public_number)| { + ( + *old_raw, + format!( + "{}:p{}", + workspace_id, + crate::workspace::encode_public_number(*public_number) + ), + ) + }) + .collect(); let mut public_pane_numbers = HashMap::new(); - let mut next_public_pane_number = 1; + let mut next_public_tab_number = snap + .public_tab_numbers + .iter() + .copied() + .max() + .and_then(|max| max.checked_add(1)) + .unwrap_or(1) + .max(snap.next_public_tab_number); let mut failed_imports = 0; for (idx, tab_snap) in snap.tabs.iter().enumerate() { @@ -298,14 +363,33 @@ fn restore_workspace( runtime_context, resumed_agent_sessions, imported_panes, + &public_pane_ids_by_old_raw, ); failed_imports += tab_failed_imports; - let Some((tab, restored_terminals, restored_runtimes)) = restored_tab else { + let Some((mut tab, restored_terminals, restored_runtimes, reverse_id_map)) = restored_tab + else { continue; }; + if let Some(public_tab_number) = snap.public_tab_numbers.get(idx).copied() { + tab.number = public_tab_number; + } + next_public_tab_number = next_public_tab_number.max(tab.number + 1); for pane_id in tab.layout.pane_ids() { - public_pane_numbers.insert(pane_id, next_public_pane_number); - next_public_pane_number += 1; + let public_number = public_pane_numbers_by_old_raw + .get( + &reverse_id_map + .get(&pane_id) + .copied() + .unwrap_or(pane_id.raw()), + ) + .copied() + .unwrap_or_else(|| { + let number = next_public_pane_number; + next_public_pane_number += 1; + number + }); + public_pane_numbers.insert(pane_id, public_number); + next_public_pane_number = next_public_pane_number.max(public_number + 1); } terminals.extend(restored_terminals); terminal_runtimes.extend(restored_runtimes); @@ -320,10 +404,7 @@ fn restore_workspace( ( Some(Workspace { - id: snap - .id - .clone() - .unwrap_or_else(crate::workspace::generate_workspace_id), + id: workspace_id, custom_name: snap.custom_name.clone(), identity_cwd: snap.identity_cwd.clone(), cached_git_branch: crate::workspace::git_branch(&snap.identity_cwd), @@ -332,6 +413,7 @@ fn restore_workspace( worktree_space, public_pane_numbers, next_public_pane_number, + next_public_tab_number, active_tab: snap.active_tab.min(tabs.len().saturating_sub(1)), tabs, #[cfg(test)] @@ -361,6 +443,7 @@ fn restore_tab( runtime_context: &RestoreRuntimeContext<'_>, resumed_agent_sessions: &mut HashSet, imported_panes: &mut HashMap, + public_pane_ids_by_old_raw: &HashMap, ) -> RestoreFailures> { let (node, id_map) = restore_node_remapped(&snap.layout); let reverse_id_map: HashMap = id_map @@ -416,6 +499,9 @@ fn restore_tab( .and_then(|plan| crate::detect::parse_agent_label(&plan.agent)); let old_pane_id = reverse_id_map.get(id).copied(); + let public_pane_id = old_pane_id + .and_then(|old_id| public_pane_ids_by_old_raw.get(&old_id)) + .map(String::as_str); let imported_runtime = old_pane_id.and_then(|old_id| imported_panes.remove(&old_id)); let was_imported = imported_runtime.is_some(); let pending_native_agent_restore = if was_imported { @@ -488,6 +574,7 @@ fn restore_tab( runtime_context.events.clone(), runtime_context.render_notify.clone(), runtime_context.render_dirty.clone(), + public_pane_id, ) } @@ -505,6 +592,7 @@ fn restore_tab( runtime_context.events.clone(), runtime_context.render_notify.clone(), runtime_context.render_dirty.clone(), + public_pane_id, ) } }; @@ -611,6 +699,7 @@ fn restore_tab( }, terminals, terminal_runtimes, + reverse_id_map, )), failed_imports, ) @@ -1057,6 +1146,10 @@ mod tests { custom_name: None, identity_cwd: cwd.clone(), worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, tabs: vec![TabSnapshot { custom_name: None, layout: LayoutSnapshot::Pane(0), @@ -1121,6 +1214,124 @@ mod tests { assert_eq!(session.session_ref.value, "opencode-session"); } + #[tokio::test] + async fn restore_preserves_public_id_mapping_after_pane_id_remap() { + let cwd = std::env::current_dir().unwrap(); + let snapshot = SessionSnapshot { + version: super::super::snapshot::SNAPSHOT_VERSION, + workspaces: vec![WorkspaceSnapshot { + id: Some("w1".into()), + custom_name: None, + identity_cwd: cwd.clone(), + worktree_space: None, + public_pane_numbers: HashMap::from([(10, 1), (20, 3)]), + next_public_pane_number: 4, + public_tab_numbers: vec![5], + next_public_tab_number: 6, + tabs: vec![TabSnapshot { + custom_name: None, + layout: LayoutSnapshot::Split { + direction: super::super::snapshot::DirectionSnapshot::Horizontal, + ratio: 0.5, + first: Box::new(LayoutSnapshot::Pane(10)), + second: Box::new(LayoutSnapshot::Pane(20)), + }, + panes: HashMap::from([ + ( + 10, + super::super::snapshot::PaneSnapshot { + cwd: cwd.clone(), + label: None, + agent_name: None, + agent_session: None, + launch_argv: None, + }, + ), + ( + 20, + super::super::snapshot::PaneSnapshot { + cwd: cwd.clone(), + label: None, + agent_name: None, + agent_session: None, + launch_argv: None, + }, + ), + ]), + zoomed: false, + focused: Some(10), + root_pane: Some(10), + }], + active_tab: 0, + }], + active: Some(0), + selected: 0, + agent_panel_scope: Default::default(), + sidebar_width: None, + sidebar_section_split: None, + collapsed_space_keys: Default::default(), + }; + let (events, _event_rx) = mpsc::channel(4); + + let (workspaces, _terminals, _runtimes) = restore( + &snapshot, + None, + 24, + 80, + 0, + test_restore_shell(), + crate::config::ShellModeConfig::NonLogin, + false, + events, + Arc::new(Notify::new()), + Arc::new(AtomicBool::new(false)), + ); + + let workspace = workspaces.first().expect("workspace should restore"); + let mut public_numbers: Vec<_> = workspace.public_pane_numbers.values().copied().collect(); + public_numbers.sort_unstable(); + assert_eq!(public_numbers, vec![1, 3]); + assert_eq!(workspace.next_public_pane_number, 4); + assert_eq!(workspace.tabs[0].number, 5); + assert_eq!(workspace.next_public_tab_number, 6); + } + + #[test] + fn legacy_restore_precomputes_missing_public_pane_numbers() { + let cwd = std::env::current_dir().unwrap(); + let snapshot = WorkspaceSnapshot { + id: Some("w1".into()), + custom_name: None, + identity_cwd: cwd, + worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, + tabs: vec![TabSnapshot { + custom_name: None, + layout: LayoutSnapshot::Split { + direction: super::super::snapshot::DirectionSnapshot::Horizontal, + ratio: 0.5, + first: Box::new(LayoutSnapshot::Pane(10)), + second: Box::new(LayoutSnapshot::Pane(20)), + }, + panes: HashMap::new(), + zoomed: false, + focused: Some(10), + root_pane: Some(10), + }], + active_tab: 0, + }; + let mut next_public_pane_number = 1; + + let public_numbers = + migrated_public_pane_numbers_by_old_raw(&snapshot, &mut next_public_pane_number); + + assert_eq!(public_numbers, HashMap::from([(10, 1), (20, 2)])); + assert_eq!(next_public_pane_number, 3); + } + #[tokio::test] #[cfg(unix)] async fn native_agent_restore_defers_runtime_launch() { @@ -1132,6 +1343,10 @@ mod tests { custom_name: None, identity_cwd: cwd.clone(), worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, tabs: vec![TabSnapshot { custom_name: None, layout: LayoutSnapshot::Pane(0), @@ -1333,6 +1548,10 @@ mod tests { custom_name: None, identity_cwd: cwd, worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, tabs: vec![TabSnapshot { custom_name: None, layout: LayoutSnapshot::Pane(0), diff --git a/src/persist/snapshot.rs b/src/persist/snapshot.rs index c04c280f..765fa2b6 100644 --- a/src/persist/snapshot.rs +++ b/src/persist/snapshot.rs @@ -57,6 +57,14 @@ pub struct WorkspaceSnapshot { pub identity_cwd: PathBuf, #[serde(default, skip_serializing_if = "Option::is_none")] pub worktree_space: Option, + #[serde(default)] + pub public_pane_numbers: HashMap, + #[serde(default)] + pub next_public_pane_number: usize, + #[serde(default)] + pub public_tab_numbers: Vec, + #[serde(default)] + pub next_public_tab_number: usize, pub tabs: Vec, #[serde(default)] pub active_tab: usize, @@ -150,6 +158,10 @@ impl From for WorkspaceSnapshot { custom_name: snap.custom_name, identity_cwd, worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, tabs: vec![tab], active_tab: 0, } @@ -284,6 +296,14 @@ fn capture_workspace( .resolved_identity_cwd_from(terminals, terminal_runtimes) .unwrap_or_else(|| ws.identity_cwd.clone()), worktree_space: ws.worktree_space.clone(), + public_pane_numbers: ws + .public_pane_numbers + .iter() + .map(|(pane_id, number)| (pane_id.raw(), *number)) + .collect(), + next_public_pane_number: ws.next_public_pane_number, + public_tab_numbers: ws.tabs.iter().map(|tab| tab.number).collect(), + next_public_tab_number: ws.next_public_tab_number, tabs: ws .tabs .iter() @@ -616,6 +636,10 @@ mod tests { custom_name: Some("pi-mono".to_string()), identity_cwd: PathBuf::from("/home/can/Projects/herdr"), worktree_space: None, + public_pane_numbers: HashMap::from([(0, 1), (1, 2)]), + next_public_pane_number: 3, + public_tab_numbers: vec![1], + next_public_tab_number: 2, tabs: vec![TabSnapshot { custom_name: Some("api".to_string()), layout: LayoutSnapshot::Split { @@ -929,6 +953,30 @@ mod tests { assert!(!tab.zoomed); } + #[test] + fn capture_contract_tracks_public_id_counters() { + let mut state = state_with_workspaces(&["one"]); + let second = state.workspaces[0].test_split(Direction::Horizontal); + let third = state.workspaces[0].test_split(Direction::Vertical); + let second_tab = state.workspaces[0].test_add_tab(None); + + state.workspaces[0].close_pane(second); + + let snapshot = capture_from_state(&state); + let workspace = &snapshot.workspaces[0]; + assert_eq!( + workspace.public_pane_numbers, + HashMap::from([ + (state.workspaces[0].tabs[0].root_pane.raw(), 1), + (third.raw(), 3), + (state.workspaces[0].tabs[second_tab].root_pane.raw(), 4), + ]) + ); + assert_eq!(workspace.next_public_pane_number, 5); + assert_eq!(workspace.public_tab_numbers, vec![1, 2]); + assert_eq!(workspace.next_public_tab_number, 3); + } + #[test] fn capture_contract_tracks_workspace_identity_and_pane_cwds() { let mut state = state_with_workspaces(&["one"]); @@ -1153,6 +1201,10 @@ mod tests { custom_name: Some("fallback test".to_string()), identity_cwd: PathBuf::from("/tmp"), worktree_space: None, + public_pane_numbers: HashMap::new(), + next_public_pane_number: 0, + public_tab_numbers: Vec::new(), + next_public_tab_number: 0, tabs: vec![TabSnapshot { custom_name: None, layout: LayoutSnapshot::Split { diff --git a/src/server/headless.rs b/src/server/headless.rs index faf43c10..ed81b015 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -7495,7 +7495,7 @@ next_tab = "" let mut server = test_headless_server(); let background = crate::workspace::Workspace::test_new("background"); let pane_id = background.tabs[0].root_pane; - let public_pane_id = format!("{}-1", background.id); + let public_pane_id = format!("{}:p1", background.id); let foreground = crate::workspace::Workspace::test_new("foreground"); server.app.state.workspaces = vec![background, foreground]; server.app.state.ensure_test_terminals(); diff --git a/src/server/notifications.rs b/src/server/notifications.rs index ed8fcb6c..63bfe944 100644 --- a/src/server/notifications.rs +++ b/src/server/notifications.rs @@ -124,6 +124,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); diff --git a/src/terminal/runtime.rs b/src/terminal/runtime.rs index ec830833..c4bf91e1 100644 --- a/src/terminal/runtime.rs +++ b/src/terminal/runtime.rs @@ -88,6 +88,7 @@ impl TerminalRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { crate::pane::PaneRuntime::spawn( pane_id, @@ -100,10 +101,13 @@ impl TerminalRuntime { events, render_notify, render_dirty, + public_pane_id, ) .map(Self) } + // Wrapper mirrors pane runtime construction arguments. + #[allow(clippy::too_many_arguments)] pub fn spawn_with_initial_history( pane_id: PaneId, rows: u16, @@ -116,6 +120,7 @@ impl TerminalRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { crate::pane::PaneRuntime::spawn_with_initial_history( pane_id, @@ -129,10 +134,13 @@ impl TerminalRuntime { events, render_notify, render_dirty, + public_pane_id, ) .map(Self) } + // Wrapper mirrors pane runtime construction arguments. + #[allow(clippy::too_many_arguments)] pub fn spawn_shell_command( pane_id: PaneId, rows: u16, @@ -145,6 +153,7 @@ impl TerminalRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { crate::pane::PaneRuntime::spawn_shell_command( pane_id, @@ -158,6 +167,7 @@ impl TerminalRuntime { events, render_notify, render_dirty, + public_pane_id, ) .map(Self) } @@ -173,6 +183,7 @@ impl TerminalRuntime { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result { crate::pane::PaneRuntime::spawn_argv_command( pane_id, @@ -185,6 +196,7 @@ impl TerminalRuntime { events, render_notify, render_dirty, + public_pane_id, ) .map(Self) } diff --git a/src/ui/mobile.rs b/src/ui/mobile.rs index 3d6911ca..33fd4443 100644 --- a/src/ui/mobile.rs +++ b/src/ui/mobile.rs @@ -1010,6 +1010,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); diff --git a/src/ui/sidebar.rs b/src/ui/sidebar.rs index 085f4371..293eeb24 100644 --- a/src/ui/sidebar.rs +++ b/src/ui/sidebar.rs @@ -1333,6 +1333,7 @@ mod tests { events, std::sync::Arc::new(tokio::sync::Notify::new()), std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + None, ) .unwrap(); diff --git a/src/workspace.rs b/src/workspace.rs index b01b5891..9e342ffd 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -3,7 +3,6 @@ use std::ops::{Deref, DerefMut}; use std::path::PathBuf; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::Arc; -use std::time::{SystemTime, UNIX_EPOCH}; use ratatui::layout::Direction; use tokio::sync::{mpsc, Notify}; @@ -71,14 +70,70 @@ impl WorkspaceGitStatusSnapshot { } static NEXT_WORKSPACE_ID: AtomicU64 = AtomicU64::new(1); +const PUBLIC_ID_ALPHABET: &[u8; 32] = b"123456789ABCDEFGHJKMNPQRSTVWXYZ0"; pub(crate) fn generate_workspace_id() -> String { - let micros = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_micros()) - .unwrap_or(0); let counter = NEXT_WORKSPACE_ID.fetch_add(1, Ordering::Relaxed); - format!("w{micros:x}{counter:x}") + format!("w{}", encode_public_number(counter as usize)) +} + +pub(crate) fn encode_public_number(mut value: usize) -> String { + if value == 0 { + return "0".to_string(); + } + + let mut encoded = Vec::new(); + while value > 0 { + let digit = (value - 1) % PUBLIC_ID_ALPHABET.len(); + encoded.push(PUBLIC_ID_ALPHABET[digit] as char); + value = (value - 1) / PUBLIC_ID_ALPHABET.len(); + } + encoded.iter().rev().collect() +} + +pub(crate) fn decode_public_number(value: &str) -> Option { + let mut decoded = 0usize; + for ch in value.chars() { + let digit = PUBLIC_ID_ALPHABET + .iter() + .position(|candidate| *candidate as char == ch)?; + decoded = decoded + .checked_mul(PUBLIC_ID_ALPHABET.len())? + .checked_add(digit + 1)?; + } + Some(decoded) +} + +pub(crate) fn public_workspace_number(id: &str) -> Option { + id.strip_prefix('w').and_then(decode_public_number) +} + +fn public_pane_id_for_number(workspace_id: &str, pane_number: usize) -> String { + format!("{workspace_id}:p{}", encode_public_number(pane_number)) +} + +pub(crate) fn reserve_workspace_ids(workspaces: &[Workspace]) { + let Some(next) = workspaces + .iter() + .filter_map(|workspace| public_workspace_number(&workspace.id)) + .max() + .and_then(|max| u64::try_from(max.checked_add(1)?).ok()) + else { + return; + }; + + let mut current = NEXT_WORKSPACE_ID.load(Ordering::Relaxed); + while current < next { + match NEXT_WORKSPACE_ID.compare_exchange_weak( + current, + next, + Ordering::Relaxed, + Ordering::Relaxed, + ) { + Ok(_) => break, + Err(observed) => current = observed, + } + } } /// A named workspace containing tabs. @@ -97,10 +152,10 @@ pub struct Workspace { pub(crate) cached_git_space: Option, /// Explicit Herdr-managed worktree grouping provenance. pub worktree_space: Option, - /// Stable-ish public pane numbers within this workspace. - /// New panes append at the end; closing a pane compacts higher numbers down. + /// Public pane numbers within this workspace. Closed pane numbers are not reused. pub public_pane_numbers: HashMap, pub(crate) next_public_pane_number: usize, + pub(crate) next_public_tab_number: usize, pub tabs: Vec, pub active_tab: usize, #[cfg(test)] @@ -187,6 +242,8 @@ impl Workspace { render_dirty: Arc, argv: Option<&[String]>, ) -> std::io::Result<(Self, TerminalState, TerminalRuntime)> { + let id = generate_workspace_id(); + let root_public_pane_id = public_pane_id_for_number(&id, 1); let (tab, terminal, runtime) = if let Some(argv) = argv { Tab::new_argv_command( 1, @@ -199,6 +256,7 @@ impl Workspace { events, render_notify, render_dirty, + Some(&root_public_pane_id), )? } else { Tab::new( @@ -212,13 +270,14 @@ impl Workspace { events, render_notify, render_dirty, + Some(&root_public_pane_id), )? }; let mut public_pane_numbers = HashMap::new(); public_pane_numbers.insert(tab.root_pane, 1); Ok(( Self { - id: generate_workspace_id(), + id, custom_name: None, identity_cwd: initial_cwd.clone(), cached_git_branch: git_branch(&initial_cwd), @@ -227,6 +286,7 @@ impl Workspace { worktree_space: None, public_pane_numbers, next_public_pane_number: 2, + next_public_tab_number: 2, tabs: vec![tab], active_tab: 0, #[cfg(test)] @@ -294,7 +354,10 @@ impl Workspace { shell_config: crate::pane::PaneShellConfig<'_>, argv: Option<&[String]>, ) -> std::io::Result<(usize, TerminalState, TerminalRuntime)> { - let number = self.tabs.len() + 1; + let number = self.next_public_tab_number; + self.next_public_tab_number += 1; + let pane_number = self.next_public_pane_number; + let public_pane_id = public_pane_id_for_number(&self.id, pane_number); let events = self .active_tab() .map(|tab| tab.events.clone()) @@ -320,6 +383,7 @@ impl Workspace { events, render_notify, render_dirty, + Some(&public_pane_id), )? } else { Tab::new( @@ -333,9 +397,10 @@ impl Workspace { events, render_notify, render_dirty, + Some(&public_pane_id), )? }; - self.register_new_pane(tab.root_pane); + self.register_new_pane_with_number(tab.root_pane, pane_number); self.tabs.push(tab); Ok((self.tabs.len() - 1, terminal, runtime)) } @@ -348,7 +413,6 @@ impl Workspace { for pane_id in tab.panes.keys() { self.unregister_pane(*pane_id); } - self.renumber_tabs(); if self.active_tab >= self.tabs.len() { self.active_tab = self.tabs.len() - 1; } else if idx <= self.active_tab && self.active_tab > 0 { @@ -376,7 +440,6 @@ impl Workspace { let active_root_pane = self.tabs.get(self.active_tab).map(|tab| tab.root_pane); let tab = self.tabs.remove(source_idx); self.tabs.insert(target_idx, tab); - self.renumber_tabs(); self.active_tab = active_root_pane .and_then(|root_pane| self.tabs.iter().position(|tab| tab.root_pane == root_pane)) .unwrap_or(target_idx); @@ -397,6 +460,8 @@ impl Workspace { host_terminal_theme: crate::terminal_theme::TerminalTheme, shell_config: crate::pane::PaneShellConfig<'_>, ) -> std::io::Result { + let pane_number = self.next_public_pane_number; + let public_pane_id = public_pane_id_for_number(&self.id, pane_number); let new_pane = self .active_tab_mut() .expect("workspace must always have at least one tab") @@ -408,8 +473,41 @@ impl Workspace { scrollback_limit_bytes, host_terminal_theme, shell_config, + Some(&public_pane_id), )?; - self.register_new_pane(new_pane.pane_id); + self.register_new_pane_with_number(new_pane.pane_id, pane_number); + Ok(new_pane) + } + + #[allow(clippy::too_many_arguments)] + pub fn split_focused_command( + &mut self, + direction: Direction, + rows: u16, + cols: u16, + cwd: Option, + command: &str, + extra_env: &[(String, String)], + scrollback_limit_bytes: usize, + host_terminal_theme: crate::terminal_theme::TerminalTheme, + ) -> std::io::Result { + let pane_number = self.next_public_pane_number; + let public_pane_id = public_pane_id_for_number(&self.id, pane_number); + let new_pane = self + .active_tab_mut() + .expect("workspace must always have at least one tab") + .split_focused_command( + direction, + rows, + cols, + cwd, + command, + extra_env, + scrollback_limit_bytes, + host_terminal_theme, + Some(&public_pane_id), + )?; + self.register_new_pane_with_number(new_pane.pane_id, pane_number); Ok(new_pane) } @@ -513,6 +611,8 @@ impl Workspace { argv: Option<&[String]>, ) -> Option> { let tab_idx = self.find_tab_index_for_pane(pane_id)?; + let pane_number = self.next_public_pane_number; + let public_pane_id = public_pane_id_for_number(&self.id, pane_number); let tab = &mut self.tabs[tab_idx]; let previous_focus = tab.layout.focused(); tab.layout.focus_pane(pane_id); @@ -525,6 +625,7 @@ impl Workspace { argv, scrollback_limit_bytes, host_terminal_theme, + Some(&public_pane_id), ) } else { match ratio { @@ -537,6 +638,7 @@ impl Workspace { scrollback_limit_bytes, host_terminal_theme, shell_config, + Some(&public_pane_id), ), None => tab.split_focused( direction, @@ -546,6 +648,7 @@ impl Workspace { scrollback_limit_bytes, host_terminal_theme, shell_config, + Some(&public_pane_id), ), } } { @@ -558,7 +661,7 @@ impl Workspace { if !focus_new_pane { tab.layout.focus_pane(previous_focus); } - self.register_new_pane(new_pane.pane_id); + self.register_new_pane_with_number(new_pane.pane_id, pane_number); Some(Ok((tab_idx, new_pane))) } @@ -593,7 +696,6 @@ impl Workspace { } self.tabs.remove(tab_idx); self.unregister_pane(pane_id); - self.renumber_tabs(); if self.active_tab >= self.tabs.len() { self.active_tab = self.tabs.len() - 1; } else if tab_idx <= self.active_tab && self.active_tab > 0 { @@ -612,6 +714,16 @@ impl Workspace { self.public_pane_numbers.get(&pane_id).copied() } + pub fn public_tab_number(&self, tab_idx: usize) -> Option { + self.tabs.get(tab_idx).map(|tab| tab.number) + } + + #[cfg(test)] + pub fn public_tab_number_for_pane(&self, pane_id: PaneId) -> Option { + let tab_idx = self.find_tab_index_for_pane(pane_id)?; + self.public_tab_number(tab_idx) + } + pub fn set_custom_name(&mut self, name: String) { self.custom_name = Some(name); } @@ -717,7 +829,6 @@ impl Workspace { } self.tabs.remove(tab_idx); self.unregister_pane(pane_id); - self.renumber_tabs(); if self.active_tab >= self.tabs.len() { self.active_tab = self.tabs.len() - 1; } else if tab_idx <= self.active_tab && self.active_tab > 0 { @@ -732,27 +843,18 @@ impl Workspace { false } + #[cfg(test)] fn register_new_pane(&mut self, pane_id: PaneId) { - self.public_pane_numbers - .insert(pane_id, self.next_public_pane_number); - self.next_public_pane_number += 1; + self.register_new_pane_with_number(pane_id, self.next_public_pane_number); + } + + fn register_new_pane_with_number(&mut self, pane_id: PaneId, number: usize) { + self.public_pane_numbers.insert(pane_id, number); + self.next_public_pane_number = self.next_public_pane_number.max(number + 1); } fn unregister_pane(&mut self, pane_id: PaneId) { - if let Some(removed_number) = self.public_pane_numbers.remove(&pane_id) { - for number in self.public_pane_numbers.values_mut() { - if *number > removed_number { - *number -= 1; - } - } - self.next_public_pane_number = self.public_pane_numbers.len() + 1; - } - } - - fn renumber_tabs(&mut self) { - for (idx, tab) in self.tabs.iter_mut().enumerate() { - tab.number = idx + 1; - } + self.public_pane_numbers.remove(&pane_id); } fn close_active_tab_and_report(&mut self) -> bool { @@ -799,6 +901,7 @@ impl Workspace { worktree_space: None, public_pane_numbers, next_public_pane_number: 2, + next_public_tab_number: 2, tabs: vec![tab], active_tab: 0, test_runtimes: HashMap::new(), @@ -827,7 +930,7 @@ impl Workspace { panes.insert(root_id, PaneState::new(TerminalId::alloc())); let tab = Tab { custom_name: name.map(str::to_string), - number: self.tabs.len() + 1, + number: self.next_public_tab_number, root_pane: root_id, layout, panes, @@ -837,6 +940,7 @@ impl Workspace { render_notify, render_dirty, }; + self.next_public_tab_number += 1; self.register_new_pane(root_id); self.tabs.push(tab); self.tabs.len() - 1 @@ -847,6 +951,92 @@ impl Workspace { mod tests { use super::*; + #[test] + fn generated_workspace_ids_are_short_base32_handles() { + let first = generate_workspace_id(); + let second = generate_workspace_id(); + + assert!(first.starts_with('w')); + assert!(second.starts_with('w')); + assert_ne!(first, second); + assert!(first.len() <= 3, "unexpectedly long workspace id: {first}"); + assert!( + second.len() <= 3, + "unexpectedly long workspace id: {second}" + ); + } + + #[test] + fn public_numbers_round_trip_readable_base32_handles() { + assert_eq!(encode_public_number(1), "1"); + assert_eq!(encode_public_number(9), "9"); + assert_eq!(encode_public_number(10), "A"); + assert_eq!(encode_public_number(31), "Z"); + assert_eq!(encode_public_number(32), "0"); + assert_eq!(encode_public_number(33), "11"); + + for value in [1, 9, 10, 31, 32, 33, 1024, 1025] { + let encoded = encode_public_number(value); + assert_eq!(decode_public_number(&encoded), Some(value)); + } + } + + #[test] + fn reserving_restored_workspace_ids_prevents_reuse() { + let mut restored = Workspace::test_new("restored"); + restored.id = "wZ".to_string(); + + reserve_workspace_ids(&[restored]); + + let generated = generate_workspace_id(); + assert_ne!(generated, "wZ"); + assert!(public_workspace_number(&generated) > public_workspace_number("wZ")); + } + + #[test] + fn pane_public_numbers_are_stable_and_not_reused_after_close() { + let mut ws = Workspace::test_new("test"); + let root = ws.tabs[0].root_pane; + let second = ws.test_split(Direction::Horizontal); + let third = ws.test_split(Direction::Vertical); + + assert_eq!(ws.public_pane_number(root), Some(1)); + assert_eq!(ws.public_pane_number(second), Some(2)); + assert_eq!(ws.public_pane_number(third), Some(3)); + + assert!(!ws.close_pane(second)); + + assert_eq!(ws.public_pane_number(root), Some(1)); + assert_eq!(ws.public_pane_number(second), None); + assert_eq!(ws.public_pane_number(third), Some(3)); + + let fourth = ws.test_split(Direction::Horizontal); + assert_eq!(ws.public_pane_number(fourth), Some(4)); + } + + #[test] + fn tab_public_numbers_are_stable_and_not_reused_after_close() { + let mut ws = Workspace::test_new("test"); + let first_root = ws.tabs[0].root_pane; + let second_tab = ws.test_add_tab(None); + let second_root = ws.tabs[second_tab].root_pane; + let third_tab = ws.test_add_tab(None); + let third_root = ws.tabs[third_tab].root_pane; + + assert_eq!(ws.public_tab_number_for_pane(first_root), Some(1)); + assert_eq!(ws.public_tab_number_for_pane(second_root), Some(2)); + assert_eq!(ws.public_tab_number_for_pane(third_root), Some(3)); + + assert!(ws.close_tab(second_tab)); + + assert_eq!(ws.public_tab_number_for_pane(first_root), Some(1)); + assert_eq!(ws.public_tab_number_for_pane(third_root), Some(3)); + + let fourth_tab = ws.test_add_tab(None); + let fourth_root = ws.tabs[fourth_tab].root_pane; + assert_eq!(ws.public_tab_number_for_pane(fourth_root), Some(4)); + } + #[test] fn workspace_identity_follows_first_tab_root_pane_cwd() { let mut ws = Workspace::test_new("ignored"); @@ -868,7 +1058,7 @@ mod tests { } #[test] - fn moving_tab_keeps_active_identity_and_renumbers_auto_tabs() { + fn moving_tab_keeps_active_identity_and_stable_tab_numbers() { let mut ws = Workspace::test_new("test"); let moved_root = ws.tabs[0].root_pane; ws.test_add_tab(Some("foo")); @@ -879,10 +1069,13 @@ mod tests { assert!(ws.move_tab(0, ws.tabs.len())); let labels: Vec<_> = ws.tabs.iter().map(|tab| tab.display_name()).collect(); - assert_eq!(labels, vec!["foo", "2", "3"]); + assert_eq!(labels, vec!["foo", "3", "1"]); assert_eq!(ws.tabs[0].custom_name.as_deref(), Some("foo")); assert!(ws.tabs[1].custom_name.is_none()); assert!(ws.tabs[2].custom_name.is_none()); + assert_eq!(ws.tabs[0].number, 2); + assert_eq!(ws.tabs[1].number, 3); + assert_eq!(ws.tabs[2].number, 1); assert_eq!(ws.tabs[2].root_pane, moved_root); assert_eq!(ws.tabs[ws.active_tab].root_pane, active_root); } diff --git a/src/workspace/tab.rs b/src/workspace/tab.rs index cdb86136..4363e05c 100644 --- a/src/workspace/tab.rs +++ b/src/workspace/tab.rs @@ -57,6 +57,7 @@ impl Tab { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result<(Self, TerminalState, TerminalRuntime)> { Self::new_with_runtime( number, @@ -70,6 +71,7 @@ impl Tab { render_notify, render_dirty, None, + public_pane_id, ) } @@ -84,6 +86,7 @@ impl Tab { events: mpsc::Sender, render_notify: Arc, render_dirty: Arc, + public_pane_id: Option<&str>, ) -> std::io::Result<(Self, TerminalState, TerminalRuntime)> { Self::new_with_runtime( number, @@ -97,6 +100,7 @@ impl Tab { render_notify, render_dirty, Some(argv), + public_pane_id, ) } @@ -113,6 +117,7 @@ impl Tab { render_notify: Arc, render_dirty: Arc, argv: Option<&[String]>, + public_pane_id: Option<&str>, ) -> std::io::Result<(Self, TerminalState, TerminalRuntime)> { let (layout, root_id) = TileLayout::new(); let runtime = if let Some(argv) = argv { @@ -127,6 +132,7 @@ impl Tab { events.clone(), render_notify.clone(), render_dirty.clone(), + public_pane_id, )? } else { TerminalRuntime::spawn( @@ -140,6 +146,7 @@ impl Tab { events.clone(), render_notify.clone(), render_dirty.clone(), + public_pane_id, )? }; @@ -195,6 +202,7 @@ impl Tab { scrollback_limit_bytes: usize, host_terminal_theme: crate::terminal_theme::TerminalTheme, shell_config: crate::pane::PaneShellConfig<'_>, + public_pane_id: Option<&str>, ) -> std::io::Result { self.split_focused_with_runtime( direction, @@ -206,6 +214,7 @@ impl Tab { host_terminal_theme, shell_config, None, + public_pane_id, ) } @@ -219,6 +228,7 @@ impl Tab { scrollback_limit_bytes: usize, host_terminal_theme: crate::terminal_theme::TerminalTheme, shell_config: crate::pane::PaneShellConfig<'_>, + public_pane_id: Option<&str>, ) -> std::io::Result { self.split_focused_with_runtime( direction, @@ -230,6 +240,7 @@ impl Tab { host_terminal_theme, shell_config, None, + public_pane_id, ) } @@ -243,6 +254,7 @@ impl Tab { extra_env: &[(String, String)], scrollback_limit_bytes: usize, host_terminal_theme: crate::terminal_theme::TerminalTheme, + public_pane_id: Option<&str>, ) -> std::io::Result { self.split_focused_with_runtime( direction, @@ -254,6 +266,7 @@ impl Tab { host_terminal_theme, crate::pane::PaneShellConfig::new("", crate::config::ShellModeConfig::NonLogin), Some(SplitCommand::Shell { command, extra_env }), + public_pane_id, ) } @@ -266,6 +279,7 @@ impl Tab { argv: &[String], scrollback_limit_bytes: usize, host_terminal_theme: crate::terminal_theme::TerminalTheme, + public_pane_id: Option<&str>, ) -> std::io::Result { self.split_focused_with_runtime( direction, @@ -277,6 +291,7 @@ impl Tab { host_terminal_theme, crate::pane::PaneShellConfig::new("", crate::config::ShellModeConfig::NonLogin), Some(SplitCommand::Argv { argv }), + public_pane_id, ) } @@ -291,6 +306,7 @@ impl Tab { host_terminal_theme: crate::terminal_theme::TerminalTheme, shell_config: crate::pane::PaneShellConfig<'_>, command: Option>, + public_pane_id: Option<&str>, ) -> std::io::Result { let previous_focus = self.layout.focused(); let new_id = match ratio { @@ -318,6 +334,7 @@ impl Tab { self.events.clone(), self.render_notify.clone(), self.render_dirty.clone(), + public_pane_id, ) } Some(SplitCommand::Argv { argv }) => TerminalRuntime::spawn_argv_command( @@ -331,6 +348,7 @@ impl Tab { self.events.clone(), self.render_notify.clone(), self.render_dirty.clone(), + public_pane_id, ), None => TerminalRuntime::spawn( new_id, @@ -343,6 +361,7 @@ impl Tab { self.events.clone(), self.render_notify.clone(), self.render_dirty.clone(), + public_pane_id, ), }; let runtime = match runtime { diff --git a/tests/api_ping.rs b/tests/api_ping.rs index df0b5908..7f4271bc 100644 --- a/tests/api_ping.rs +++ b/tests/api_ping.rs @@ -406,7 +406,7 @@ fn workspace_list_and_create_round_trip() { assert_eq!(created["result"]["workspace"]["tab_count"], 1); assert_eq!(created["result"]["tab"]["tab_id"], active_tab_id); assert_eq!(created["result"]["root_pane"]["tab_id"], active_tab_id); - assert_eq!(active_tab_id, format!("{workspace_id}:1")); + assert_eq!(active_tab_id, format!("{workspace_id}:t1")); let listed = send_request( &socket_path, @@ -436,6 +436,7 @@ fn workspace_list_and_create_round_trip() { let pane_id = panes[0]["pane_id"].as_str().unwrap().to_string(); assert_eq!(pane_id, root_pane_id); assert_eq!(panes[0]["terminal_id"], root_terminal_id); + let legacy_pane_id = format!("{workspace_id}-1"); let pane = send_request( &socket_path, @@ -451,7 +452,7 @@ fn workspace_list_and_create_round_trip() { &socket_path, &format!( r#"{{"id":"req_8","method":"pane.read","params":{{"pane_id":"{}","source":"visible"}}}}"#, - pane_id + legacy_pane_id ), ); assert_eq!(read["result"]["read"]["pane_id"], pane_id); @@ -492,10 +493,12 @@ fn workspace_list_and_create_round_trip() { &socket_path, &format!( r#"{{"id":"req_12","method":"pane.wait_for_output","params":{{"pane_id":"{}","source":"recent","lines":40,"match":{{"type":"substring","value":"gamma"}},"timeout_ms":2000}}}}"#, - pane_id + legacy_pane_id ), ); assert_eq!(waited["result"]["type"], "output_matched"); + assert_eq!(waited["result"]["pane_id"], pane_id); + assert_eq!(waited["result"]["read"]["pane_id"], pane_id); assert!(waited["result"]["matched_line"] .as_str() .unwrap() @@ -522,6 +525,8 @@ fn workspace_list_and_create_round_trip() { ), ); assert_eq!(waited_delta["result"]["type"], "output_matched"); + assert_eq!(waited_delta["result"]["pane_id"], pane_id); + assert_eq!(waited_delta["result"]["read"]["pane_id"], pane_id); assert!(waited_delta["result"]["matched_line"] .as_str() .unwrap() @@ -535,6 +540,8 @@ fn workspace_list_and_create_round_trip() { ), ); assert_eq!(waited_regex["result"]["type"], "output_matched"); + assert_eq!(waited_regex["result"]["pane_id"], pane_id); + assert_eq!(waited_regex["result"]["read"]["pane_id"], pane_id); assert!(waited_regex["result"]["matched_line"] .as_str() .unwrap() @@ -579,7 +586,7 @@ fn tab_methods_round_trip_over_socket() { .as_str() .unwrap() .to_string(); - assert_eq!(first_tab_id, format!("{workspace_id}:1")); + assert_eq!(first_tab_id, format!("{workspace_id}:t1")); let tab_created = send_request( &socket_path, @@ -603,7 +610,7 @@ fn tab_methods_round_trip_over_socket() { .to_string(); assert!(second_root_terminal_id.starts_with("term_")); assert_ne!(second_root_terminal_id, second_root_pane_id); - assert_eq!(second_tab_id, format!("{workspace_id}:2")); + assert_eq!(second_tab_id, format!("{workspace_id}:t2")); assert_eq!(tab_created["result"]["tab"]["focused"], true); assert_eq!(tab_created["result"]["root_pane"]["tab_id"], second_tab_id); @@ -698,7 +705,6 @@ fn pane_info_reports_foreground_cwd_without_changing_pane_cwd() { .as_str() .unwrap() .to_string(); - let command = format!( "/bin/sh -c 'cd {} && printf %s $$ > {} && touch {} && sleep 30'", foreground.display(), @@ -1044,7 +1050,7 @@ fn tab_create_with_no_focus_preserves_active_tab() { .as_str() .unwrap() .to_string(); - assert_eq!(second_tab_id, format!("{workspace_id}:2")); + assert_eq!(second_tab_id, format!("{workspace_id}:t2")); assert_eq!(tab_created["result"]["tab"]["focused"], false); let tab_list = send_request( @@ -1140,7 +1146,7 @@ fn events_subscribe_streams_workspace_tab_and_agent_events() { let workspace_focused = event_by_kind(&initial_events, "workspace_focused"); assert_eq!(workspace_focused["data"]["workspace_id"], workspace_id); - let first_tab_id = format!("{workspace_id}:1"); + let first_tab_id = format!("{workspace_id}:t1"); let tab_created = event_by_kind(&initial_events, "tab_created"); assert_eq!(tab_created["data"]["tab"]["tab_id"], first_tab_id); let tab_focused = event_by_kind(&initial_events, "tab_focused"); @@ -1186,7 +1192,7 @@ fn events_subscribe_streams_workspace_tab_and_agent_events() { .as_str() .unwrap() .to_string(); - assert_eq!(second_tab_id, format!("{workspace_id}:2")); + assert_eq!(second_tab_id, format!("{workspace_id}:t2")); let created_tab_event = wait_for_event(&mut reader, "tab_created", Duration::from_secs(2)); assert_eq!(created_tab_event["data"]["tab"]["tab_id"], second_tab_id); @@ -1227,11 +1233,14 @@ fn events_subscribe_streams_pane_split_and_close_events() { base.display() ), ); + let pane_id = created["result"]["root_pane"]["pane_id"] + .as_str() + .unwrap() + .to_string(); let workspace_id = created["result"]["workspace"]["workspace_id"] .as_str() .unwrap() .to_string(); - let pane_id = format!("{workspace_id}-1"); let mut reader = open_subscription( &socket_path, @@ -1265,8 +1274,8 @@ fn events_subscribe_streams_pane_split_and_close_events() { let closed = send_request( &socket_path, &format!( - r#"{{"id":"req_pc_3","method":"pane.close","params":{{"pane_id":"{}"}}}}"#, - split_pane_id + r#"{{"id":"req_pc_3","method":"pane.close","params":{{"pane_id":"{}-2"}}}}"#, + workspace_id ), ); assert_eq!(closed["result"]["type"], "ok"); @@ -1330,8 +1339,8 @@ fn events_subscribe_streams_tab_and_workspace_close_events() { let closed_tab = send_request( &socket_path, &format!( - r#"{{"id":"req_tc_3","method":"tab.close","params":{{"tab_id":"{}"}}}}"#, - second_tab_id + r#"{{"id":"req_tc_3","method":"tab.close","params":{{"tab_id":"{}:2"}}}}"#, + workspace_id ), ); assert_eq!(closed_tab["result"]["type"], "ok"); @@ -1355,10 +1364,7 @@ fn events_subscribe_streams_tab_and_workspace_close_events() { let closed_ws = send_request( &socket_path, - &format!( - r#"{{"id":"req_tc_5","method":"workspace.close","params":{{"workspace_id":"{}"}}}}"#, - workspace_id - ), + r#"{"id":"req_tc_5","method":"workspace.close","params":{"workspace_id":"1"}}"#, ); assert_eq!(closed_ws["result"]["type"], "ok"); @@ -1407,10 +1413,10 @@ fn pane_report_agent_updates_effective_state() { base.display() ), ); - let pane_id = created["result"]["workspace"]["workspace_id"] + let pane_id = created["result"]["root_pane"]["pane_id"] .as_str() - .map(|workspace_id| format!("{}-1", workspace_id)) - .unwrap(); + .unwrap() + .to_string(); let send_pi = send_request( &socket_path, @@ -1597,10 +1603,10 @@ fn pane_report_agent_accepts_unknown_agent_labels() { base.display() ), ); - let pane_id = created["result"]["workspace"]["workspace_id"] + let pane_id = created["result"]["root_pane"]["pane_id"] .as_str() - .map(|workspace_id| format!("{}-1", workspace_id)) - .unwrap(); + .unwrap() + .to_string(); let hook = send_request( &socket_path, @@ -1670,10 +1676,10 @@ fn pane_release_agent_suppresses_reacquire_during_graceful_exit() { base.display() ), ); - let pane_id = created["result"]["workspace"]["workspace_id"] + let pane_id = created["result"]["root_pane"]["pane_id"] .as_str() - .map(|workspace_id| format!("{}-1", workspace_id)) - .unwrap(); + .unwrap() + .to_string(); let send_pi = send_request( &socket_path, @@ -1810,10 +1816,10 @@ fn pane_clear_agent_authority_restores_fallback_state() { base.display() ), ); - let pane_id = created["result"]["workspace"]["workspace_id"] + let pane_id = created["result"]["root_pane"]["pane_id"] .as_str() - .map(|workspace_id| format!("{}-1", workspace_id)) - .unwrap(); + .unwrap() + .to_string(); let send_pi = send_request( &socket_path, @@ -1935,7 +1941,10 @@ fn events_subscribe_streams_output_and_agent_status_events() { base.display() ), ); - assert!(created["result"]["workspace"]["workspace_id"].is_string()); + let workspace_id = created["result"]["workspace"]["workspace_id"] + .as_str() + .unwrap() + .to_string(); let panes = send_request( &socket_path, @@ -1945,12 +1954,13 @@ fn events_subscribe_streams_output_and_agent_status_events() { .as_str() .unwrap() .to_string(); + let legacy_pane_id = format!("{workspace_id}-1"); let mut reader = open_subscription( &socket_path, &format!( r#"{{"id":"sub_1","method":"events.subscribe","params":{{"subscriptions":[{{"type":"pane.output_matched","pane_id":"{}","source":"recent","lines":40,"match":{{"type":"substring","value":"hello from socket"}}}},{{"type":"pane.agent_status_changed","pane_id":"{}","agent_status":"idle"}}]}}}}"#, - pane_id, pane_id, + legacy_pane_id, legacy_pane_id, ), ); @@ -1978,6 +1988,7 @@ fn events_subscribe_streams_output_and_agent_status_events() { let output_event = reader.read_json_line(Duration::from_secs(3)); assert_eq!(output_event["event"], "pane.output_matched"); assert_eq!(output_event["data"]["pane_id"], pane_id); + assert_eq!(output_event["data"]["read"]["pane_id"], pane_id); assert!(output_event["data"]["matched_line"] .as_str() .unwrap() @@ -2062,7 +2073,10 @@ fn pane_info_and_subscriptions_expose_done_agent_status() { .as_str() .unwrap() .to_string(); - let background_pane_id = format!("{}-1", workspace_id); + let background_pane_id = created["result"]["root_pane"]["pane_id"] + .as_str() + .unwrap() + .to_string(); let tab_created = send_request( &socket_path, @@ -2178,10 +2192,10 @@ fn metadata_status_subscription_filter_and_ttl_expiry_are_observable() { base.display() ), ); - let pane_id = created["result"]["workspace"]["workspace_id"] + let pane_id = created["result"]["root_pane"]["pane_id"] .as_str() - .map(|workspace_id| format!("{}-1", workspace_id)) - .unwrap(); + .unwrap() + .to_string(); let report_agent = send_request( &socket_path, diff --git a/tests/cli_wrapper.rs b/tests/cli_wrapper.rs index 117afe97..287be14b 100644 --- a/tests/cli_wrapper.rs +++ b/tests/cli_wrapper.rs @@ -1937,7 +1937,7 @@ fn tab_management_commands_work() { .as_str() .unwrap() .to_string(); - assert_eq!(second_tab_id, format!("{workspace_id}:2")); + assert_eq!(second_tab_id, format!("{workspace_id}:t2")); let listed_tabs = run_cli(&socket_path, &["tab", "list", "--workspace", &workspace_id]); assert!(listed_tabs.status.success()); @@ -2209,15 +2209,13 @@ fn pane_run_read_and_wait_commands_work() { let herdr = spawn_herdr(&config_home, &runtime_dir, &socket_path); wait_for_socket(&socket_path, Duration::from_secs(5)); - let created = send_request( + send_request( &socket_path, &format!( r#"{{"id":"req_cli_1","method":"workspace.create","params":{{"cwd":"{}","focus":true}}}}"#, base.display() ), ); - assert!(created["result"]["workspace"]["workspace_id"].is_string()); - let create = run_cli( &socket_path, &[ @@ -2461,7 +2459,7 @@ fn closing_workspace_terminates_processes_inside_it() { } #[test] -fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { +fn workspace_ids_and_public_pane_ids_are_stable() { let base = unique_test_dir(); let config_home = base.join("config"); let runtime_dir = base.join("runtime"); @@ -2485,7 +2483,7 @@ fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { ); assert_eq!( split_12_json["result"]["pane"]["pane_id"], - format!("{ws1_id}-2") + format!("{ws1_id}:p2") ); let split_13_json = run_cli_json( @@ -2494,7 +2492,7 @@ fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { ); assert_eq!( split_13_json["result"]["pane"]["pane_id"], - format!("{ws1_id}-3") + format!("{ws1_id}:p3") ); let ws2_json = run_cli_json( @@ -2520,7 +2518,7 @@ fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { ); assert_eq!( ws2_split_json["result"]["pane"]["pane_id"], - format!("{ws2_id}-2") + format!("{ws2_id}:p2") ); let ws3_json = run_cli_json( @@ -2565,7 +2563,7 @@ fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { let ws3_panes_json = run_cli_json(&socket_path, &["pane", "list", "--workspace", &ws3_id]); assert_eq!( ws3_panes_json["result"]["panes"][0]["pane_id"], - format!("{ws3_id}-1") + format!("{ws3_id}:p1") ); let close_middle = run_cli(&socket_path, &["pane", "close", &format!("{ws1_id}-2")]); @@ -2582,7 +2580,33 @@ fn workspace_ids_are_stable_and_pane_numbers_stay_compact() { .iter() .map(|pane| pane["pane_id"].as_str().unwrap().to_string()) .collect(); - assert_eq!(pane_ids, vec![format!("{ws1_id}-1"), format!("{ws1_id}-2")]); + assert_eq!( + pane_ids, + vec![format!("{ws1_id}:p1"), format!("{ws1_id}:p3")] + ); + + let closed_lookup = run_cli(&socket_path, &["pane", "get", &format!("{ws1_id}:p2")]); + assert!( + !closed_lookup.status.success(), + "closed pane id should not retarget: {}", + String::from_utf8_lossy(&closed_lookup.stdout) + ); + + let split_14_json = run_cli_json( + &socket_path, + &[ + "pane", + "split", + &format!("{ws1_id}:p1"), + "--direction", + "right", + "--no-focus", + ], + ); + assert_eq!( + split_14_json["result"]["pane"]["pane_id"], + format!("{ws1_id}:p4") + ); cleanup_spawned_herdr(herdr, base); } @@ -2604,7 +2628,10 @@ fn pane_shell_gets_herdr_socket_and_pane_env() { base.display() ), ); - assert!(created["result"]["workspace"]["workspace_id"].is_string()); + let pane_id = created["result"]["root_pane"]["pane_id"] + .as_str() + .unwrap() + .to_string(); let env_capture = base.join("pane-env.txt"); let ran = run_cli( @@ -2626,7 +2653,7 @@ fn pane_shell_gets_herdr_socket_and_pane_env() { while Instant::now() < deadline { if env_capture.exists() { text = fs::read_to_string(&env_capture).unwrap(); - if text.contains(&socket_path.display().to_string()) && text.contains("p_") { + if text.contains(&socket_path.display().to_string()) && text.contains(&pane_id) { break; } } @@ -2637,7 +2664,7 @@ fn pane_shell_gets_herdr_socket_and_pane_env() { text.contains(&socket_path.display().to_string()), "env file was: {text:?}" ); - assert!(text.contains("p_"), "env file was: {text:?}"); + assert!(text.contains(&pane_id), "env file was: {text:?}"); cleanup_spawned_herdr(herdr, base); } @@ -2734,7 +2761,7 @@ fn wait_agent_status_exits_immediately_when_status_already_matches() { .as_str() .unwrap() .to_string(); - let pane_id = format!("{workspace_id}-1"); + let pane_id = format!("{workspace_id}:p1"); let reported = send_request( &socket_path, diff --git a/tests/live_handoff.rs b/tests/live_handoff.rs index c56e7c3f..d7ad493d 100644 --- a/tests/live_handoff.rs +++ b/tests/live_handoff.rs @@ -895,13 +895,13 @@ pathlib.Path({received:?}).write_text(data.hex()) } #[test] -fn live_handoff_accepts_old_pane_id_from_child_env() { +fn live_handoff_accepts_canonical_pane_id_from_child_env() { let _lock = test_lock(); let base = unique_test_dir(); let config_home = base.join("config"); let runtime_dir = base.join("runtime"); let api_socket = runtime_dir.join("herdr.sock"); - let pane_id_marker = base.join("old-pane-id"); + let pane_id_marker = base.join("pane-id"); let spawned = spawn_server(&config_home, &runtime_dir, &api_socket); wait_for_socket(&api_socket, Duration::from_secs(10)); @@ -927,9 +927,9 @@ fn live_handoff_accepts_old_pane_id_from_child_env() { "params": {"pane_id": pane_id, "text": format!("printf '%s' \"$HERDR_PANE_ID\" > {}", pane_id_marker.display()), "keys": ["Enter"]} }), )); - let old_pane_id = wait_for_file_contains(&pane_id_marker, "p_", Duration::from_secs(5)); + let old_pane_id = wait_for_file_contains(&pane_id_marker, &pane_id, Duration::from_secs(5)); assert!( - old_pane_id.starts_with("p_"), + old_pane_id == pane_id, "unexpected pane id from env: {old_pane_id:?}" );