From e9f2409b63c92273b29e5158760ef1c98793a56e Mon Sep 17 00:00:00 2001 From: Edwin Date: Fri, 3 Jul 2026 04:25:43 +0900 Subject: [PATCH] Fix Program run shimmer seeding --- crates/cli/src/app.rs | 142 ++++++++++++++++++++++++++++ crates/cli/src/app/program_popup.rs | 128 +++++++++++++++++-------- crates/daemon/src/session.rs | 81 ++++++++++++++++ 3 files changed, 312 insertions(+), 39 deletions(-) diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index 649d0b6c..c098fda5 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -14134,6 +14134,148 @@ mod tests { server.abort(); } + #[tokio::test] + async fn program_dirty_run_seeds_optimistic_pending_before_save_response() { + let (mut app, _dir, server) = empty_app().await; + let id = agentd_protocol::program_block_id; + let saved = "# Todo\n\n- settled\n\n- pending\n\n- untouched\n"; + let edited = "# Todo\n\n- settled\n\n- pending\n\n- user changed\n"; + + app.program_popup = Some(program_popup_for_test("s1", saved, 0)); + app.program_popup.as_mut().unwrap().buffer = edited.to_string(); + app.start_program_run("s1", saved, false, ""); + app.program_runs.get_mut("s1").expect("run").pending = HashSet::from([id("- pending")]); + app.start_program_run("s1", edited, false, saved); + + let pending = &app.program_runs["s1"].pending; + assert_eq!(pending.len(), 2); + assert!(pending.contains(&id("- pending"))); + assert!( + pending.contains(&id("- user changed")), + "dirty user edits must shimmer before any save RPC response" + ); + assert!(!pending.contains(&id("- settled"))); + assert!(!pending.contains(&id("- untouched"))); + server.abort(); + } + + #[tokio::test] + async fn program_execute_sends_explicit_shimmer_for_edited_and_pending_blocks() { + use agentd_protocol::ipc_method; + use serde_json::Value; + use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; + use tokio::net::UnixListener; + use tokio::sync::mpsc; + + let dir = tempfile::tempdir().expect("tempdir"); + let sock = dir.path().join("construct.sock"); + let listener = UnixListener::bind(&sock).expect("bind mock daemon"); + let (tx, mut rx) = mpsc::unbounded_channel::<(String, Value)>(); + let server = tokio::spawn(async move { + loop { + let Ok((stream, _)) = listener.accept().await else { + break; + }; + let tx = tx.clone(); + tokio::spawn(async move { + let (reader, mut writer) = stream.into_split(); + let mut reader = BufReader::new(reader); + let mut line = String::new(); + loop { + line.clear(); + let Ok(n) = reader.read_line(&mut line).await else { + break; + }; + if n == 0 { + break; + } + let req: Value = serde_json::from_str(&line).expect("json request"); + let id = req.get("id").cloned().unwrap_or(Value::Null); + let method = req + .get("method") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(); + let params = req.get("params").cloned().unwrap_or(Value::Null); + let _ = tx.send((method.clone(), params.clone())); + let session_id = params + .get("session_id") + .and_then(Value::as_str) + .unwrap_or("s1"); + let markdown = params + .get("markdown") + .and_then(Value::as_str) + .unwrap_or("# Todo\n\n- settled\n\n- pending\n\n- user changed\n"); + let result = match method.as_str() { + ipc_method::PROGRAM_UPDATE => serde_json::json!({ + "program": { + "session_id": session_id, + "markdown": markdown, + "version": 2, + "updated_at_ms": 0, + "template_id": null + }, + "blocks": [] + }), + ipc_method::PROGRAM_EXECUTE => serde_json::json!({ + "program": { + "session_id": session_id, + "markdown": "# Todo\n\n- settled\n\n- pending\n\n- user changed\n", + "version": 2, + "updated_at_ms": 0, + "template_id": null + }, + "prompt": "run", + "active_run": null, + "blocks": [] + }), + _ => Value::Null, + }; + let resp = + serde_json::json!({ "jsonrpc": "2.0", "id": id, "result": result }); + if writer + .write_all((resp.to_string() + "\n").as_bytes()) + .await + .is_err() + { + break; + } + } + }); + } + }); + + let client = Client::connect(&sock).await.expect("client connects"); + let mut summary = summary_with_kind(agentd_protocol::SessionKind::User); + summary.id = "s1".into(); + let mut app = test_app(client, vec![summary]); + let id = agentd_protocol::program_block_id; + let saved = "# Todo\n\n- settled\n\n- pending\n\n- untouched\n"; + let edited = "# Todo\n\n- settled\n\n- pending\n\n- user changed\n"; + app.program_popup = Some(program_popup_for_test("s1", saved, 0)); + app.program_popup.as_mut().unwrap().buffer = edited.to_string(); + app.start_program_run("s1", saved, false, ""); + app.program_runs.get_mut("s1").expect("run").pending = HashSet::from([id("- pending")]); + + assert!(app.execute_program_popup(None, None).await); + + let mut execute_params = None; + while let Some((method, params)) = rx.recv().await { + if method == ipc_method::PROGRAM_EXECUTE { + execute_params = Some(params); + break; + } + } + let params = execute_params.expect("program.execute params"); + assert_eq!(params.get("base_version").and_then(Value::as_u64), Some(2)); + assert_eq!( + params.get("shimmer").cloned(), + Some(serde_json::json!([false, false, true, true])), + "explicit shimmer must preserve still-pending and user-edited blocks" + ); + server.abort(); + } + #[tokio::test] async fn program_optimistic_selection_run_adds_to_existing_shimmer_scope() { // A selection run must not clear shimmer another in-flight run diff --git a/crates/cli/src/app/program_popup.rs b/crates/cli/src/app/program_popup.rs index 8e3db13b..01be3d56 100644 --- a/crates/cli/src/app/program_popup.rs +++ b/crates/cli/src/app/program_popup.rs @@ -378,10 +378,42 @@ impl App { .map(|popup| popup.saved_markdown.clone()) .unwrap_or_default(); + let selection = + selection.map(|selection| program_normalize_smart_clip_instance_ids(&selection)); + let is_selection = selection.is_some(); + let pre_save_run_body = match selection.as_deref() { + Some(sel) => sel.to_string(), + None => self + .program_popup + .as_ref() + .map(|popup| program_normalize_smart_clip_instance_ids(&popup.buffer)) + .unwrap_or_default(), + }; + let selected_block_ids = selected_block_ids.or_else(|| { + is_selection + .then(|| { + self.program_popup + .as_ref() + .and_then(Self::selected_program_block_ids) + }) + .flatten() + }); + let pending = match selected_block_ids { + Some(ids) if is_selection => self.program_run_pending_with_existing(&session_id, ids), + _ => self.program_run_pending_for_body( + &session_id, + &pre_save_run_body, + is_selection, + &prev_saved, + ), + }; + self.start_program_run_with_pending(&session_id, pending); + let dirty = self.program_popup.as_ref().is_some_and(|popup| { program_normalize_smart_clip_instance_ids(&popup.buffer) != popup.saved_markdown }); if dirty && !self.save_program_popup().await { + self.program_runs.remove(&session_id); return false; } @@ -389,9 +421,6 @@ impl App { .program_popup .as_ref() .map(|popup| popup.program.version); - let selection = - selection.map(|selection| program_normalize_smart_clip_instance_ids(&selection)); - let is_selection = selection.is_some(); // Optimistic feedback (spec 0042): start the Run shimmer the instant // Run is pressed, before the execute round trip, so the affordance // covers the agent's latency rather than the request's. The executed @@ -404,24 +433,36 @@ impl App { .map(|popup| popup.buffer.clone()) .unwrap_or_default(), }; - let selected_block_ids = selected_block_ids.or_else(|| { - self.program_popup - .as_ref() - .and_then(Self::selected_program_block_ids) - }); - match selected_block_ids { - Some(pending) if is_selection => { - let pending = self.program_run_pending_with_existing(&session_id, pending); - self.start_program_run_with_pending(&session_id, pending) - } - _ => self.start_program_run(&session_id, &run_body, is_selection, &prev_saved), - } + let selected_block_ids = is_selection + .then(|| { + self.program_popup + .as_ref() + .and_then(Self::selected_program_block_ids) + }) + .flatten(); + let pending = match selected_block_ids { + Some(ids) if is_selection => self.program_run_pending_with_existing(&session_id, ids), + _ => self.program_run_pending_for_body( + &session_id, + &run_body, + is_selection, + &prev_saved, + ), + }; + self.start_program_run_with_pending(&session_id, pending.clone()); + let shimmer = if is_selection { + Self::program_run_all_shimmer_for_body(&run_body) + } else { + Self::program_run_shimmer_for_body(&run_body, &pending) + }; let params = agentd_protocol::ProgramExecuteParams { session_id: session_id.clone(), selection, base_version, - // Optimistic full-region shimmer; the run's planning pass narrows it. - shimmer: None, + // Echo the TUI's optimistic pending set so a mid-flight full re-Run + // cannot be narrowed back to old pending refs before the planning + // pass sees user-edited blocks. + shimmer, }; match self.client.program_execute(params).await { Ok(result) => { @@ -462,6 +503,7 @@ impl App { /// running one snippet must not dim blocks another in-flight run is still /// working on; a fresh run with nothing in flight shimmers just the /// selected region. + #[cfg(test)] pub(super) fn start_program_run( &mut self, session_id: &str, @@ -469,18 +511,22 @@ impl App { is_selection: bool, prev_saved: &str, ) { + let pending = self.program_run_pending_for_body(session_id, body, is_selection, prev_saved); + self.start_program_run_with_pending(session_id, pending); + } + + pub(super) fn program_run_pending_for_body( + &self, + session_id: &str, + body: &str, + is_selection: bool, + prev_saved: &str, + ) -> HashSet { let body_ids = program_run_pending_ids(body); if body_ids.is_empty() { - // Empty body has nothing to shimmer; drop any stale run. - self.program_runs.remove(session_id); - return; + return HashSet::new(); } - let pending: HashSet = match self.program_runs.get(session_id) { - // Re-Run mid-flight: union of the user's fresh edits (blocks present - // now but not in the last synced content) and blocks that never - // settled under the prior run. Agent-settled blocks are in - // `prev_saved` and absent from `old.pending`, so both terms skip - // them and they stop re-shimmering. + match self.program_runs.get(session_id) { Some(old) if !is_selection => { let prev_ids = program_run_pending_ids(prev_saved); let narrowed: HashSet = body_ids @@ -488,27 +534,31 @@ impl App { .chain(body_ids.intersection(&old.pending)) .cloned() .collect(); - // A run can already exist over this session with nothing left - // that overlaps the fresh press — e.g. the prior run was - // scoped to a selection, or its pending set had transiently - // emptied mid-turn (spec 0042) without being cleared yet. - // Pressing Run again must still give immediate feedback for - // this new request rather than silently going dark, so an - // empty narrowing falls back to the whole executed body — - // mirroring the daemon's own mid-flight fallback. if narrowed.is_empty() { body_ids } else { narrowed } } - // A selection run keeps any shimmer already in flight and adds - // its own scope on top of it (see `program_run_pending_with_existing`). Some(old) => old.pending.union(&body_ids).cloned().collect(), - // Fresh run, nothing in flight: shimmer just the executed body. None => body_ids, - }; - self.start_program_run_with_pending(session_id, pending); + } + } + + pub(super) fn program_run_shimmer_for_body( + body: &str, + pending: &HashSet, + ) -> Option> { + let shimmer: Vec = agentd_protocol::program_block_spans(body) + .into_iter() + .map(|span| pending.contains(&span.id)) + .collect(); + (!shimmer.is_empty()).then_some(shimmer) + } + + fn program_run_all_shimmer_for_body(body: &str) -> Option> { + let len = agentd_protocol::program_block_spans(body).len(); + (len > 0).then(|| vec![true; len]) } /// Union `ids` with the pending set of any Run already in flight for diff --git a/crates/daemon/src/session.rs b/crates/daemon/src/session.rs index e422aaec..e053fae2 100644 --- a/crates/daemon/src/session.rs +++ b/crates/daemon/src/session.rs @@ -6648,6 +6648,87 @@ mod tests { assert!(!run.agent_managed); } + #[tokio::test] + async fn program_full_rerun_with_explicit_shimmer_includes_user_edited_block() { + use agentd_protocol::SessionState; + + let md = "# A\n\n# B\n"; + let edited = "# A\n\n# B edited\n"; + let (mgr, storage, id) = program_test_mgr(md).await; + mgr.start_program_run(&id, md, false, None) + .expect("start full run"); + mgr.note_session_state_for_program_run(&id, SessionState::Running); + let get = mgr.program_get(&id).await.expect("get"); + let block_ref = |needle: &str| { + get.blocks + .iter() + .find(|b| b.text.contains(needle)) + .unwrap_or_else(|| panic!("missing block {needle:?}")) + .id + .clone() + }; + let a_ref = block_ref("# A"); + let b_ref = block_ref("# B"); + + declare( + &mgr, + &id, + "# A", + vec![ + agentd_protocol::ProgramShimmerDecl { + id: a_ref.clone(), + shimmer: true, + tooltip: Some("Working A".into()), + }, + agentd_protocol::ProgramShimmerDecl { + id: b_ref, + shimmer: false, + tooltip: None, + }, + ], + ) + .await; + assert_eq!( + mgr.program_run_snapshot(&id) + .expect("narrowed run") + .pending_block_refs, + vec![a_ref.clone()] + ); + + storage + .update_program( + &id, + edited.to_string(), + agentd_protocol::ProgramUpdateActor::Human, + None, + None, + None, + ) + .expect("save edited program"); + let edited_get = mgr.program_get(&id).await.expect("edited get"); + let edited_b_ref = edited_get + .blocks + .iter() + .find(|b| b.text.contains("# B edited")) + .expect("edited B block") + .id + .clone(); + + let run = mgr + .start_program_run_with_dispatch_state(&id, edited, false, Some(&[true, true]), true) + .expect("explicit full re-run"); + + assert!( + run.pending_block_refs.contains(&a_ref), + "still-pending block A remains pending" + ); + assert!( + run.pending_block_refs.contains(&edited_b_ref), + "user-edited block B is seeded pending by explicit shimmer" + ); + assert_eq!(run.pending_block_refs.len(), 2); + } + // An edit's shimmer declaration for an id that no longer exists is dropped // (fail closed), and other blocks are untouched. #[tokio::test]