Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmux-tui/bindings/cpp/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/go/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/java/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/python/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/rust/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
41 changes: 35 additions & 6 deletions cmux-tui/bindings/rust/src/resource/handles/tab_update.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<String>,
/// 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<String>,
}

impl Tab {
Expand All @@ -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<MutationResult<TabSnapshot>> {
self.update_with(options, MutationOptions::unique()?)
}
Expand All @@ -69,15 +77,16 @@ impl Tab {
}

fn tab_update_params(params: Params, options: TabUpdateOptions) -> Result<Params> {
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 {
Expand Down Expand Up @@ -119,5 +128,25 @@ fn tab_update_params(params: Params, options: TabUpdateOptions) -> Result<Params
return Err(Error::InvalidArgument("tab icon must contain 1 to 128 bytes".to_string()));
}
}
match attention {
Update::Unchanged => {}
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
&& !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 one line of 1 to {TAB_ATTENTION_REASON_MAX_BYTES} bytes"
)));
}
}
Ok(params)
}
23 changes: 21 additions & 2 deletions cmux-tui/bindings/rust/tests/typed_state_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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!({})));
});
Expand All @@ -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 [
Expand All @@ -259,6 +267,17 @@ 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() },
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(_))));
}
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/typescript/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/bindings/zig/.cmux-resource-api.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"catalog_sha256": "415b38e0d3938e494b511947d277fedf35d2dc32949374e1584b98caef0353c8",
"catalog_sha256": "62219c3d79443e45ff46961fd4a9e27bf0bd1ba18300e2730d5a78c586e934d5",
"operations": {
"agent.list": {
"class": "read"
Expand Down
2 changes: 1 addition & 1 deletion cmux-tui/crates/cmux-app-host/generated/cmux-app.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1513,7 +1513,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<Cmux.TabSnapshot>>
/** `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<string>; forward?: Array<string>; owner?: string; icon?: string | null; marked_unread?: boolean; expected_revision?: string }, Cmux.MutationResult<Cmux.TabSnapshot>>
update: CmuxOp<{ machine?: string; session?: string; workspace?: string; screen?: string; pane?: string; tab: string; zoom?: number | null; back?: Array<string>; forward?: Array<string>; owner?: string; icon?: string | null; marked_unread?: boolean; attention?: string | null; expected_revision?: string }, Cmux.MutationResult<Cmux.TabSnapshot>>
}
tab_group: {
/** `tab_group.add_tabs` (mutation, scope `tab_group:write`) */
Expand Down
4 changes: 2 additions & 2 deletions cmux-tui/crates/cmux-tui-core/src/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
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,
};
use worker::{is_cdp_timeout_error, start_browser_worker, start_surface_thread};

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum BrowserSource {
Expand Down
5 changes: 3 additions & 2 deletions cmux-tui/crates/cmux-tui-core/src/browser/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1036,3 +1036,48 @@ 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 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 = None;
let mut asked: Vec<String> = Vec::new();

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, |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(
browser,
&json!({"frameId": "main-frame", "url": "https://login.test/#done"}),
browser.frame_epoch.advance_same_document(),
);
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 same-document navigation is a page change"
);

let _ = browser.set_title("Signed in".to_string());
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"
);
}
47 changes: 47 additions & 0 deletions cmux-tui/crates/cmux-tui-core/src/browser/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 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<String> = None;
while let Some(event) = events.recv() {
let Surface::Browser(browser) = surface.as_ref() else { break };
let seen_url = &mut seen_url;
match event {
CdpEvent::ScreencastFrame(frame) => {
let frame_epoch = frame.frame_epoch;
Expand Down Expand Up @@ -88,6 +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_attention_clear(&mux, id),
);
if (url_changed || title_changed)
&& let Some(mux) = mux.upgrade()
{
Expand All @@ -99,6 +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_attention_clear(&mux, id),
);
if let Some(mux) = mux.upgrade() {
mux.emit(MuxEvent::TitleChanged {
surface: id,
Expand Down Expand Up @@ -126,6 +142,11 @@ pub(super) fn start_surface_thread(
frame_epoch,
} => {
let _ = handle_same_document_navigated(browser, &params, frame_epoch);
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 {
Expand Down Expand Up @@ -185,6 +206,32 @@ pub(super) fn start_surface_thread(
Ok(())
}

/// 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<String>,
browser: &BrowserSurface,
clear: impl FnOnce(&str),
) {
let url = browser.url();
if url.is_empty() || seen_url.as_deref() == Some(url.as_str()) {
return;
}
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<Mux>, 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<Surface>,
rx: Receiver<SequencedBrowserCommand>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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_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());
Expand All @@ -107,6 +110,9 @@ impl Mux {
}
self.emit_tab_changed(surface);
}
if let Some(page) = &navigated_to {
self.clear_attention_after_navigation(actor, surface, page);
}
Ok((record, changed))
}

Expand Down
5 changes: 5 additions & 0 deletions cmux-tui/crates/cmux-tui-core/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,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";
Expand Down
Loading
Loading