diff --git a/plugins/agent/native/src/owner.rs b/plugins/agent/native/src/owner.rs index 654b8317f2..1d206c6dd3 100644 --- a/plugins/agent/native/src/owner.rs +++ b/plugins/agent/native/src/owner.rs @@ -993,7 +993,7 @@ impl Owner { .get_session::(&input.session_id) .await .map_err(|_| "agent_store_unavailable")?; - if session.is_none_or(|session| session.archived) { + if session.is_none() { return Ok(MarkReadResult::Rejected { reason: Rejection::SessionUnavailable, }); diff --git a/plugins/agent/native/src/owner/tests/session_availability.rs b/plugins/agent/native/src/owner/tests/session_availability.rs index 6d6d39c04b..b68951c2e2 100644 --- a/plugins/agent/native/src/owner/tests/session_availability.rs +++ b/plugins/agent/native/src/owner/tests/session_availability.rs @@ -43,6 +43,8 @@ async fn session_read_guards_reject_foreign_missing_and_logically_removed_sessio ]; for id in ["foreign_session", missing.as_str(), session.as_str()] { assert_eq!(fixture.owner.require_session(id).await.unwrap_err(), "agent_session_unavailable"); + assert_eq!(fixture.mutation(2, "markRead", json!({"sessionId":id,"messageId":"unavailable-tail"}), Uuid::new_v4()).await.unwrap(), + json!({"kind":"rejected","reason":"session_unavailable"})); for (method, input) in &requests { let mut input = input.clone(); input["sessionId"] = json!(id); @@ -127,3 +129,99 @@ async fn history_visibility_does_not_decode_configuration_but_source_still_requi .await .unwrap(); } + +#[tokio::test] +async fn archived_session_mark_read_acknowledges_only_the_exact_tail_without_restoring() { + tokio::time::timeout(Duration::from_secs(30), async { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let fixture = Fixture::new( + &format!("http://{}/v1", listener.local_addr().unwrap()), + None, + ) + .await; + let session = fixture.create("Archived unread conversation").await; + let mut tails = Vec::new(); + for _ in 0..2 { + fixture.send(&session).await.unwrap(); + let (mut socket, _) = listener.accept().await.unwrap(); + request(&mut socket).await; + response(&mut socket).await; + fixture.drained().await; + let observed = fixture + .query("observe", json!({"sessionId":session})) + .await + .unwrap(); + assert_eq!(observed["session"]["hasUnread"], true); + tails.push( + observed["session"]["tailMessageId"] + .as_str() + .unwrap() + .to_owned(), + ); + } + assert_ne!(tails[0], tails[1]); + let revision = fixture.owner.session(&session).await.unwrap().revision; + let archived = fixture + .mutation( + 2, + "changeSession", + json!({ + "sessionId":session,"expectedRevision":revision,"action":"archive" + }), + Uuid::new_v4(), + ) + .await + .unwrap(); + assert_eq!(archived["kind"], "changed"); + assert_eq!(archived["session"]["archived"], true); + assert_eq!(archived["session"]["hasUnread"], true); + + let stale = fixture + .mutation( + 2, + "markRead", + json!({ + "sessionId":session,"messageId":tails[0] + }), + Uuid::new_v4(), + ) + .await + .unwrap(); + assert_eq!(stale["kind"], "observed"); + assert_eq!(stale["session"]["hasUnread"], true); + assert_eq!( + stale["session"]["revision"], + archived["session"]["revision"] + ); + + let read = fixture + .mutation( + 2, + "markRead", + json!({ + "sessionId":session,"messageId":tails[1] + }), + Uuid::new_v4(), + ) + .await + .unwrap(); + assert_eq!(read["kind"], "observed"); + assert_eq!(read["session"]["archived"], true); + assert_eq!(read["session"]["hasUnread"], false); + let observed = fixture + .query("observe", json!({"sessionId":session})) + .await + .unwrap(); + assert_eq!(observed["session"]["archived"], true); + assert_eq!(observed["session"]["hasUnread"], false); + let stored = fixture.owner.session(&session).await.unwrap(); + assert_eq!( + stored.read_state.last_read_message_id.as_deref(), + Some(tails[1].as_str()) + ); + fixture.app.shutdown().await.unwrap(); + fixture.config.shutdown().await.unwrap(); + }) + .await + .unwrap(); +} diff --git a/products/maka/terminal-shell/tests/real_host/desktop_new_session.rs b/products/maka/terminal-shell/tests/real_host/desktop_new_session.rs index c3d97f2029..ea3a74a4f1 100644 --- a/products/maka/terminal-shell/tests/real_host/desktop_new_session.rs +++ b/products/maka/terminal-shell/tests/real_host/desktop_new_session.rs @@ -2264,7 +2264,7 @@ async fn desktop_parent_reviews_child_access_and_opens_attributed_sender_beside( } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn desktop_child_conversations_expand_archived_ancestors_and_open_a_grandchild() { +async fn desktop_child_conversations_manage_nested_and_archived_workers() { use maka_agent_api::session::StoredSessionConfiguration; let fixture = Fixture::open().await; let original = fixture @@ -2280,9 +2280,11 @@ async fn desktop_child_conversations_expand_archived_ancestors_and_open_a_grandc let prefix = fixture.session.rsplit_once('_').unwrap().0; let child = format!("{prefix}_{}", Uuid::new_v4().simple()); let grandchild = format!("{prefix}_{}", Uuid::new_v4().simple()); + let disposable = format!("{prefix}_{}", Uuid::new_v4().simple()); for (id, parent, title) in [ (&child, &fixture.session, "Archived worker"), (&grandchild, &child, "Verification worker"), + (&disposable, &fixture.session, "Disposable worker"), ] { let mut configuration = original.clone(); configuration.parent_session_id = Some(parent.clone()); @@ -2334,6 +2336,87 @@ async fn desktop_child_conversations_expand_archived_ancestors_and_open_a_grandc "deep rows stay inside the dialog's viewport" ); desktop.artifact(&format!("child-conversations-{theme}-{layout}")); + for (id, title, action, archived) in [ + ( + &grandchild, + "Verification worker", + "Archive conversation", + true, + ), + ( + &grandchild, + "Verification worker", + "Restore conversation", + false, + ), + (&child, "Archived worker", "Restore conversation", false), + ( + &disposable, + "Disposable worker", + "Archive conversation", + true, + ), + ] { + desktop.right_click(&format!("Open {title}")).await; + desktop.control(action).await; + desktop.artifact(&format!( + "child-menu-{}-{theme}-{layout}", + if archived { "archive" } else { "restore" } + )); + desktop.press(action).await; + desktop + .wait("tree action did not change the targeted worker", async { + loop { + let observed = desktop + .client + .query("agent", "observe", json!({"sessionId":id})) + .await + .unwrap(); + if observed["session"]["archived"] == archived { + break; + } + tokio::task::yield_now().await; + } + }) + .await; + desktop + .until("tree action menu did not settle", || { + (desktop.controls(action).is_empty() + && desktop.controls("Close child conversations").len() == 1) + .then_some(()) + }) + .await; + desktop.control("Open Verification worker").await; + } + desktop.press("Actions for Disposable worker").await; + desktop.press("Delete conversation").await; + desktop.control("Confirm delete conversation").await; + assert!( + fixture + .store + .get_session::(&disposable) + .await + .unwrap() + .is_some(), + "opening delete confirmation must not delete the worker" + ); + desktop.artifact(&format!("child-delete-confirmation-{theme}-{layout}")); + desktop.press("Confirm delete conversation").await; + desktop + .until("deleted worker remained in the tree", || { + (desktop.controls("Open Disposable worker").is_empty() + && desktop.controls("Close child conversations").len() == 1) + .then_some(()) + }) + .await; + assert!( + fixture + .store + .get_session::(&disposable) + .await + .unwrap() + .is_none() + ); let previous = desktop.platform.text_input_target(); desktop.press("Open Verification worker").await; desktop.target_after(previous).await; diff --git a/products/maka/workbench/app/SessionActions.tsx b/products/maka/workbench/app/SessionActions.tsx index 8ac9b7b913..4091f89b9a 100644 --- a/products/maka/workbench/app/SessionActions.tsx +++ b/products/maka/workbench/app/SessionActions.tsx @@ -360,6 +360,16 @@ export function SessionActions(props: { session={session()} current={() => active() && page() === 'children'} close={props.close} + renderActions={(target) => ( + target.run(() => props.change(action))} + /> + )} openSession={(id) => { props.close(); props.openSession?.(id); diff --git a/products/maka/workbench/app/SessionTree.tsx b/products/maka/workbench/app/SessionTree.tsx index 31ea006829..c0e9ce1fb5 100644 --- a/products/maka/workbench/app/SessionTree.tsx +++ b/products/maka/workbench/app/SessionTree.tsx @@ -17,7 +17,18 @@ * under the License. */ -import { cleanup, computed, inputAction, List, Scroll, signal, Text, View } from 'nsh'; +import { + cleanup, + computed, + inputAction, + List, + Scroll, + signal, + Text, + View, + type ViewRef, + type JSX, +} from 'nsh'; import { Button, Dialog } from 'nsh/base'; import type { AgentSession, AgentSessionPage } from '../../../../packages/plugin-sdk/src/index.js'; import { useHost } from './host'; @@ -39,35 +50,67 @@ import { textTooltip, } from './ui'; -type TreeSession = Pick & - Partial> & { workspace?: Pick }; -type Branch = { page: AgentSessionPage | null; loading: boolean; error: UiText }; +type TreeSession = SessionActionTarget & Partial>; +export type TreeActions = { + session: SessionActionTarget; + anchor(): ViewRef | null; + run(operation: () => Promise): Promise; + current(): boolean; + close(): void; +}; +type Branch = { + page: AgentSessionPage | null; + loading: boolean; + refreshing: boolean; + error: UiText; +}; type Row = { key: string; depth: number } & ( - | { kind: 'session'; session: TreeSession } + | { kind: 'session'; session: TreeSession; parentId: string | null } | { kind: 'branch'; parent: TreeSession } ); +function actionTarget(session: TreeSession): SessionActionTarget { + return { + id: session.id, + revision: session.revision, + title: session.title, + labels: [...session.labels], + archived: session.archived, + pinned: session.pinned, + workspace: { path: session.workspace.path }, + }; +} + export function SessionTree(props: { session: SessionActionTarget; current(): boolean; close(): void; openSession(id: string): void; + renderActions(target: TreeActions): JSX.Element; }) { const host = useHost(); + const actions = signal(null); + const busy = signal(false); const branches = signal>(new Map()); const expanded = signal>(new Set([props.session.id])); const viewportWidth = signal(0); + const requests = new Map(); let alive = true; cleanup(() => { alive = false; }); const active = () => alive && props.current(); + const interactive = () => active() && !busy(); const update = (id: string, branch: Branch) => branches.set(new Map(branches()).set(id, branch)); const load = async (id: string, restart = false) => { const previous = branches().get(id); - if (!active() || previous?.loading) return; - const page = restart ? null : (previous?.page ?? null); - update(id, { page, loading: true, error: '' }); + if (!active() || (!restart && (previous?.loading || previous?.refreshing))) return; + const request = (requests.get(id) ?? 0) + 1; + requests.set(id, request); + const displayed = previous?.page ?? null; + const page = restart ? null : displayed; + const quiet = restart && Boolean(displayed?.sessions.length); + update(id, { page: displayed, loading: !quiet, refreshing: quiet, error: '' }); try { const next = await host.query('agent', 'list', { filter: 'all', @@ -77,20 +120,27 @@ export function SessionTree(props: { ? { kind: 'after', revision: page.revision, cursor: page.after } : { kind: 'start' }, }); - if (!active()) return; + if (!active() || requests.get(id) !== request) return; if (page && next.revision !== page.revision) throw new UiError('The conversation list changed. Refresh to continue.'); update(id, { page: page ? { ...next, sessions: [...page.sessions, ...next.sessions] } : next, loading: false, + refreshing: false, error: '', }); } catch (reason) { - if (active()) update(id, { page, loading: false, error: errorText(reason) }); + if (active() && requests.get(id) === request) + update(id, { + page: displayed, + loading: false, + refreshing: false, + error: errorText(reason), + }); } }; const toggle = (id: string) => { - if (!active()) return; + if (!interactive()) return; const next = new Set(expanded()); if (next.has(id)) next.delete(id); else { @@ -100,19 +150,23 @@ export function SessionTree(props: { expanded.set(next); }; const open = (id: string) => { - if (active()) props.openSession(id); + if (interactive()) props.openSession(id); }; const close = () => { - if (active()) props.close(); + if (interactive()) props.close(); }; - const rows = computed(() => { - const rows: Row[] = []; + const tree = computed(() => { + const keys: string[] = []; + const rowsByKey = new Map(); + let maxDepth = 0; const stack: Row[] = [ - { kind: 'session', key: props.session.id, depth: 0, session: props.session }, + { kind: 'session', key: props.session.id, depth: 0, session: props.session, parentId: null }, ]; while (stack.length) { const row = stack.pop()!; - rows.push(row); + keys.push(row.key); + rowsByKey.set(row.key, row); + maxDepth = Math.max(maxDepth, row.depth); if (row.kind !== 'session' || !expanded().has(row.session.id)) continue; const branch = branches().get(row.session.id); if (!branch) continue; @@ -135,19 +189,19 @@ export function SessionTree(props: { key: children[index]!.id, depth: row.depth + 1, session: children[index]!, + parentId: row.session.id, }); } - return rows; + return { keys, rowsByKey, maxDepth }; }); - const contentWidth = () => - Math.max( - viewportWidth(), - 320 + rows().reduce((depth, row) => Math.max(depth, row.depth), 0) * 16, - ); + const contentWidth = () => Math.max(viewportWidth(), 320 + tree().maxDepth * 16); void load(props.session.id); return ( tr('Close'))} + disabled={!interactive()} onPress={inputAction(close, null)} style={{ ...controlStyle, width: 32, alignItems: 'center', justifyContent: 'center' }} hoverStyle={controlHoverStyle()} focusStyle={controlFocusStyle()} + disabledStyle={controlDisabledStyle} > @@ -195,24 +251,31 @@ export function SessionTree(props: { style={{ flex: 1, minHeight: 0 }} > row.key} + each={tree().keys} + keyExtractor={(key) => key} estimatedItemSize={48} role="tree" accessibilityLabel={tr('Child conversations')} style={{ width: contentWidth(), flex: 1, minHeight: 0 }} > {(item) => ( - row.key}> - {(value) => { - const row = value(); - if (row.kind === 'branch') { - const branch = () => branches().get(row.parent.id)!; + key}> + {(key) => { + let last = tree().rowsByKey.get(key())!; + const currentRow = () => { + const next = tree().rowsByKey.get(key()); + if (next) last = next; + return last; + }; + const initial = currentRow(); + if (initial.kind === 'branch') { + const parent = () => (currentRow() as Extract).parent; + const branch = () => branches().get(parent().id)!; return ( @@ -252,12 +317,14 @@ export function SessionTree(props: { > @@ -266,7 +333,46 @@ export function SessionTree(props: { ); } - const session = () => (value() as Extract).session; + const session = () => (currentRow() as Extract).session; + const present = () => tree().rowsByKey.has(key()); + const actionTrigger = signal(null); + cleanup(() => { + actionTrigger.set(null); + }); + const showActions = (target: SessionActionTarget) => { + const current = session(); + if ( + !interactive() || + !present() || + !actionTrigger() || + current.id !== target.id || + current.revision !== target.revision + ) + return; + const menu: TreeActions = { + session: target, + anchor: actionTrigger, + run: async (operation) => { + busy.set(true); + try { + await operation(); + if (initial.parentId === null) { + if (active()) props.close(); + } else { + await load(initial.parentId, true); + if (alive && actions() === menu) actions.set(null); + } + } finally { + if (alive) busy.set(false); + } + }, + current: () => active() && actions() === menu, + close: () => { + if (actions() === menu) actions.set(null); + }, + }; + actions.set(menu); + }; const branch = () => branches().get(session().id); const leaf = () => branch()?.page?.sessions.length === 0 && !branch()?.page?.after; @@ -286,14 +392,19 @@ export function SessionTree(props: { return ( session().title)} - disabled={!active()} + disabled={!interactive() || !present()} onPress={inputAction(open, session().id)} style={{ ...controlStyle, @@ -349,6 +462,26 @@ export function SessionTree(props: { + ); }} @@ -357,6 +490,10 @@ export function SessionTree(props: { + {/* Create menus inside the tree's modal scope. */} + target}> + {(item) => props.renderActions(item())} + ); }