From 78947df455bf970b01e12efbd3155968fd6b35b9 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 00:37:02 -0700 Subject: [PATCH 1/5] feat(daemon, cli): agents mark the tab where the person must act (tab-attention-v1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cmux attention request --tab --reason ` and `cmux attention clear --tab ` send `tab.update {attention}` (MCP tab_update with attention). The daemon keeps `{reason, at_ms}` on the tab record and every client sees it as the tab's extra.attention. It never moves focus. The mark clears on the agent's clear, when a browser tab's page goes to another URL (frontend-rendered and CDP browsers), or when the tab closes, so a reopened tab does not ask again. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16037-16c2-7a73-b063-2e37ed40b9c4&host=cosmic-gray --- cmux-tui/bindings/cpp/.cmux-resource-api.json | 2 +- cmux-tui/bindings/go/.cmux-resource-api.json | 2 +- .../bindings/java/.cmux-resource-api.json | 2 +- .../bindings/python/.cmux-resource-api.json | 2 +- .../bindings/rust/.cmux-resource-api.json | 2 +- .../typescript/.cmux-resource-api.json | 2 +- cmux-tui/bindings/zig/.cmux-resource-api.json | 2 +- .../cmux-app-host/generated/cmux-app.d.ts | 2 +- .../cmux-tui-core/src/browser/worker.rs | 4 + .../presentation/frontend_browser_history.rs | 6 + cmux-tui/crates/cmux-tui-core/src/server.rs | 5 + .../cmux-tui-core/src/server/capabilities.rs | 1 + cmux-tui/crates/cmux-tui-core/src/state.rs | 2 + .../crates/cmux-tui-core/src/state/router.rs | 22 ++- .../crates/cmux-tui-core/src/state/store.rs | 1 + .../src/state/tab_attention_tests.rs | 112 ++++++++++++ .../src/state/tab_state_store.rs | 169 +++++++++++++++++- .../crates/cmux-tui-core/src/state/tabs.rs | 40 +++++ .../crates/cmux-tui-core/src/state/values.rs | 11 +- .../src/workspace_registry/resource_store.rs | 3 + cmux-tui/crates/cmux-tui/src/cli.rs | 3 + cmux-tui/crates/cmux-tui/src/cli/command.rs | 3 + .../crates/cmux-tui/src/cli/command/state.rs | 28 +++ .../cmux-tui/src/cli/command/state/tests.rs | 18 ++ .../tests/transport_path_coverage_tests.rs | 2 + cmux-tui/crates/cmux-tui/src/cli/docs.rs | 7 +- .../crates/cmux-tui/src/cli/scope_help.rs | 15 ++ cmux-tui/crates/cmux-tui/src/cli/surface.rs | 1 + cmux-tui/docs/protocol.md | 8 + cmux-tui/spec/cli.md | 2 + cmux-tui/spec/resource-api-v2.md | 4 +- cmux-tui/spec/resource-operations-v2.json | 15 +- 32 files changed, 477 insertions(+), 21 deletions(-) create mode 100644 cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs diff --git a/cmux-tui/bindings/cpp/.cmux-resource-api.json b/cmux-tui/bindings/cpp/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/cpp/.cmux-resource-api.json +++ b/cmux-tui/bindings/cpp/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/go/.cmux-resource-api.json b/cmux-tui/bindings/go/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/go/.cmux-resource-api.json +++ b/cmux-tui/bindings/go/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/java/.cmux-resource-api.json b/cmux-tui/bindings/java/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/java/.cmux-resource-api.json +++ b/cmux-tui/bindings/java/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/python/.cmux-resource-api.json b/cmux-tui/bindings/python/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/python/.cmux-resource-api.json +++ b/cmux-tui/bindings/python/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/rust/.cmux-resource-api.json b/cmux-tui/bindings/rust/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/rust/.cmux-resource-api.json +++ b/cmux-tui/bindings/rust/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/typescript/.cmux-resource-api.json b/cmux-tui/bindings/typescript/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/typescript/.cmux-resource-api.json +++ b/cmux-tui/bindings/typescript/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/zig/.cmux-resource-api.json b/cmux-tui/bindings/zig/.cmux-resource-api.json index 99ea187506ae..30819d76de4c 100644 --- a/cmux-tui/bindings/zig/.cmux-resource-api.json +++ b/cmux-tui/bindings/zig/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8", + "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/crates/cmux-app-host/generated/cmux-app.d.ts b/cmux-tui/crates/cmux-app-host/generated/cmux-app.d.ts index 6b458abbf532..41b58b35788e 100644 --- a/cmux-tui/crates/cmux-app-host/generated/cmux-app.d.ts +++ b/cmux-tui/crates/cmux-app-host/generated/cmux-app.d.ts @@ -1494,7 +1494,7 @@ interface CmuxGlobal { /** `tab.unpin` (mutation, scope `workspace:write`) */ unpin: CmuxOp<{ machine?: string; session?: string; workspace?: string; screen?: string; pane?: string; tab: string; expected_revision?: string }, Cmux.MutationResult> /** `tab.update` (mutation, scope `workspace:write`) */ - update: CmuxOp<{ machine?: string; session?: string; workspace?: string; screen?: string; pane?: string; tab: string; zoom?: number | null; back?: Array; forward?: Array; owner?: string; icon?: string | null; marked_unread?: boolean; expected_revision?: string }, Cmux.MutationResult> + update: CmuxOp<{ machine?: string; session?: string; workspace?: string; screen?: string; pane?: string; tab: string; zoom?: number | null; back?: Array; forward?: Array; owner?: string; icon?: string | null; marked_unread?: boolean; attention?: string | null; expected_revision?: string }, Cmux.MutationResult> } tab_group: { /** `tab_group.add_tabs` (mutation, scope `tab_group:write`) */ diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs index 9258c2fcc09e..a91cceb05b15 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs @@ -95,6 +95,10 @@ pub(super) fn start_surface_thread( surface: id, title: browser.title().into(), }); + // tab-attention-v1: the marked page is gone. + if url_changed { + mux.clear_attention_after_navigation(&crate::Actor::Daemon, id); + } } } CdpEvent::FrameNavigated { params, frame_epoch, .. } => { diff --git a/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs b/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs index 2fe8f17e340e..3b994a99c858 100644 --- a/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs +++ b/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs @@ -72,6 +72,7 @@ impl Mux { if let Some(owner) = &owner { crate::state::window_record_store::validate_key("owner", owner)?; } + let previous_url = runtime.browser_url(); let (mut record, mut changed) = { let mut registry = self.workspace_registry.lock().unwrap(); let result = registry.update_frontend_browser( @@ -97,6 +98,8 @@ impl Mux { record.owner = Some(owner); changed = true; } + // tab-attention-v1: the page the agent asked the person to act on is gone. + let navigated = url.as_ref().is_some_and(|url| previous_url.as_ref() != Some(url)); if changed { if let Some(browser) = runtime.as_browser() { browser.set_frontend_location(url, title.clone()); @@ -107,6 +110,9 @@ impl Mux { } self.emit_tab_changed(surface); } + if navigated { + self.clear_attention_after_navigation(actor, surface); + } Ok((record, changed)) } diff --git a/cmux-tui/crates/cmux-tui-core/src/server.rs b/cmux-tui/crates/cmux-tui-core/src/server.rs index 8d9a6a779c47..3736e65ea001 100644 --- a/cmux-tui/crates/cmux-tui-core/src/server.rs +++ b/cmux-tui/crates/cmux-tui-core/src/server.rs @@ -446,6 +446,11 @@ pub const NOTIFICATION_MARK_UNREAD_CAPABILITY: &str = "notification-mark-unread- /// resource's `extra.marked_unread`. It needs no notification; clients clear /// it when the person views the tab. pub const TAB_MARK_UNREAD_CAPABILITY: &str = "tab-mark-unread-v1"; +/// An agent's "needs you here" mark on a tab: `attention` (a reason, or null +/// to clear) on `tab.update` and the tab resource's `extra.attention` +/// (`{reason, at_ms}`). Viewing the tab keeps it; the agent's clear, a browser +/// tab's navigation to another page, or the tab's close clears it. +pub const TAB_ATTENTION_CAPABILITY: &str = "tab-attention-v1"; /// A workspace's description: `description` on `workspace.update` (null /// clears it) and the workspace snapshot's `extra.description` while set. pub const WORKSPACE_DESCRIPTION_CAPABILITY: &str = "workspace-description-v1"; diff --git a/cmux-tui/crates/cmux-tui-core/src/server/capabilities.rs b/cmux-tui/crates/cmux-tui-core/src/server/capabilities.rs index 8522c6988354..d30ca540c995 100644 --- a/cmux-tui/crates/cmux-tui-core/src/server/capabilities.rs +++ b/cmux-tui/crates/cmux-tui-core/src/server/capabilities.rs @@ -249,6 +249,7 @@ pub(super) fn advertised_capabilities( WORKSPACE_PIN_CAPABILITY, NOTIFICATION_MARK_UNREAD_CAPABILITY, TAB_MARK_UNREAD_CAPABILITY, + TAB_ATTENTION_CAPABILITY, WORKSPACE_DESCRIPTION_CAPABILITY, WORKSPACE_STATUS_CAPABILITY, WORKSPACE_CREATE_FOCUS_CAPABILITY, diff --git a/cmux-tui/crates/cmux-tui-core/src/state.rs b/cmux-tui/crates/cmux-tui-core/src/state.rs index 0e2f2d867c3f..f6ab7a587bb7 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state.rs @@ -108,6 +108,8 @@ pub(crate) mod sidebar_doc_order; pub(crate) mod sidebar_doc_store; pub(crate) mod sidebar_doc_sync; #[cfg(test)] +mod tab_attention_tests; +#[cfg(test)] mod tab_changed_pin_tests; pub(crate) use cmux_layout_reducer::sidebar_layout; pub(crate) mod sidebar_layout_ops; diff --git a/cmux-tui/crates/cmux-tui-core/src/state/router.rs b/cmux-tui/crates/cmux-tui-core/src/state/router.rs index f52ccd41481a..433a5dc49699 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/router.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/router.rs @@ -204,6 +204,22 @@ fn optional_bool(fields: &Map, name: &str) -> Result } } +/// `None` when absent, `Some(None)` for null; a present value must be a string. +fn nullable_reason( + fields: &Map, + name: &str, +) -> Result>, ResourceError> { + match fields.get(name) { + None => Ok(None), + Some(Value::Null) => Ok(Some(None)), + Some(Value::String(value)) => Ok(Some(Some(value.clone()))), + Some(_) => Err(validation_error( + &format!("{name} must be a string or null"), + json!({"field": name}), + )), + } +} + fn index(fields: &Map, name: &str) -> Option { fields.get(name).and_then(Value::as_u64).and_then(|value| usize::try_from(value).ok()) } @@ -317,7 +333,10 @@ pub(crate) fn dispatch( patch_result(mux, commit) } Op::TabUpdate => { - require_any(fields, &["zoom", "back", "forward", "owner", "icon", "marked_unread"])?; + require_any( + fields, + &["zoom", "back", "forward", "owner", "icon", "marked_unread", "attention"], + )?; let update = TabStateUpdate { zoom: fields.get("zoom").map(Value::as_f64), back: fields.contains_key("back").then(|| strings(fields, "back")), @@ -326,6 +345,7 @@ pub(crate) fn dispatch( icon: nullable_string(fields, "icon"), emulation: None, marked_unread: optional_bool(fields, "marked_unread")?, + attention: nullable_reason(fields, "attention")?, }; let commit = mux .state_update_tab(strip_request(&request)?, selectors.clone(), update) diff --git a/cmux-tui/crates/cmux-tui-core/src/state/store.rs b/cmux-tui/crates/cmux-tui-core/src/state/store.rs index d73e3c715248..1b0aa41b6e65 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/store.rs @@ -82,6 +82,7 @@ pub(crate) fn create_state_schema(transaction: &Transaction<'_>) -> anyhow::Resu )?; super::tab_state_store::add_tab_icon_column(transaction)?; super::tab_state_store::add_tab_marked_unread_column(transaction)?; + super::tab_state_store::add_tab_attention_column(transaction)?; super::browser_devices::add_tab_emulation_column(transaction)?; super::browser_devices::create_schema(transaction)?; crate::workspace_registry::presentation_store::add_group_icon_column( diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs new file mode 100644 index 000000000000..96febbe61123 --- /dev/null +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs @@ -0,0 +1,112 @@ +//! tab-attention-v1: an agent marks the tab where the person must act. The +//! mark rides on the tab record (`extra.attention`), never moves focus, and +//! clears on the agent's clear, a browser tab's navigation, or the tab's +//! close; a reopened tab does not bring it back. + +use serde_json::json; + +use super::tests::{error_code, mutate, read, send, snapshot, tab_id, terminal_tabs}; +use crate::mux::*; +use crate::state::prelude::*; +use crate::surface::SurfaceOptions; +use crate::workspace_registry::FrontendBrowserRecord; + +const REASON: &str = "Confirm GitHub access"; + +fn browser_tab(mux: &Arc) -> SurfaceId { + let terminal = terminal_tabs(mux, 1)[0]; + let pane = mux.with_state(|state| state.pane_of(terminal)).unwrap(); + let record = FrontendBrowserRecord { + engine: "webkit".into(), + url: "https://github.com/settings/apps".into(), + title: None, + favicon_url: None, + profile_id: None, + owner: None, + }; + mux.new_frontend_browser_tab(Some(pane), record, None).unwrap().id +} + +fn listed_attention(mux: &Mux, tab: &str) -> Value { + snapshot(mux)["tabs"] + .as_array() + .unwrap() + .iter() + .find(|listed| listed["id"] == tab) + .map(|listed| listed["extra"]["attention"].clone()) + .unwrap_or(Value::Null) +} + +fn focused(mux: &Mux) -> Option { + mux.active_surface() +} + +#[test] +fn an_agent_marks_a_tab_without_moving_focus_and_clears_it() { + let mux = Mux::new_for_test("state-tab-attention", SurfaceOptions::default()); + let tabs = terminal_tabs(&mux, 2); + let tab = tab_id(&mux, tabs[0]); + let focus_before = focused(&mux); + let marked = + mutate(&mux, "tab.update", json!({"tab": tab, "attention": REASON}), "attention-1"); + assert_eq!(marked["extra"]["attention"]["reason"], REASON); + assert!(marked["extra"]["attention"]["at_ms"].as_u64().is_some()); + assert_eq!(listed_attention(&mux, &tab)["reason"], REASON); + assert_eq!(focused(&mux), focus_before, "a request never moves focus"); + let zoomed = mutate(&mux, "tab.update", json!({"tab": tab, "zoom": 1.5}), "attention-2"); + assert_eq!(zoomed["extra"]["attention"]["reason"], REASON, "another field keeps the mark"); + let cleared = mutate(&mux, "tab.update", json!({"tab": tab, "attention": null}), "attention-3"); + assert!(cleared["extra"].get("attention").is_none()); + assert_eq!(cleared["extra"]["zoom"], 1.5); + for bad in [json!(""), json!("two\nlines"), json!(true), json!("x".repeat(201))] { + assert_eq!( + error_code(send( + &mux, + "tab.update", + json!({"tab": tab, "attention": bad}), + Some(&format!("attention-bad-{bad}")) + )), + "validation.invalid", + "{bad}" + ); + } +} + +#[test] +fn a_browser_tab_drops_its_mark_when_its_page_changes_but_not_on_a_title_change() { + let mux = Mux::new_for_test("state-tab-attention-nav", SurfaceOptions::default()); + let browser = browser_tab(&mux); + let tab = tab_id(&mux, browser); + mutate(&mux, "tab.update", json!({"tab": tab, "attention": REASON}), "attention-nav-1"); + mux.update_frontend_browser_tab(browser, None, Some("GitHub".into()), None).unwrap(); + assert_eq!(listed_attention(&mux, &tab)["reason"], REASON, "a title alone keeps the mark"); + mux.update_frontend_browser_tab( + browser, + Some("https://github.com/settings/apps".into()), + None, + None, + ) + .unwrap(); + assert_eq!(listed_attention(&mux, &tab)["reason"], REASON, "the same page keeps the mark"); + mux.update_frontend_browser_tab(browser, Some("https://github.com/".into()), None, None) + .unwrap(); + assert!(listed_attention(&mux, &tab).is_null(), "another page clears the mark"); +} + +#[test] +fn a_closed_tab_drops_its_mark_and_a_reopen_does_not_bring_it_back() { + let mux = Mux::new_for_test("state-tab-attention-close", SurfaceOptions::default()); + let tabs = terminal_tabs(&mux, 2); + let tab = tab_id(&mux, tabs[1]); + mutate(&mux, "tab.update", json!({"tab": tab, "attention": REASON}), "attention-close-1"); + mutate(&mux, "tab.close", json!({"tab": tab}), "attention-close-2"); + let closed = read(&mux, "closed.list", json!({})); + let reopened = mutate( + &mux, + "closed.reopen", + json!({"closed": closed[0]["id"].clone()}), + "attention-close-3", + ); + let reopened = reopened["tab_ids"][0].as_str().unwrap(); + assert!(listed_attention(&mux, reopened).is_null()); +} diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs index 8971d3a39553..642cbcf02146 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs @@ -91,6 +91,32 @@ pub(crate) fn add_tab_marked_unread_column(transaction: &Transaction<'_>) -> any Ok(()) } +/// Additive: an agent's "needs you here" mark on a tab (`tab-attention-v1`). +/// Older builds name their columns, so they keep reading and writing the +/// table; an older build that empties a row of every field it knows drops +/// the mark with it. +pub(crate) fn add_tab_attention_column(transaction: &Transaction<'_>) -> anyhow::Result<()> { + let has_column = transaction + .prepare("PRAGMA table_info(tab_state)")? + .query_map([], |row| row.get::<_, String>(1))? + .collect::, _>>()? + .iter() + .any(|column| column == "attention_json"); + if !has_column { + transaction.execute_batch("ALTER TABLE tab_state ADD COLUMN attention_json TEXT;")?; + } + Ok(()) +} + +/// Characters an attention reason keeps at most. +pub(crate) const MAX_ATTENTION_REASON_CHARS: usize = 200; + +/// The stored attention mark: why the agent needs the person in this tab, +/// and when it asked. Shown as the tab's `extra.attention`. +pub(crate) fn attention_value(reason: &str, at_ms: u64) -> Value { + json!({"reason": reason, "at_ms": at_ms}) +} + /// A partial tab state update: `None` keeps a field, `Some(None)` clears /// the zoom or the icon. #[derive(Debug, Clone, Default, PartialEq)] @@ -112,6 +138,11 @@ pub(crate) struct TabStateUpdate { /// apart from notification markers: `Some(true)` sets the mark, /// `Some(false)` clears it. Clients clear it when the tab is viewed. pub(crate) marked_unread: Option, + /// An agent needs the person to act in this tab (`tab-attention-v1`): + /// `Some(Some(reason))` sets the mark, `Some(None)` clears it. Viewing + /// the tab keeps it; the agent's clear, a browser tab's navigation to + /// another page, or the tab's close clears it. + pub(crate) attention: Option>, } impl TabStateUpdate { @@ -128,6 +159,20 @@ impl TabStateUpdate { if let Some(Some(icon)) = &self.icon { crate::workspace_registry::presentation_store::validate_icon_value(icon)?; } + if let Some(Some(reason)) = &self.attention { + anyhow::ensure!( + !reason.trim().is_empty(), + "bad request: attention needs a reason the person can read" + ); + anyhow::ensure!( + reason.chars().count() <= MAX_ATTENTION_REASON_CHARS, + "bad request: an attention reason holds at most {MAX_ATTENTION_REASON_CHARS} characters" + ); + anyhow::ensure!( + !reason.chars().any(char::is_control), + "bad request: an attention reason is one line of text" + ); + } for list in [&self.back, &self.forward].into_iter().flatten() { anyhow::ensure!( list.len() <= MAX_HISTORY_URLS, @@ -177,6 +222,15 @@ pub(crate) fn update_tab_state( params![tab_id, stored], )?; } + if let Some(attention) = &update.attention { + let stored = attention + .as_deref() + .map(|reason| attention_value(reason.trim(), crate::mux::now_ms()).to_string()); + transaction.execute( + "UPDATE tab_state SET attention_json = ?2 WHERE tab_id = ?1", + params![tab_id, stored], + )?; + } for (column, list) in [("back_json", &update.back), ("forward_json", &update.forward)] { if let Some(list) = list { let stored = (!list.is_empty()).then(|| serde_json::to_string(list)).transpose()?; @@ -186,12 +240,7 @@ pub(crate) fn update_tab_state( )?; } } - transaction.execute( - "DELETE FROM tab_state - WHERE tab_id = ?1 AND zoom IS NULL AND back_json IS NULL AND forward_json IS NULL - AND icon IS NULL AND emulation_json IS NULL AND marked_unread IS NULL", - [tab_id], - )?; + drop_empty_tab_state_row(transaction, tab_id)?; if let Some(owner) = &update.owner { let updated = transaction.execute( "UPDATE frontend_browser_tabs SET owner = ?2 @@ -203,6 +252,47 @@ pub(crate) fn update_tab_state( Ok(()) } +/// Delete a tab's state row once it holds no field. +fn drop_empty_tab_state_row(transaction: &Transaction<'_>, tab_id: &str) -> anyhow::Result<()> { + transaction.execute( + "DELETE FROM tab_state + WHERE tab_id = ?1 AND zoom IS NULL AND back_json IS NULL AND forward_json IS NULL + AND icon IS NULL AND emulation_json IS NULL AND marked_unread IS NULL + AND attention_json IS NULL", + [tab_id], + )?; + Ok(()) +} + +/// Whether a tab holds an attention mark (`tab-attention-v1`). +pub(crate) fn tab_has_attention(connection: &Connection, tab_id: &str) -> anyhow::Result { + Ok(connection + .query_row( + "SELECT attention_json IS NOT NULL FROM tab_state WHERE tab_id = ?1", + [tab_id], + |row| row.get::<_, bool>(0), + ) + .optional()? + .unwrap_or(false)) +} + +/// Drop a closing tab's attention mark, so a reopened tab (which keeps its +/// id) does not ask for the person again. +pub(crate) fn clear_closed_tab_attention( + transaction: &Transaction<'_>, + tab_id: &str, +) -> anyhow::Result<()> { + let cleared = transaction.execute( + "UPDATE tab_state SET attention_json = NULL + WHERE tab_id = ?1 AND attention_json IS NOT NULL", + [tab_id], + )?; + if cleared != 0 { + drop_empty_tab_state_row(transaction, tab_id)?; + } + Ok(()) +} + /// Replace every tab group row in the caller's transaction. pub(crate) fn write_tab_groups( transaction: &Transaction<'_>, @@ -415,3 +505,70 @@ mod marked_unread_tests { assert_eq!(rows, 0); } } + +#[cfg(test)] +mod attention_tests { + use super::*; + + /// A tab_state table without the attention column gains it; a mark alone + /// keeps the row with its reason, clearing it deletes the emptied row, and + /// a closing tab drops its mark. + #[test] + fn tab_attention_column_is_added_and_cleared_by_clear_or_close() { + let mut connection = Connection::open_in_memory().unwrap(); + connection + .execute_batch( + "CREATE TABLE tab_state (tab_id TEXT PRIMARY KEY NOT NULL, zoom REAL, + back_json TEXT, forward_json TEXT, icon TEXT, emulation_json TEXT, + marked_unread INTEGER);", + ) + .unwrap(); + for _ in 0..2 { + let transaction = connection.transaction().unwrap(); + add_tab_attention_column(&transaction).unwrap(); + transaction.commit().unwrap(); + } + let rows = |connection: &Connection| -> i64 { + connection.query_row("SELECT COUNT(*) FROM tab_state", [], |row| row.get(0)).unwrap() + }; + let mark = TabStateUpdate { + attention: Some(Some("Confirm GitHub access".into())), + ..Default::default() + }; + let transaction = connection.transaction().unwrap(); + update_tab_state(&transaction, "tab_a", &mark).unwrap(); + update_tab_state(&transaction, "tab_b", &mark).unwrap(); + transaction.commit().unwrap(); + assert!(tab_has_attention(&connection, "tab_a").unwrap()); + let stored: String = connection + .query_row("SELECT attention_json FROM tab_state WHERE tab_id = 'tab_a'", [], |row| { + row.get(0) + }) + .unwrap(); + let stored: Value = serde_json::from_str(&stored).unwrap(); + assert_eq!(stored["reason"], "Confirm GitHub access"); + assert!(stored["at_ms"].as_u64().is_some()); + let transaction = connection.transaction().unwrap(); + let clear = TabStateUpdate { attention: Some(None), ..Default::default() }; + update_tab_state(&transaction, "tab_a", &clear).unwrap(); + clear_closed_tab_attention(&transaction, "tab_b").unwrap(); + clear_closed_tab_attention(&transaction, "tab_never_marked").unwrap(); + transaction.commit().unwrap(); + assert!(!tab_has_attention(&connection, "tab_a").unwrap()); + assert!(!tab_has_attention(&connection, "tab_b").unwrap()); + assert_eq!(rows(&connection), 0); + } + + #[test] + fn an_attention_reason_is_one_short_readable_line() { + let with = |reason: &str| TabStateUpdate { + attention: Some(Some(reason.to_string())), + ..Default::default() + }; + assert!(with("Confirm GitHub access").validate().is_ok()); + assert!(with(" ").validate().is_err()); + assert!(with("two\nlines").validate().is_err()); + assert!(with(&"x".repeat(MAX_ATTENTION_REASON_CHARS + 1)).validate().is_err()); + assert!(TabStateUpdate { attention: Some(None), ..Default::default() }.validate().is_ok()); + } +} diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs index 5cde876ef0aa..18cf2cee354e 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs @@ -132,6 +132,46 @@ impl Mux { Ok(commit) } + /// `tab-attention-v1`: a browser tab whose page went to another URL + /// drops its attention mark, since the page the agent asked the person to + /// act on is gone (the person confirmed, logged in, or left). One read + /// when the tab holds no mark; a commit restating the tab when it does. + pub(crate) fn clear_attention_after_navigation(&self, actor: &Actor, surface: SurfaceId) { + let Some(tab) = self.with_state(|state| { + state.resource_indexes.tab_ids.get(&surface).map(|id| id.as_str().to_string()) + }) else { + return; + }; + let marked = self.personal_state_read(|connection| { + crate::state::tab_state_store::tab_has_attention(connection, &tab) + }); + if !matches!(marked, Ok(true)) { + return; + } + let fingerprint = serde_json::json!({ + "operation": "tab.attention.navigated", + "tab": tab, + "nonce": crate::workspace_registry::new_uuid_v4(), + }); + let result = self.commit_state( + &WorkspaceMutation::local("cmux-tui-tab-attention", actor.clone()), + "tab.attention.navigated", + &fingerprint, + None, + StateEffects::PRESENTATION, + |transaction, _| { + let update = TabStateUpdate { attention: Some(None), ..Default::default() }; + crate::state::tab_state_store::update_tab_state(transaction, &tab, &update)?; + let changes = + crate::state::values::fresh_upserts(transaction, &[], &[], &[tab.clone()])?; + Ok(StateChanges::new(serde_json::json!({"tab": tab}), changes)) + }, + ); + if let Err(error) = result { + eprintln!("cmux-tui: clearing the attention mark of {tab} after navigation failed: {error:#}"); + } + } + /// Store the owner (hosting app's install id) of a frontend-rendered /// browser tab on its record, restating the tab on `session.events`. /// The raw `update-frontend-browser-tab {owner}` path; `tab.update` diff --git a/cmux-tui/crates/cmux-tui-core/src/state/values.rs b/cmux-tui/crates/cmux-tui-core/src/state/values.rs index 031d0d0a6bac..54e0384fc091 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/values.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/values.rs @@ -163,7 +163,8 @@ fn tab_extra(connection: &Connection, tab_id: &str) -> anyhow::Result anyhow::Result>(3)?, row.get::<_, Option>(4)?, row.get::<_, Option>(5)?, + row.get::<_, Option>(6)?, )) }, ) .optional()?; - if let Some((zoom, back, forward, icon, emulation, marked_unread)) = state { + if let Some((zoom, back, forward, icon, emulation, marked_unread, attention)) = state { if let Some(emulation) = emulation.as_deref().and_then(crate::state::browser_devices::resolve) { @@ -194,6 +196,11 @@ fn tab_extra(connection: &Connection, tab_id: &str) -> anyhow::Result Cow::Borrowed(scope_help::BROWSER_HELP), "notification" => Cow::Borrowed(scope_help::NOTIFICATION_HELP), + "attention" => Cow::Borrowed(scope_help::ATTENTION_HELP), "agent" => Cow::Borrowed(AGENT_HELP), "room" => Cow::Borrowed(scope_help::ROOM_HELP), "closed" => Cow::Borrowed(scope_help::CLOSED_HELP), @@ -908,6 +910,7 @@ const ROOT_HELP_SCOPES_SUFFIX: &str = "\ terminal Read, write, and attach to terminals browser Navigate and attach to browsers notification List and create notifications + attention Mark the tab where the person must act (no focus change) agent List and report agent state room Organize workspaces into rooms closed List and reopen closed tabs, screens, workspaces diff --git a/cmux-tui/crates/cmux-tui/src/cli/command.rs b/cmux-tui/crates/cmux-tui/src/cli/command.rs index 246c37d141e7..c573a63cac2a 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/command.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/command.rs @@ -181,6 +181,9 @@ pub(super) fn parse(args: &[String], surface: super::Surface) -> Result parse_terminal(&tokens.words[1..], &mut selectors, &mut tokens.flags)?, "browser" => parse_browser(&tokens.words[1..], &mut selectors, &mut tokens.flags)?, "notification" => parse_notification(&tokens.words[1..], &mut tokens.flags)?, + "attention" => { + state::parse_attention(&strs(&tokens.words[1..]), &mut selectors, &mut tokens.flags)? + } "room" => state::parse_room(&strs(&tokens.words[1..]), &mut tokens.flags)?, "closed" => state::parse_closed(&strs(&tokens.words[1..]), &mut tokens.flags)?, "git" => git::parse_git(&strs(&tokens.words[1..]), &mut tokens.flags)?, diff --git a/cmux-tui/crates/cmux-tui/src/cli/command/state.rs b/cmux-tui/crates/cmux-tui/src/cli/command/state.rs index 857534f243b5..134431c5abac 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/command/state.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/command/state.rs @@ -546,6 +546,34 @@ pub(super) fn tab_change( params.send(operation, selectors, flags) } +/// `attention request --tab --reason ` and `attention clear +/// --tab ` (`tab-attention-v1`): an agent marks the tab where the person +/// must act (confirm, log in, approve, pick), or drops that mark. Both are +/// `tab.update {attention}`, the one mutation path every surface uses; neither +/// moves focus. +pub(super) fn parse_attention( + words: &[&str], + selectors: &mut Selectors, + flags: &mut Flags, +) -> Result { + let mut params = Params::default(); + match words { + ["request"] => { + let reason = flags.required("reason")?; + let reason = reason.trim(); + if reason.is_empty() { + return Err(UsageError::new("--reason cannot be empty")); + } + params.insert("attention", Value::String(reason.to_string())); + } + ["clear"] => params.insert("attention", Value::Null), + _ => return usage("attention"), + } + let tab = flags.required("tab")?; + selectors.insert("tab", "tab", &tab)?; + params.send(Op::TabUpdate, selectors, flags) +} + /// `tab group …`. Group operations take no topology selectors: a group is /// addressed by its id or name, its tabs and panes by public id. pub(super) fn parse_tab_group( diff --git a/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs b/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs index df567479f793..5c05d373cc33 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs @@ -623,3 +623,21 @@ fn workspace_notes_task_lane_unread_mark_and_closed_ids() { ("workspace.close".into(), json!({"workspace": WS, "report_closed_ids": true})) ); } + +/// tab-attention-v1: `cmux attention request|clear --tab` is `tab.update +/// {attention}`, the one path the app and MCP use too. +#[test] +fn attention_request_and_clear_send_tab_update() { + assert_eq!( + sent(&["attention", "request", "--tab", TAB, "--reason", " Confirm GitHub access "]), + ("tab.update".into(), json!({"tab": TAB, "attention": "Confirm GitHub access"})) + ); + assert_eq!( + sent(&["attention", "clear", "--tab", TAB]), + ("tab.update".into(), json!({"tab": TAB, "attention": null})) + ); + assert!(rejects(&["attention", "request", "--tab", TAB]).contains("--reason")); + assert!(rejects(&["attention", "request", "--tab", TAB, "--reason", " "]).contains("empty")); + assert!(rejects(&["attention", "clear"]).contains("--tab")); + assert!(rejects(&["attention", "clear", "--tab", "ws_1"]).contains("tab")); +} diff --git a/cmux-tui/crates/cmux-tui/src/cli/command/tests/transport_path_coverage_tests.rs b/cmux-tui/crates/cmux-tui/src/cli/command/tests/transport_path_coverage_tests.rs index d390046f828e..84b7b92b1cc3 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/command/tests/transport_path_coverage_tests.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/command/tests/transport_path_coverage_tests.rs @@ -67,6 +67,8 @@ fn every_safe_transport_operation_has_a_noun_first_path() { "tab.create_browser", ), (vec!["tab", TAB, "update", "--marked-unread", "true"], "tab.update"), + (vec!["attention", "request", "--tab", TAB, "--reason", "Confirm"], "tab.update"), + (vec!["attention", "clear", "--tab", TAB], "tab.update"), (vec!["workspace", WORKSPACE, "close", "--report-closed-ids"], "workspace.close"), ( vec![ diff --git a/cmux-tui/crates/cmux-tui/src/cli/docs.rs b/cmux-tui/crates/cmux-tui/src/cli/docs.rs index b687bcbc5cb8..e416ae89de1b 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/docs.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/docs.rs @@ -86,7 +86,12 @@ pub(super) const GUIDES: &[Guide] = &[ status on a workspace's sidebar row: workspace_status_set {workspace, key, text, state, \ progress, ttl_ms} (CLI `cmux workspace WS status set KEY TEXT`), workspace_progress_set \ and workspace_log_append; clear them when the work is done. Ask for attention: \ - notification_create {title, body} (desktop: false shows no banner). The focus tools \ + notification_create {title, body} (desktop: false shows no banner). When the user must \ + act in a tab (confirm, log in, approve, pick), mark it: tab_update {tab, attention: \ + \"one-line reason\"} (CLI `cmux attention request --tab TAB --reason TEXT`); it never \ + moves focus, and tab_update {tab, attention: null} (`cmux attention clear --tab TAB`) \ + drops it. Tell the user the workspace and tab by their titles, never a ws_ or tab_ id. \ + The focus tools \ (tab_focus, pane_focus, screen_focus, workspace_focus, agents_tab_select) and focus: true \ move the user's focus: use them only when the user asked. The selector `current` is what \ the user looks at, not your own workspace.", diff --git a/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs b/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs index d3ed67355f09..5cf038545eab 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs @@ -47,6 +47,21 @@ session; --workspace clears that workspace's rows and those of its terminals. `c notify` takes the flags of the macOS `cmux notify` (`cmux notify --help`). "; +pub(super) const ATTENTION_HELP: &str = "\ +USAGE + cmux attention request --tab --reason + cmux attention clear --tab + +request marks the tab where the person must act (confirm, log in, approve, +pick): the tab and its workspace show as needing them, and the app lists it +under Go to What Needs Me. The reason is one line they read (at most 200 +characters). It never moves focus; to take the person there when they ask, run +`cmux tab focus`. The mark stays while they look at the tab and clears +when you clear it, when a browser tab goes to another page, or when the tab +closes. Name the workspace and tab by their titles when you talk to the person, +never by a ws_ or tab_ id. +"; + pub(super) const NOTIFY_HELP: &str = "\ USAGE cmux notify [--title ] [--subtitle ] [--body ] diff --git a/cmux-tui/crates/cmux-tui/src/cli/surface.rs b/cmux-tui/crates/cmux-tui/src/cli/surface.rs index 0f9db5f46303..426cc3561c6e 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/surface.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/surface.rs @@ -18,6 +18,7 @@ pub(super) const CMUX_SCOPES: &[&str] = &[ "terminal", "browser", "notification", + "attention", "agent", "room", "closed", diff --git a/cmux-tui/docs/protocol.md b/cmux-tui/docs/protocol.md index ea5f5e343193..3709f3f56ea3 100644 --- a/cmux-tui/docs/protocol.md +++ b/cmux-tui/docs/protocol.md @@ -130,6 +130,14 @@ Unread) on any tab, separate from notification markers. The daemon keeps it on the tab record until a frontend clears it, which the Mac app does when the tab comes into view. +`tab-attention-v1` adds `attention` (a one-line reason, or null to clear) to +`tab.update` and, while set, `attention: {reason, at_ms}` to the tab's `extra`. +An agent sets it when the person must act in that tab (confirm, log in, +approve, pick); `cmux attention request --tab --reason ` and +`cmux attention clear --tab ` send it. It never moves focus. Viewing +the tab keeps it; the agent's clear, a browser tab's navigation to another +URL, or the tab's close clears it. + `workspace-description-v1` adds `description` (string, 1-4096 characters, line breaks allowed; null clears it) to `workspace.update` and, while set, `description` to the workspace's `extra`. `workspace-status-v1` adds diff --git a/cmux-tui/spec/cli.md b/cmux-tui/spec/cli.md index b37ba4ad9247..70e8f8a94420 100644 --- a/cmux-tui/spec/cli.md +++ b/cmux-tui/spec/cli.md @@ -567,6 +567,8 @@ notification list notification create --title --body [--subtitle ] [--level ] [--terminal ] notification clear [--terminal ] notification ack --client ... +attention request --tab --reason +attention clear --tab git status [--path |--workspace|--screen|--pane|--tab|--terminal ] git diff [TARGET] [--scope uncommitted|unstaged|staged|committed|branch] [--patch] [--max-patch-bytes ] [--max-files ] [--context-lines ] [...] diff --git a/cmux-tui/spec/resource-api-v2.md b/cmux-tui/spec/resource-api-v2.md index cfe5a4192b3d..29bcd0a4f085 100644 --- a/cmux-tui/spec/resource-api-v2.md +++ b/cmux-tui/spec/resource-api-v2.md @@ -87,7 +87,7 @@ lowercase hexadecimal digits. Older records keep the IDs they already have | App workspaces (`app-screens-v1`, one per installed app), app tabs | shared | `workspace.ensure_app` | | Workspace agent folder (where new agent chats start; set only by the user) | shared | `workspace.agent_folder.set` | | Web links of a workspace (open on its right side, reuse its browser tab) | shared | `workspace.open_url` | -| Tab pin, zoom, user icon, manual unread mark, browser back/forward, browser owner | shared | `tab.pin`, `tab.unpin`, `tab.update` | +| Tab pin, zoom, user icon, manual unread mark, attention mark, browser back/forward, browser owner | shared | `tab.pin`, `tab.unpin`, `tab.update` | | Tab groups | shared | `tab_group.*` | | Screen pin, color, icon, order; screen groups | shared | `screen.update`, `screen.move`, `screen_group.*` | | Closed history (one group per close gesture, kept until deleted) | shared | `closed.list`, `closed.reopen`, `closed.delete` | @@ -122,7 +122,7 @@ workspace, absent for a normal one), `agent_folder` (absent until set), `descrip (`workspace-description-v1`, absent until set), `task_status` (`todo`, `working`, `needs-attention`, `review` or `done`, `workspace-status-v1`; absent while automatic); tab `pinned`, `tab_group_id`, `zoom`, `icon` (the user icon, absent until set), `marked_unread` -(`true` while the person marked the tab unread by hand, `tab-mark-unread-v1`; absent otherwise), `back`, `forward`, `owner` (the install id of the app +(`true` while the person marked the tab unread by hand, `tab-mark-unread-v1`; absent otherwise), `attention` (`{reason, at_ms}` while an agent needs the person in the tab, `tab-attention-v1`; absent otherwise), `back`, `forward`, `owner` (the install id of the app that hosts a frontend-rendered browser, its record's only writer), `relaunch` (`{cwd}` for a tab kept by `shutdown-daemon {end_terminals, keep_layout}`, absent otherwise); screen `pinned`, `color`, `icon`, diff --git a/cmux-tui/spec/resource-operations-v2.json b/cmux-tui/spec/resource-operations-v2.json index 48b8df3a7a6b..728aae4e768c 100644 --- a/cmux-tui/spec/resource-operations-v2.json +++ b/cmux-tui/spec/resource-operations-v2.json @@ -29731,6 +29731,19 @@ }, "description": "tab-mark-unread-v1: true marks the tab unread by hand (no notification needed), false clears the mark. Shown as the tab's extra.marked_unread (present only while marked) on every client; a client clears it when the person views the tab." }, + "attention": { + "required": false, + "type": { + "kind": "nullable", + "value": { + "kind": "primitive", + "name": "string", + "min_length": 1, + "max_length": 200 + } + }, + "description": "tab-attention-v1: an agent needs the person to act in this tab (confirm, log in, approve, pick). The reason is one line the person reads, at most 200 characters; null clears the mark. Shown as the tab's extra.attention {reason, at_ms} (present only while marked) on every client, which draw it as needs-you. It never moves focus. Viewing the tab keeps it; the agent's clear, a browser tab's navigation to another page, or the tab's close clears it." + }, "expected_revision": { "required": false, "type": { @@ -29742,7 +29755,7 @@ }, "extra": false, "constraints": [ - "At least one field is present. back and forward apply only to browser tabs; owner only to frontend-rendered browser tabs; icon applies to every tab kind." + "At least one field is present. back and forward apply only to browser tabs; owner only to frontend-rendered browser tabs; icon and attention apply to every tab kind." ] }, "result": { From a6e4474cec023b27318fae5789bc0f5c9a8aff4b Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 01:07:38 -0700 Subject: [PATCH 2/5] tab-attention: clear the mark on every CDP URL change; review fixes - The CDP event thread now compares the page URL against the one it last saw after target-info, frame and same-document navigation, so a mark clears even when another setter moved the URL first. Test added. - clippy: slice::from_ref; rustfmt the eprintln. - CLI test asserts the real 'not an id' message. - Bad-value test uses short idempotency keys. - The reason limit is 200 UTF-8 bytes, matching the catalog's max_length. - Help no longer names palette UI this PR does not ship. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16037-16c2-7a73-b063-2e37ed40b9c4&host=cosmic-gray --- cmux-tui/bindings/cpp/.cmux-resource-api.json | 2 +- cmux-tui/bindings/go/.cmux-resource-api.json | 2 +- .../bindings/java/.cmux-resource-api.json | 2 +- .../bindings/python/.cmux-resource-api.json | 2 +- .../bindings/rust/.cmux-resource-api.json | 2 +- .../typescript/.cmux-resource-api.json | 2 +- cmux-tui/bindings/zig/.cmux-resource-api.json | 2 +- cmux-tui/crates/cmux-tui-core/src/browser.rs | 2 +- .../cmux-tui-core/src/browser/tests/mod.rs | 5 +-- .../src/browser/tests/navigation_barriers.rs | 30 ++++++++++++++++++ .../cmux-tui-core/src/browser/worker.rs | 31 ++++++++++++++++--- .../src/state/tab_attention_tests.rs | 5 +-- .../src/state/tab_state_store.rs | 10 +++--- .../crates/cmux-tui-core/src/state/tabs.rs | 12 +++++-- .../cmux-tui/src/cli/command/state/tests.rs | 2 +- .../crates/cmux-tui/src/cli/scope_help.rs | 7 ++--- cmux-tui/spec/resource-operations-v2.json | 2 +- 17 files changed, 90 insertions(+), 30 deletions(-) diff --git a/cmux-tui/bindings/cpp/.cmux-resource-api.json b/cmux-tui/bindings/cpp/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/cpp/.cmux-resource-api.json +++ b/cmux-tui/bindings/cpp/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/go/.cmux-resource-api.json b/cmux-tui/bindings/go/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/go/.cmux-resource-api.json +++ b/cmux-tui/bindings/go/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/java/.cmux-resource-api.json b/cmux-tui/bindings/java/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/java/.cmux-resource-api.json +++ b/cmux-tui/bindings/java/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/python/.cmux-resource-api.json b/cmux-tui/bindings/python/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/python/.cmux-resource-api.json +++ b/cmux-tui/bindings/python/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/rust/.cmux-resource-api.json b/cmux-tui/bindings/rust/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/rust/.cmux-resource-api.json +++ b/cmux-tui/bindings/rust/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/typescript/.cmux-resource-api.json b/cmux-tui/bindings/typescript/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/typescript/.cmux-resource-api.json +++ b/cmux-tui/bindings/typescript/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/bindings/zig/.cmux-resource-api.json b/cmux-tui/bindings/zig/.cmux-resource-api.json index 30819d76de4c..e2326d24a74b 100644 --- a/cmux-tui/bindings/zig/.cmux-resource-api.json +++ b/cmux-tui/bindings/zig/.cmux-resource-api.json @@ -1,5 +1,5 @@ { - "catalog_sha256": "fa9828c831006d69dea3ec00804e6a77d19dad6e755e0d31f0c67d02c83e1fe7", + "catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5", "operations": { "agent.list": { "class": "read" diff --git a/cmux-tui/crates/cmux-tui-core/src/browser.rs b/cmux-tui/crates/cmux-tui-core/src/browser.rs index 6c873bbf3a3c..f7dd4df6e395 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser.rs @@ -43,7 +43,7 @@ pub(crate) use runtime::{new_surface, new_surface_with_resource_identity}; use worker::{is_cdp_timeout_error, start_browser_worker, start_surface_thread}; #[cfg(test)] use worker::{ - next_pointer_lifecycle_deadline, record_browser_worker_result, + clear_attention_if_page_changed, next_pointer_lifecycle_deadline, record_browser_worker_result, release_abandoned_pointer_presses, take_latest_worker_commands, }; diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/tests/mod.rs b/cmux-tui/crates/cmux-tui-core/src/browser/tests/mod.rs index 79fe9bf94f0c..576d435fda41 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/tests/mod.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/tests/mod.rs @@ -5,8 +5,9 @@ use super::{ AUTHORITY_CAPTURE_ATTEMPTS, BROWSER_COMMAND_QUEUE_CAPACITY, BrowserCaptureOptions, BrowserCommand, BrowserFrame, BrowserSession, BrowserSource, BrowserStatus, MAX_RECONFIGURE_WAITERS_PER_RESERVATION, SequencedBrowserCommand, capture_scale_for, - handle_frame_navigated, handle_same_document_navigated, new_surface, normalize_url, - runtime_endpoint, scaled_pixels, start_surface_thread, take_latest_worker_commands, + clear_attention_if_page_changed, handle_frame_navigated, handle_same_document_navigated, + new_surface, normalize_url, runtime_endpoint, scaled_pixels, start_surface_thread, + take_latest_worker_commands, }; use crate::{Mux, MuxEvent, Surface, SurfaceOptions}; use serde_json::{Value, json}; diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs index 8a666f0eaa0b..08c6e597d312 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs @@ -1036,3 +1036,33 @@ fn response_level_navigation_failure_clears_frame_epoch_reservation() { runtime.shutdown(); server.join().unwrap(); } + +/// tab-attention-v1: the frame and same-document setters move the URL +/// before any target-info event, so the mark check compares against the +/// URL the event thread last saw, not one setter's return value. +#[test] +fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { + let surface = test_surface(); + let browser = surface.as_browser().expect("browser surface"); + let mut seen = browser.url(); + + clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + assert_eq!(seen, "https://example.test", "no navigation, no change"); + + let epoch = browser.frame_epoch.advance_navigation(); + handle_frame_navigated(browser, json!({"frame": {"url": "https://login.test"}}), epoch); + clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + assert_eq!(seen, "https://login.test", "a frame navigation is a page change"); + + let _ = handle_same_document_navigated( + browser, + &json!({"frameId": "main-frame", "url": "https://login.test/#done"}), + browser.frame_epoch.advance_same_document(), + ); + clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + assert_eq!(seen, "https://login.test/#done", "a same-document navigation is a page change"); + + let _ = browser.set_title("Signed in".to_string()); + clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + assert_eq!(seen, "https://login.test/#done", "a title change is not a page change"); +} diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs index a91cceb05b15..d34798219da6 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs @@ -12,8 +12,14 @@ pub(super) fn start_surface_thread( ) -> anyhow::Result<()> { let id = surface.id; std::thread::Builder::new().name(format!("browser-surface-{id}-events")).spawn(move || { + // tab-attention-v1: the page URL this thread last saw. Any setter + // (target info, frame navigation, same-document navigation, the + // navigate hold) may move the URL first, so compare against what + // this thread saw rather than trust one setter's return value. + let mut seen_url: Option = None; while let Some(event) = events.recv() { let Surface::Browser(browser) = surface.as_ref() else { break }; + let seen_url = seen_url.get_or_insert_with(|| browser.url()); match event { CdpEvent::ScreencastFrame(frame) => { let frame_epoch = frame.frame_epoch; @@ -88,6 +94,7 @@ pub(super) fn start_surface_thread( let url_changed = if info.url.is_empty() { false } else { browser.set_url(info.url) }; let title_changed = browser.set_title(title); + clear_attention_if_page_changed(seen_url, browser, &mux, id); if (url_changed || title_changed) && let Some(mux) = mux.upgrade() { @@ -95,14 +102,11 @@ pub(super) fn start_surface_thread( surface: id, title: browser.title().into(), }); - // tab-attention-v1: the marked page is gone. - if url_changed { - mux.clear_attention_after_navigation(&crate::Actor::Daemon, id); - } } } CdpEvent::FrameNavigated { params, frame_epoch, .. } => { handle_frame_navigated(browser, params, frame_epoch); + clear_attention_if_page_changed(seen_url, browser, &mux, id); if let Some(mux) = mux.upgrade() { mux.emit(MuxEvent::TitleChanged { surface: id, @@ -130,6 +134,7 @@ pub(super) fn start_surface_thread( frame_epoch, } => { let _ = handle_same_document_navigated(browser, ¶ms, frame_epoch); + clear_attention_if_page_changed(seen_url, browser, &mux, id); if browser.needs_same_document_paint() { let _ = browser.enqueue_latest_authority( BrowserCommand::AuthorizeSameDocumentPaint { @@ -189,6 +194,24 @@ pub(super) fn start_surface_thread( Ok(()) } +/// tab-attention-v1: drop the tab's "needs you" mark once its page URL +/// differs from the one `seen_url` last recorded. The marked page is gone. +pub(super) fn clear_attention_if_page_changed( + seen_url: &mut String, + browser: &BrowserSurface, + mux: &Weak, + id: SurfaceId, +) { + let url = browser.url(); + if url.is_empty() || url == *seen_url { + return; + } + *seen_url = url; + if let Some(mux) = mux.upgrade() { + mux.clear_attention_after_navigation(&crate::Actor::Daemon, id); + } +} + pub(super) fn start_browser_worker( surface: Arc, rx: Receiver, diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs index 96febbe61123..0c21b5996567 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs @@ -58,13 +58,14 @@ fn an_agent_marks_a_tab_without_moving_focus_and_clears_it() { let cleared = mutate(&mux, "tab.update", json!({"tab": tab, "attention": null}), "attention-3"); assert!(cleared["extra"].get("attention").is_none()); assert_eq!(cleared["extra"]["zoom"], 1.5); - for bad in [json!(""), json!("two\nlines"), json!(true), json!("x".repeat(201))] { + let bad_values = [json!(""), json!("two\nlines"), json!(true), json!("x".repeat(201))]; + for (index, bad) in bad_values.into_iter().enumerate() { assert_eq!( error_code(send( &mux, "tab.update", json!({"tab": tab, "attention": bad}), - Some(&format!("attention-bad-{bad}")) + Some(&format!("attention-bad-{index}")) )), "validation.invalid", "{bad}" diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs index 642cbcf02146..7057d8e785b6 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs @@ -108,8 +108,8 @@ pub(crate) fn add_tab_attention_column(transaction: &Transaction<'_>) -> anyhow: Ok(()) } -/// Characters an attention reason keeps at most. -pub(crate) const MAX_ATTENTION_REASON_CHARS: usize = 200; +/// UTF-8 bytes an attention reason holds at most (the catalog's max_length unit). +pub(crate) const MAX_ATTENTION_REASON_BYTES: usize = 200; /// The stored attention mark: why the agent needs the person in this tab, /// and when it asked. Shown as the tab's `extra.attention`. @@ -165,8 +165,8 @@ impl TabStateUpdate { "bad request: attention needs a reason the person can read" ); anyhow::ensure!( - reason.chars().count() <= MAX_ATTENTION_REASON_CHARS, - "bad request: an attention reason holds at most {MAX_ATTENTION_REASON_CHARS} characters" + reason.len() <= MAX_ATTENTION_REASON_BYTES, + "bad request: an attention reason holds at most {MAX_ATTENTION_REASON_BYTES} bytes" ); anyhow::ensure!( !reason.chars().any(char::is_control), @@ -568,7 +568,7 @@ mod attention_tests { assert!(with("Confirm GitHub access").validate().is_ok()); assert!(with(" ").validate().is_err()); assert!(with("two\nlines").validate().is_err()); - assert!(with(&"x".repeat(MAX_ATTENTION_REASON_CHARS + 1)).validate().is_err()); + assert!(with(&"x".repeat(MAX_ATTENTION_REASON_BYTES + 1)).validate().is_err()); assert!(TabStateUpdate { attention: Some(None), ..Default::default() }.validate().is_ok()); } } diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs index 18cf2cee354e..ef92ad110f75 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs @@ -162,13 +162,19 @@ impl Mux { |transaction, _| { let update = TabStateUpdate { attention: Some(None), ..Default::default() }; crate::state::tab_state_store::update_tab_state(transaction, &tab, &update)?; - let changes = - crate::state::values::fresh_upserts(transaction, &[], &[], &[tab.clone()])?; + let changes = crate::state::values::fresh_upserts( + transaction, + &[], + &[], + std::slice::from_ref(&tab), + )?; Ok(StateChanges::new(serde_json::json!({"tab": tab}), changes)) }, ); if let Err(error) = result { - eprintln!("cmux-tui: clearing the attention mark of {tab} after navigation failed: {error:#}"); + eprintln!( + "cmux-tui: clearing the attention mark of {tab} after navigation failed: {error:#}" + ); } } diff --git a/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs b/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs index 5c05d373cc33..8dd4d21c480a 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/command/state/tests.rs @@ -639,5 +639,5 @@ fn attention_request_and_clear_send_tab_update() { assert!(rejects(&["attention", "request", "--tab", TAB]).contains("--reason")); assert!(rejects(&["attention", "request", "--tab", TAB, "--reason", " "]).contains("empty")); assert!(rejects(&["attention", "clear"]).contains("--tab")); - assert!(rejects(&["attention", "clear", "--tab", "ws_1"]).contains("tab")); + assert!(rejects(&["attention", "clear", "--tab", "ws_1"]).contains("not an id")); } diff --git a/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs b/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs index 5cf038545eab..12b4c0385bb5 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/scope_help.rs @@ -53,10 +53,9 @@ USAGE cmux attention clear --tab request marks the tab where the person must act (confirm, log in, approve, -pick): the tab and its workspace show as needing them, and the app lists it -under Go to What Needs Me. The reason is one line they read (at most 200 -characters). It never moves focus; to take the person there when they ask, run -`cmux tab focus`. The mark stays while they look at the tab and clears +pick): the tab and its workspace show as needing them in every cmux client. +The reason is one line they read (at most 200 bytes). It never moves focus; +to take the person there when they ask, run `cmux tab focus`. The mark stays while they look at the tab and clears when you clear it, when a browser tab goes to another page, or when the tab closes. Name the workspace and tab by their titles when you talk to the person, never by a ws_ or tab_ id. diff --git a/cmux-tui/spec/resource-operations-v2.json b/cmux-tui/spec/resource-operations-v2.json index 728aae4e768c..f03618c1e550 100644 --- a/cmux-tui/spec/resource-operations-v2.json +++ b/cmux-tui/spec/resource-operations-v2.json @@ -29742,7 +29742,7 @@ "max_length": 200 } }, - "description": "tab-attention-v1: an agent needs the person to act in this tab (confirm, log in, approve, pick). The reason is one line the person reads, at most 200 characters; null clears the mark. Shown as the tab's extra.attention {reason, at_ms} (present only while marked) on every client, which draw it as needs-you. It never moves focus. Viewing the tab keeps it; the agent's clear, a browser tab's navigation to another page, or the tab's close clears it." + "description": "tab-attention-v1: an agent needs the person to act in this tab (confirm, log in, approve, pick). The reason is one line the person reads, at most 200 bytes of UTF-8; null clears the mark. Shown as the tab's extra.attention {reason, at_ms} (present only while marked) on every client, which draw it as needs-you. It never moves focus. Viewing the tab keeps it; the agent's clear, a browser tab's navigation to another page, or the tab's close clears it." }, "expected_revision": { "required": false, From 5decfe582f464efbb28fd2c71c8d3cf583432c0c Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 22 Oct 2026 21:34:29 -0700 Subject: [PATCH 3/5] tab-attention-v1: a navigation clears only a mark set on another page, in its commit Review (codex xhigh) BLOCK at 53b2f384: - The CDP worker seeded its last-seen URL from the first event, so a mark set before the worker saw the old page survived a navigation whose URL moved first. - The navigation clear read the mark, then cleared it in a later commit, so a mark an agent set on the new page in between was dropped. A browser tab's mark now records the page it was set on (attention_json.page, not shown in extra.attention). The clear runs one conditional UPDATE in its commit: it drops the mark only when the mark's page differs from the page the tab is on, so a late or repeated clear for the new page keeps that page's mark, and the worker's first event always asks. The worker's seen URL now only skips the read for a page it already asked about. Also: the Rust SDK's TabUpdateOptions gains attention (set and clear), and a reason with U+2028 or U+2029 is refused as more than one line. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16037-16c2-7a73-b063-2e37ed40b9c4&host=cosmic-gray --- .../rust/src/resource/handles/tab_update.rs | 35 ++++- .../bindings/rust/tests/typed_state_ops.rs | 14 +- .../src/browser/tests/navigation_barriers.rs | 23 ++- .../cmux-tui-core/src/browser/worker.rs | 23 +-- .../presentation/frontend_browser_history.rs | 6 +- .../src/state/tab_attention_tests.rs | 16 +- .../src/state/tab_state_store.rs | 141 +++++++++++++++++- .../crates/cmux-tui-core/src/state/tabs.rs | 44 ++++-- .../crates/cmux-tui-core/src/state/values.rs | 5 +- 9 files changed, 257 insertions(+), 50 deletions(-) diff --git a/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs b/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs index 5d28c3bef0be..c6a79afd96b9 100644 --- a/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs +++ b/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs @@ -1,13 +1,16 @@ //! `tab.pin`, `tab.unpin`, and `tab.update` on a tab handle. The result's //! `TabSnapshot.extra` carries `pinned`, `zoom`, a browser tab's `back` and -//! `forward` URL lists, a frontend-rendered browser's `owner`, and the -//! user's `icon`. +//! `forward` URL lists, a frontend-rendered browser's `owner`, the +//! user's `icon`, and an agent's `attention` mark (`tab-attention-v1`). use super::super::*; /// Most URLs `tab.update` accepts in `back` or `forward`. pub const TAB_HISTORY_MAX_URLS: usize = 20; +/// Most UTF-8 bytes an attention reason holds (`tab-attention-v1`). +pub const TAB_ATTENTION_REASON_MAX_BYTES: usize = 200; + /// Fields of `tab.update`. At least one field must be present. `back` and /// `forward` apply only to browser tabs; `owner` only to frontend-rendered /// browser tabs. @@ -26,6 +29,11 @@ pub struct TabUpdateOptions { /// The tab's user icon: an SF Symbol name or one emoji (the daemon /// validates the shape). `Update::Clear` sends `null`. pub icon: Update, + /// An agent needs the person to act in this tab (`tab-attention-v1`): + /// one line the person reads, at most 200 bytes. It never moves focus; + /// the tab's navigation to another page or its close clears it. + /// `Update::Clear` sends `null` (the agent's clear). + pub attention: Update, } impl Tab { @@ -48,8 +56,8 @@ impl Tab { mutation_snapshot(self.client().mutate(ops::TAB_UNPIN, self.params(), mutation)?, "tab") } - /// Sets the tab's zoom, browser history lists, frontend owner, or icon - /// with a fresh idempotency key. + /// Sets the tab's zoom, browser history lists, frontend owner, icon, or + /// attention mark with a fresh idempotency key. pub fn update(&self, options: TabUpdateOptions) -> Result> { self.update_with(options, MutationOptions::unique()?) } @@ -69,15 +77,16 @@ impl Tab { } fn tab_update_params(params: Params, options: TabUpdateOptions) -> Result { - let TabUpdateOptions { zoom, back, forward, owner, icon } = options; + let TabUpdateOptions { zoom, back, forward, owner, icon, attention } = options; if matches!(zoom, Update::Unchanged) && matches!(icon, Update::Unchanged) + && matches!(attention, Update::Unchanged) && back.is_none() && forward.is_none() && owner.is_none() { return Err(Error::InvalidArgument( - "tab update must set zoom, back, forward, owner, or icon".to_string(), + "tab update must set zoom, back, forward, owner, icon, or attention".to_string(), )); } let mut params = match zoom { @@ -119,5 +128,19 @@ fn tab_update_params(params: Params, options: TabUpdateOptions) -> Result {} + Update::Clear => params = params.value("attention", Value::Null), + Update::Set(reason) + if !reason.trim().is_empty() && reason.len() <= TAB_ATTENTION_REASON_MAX_BYTES => + { + params = params.string("attention", reason); + } + Update::Set(_) => { + return Err(Error::InvalidArgument(format!( + "tab attention needs a reason of 1 to {TAB_ATTENTION_REASON_MAX_BYTES} bytes" + ))); + } + } Ok(params) } diff --git a/cmux-tui/bindings/rust/tests/typed_state_ops.rs b/cmux-tui/bindings/rust/tests/typed_state_ops.rs index 45f9e3711dbc..3fe83456b2cf 100644 --- a/cmux-tui/bindings/rust/tests/typed_state_ops.rs +++ b/cmux-tui/bindings/rust/tests/typed_state_ops.rs @@ -220,6 +220,7 @@ fn tab_pin_unpin_and_update_send_their_operations_and_decode_tab_snapshots() { assert_eq!(params["forward"], json!([])); assert_eq!(params["owner"], "install-a"); assert_eq!(params["icon"], "hammer.fill"); + assert_eq!(params["attention"], "Log in to GitHub"); mutation_ok( stream, &update, @@ -229,6 +230,7 @@ fn tab_pin_unpin_and_update_send_their_operations_and_decode_tab_snapshots() { let clear = request(reader, "tab.update"); assert_eq!(clear["params"]["zoom"], Value::Null); assert_eq!(clear["params"]["icon"], Value::Null); + assert_eq!(clear["params"]["attention"], Value::Null); assert!(clear["params"].get("back").is_none()); mutation_ok(stream, &clear, tab_snapshot(json!({}))); }); @@ -244,12 +246,18 @@ fn tab_pin_unpin_and_update_send_their_operations_and_decode_tab_snapshots() { forward: Some(vec![]), owner: Some("install-a".into()), icon: Update::Set("hammer.fill".into()), + attention: Update::Set("Log in to GitHub".into()), }) .unwrap(); assert_eq!(updated.value.extra["owner"], "install-a"); assert_eq!(updated.value.extra["icon"], "hammer.fill"); - tab.update(TabUpdateOptions { zoom: Update::Clear, icon: Update::Clear, ..Default::default() }) - .unwrap(); + tab.update(TabUpdateOptions { + zoom: Update::Clear, + icon: Update::Clear, + attention: Update::Clear, + ..Default::default() + }) + .unwrap(); // Catalog limits are refused before any request. for invalid in [ @@ -259,6 +267,8 @@ fn tab_pin_unpin_and_update_send_their_operations_and_decode_tab_snapshots() { TabUpdateOptions { back: Some(vec![String::new(); 21]), ..Default::default() }, TabUpdateOptions { owner: Some(String::new()), ..Default::default() }, TabUpdateOptions { icon: Update::Set(String::new()), ..Default::default() }, + TabUpdateOptions { attention: Update::Set(" ".into()), ..Default::default() }, + TabUpdateOptions { attention: Update::Set("x".repeat(201)), ..Default::default() }, ] { assert!(matches!(tab.update(invalid), Err(Error::InvalidArgument(_)))); } diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs index 08c6e597d312..64250c30e81c 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs @@ -1038,21 +1038,22 @@ fn response_level_navigation_failure_clears_frame_epoch_reservation() { } /// tab-attention-v1: the frame and same-document setters move the URL -/// before any target-info event, so the mark check compares against the -/// URL the event thread last saw, not one setter's return value. +/// before any target-info event, so each check asks about the page the tab +/// is on now; the thread's first check asks even when the URL already moved +/// (the mark, not the thread, knows which page it was set on). #[test] fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { let surface = test_surface(); let browser = surface.as_browser().expect("browser surface"); - let mut seen = browser.url(); + let mut seen = None; clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); - assert_eq!(seen, "https://example.test", "no navigation, no change"); + assert_eq!(seen.as_deref(), Some("https://example.test"), "the first event asks"); let epoch = browser.frame_epoch.advance_navigation(); handle_frame_navigated(browser, json!({"frame": {"url": "https://login.test"}}), epoch); clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); - assert_eq!(seen, "https://login.test", "a frame navigation is a page change"); + assert_eq!(seen.as_deref(), Some("https://login.test"), "a frame navigation is a page change"); let _ = handle_same_document_navigated( browser, @@ -1060,9 +1061,17 @@ fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { browser.frame_epoch.advance_same_document(), ); clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); - assert_eq!(seen, "https://login.test/#done", "a same-document navigation is a page change"); + assert_eq!( + seen.as_deref(), + Some("https://login.test/#done"), + "a same-document navigation is a page change" + ); let _ = browser.set_title("Signed in".to_string()); clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); - assert_eq!(seen, "https://login.test/#done", "a title change is not a page change"); + assert_eq!( + seen.as_deref(), + Some("https://login.test/#done"), + "a title change is not a page change" + ); } diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs index d34798219da6..3e20521c7e69 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs @@ -12,14 +12,14 @@ pub(super) fn start_surface_thread( ) -> anyhow::Result<()> { let id = surface.id; std::thread::Builder::new().name(format!("browser-surface-{id}-events")).spawn(move || { - // tab-attention-v1: the page URL this thread last saw. Any setter - // (target info, frame navigation, same-document navigation, the - // navigate hold) may move the URL first, so compare against what - // this thread saw rather than trust one setter's return value. + // tab-attention-v1: the page URL this thread last asked about, so + // repeated events on one page read nothing. Whether a mark goes is + // decided against the page the mark recorded, not this, so the first + // event (even one after the URL already moved) still clears it. let mut seen_url: Option = None; while let Some(event) = events.recv() { let Surface::Browser(browser) = surface.as_ref() else { break }; - let seen_url = seen_url.get_or_insert_with(|| browser.url()); + let seen_url = &mut seen_url; match event { CdpEvent::ScreencastFrame(frame) => { let frame_epoch = frame.frame_epoch; @@ -194,22 +194,23 @@ pub(super) fn start_surface_thread( Ok(()) } -/// tab-attention-v1: drop the tab's "needs you" mark once its page URL -/// differs from the one `seen_url` last recorded. The marked page is gone. +/// tab-attention-v1: the tab is on `browser.url()`; drop a "needs you" mark +/// set on another page (the marked page is gone). `seen_url` only skips the +/// read for a page this thread already asked about. pub(super) fn clear_attention_if_page_changed( - seen_url: &mut String, + seen_url: &mut Option, browser: &BrowserSurface, mux: &Weak, id: SurfaceId, ) { let url = browser.url(); - if url.is_empty() || url == *seen_url { + if url.is_empty() || seen_url.as_deref() == Some(url.as_str()) { return; } - *seen_url = url; if let Some(mux) = mux.upgrade() { - mux.clear_attention_after_navigation(&crate::Actor::Daemon, id); + mux.clear_attention_after_navigation(&crate::Actor::Daemon, id, &url); } + *seen_url = Some(url); } pub(super) fn start_browser_worker( diff --git a/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs b/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs index 3b994a99c858..a26cf56fe382 100644 --- a/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs +++ b/cmux-tui/crates/cmux-tui-core/src/mux/presentation/frontend_browser_history.rs @@ -99,7 +99,7 @@ impl Mux { changed = true; } // tab-attention-v1: the page the agent asked the person to act on is gone. - let navigated = url.as_ref().is_some_and(|url| previous_url.as_ref() != Some(url)); + let navigated_to = url.clone().filter(|url| previous_url.as_ref() != Some(url)); if changed { if let Some(browser) = runtime.as_browser() { browser.set_frontend_location(url, title.clone()); @@ -110,8 +110,8 @@ impl Mux { } self.emit_tab_changed(surface); } - if navigated { - self.clear_attention_after_navigation(actor, surface); + if let Some(page) = &navigated_to { + self.clear_attention_after_navigation(actor, surface, page); } Ok((record, changed)) } diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs index 0c21b5996567..81ed60ada208 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs @@ -58,7 +58,13 @@ fn an_agent_marks_a_tab_without_moving_focus_and_clears_it() { let cleared = mutate(&mux, "tab.update", json!({"tab": tab, "attention": null}), "attention-3"); assert!(cleared["extra"].get("attention").is_none()); assert_eq!(cleared["extra"]["zoom"], 1.5); - let bad_values = [json!(""), json!("two\nlines"), json!(true), json!("x".repeat(201))]; + let bad_values = [ + json!(""), + json!("two\nlines"), + json!("two\u{2028}lines"), + json!(true), + json!("x".repeat(201)), + ]; for (index, bad) in bad_values.into_iter().enumerate() { assert_eq!( error_code(send( @@ -92,6 +98,14 @@ fn a_browser_tab_drops_its_mark_when_its_page_changes_but_not_on_a_title_change( mux.update_frontend_browser_tab(browser, Some("https://github.com/".into()), None, None) .unwrap(); assert!(listed_attention(&mux, &tab).is_null(), "another page clears the mark"); + // The agent marks the page the tab is on now; that page's navigation + // clear, arriving after the mark, keeps it (it decides in its own commit). + mutate(&mux, "tab.update", json!({"tab": tab, "attention": REASON}), "attention-nav-2"); + assert!(listed_attention(&mux, &tab).get("page").is_none(), "extra.attention shows no page"); + mux.clear_attention_after_navigation(&crate::Actor::Daemon, browser, "https://github.com/"); + assert_eq!(listed_attention(&mux, &tab)["reason"], REASON, "a late clear for its own page"); + mux.clear_attention_after_navigation(&crate::Actor::Daemon, browser, "https://github.com/x"); + assert!(listed_attention(&mux, &tab).is_null(), "the next page clears it"); } #[test] diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs index 7057d8e785b6..50d4840b062e 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_state_store.rs @@ -112,9 +112,26 @@ pub(crate) fn add_tab_attention_column(transaction: &Transaction<'_>) -> anyhow: pub(crate) const MAX_ATTENTION_REASON_BYTES: usize = 200; /// The stored attention mark: why the agent needs the person in this tab, -/// and when it asked. Shown as the tab's `extra.attention`. -pub(crate) fn attention_value(reason: &str, at_ms: u64) -> Value { - json!({"reason": reason, "at_ms": at_ms}) +/// and when it asked. Shown as the tab's `extra.attention`. A browser tab's +/// mark also records the page it was set on (`page`, never shown): the mark +/// clears once the tab is on another page, decided in the clearing +/// transaction ([`clear_attention_off_page`]), so a mark set on the new page +/// survives a navigation event that arrives after it. +pub(crate) fn attention_value(reason: &str, at_ms: u64, page: Option<&str>) -> Value { + match page { + Some(page) => json!({"reason": reason, "at_ms": at_ms, "page": page}), + None => json!({"reason": reason, "at_ms": at_ms}), + } +} + +/// The mark as `extra.attention` shows it: `{reason, at_ms}`, without the +/// page it was set on. +pub(crate) fn shown_attention(stored: &str) -> anyhow::Result { + let mut value: Value = serde_json::from_str(stored)?; + if let Some(object) = value.as_object_mut() { + object.remove("page"); + } + Ok(value) } /// A partial tab state update: `None` keeps a field, `Some(None)` clears @@ -143,6 +160,10 @@ pub(crate) struct TabStateUpdate { /// the tab keeps it; the agent's clear, a browser tab's navigation to /// another page, or the tab's close clears it. pub(crate) attention: Option>, + /// The page a browser tab shows when a new attention mark is set, stored + /// with it. Filled by `state_update_tab` from the tab's surface, never + /// from the request. + pub(crate) attention_page: Option, } impl TabStateUpdate { @@ -168,8 +189,10 @@ impl TabStateUpdate { reason.len() <= MAX_ATTENTION_REASON_BYTES, "bad request: an attention reason holds at most {MAX_ATTENTION_REASON_BYTES} bytes" ); + // One line: no control characters and no Unicode line or + // paragraph separator (U+2028, U+2029, not `is_control`). anyhow::ensure!( - !reason.chars().any(char::is_control), + !reason.chars().any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}')), "bad request: an attention reason is one line of text" ); } @@ -223,9 +246,10 @@ pub(crate) fn update_tab_state( )?; } if let Some(attention) = &update.attention { - let stored = attention - .as_deref() - .map(|reason| attention_value(reason.trim(), crate::mux::now_ms()).to_string()); + let stored = attention.as_deref().map(|reason| { + attention_value(reason.trim(), crate::mux::now_ms(), update.attention_page.as_deref()) + .to_string() + }); transaction.execute( "UPDATE tab_state SET attention_json = ?2 WHERE tab_id = ?1", params![tab_id, stored], @@ -265,6 +289,7 @@ fn drop_empty_tab_state_row(transaction: &Transaction<'_>, tab_id: &str) -> anyh } /// Whether a tab holds an attention mark (`tab-attention-v1`). +#[cfg(test)] pub(crate) fn tab_has_attention(connection: &Connection, tab_id: &str) -> anyhow::Result { Ok(connection .query_row( @@ -276,6 +301,48 @@ pub(crate) fn tab_has_attention(connection: &Connection, tab_id: &str) -> anyhow .unwrap_or(false)) } +/// Whether a tab holds an attention mark (`tab-attention-v1`) set on a page +/// other than `page`: a cheap read before a navigation's clearing commit. +/// The commit decides again ([`clear_attention_off_page`]). +pub(crate) fn tab_has_attention_off_page( + connection: &Connection, + tab_id: &str, + page: &str, +) -> anyhow::Result { + Ok(connection + .query_row( + &format!("SELECT 1 FROM tab_state WHERE tab_id = ?1 AND {ATTENTION_OFF_PAGE}"), + params![tab_id, page], + |_| Ok(()), + ) + .optional()? + .is_some()) +} + +/// A mark set on another page than `?2`, or one that recorded no page. +const ATTENTION_OFF_PAGE: &str = "attention_json IS NOT NULL + AND coalesce(json_extract(attention_json, '$.page'), '') <> ?2"; + +/// A navigation's clear: drop the tab's mark only when it was set on a page +/// other than `page`, in the clearing transaction, so a mark an agent sets on +/// the new page (before this commit) stays. Returns whether it cleared one. +pub(crate) fn clear_attention_off_page( + transaction: &Transaction<'_>, + tab_id: &str, + page: &str, +) -> anyhow::Result { + let cleared = transaction.execute( + &format!( + "UPDATE tab_state SET attention_json = NULL WHERE tab_id = ?1 AND {ATTENTION_OFF_PAGE}" + ), + params![tab_id, page], + )?; + if cleared != 0 { + drop_empty_tab_state_row(transaction, tab_id)?; + } + Ok(cleared != 0) +} + /// Drop a closing tab's attention mark, so a reopened tab (which keeps its /// id) does not ask for the person again. pub(crate) fn clear_closed_tab_attention( @@ -568,7 +635,67 @@ mod attention_tests { assert!(with("Confirm GitHub access").validate().is_ok()); assert!(with(" ").validate().is_err()); assert!(with("two\nlines").validate().is_err()); + assert!(with("two\u{2028}lines").validate().is_err(), "a line separator is two lines"); + assert!(with("two\u{2029}lines").validate().is_err(), "a paragraph separator is two lines"); assert!(with(&"x".repeat(MAX_ATTENTION_REASON_BYTES + 1)).validate().is_err()); assert!(TabStateUpdate { attention: Some(None), ..Default::default() }.validate().is_ok()); } + + /// A navigation clears a mark set on another page, and only that: a mark + /// an agent sets on the new page before the navigation's commit stays, + /// which a read outside the clearing transaction could not promise. + #[test] + fn a_navigation_clears_only_a_mark_set_on_another_page() { + let mut connection = Connection::open_in_memory().unwrap(); + connection + .execute_batch( + "CREATE TABLE tab_state (tab_id TEXT PRIMARY KEY NOT NULL, zoom REAL, + back_json TEXT, forward_json TEXT, icon TEXT, emulation_json TEXT, + marked_unread INTEGER, attention_json TEXT);", + ) + .unwrap(); + let mark = |reason: &str, page: Option<&str>| TabStateUpdate { + attention: Some(Some(reason.into())), + attention_page: page.map(str::to_string), + ..Default::default() + }; + let (a, b) = ("https://github.com/login", "https://github.com/settings"); + let transaction = connection.transaction().unwrap(); + update_tab_state(&transaction, "tab_a", &mark("Log in to GitHub", Some(a))).unwrap(); + update_tab_state(&transaction, "tab_t", &mark("Approve the prompt", None)).unwrap(); + transaction.commit().unwrap(); + let stored: String = connection + .query_row("SELECT attention_json FROM tab_state WHERE tab_id = 'tab_a'", [], |row| { + row.get(0) + }) + .unwrap(); + let shown = shown_attention(&stored).unwrap(); + assert_eq!(shown["reason"], "Log in to GitHub"); + assert!(shown.get("page").is_none(), "extra.attention shows no page: {shown}"); + + assert!(!tab_has_attention_off_page(&connection, "tab_a", a).unwrap()); + assert!(tab_has_attention_off_page(&connection, "tab_a", b).unwrap()); + let transaction = connection.transaction().unwrap(); + assert!(!clear_attention_off_page(&transaction, "tab_a", a).unwrap(), "same page"); + transaction.commit().unwrap(); + assert!(tab_has_attention(&connection, "tab_a").unwrap()); + + // The agent marks the new page before the navigation's clear commits. + let transaction = connection.transaction().unwrap(); + update_tab_state(&transaction, "tab_a", &mark("Approve the scopes", Some(b))).unwrap(); + assert!(!clear_attention_off_page(&transaction, "tab_a", b).unwrap(), "a stale clear"); + transaction.commit().unwrap(); + assert!(tab_has_attention(&connection, "tab_a").unwrap(), "the new page's mark stays"); + + let transaction = connection.transaction().unwrap(); + assert!(clear_attention_off_page(&transaction, "tab_a", a).unwrap(), "went back to a"); + assert!(clear_attention_off_page(&transaction, "tab_t", a).unwrap(), "a mark with no page"); + assert!(!clear_attention_off_page(&transaction, "tab_never_marked", a).unwrap()); + transaction.commit().unwrap(); + assert!(!tab_has_attention(&connection, "tab_a").unwrap()); + assert!(!tab_has_attention(&connection, "tab_t").unwrap()); + let rows: i64 = + connection.query_row("SELECT COUNT(*) FROM tab_state", [], |row| row.get(0)).unwrap(); + assert_eq!(rows, 0, "a row left with no field is deleted"); + } } diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs index ef92ad110f75..a2d64d90f9e9 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs @@ -125,6 +125,17 @@ impl Mux { frontend || update.owner.is_none(), "bad request: owner applies only to frontend browser tabs" ); + let mut update = update; + // tab-attention-v1: a browser tab's new mark records the page + // it was set on, so only a later page clears it. + if matches!(update.attention, Some(Some(_))) { + update.attention_page = state + .surfaces + .get(&surface) + .and_then(|runtime| runtime.as_browser()) + .map(|browser| browser.url()) + .filter(|url| !url.is_empty()); + } edit.tab_state.push((tab.clone(), update)); edit.result = StripResult::Tab(tab); Ok(()) @@ -132,18 +143,28 @@ impl Mux { Ok(commit) } - /// `tab-attention-v1`: a browser tab whose page went to another URL - /// drops its attention mark, since the page the agent asked the person to + /// `tab-attention-v1`: a browser tab now on `page` drops an attention + /// mark set on another page, since the page the agent asked the person to /// act on is gone (the person confirmed, logged in, or left). One read - /// when the tab holds no mark; a commit restating the tab when it does. - pub(crate) fn clear_attention_after_navigation(&self, actor: &Actor, surface: SurfaceId) { + /// when the tab holds no such mark; otherwise a commit that decides again + /// in its own transaction (a mark set on `page` meanwhile stays) and + /// restates the tab when it cleared one. + pub(crate) fn clear_attention_after_navigation( + &self, + actor: &Actor, + surface: SurfaceId, + page: &str, + ) { + if page.is_empty() { + return; + } let Some(tab) = self.with_state(|state| { state.resource_indexes.tab_ids.get(&surface).map(|id| id.as_str().to_string()) }) else { return; }; let marked = self.personal_state_read(|connection| { - crate::state::tab_state_store::tab_has_attention(connection, &tab) + crate::state::tab_state_store::tab_has_attention_off_page(connection, &tab, page) }); if !matches!(marked, Ok(true)) { return; @@ -160,15 +181,14 @@ impl Mux { None, StateEffects::PRESENTATION, |transaction, _| { - let update = TabStateUpdate { attention: Some(None), ..Default::default() }; - crate::state::tab_state_store::update_tab_state(transaction, &tab, &update)?; - let changes = crate::state::values::fresh_upserts( + let cleared = crate::state::tab_state_store::clear_attention_off_page( transaction, - &[], - &[], - std::slice::from_ref(&tab), + &tab, + page, )?; - Ok(StateChanges::new(serde_json::json!({"tab": tab}), changes)) + let restated: &[String] = if cleared { std::slice::from_ref(&tab) } else { &[] }; + let changes = crate::state::values::fresh_upserts(transaction, &[], &[], restated)?; + Ok(StateChanges::new(serde_json::json!({"tab": tab, "cleared": cleared}), changes)) }, ); if let Err(error) = result { diff --git a/cmux-tui/crates/cmux-tui-core/src/state/values.rs b/cmux-tui/crates/cmux-tui-core/src/state/values.rs index 54e0384fc091..856c98a0301d 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/values.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/values.rs @@ -199,7 +199,10 @@ fn tab_extra(connection: &Connection, tab_id: &str) -> anyhow::Result Date: Thu, 22 Oct 2026 22:02:45 -0700 Subject: [PATCH 4/5] tab-attention-v1: the router's TabStateUpdate names attention_page; browser.rs imports in cargo fmt order Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16037-16c2-7a73-b063-2e37ed40b9c4&host=cosmic-gray --- cmux-tui/crates/cmux-tui-core/src/browser.rs | 2 +- cmux-tui/crates/cmux-tui-core/src/state/router.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/browser.rs b/cmux-tui/crates/cmux-tui-core/src/browser.rs index f7dd4df6e395..7b9794421168 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser.rs @@ -40,12 +40,12 @@ mod worker; use runtime::runtime_endpoint; use runtime::{browser_geometry_locked, capture_scale_for, scaled_pixels}; pub(crate) use runtime::{new_surface, new_surface_with_resource_identity}; -use worker::{is_cdp_timeout_error, start_browser_worker, start_surface_thread}; #[cfg(test)] use worker::{ clear_attention_if_page_changed, next_pointer_lifecycle_deadline, record_browser_worker_result, release_abandoned_pointer_presses, take_latest_worker_commands, }; +use worker::{is_cdp_timeout_error, start_browser_worker, start_surface_thread}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum BrowserSource { diff --git a/cmux-tui/crates/cmux-tui-core/src/state/router.rs b/cmux-tui/crates/cmux-tui-core/src/state/router.rs index 433a5dc49699..0b0d12b8562f 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/router.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/router.rs @@ -346,6 +346,7 @@ pub(crate) fn dispatch( emulation: None, marked_unread: optional_bool(fields, "marked_unread")?, attention: nullable_reason(fields, "attention")?, + attention_page: None, }; let commit = mux .state_update_tab(strip_request(&request)?, selectors.clone(), update) From 12f9a15f0f031f9a491c1ec6e6fcf61c80d8e295 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Fri, 23 Oct 2026 02:00:04 -0700 Subject: [PATCH 5/5] tab-attention-v1: a stale navigation clear keeps a newer page's mark; SDK refuses a multi-line reason The navigation clear now decides in its commit against the tab's current page: a late callback for a page the tab already left clears nothing, so a mark the agent set on the new page stays when two navigations overlap. The Rust SDK refuses what the daemon refuses (a control character, U+2028 or U+2029). The worker's page-change check takes its clear as a closure, so its test proves the clear is asked once per page. Leo-ask: ask-12231fbe5e Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a16037-16c2-7a73-b063-2e37ed40b9c4&host=cosmic-gray --- .../rust/src/resource/handles/tab_update.rs | 10 ++++- .../bindings/rust/tests/typed_state_ops.rs | 9 ++++ .../src/browser/tests/navigation_barriers.rs | 14 +++++-- .../cmux-tui-core/src/browser/worker.rs | 41 ++++++++++++++----- .../src/state/tab_attention_tests.rs | 11 ++++- .../crates/cmux-tui-core/src/state/tabs.rs | 20 ++++++--- 6 files changed, 81 insertions(+), 24 deletions(-) diff --git a/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs b/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs index c6a79afd96b9..f363371f3cf5 100644 --- a/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs +++ b/cmux-tui/bindings/rust/src/resource/handles/tab_update.rs @@ -131,14 +131,20 @@ fn tab_update_params(params: Params, options: TabUpdateOptions) -> Result {} Update::Clear => params = params.value("attention", Value::Null), + // The daemon's rule: one readable line (no control character, no + // U+2028 or U+2029) of at most TAB_ATTENTION_REASON_MAX_BYTES. Update::Set(reason) - if !reason.trim().is_empty() && reason.len() <= TAB_ATTENTION_REASON_MAX_BYTES => + if !reason.trim().is_empty() + && reason.len() <= TAB_ATTENTION_REASON_MAX_BYTES + && !reason + .chars() + .any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}')) => { params = params.string("attention", reason); } Update::Set(_) => { return Err(Error::InvalidArgument(format!( - "tab attention needs a reason of 1 to {TAB_ATTENTION_REASON_MAX_BYTES} bytes" + "tab attention needs one line of 1 to {TAB_ATTENTION_REASON_MAX_BYTES} bytes" ))); } } diff --git a/cmux-tui/bindings/rust/tests/typed_state_ops.rs b/cmux-tui/bindings/rust/tests/typed_state_ops.rs index 3fe83456b2cf..2a957703c605 100644 --- a/cmux-tui/bindings/rust/tests/typed_state_ops.rs +++ b/cmux-tui/bindings/rust/tests/typed_state_ops.rs @@ -269,6 +269,15 @@ fn tab_pin_unpin_and_update_send_their_operations_and_decode_tab_snapshots() { TabUpdateOptions { icon: Update::Set(String::new()), ..Default::default() }, TabUpdateOptions { attention: Update::Set(" ".into()), ..Default::default() }, TabUpdateOptions { attention: Update::Set("x".repeat(201)), ..Default::default() }, + TabUpdateOptions { attention: Update::Set("two\nlines".into()), ..Default::default() }, + TabUpdateOptions { + attention: Update::Set("two\u{2028}lines".into()), + ..Default::default() + }, + TabUpdateOptions { + attention: Update::Set("two\u{2029}lines".into()), + ..Default::default() + }, ] { assert!(matches!(tab.update(invalid), Err(Error::InvalidArgument(_)))); } diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs index 64250c30e81c..121d3782a538 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/tests/navigation_barriers.rs @@ -1046,13 +1046,14 @@ fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { let surface = test_surface(); let browser = surface.as_browser().expect("browser surface"); let mut seen = None; + let mut asked: Vec = Vec::new(); - clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + clear_attention_if_page_changed(&mut seen, browser, |url| asked.push(url.to_string())); assert_eq!(seen.as_deref(), Some("https://example.test"), "the first event asks"); let epoch = browser.frame_epoch.advance_navigation(); handle_frame_navigated(browser, json!({"frame": {"url": "https://login.test"}}), epoch); - clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + clear_attention_if_page_changed(&mut seen, browser, |url| asked.push(url.to_string())); assert_eq!(seen.as_deref(), Some("https://login.test"), "a frame navigation is a page change"); let _ = handle_same_document_navigated( @@ -1060,7 +1061,7 @@ fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { &json!({"frameId": "main-frame", "url": "https://login.test/#done"}), browser.frame_epoch.advance_same_document(), ); - clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + clear_attention_if_page_changed(&mut seen, browser, |url| asked.push(url.to_string())); assert_eq!( seen.as_deref(), Some("https://login.test/#done"), @@ -1068,10 +1069,15 @@ fn every_navigation_setter_counts_as_a_page_change_for_the_attention_mark() { ); let _ = browser.set_title("Signed in".to_string()); - clear_attention_if_page_changed(&mut seen, browser, &Weak::new(), surface.id); + clear_attention_if_page_changed(&mut seen, browser, |url| asked.push(url.to_string())); assert_eq!( seen.as_deref(), Some("https://login.test/#done"), "a title change is not a page change" ); + assert_eq!( + asked, + ["https://example.test", "https://login.test", "https://login.test/#done"], + "the worker asks the mux to clear once per page, never on a title change" + ); } diff --git a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs index 3e20521c7e69..066f5ee55a17 100644 --- a/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs +++ b/cmux-tui/crates/cmux-tui-core/src/browser/worker.rs @@ -94,7 +94,11 @@ pub(super) fn start_surface_thread( let url_changed = if info.url.is_empty() { false } else { browser.set_url(info.url) }; let title_changed = browser.set_title(title); - clear_attention_if_page_changed(seen_url, browser, &mux, id); + clear_attention_if_page_changed( + seen_url, + browser, + mux_attention_clear(&mux, id), + ); if (url_changed || title_changed) && let Some(mux) = mux.upgrade() { @@ -106,7 +110,11 @@ pub(super) fn start_surface_thread( } CdpEvent::FrameNavigated { params, frame_epoch, .. } => { handle_frame_navigated(browser, params, frame_epoch); - clear_attention_if_page_changed(seen_url, browser, &mux, id); + clear_attention_if_page_changed( + seen_url, + browser, + mux_attention_clear(&mux, id), + ); if let Some(mux) = mux.upgrade() { mux.emit(MuxEvent::TitleChanged { surface: id, @@ -134,7 +142,11 @@ pub(super) fn start_surface_thread( frame_epoch, } => { let _ = handle_same_document_navigated(browser, ¶ms, frame_epoch); - clear_attention_if_page_changed(seen_url, browser, &mux, id); + clear_attention_if_page_changed( + seen_url, + browser, + mux_attention_clear(&mux, id), + ); if browser.needs_same_document_paint() { let _ = browser.enqueue_latest_authority( BrowserCommand::AuthorizeSameDocumentPaint { @@ -194,25 +206,32 @@ pub(super) fn start_surface_thread( Ok(()) } -/// tab-attention-v1: the tab is on `browser.url()`; drop a "needs you" mark -/// set on another page (the marked page is gone). `seen_url` only skips the -/// read for a page this thread already asked about. +/// tab-attention-v1: the tab is on `browser.url()`; `clear` drops a "needs +/// you" mark set on another page (the marked page is gone). `seen_url` only +/// skips the call for a page this thread already asked about. pub(super) fn clear_attention_if_page_changed( seen_url: &mut Option, browser: &BrowserSurface, - mux: &Weak, - id: SurfaceId, + clear: impl FnOnce(&str), ) { let url = browser.url(); if url.is_empty() || seen_url.as_deref() == Some(url.as_str()) { return; } - if let Some(mux) = mux.upgrade() { - mux.clear_attention_after_navigation(&crate::Actor::Daemon, id, &url); - } + clear(&url); *seen_url = Some(url); } +/// The worker's `clear`: the mux's navigation clear for surface `id`, while +/// the mux lives. +fn mux_attention_clear(mux: &Weak, id: SurfaceId) -> impl FnOnce(&str) + '_ { + move |url| { + if let Some(mux) = mux.upgrade() { + mux.clear_attention_after_navigation(&crate::Actor::Daemon, id, url); + } + } +} + pub(super) fn start_browser_worker( surface: Arc, rx: Receiver, diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs index 81ed60ada208..6cea3e790cd8 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tab_attention_tests.rs @@ -104,7 +104,16 @@ fn a_browser_tab_drops_its_mark_when_its_page_changes_but_not_on_a_title_change( assert!(listed_attention(&mux, &tab).get("page").is_none(), "extra.attention shows no page"); mux.clear_attention_after_navigation(&crate::Actor::Daemon, browser, "https://github.com/"); assert_eq!(listed_attention(&mux, &tab)["reason"], REASON, "a late clear for its own page"); - mux.clear_attention_after_navigation(&crate::Actor::Daemon, browser, "https://github.com/x"); + // A stale clear for a page the tab already left (two navigations + // overlapping) keeps the mark the agent set on the page it is on now. + mux.clear_attention_after_navigation( + &crate::Actor::Daemon, + browser, + "https://github.com/settings/apps", + ); + assert_eq!(listed_attention(&mux, &tab)["reason"], REASON, "a stale clear dropped the mark"); + mux.update_frontend_browser_tab(browser, Some("https://github.com/x".into()), None, None) + .unwrap(); assert!(listed_attention(&mux, &tab).is_null(), "the next page clears it"); } diff --git a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs index a2d64d90f9e9..0479fedd3e0d 100644 --- a/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs +++ b/cmux-tui/crates/cmux-tui-core/src/state/tabs.rs @@ -180,12 +180,20 @@ impl Mux { &fingerprint, None, StateEffects::PRESENTATION, - |transaction, _| { - let cleared = crate::state::tab_state_store::clear_attention_off_page( - transaction, - &tab, - page, - )?; + |transaction, state| { + // A late callback for a page the tab has already left clears + // nothing: the mark may be the newer page's, set since. + let current = state + .surfaces + .get(&surface) + .and_then(|runtime| runtime.as_browser()) + .map(|browser| browser.url()); + let cleared = current.as_deref() == Some(page) + && crate::state::tab_state_store::clear_attention_off_page( + transaction, + &tab, + page, + )?; let restated: &[String] = if cleared { std::slice::from_ref(&tab) } else { &[] }; let changes = crate::state::values::fresh_upserts(transaction, &[], &[], restated)?; Ok(StateChanges::new(serde_json::json!({"tab": tab, "cleared": cleared}), changes))