Draw a background task as what it ran, and go there on a tap
A card in the session's panel said "background command" under every description -- and for Codex, which names a terminal by a process id and gives no description at all, that phrase was the whole of every card. Both halves of the answer are in the transcript rather than in what the provider says: a driver now reports which tool call its task belongs to (Claude's `task_started` carries the `tool_use_id`, Codex's terminal list the `itemId`), and `LiveSession::background_tasks` resolves those ids against the transcript into a sequence number and, where the provider said nothing, the command the call was made with. So the card draws the command, and the kind shrinks to a mark beside it whose name is what a screen reader is given. Tapping one goes to that call in the transcript, opened, which is where a backgrounded command's output already lands -- rather than drawing a second copy of it beside the panel. The journey is the one a reopened session already makes to put a reader back where they stopped, now one function (`travelTo`). It has to release the held backlog first: events arriving while the reader is away from the newest end are held rather than applied, so a task started since they scrolled back was in no row at all and the tap looked like it had done nothing. Verified against the sandbox on the emulator: the panel draws `sleep 120 && echo done` for an echo session's `/background`, and tapping it lands on that Bash card with its output showing.
This commit is contained in:
1 parent
3b309766d7
commit
cedb18e8c1
18 files changed
+635
-151
No files matched your search
+59
-20
@@ -9,7 +9,7 @@
|
||||
|
||||
mod translate;
|
||||
|
||||
use std::collections::{HashSet, VecDeque};
|
||||
use std::collections::{BTreeMap, VecDeque};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
@@ -38,7 +38,11 @@ const STATE_FILE: &str = "codex-state.json";
|
||||
const POLL: std::time::Duration = std::time::Duration::from_millis(50);
|
||||
const BACKGROUND_POLL: std::time::Duration = std::time::Duration::from_secs(1);
|
||||
|
||||
type BackgroundProcesses = Arc<Mutex<HashSet<String>>>;
|
||||
/// The background terminals app-server says are alive, by process id, each
|
||||
/// against the transcript item whose call started it where the list named
|
||||
/// one. The item id is what the panel draws a command rather than a process
|
||||
/// number for, and what takes a reader to the call.
|
||||
pub(super) type BackgroundProcesses = Arc<Mutex<BTreeMap<String, Option<String>>>>;
|
||||
|
||||
#[derive(Default)]
|
||||
struct BackgroundQuery {
|
||||
@@ -49,7 +53,7 @@ struct BackgroundQuery {
|
||||
struct PendingBackgroundQuery {
|
||||
request: String,
|
||||
thread: String,
|
||||
found: HashSet<String>,
|
||||
found: BTreeMap<String, Option<String>>,
|
||||
dirty: bool,
|
||||
}
|
||||
|
||||
@@ -194,7 +198,7 @@ impl CodexDriver {
|
||||
}
|
||||
});
|
||||
|
||||
let background_processes = Arc::new(Mutex::new(HashSet::new()));
|
||||
let background_processes = Arc::new(Mutex::new(BTreeMap::new()));
|
||||
let inner = Arc::new(Inner {
|
||||
sink,
|
||||
state: Mutex::new(state),
|
||||
@@ -672,16 +676,25 @@ pub(super) fn background_tasks(
|
||||
id,
|
||||
description: Some(title),
|
||||
kind: BackgroundTaskKind::Agent,
|
||||
// A subagent is opened from the list below this one, which is
|
||||
// where its own transcript is read; the call that spawned it is
|
||||
// not what a reader of this row wants.
|
||||
call: None,
|
||||
})
|
||||
.collect();
|
||||
let mut running: Vec<String> = processes.lock().unwrap().iter().cloned().collect();
|
||||
running.sort();
|
||||
tasks.extend(running.into_iter().map(|id| BackgroundTask {
|
||||
// In process-id order, which is the order the map holds them in: the list
|
||||
// itself is a snapshot with no order of its own, and two fetches must not
|
||||
// disagree about how it reads.
|
||||
let running = processes.lock().unwrap().clone();
|
||||
tasks.extend(running.into_iter().map(|(id, item)| BackgroundTask {
|
||||
id,
|
||||
// The terminal list carries a process id and no name, and a number
|
||||
// drawn as a name is worse than the panel saying it does not know.
|
||||
// What the panel draws instead is the command, resolved from the item
|
||||
// below by whoever answers the route.
|
||||
description: None,
|
||||
kind: BackgroundTaskKind::Command,
|
||||
call: item,
|
||||
}));
|
||||
tasks
|
||||
}
|
||||
@@ -707,7 +720,7 @@ fn refresh_background_terminals(inner: &Inner) {
|
||||
query.pending = Some(PendingBackgroundQuery {
|
||||
request: request.clone(),
|
||||
thread: thread.clone(),
|
||||
found: HashSet::new(),
|
||||
found: BTreeMap::new(),
|
||||
dirty: false,
|
||||
});
|
||||
request
|
||||
@@ -755,11 +768,17 @@ fn handle_background_response(inner: &Inner, line: &Value) -> bool {
|
||||
query.pending = None;
|
||||
return true;
|
||||
}
|
||||
for process in terminals
|
||||
.iter()
|
||||
.filter_map(|terminal| terminal.get("processId").and_then(Value::as_str))
|
||||
{
|
||||
pending.found.insert(process.to_string());
|
||||
for terminal in terminals {
|
||||
let Some(process) = terminal.get("processId").and_then(Value::as_str) else {
|
||||
continue;
|
||||
};
|
||||
pending.found.insert(
|
||||
process.to_string(),
|
||||
terminal
|
||||
.get("itemId")
|
||||
.and_then(Value::as_str)
|
||||
.map(str::to_string),
|
||||
);
|
||||
}
|
||||
if let Some(cursor) = line.pointer("/result/nextCursor").and_then(Value::as_str) {
|
||||
let thread = pending.thread.clone();
|
||||
@@ -1582,7 +1601,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
};
|
||||
@@ -1598,7 +1617,7 @@ mod tests {
|
||||
"id": request_id,
|
||||
"result": {
|
||||
"data": [
|
||||
{"processId": "process-a"},
|
||||
{"processId": "process-a", "itemId": "item-a"},
|
||||
{"processId": "process-b"}
|
||||
],
|
||||
"nextCursor": null
|
||||
@@ -1606,6 +1625,26 @@ mod tests {
|
||||
})
|
||||
));
|
||||
assert_eq!(background_task_count(&inner), 2);
|
||||
// The item the terminal belongs to is what the panel draws a command
|
||||
// rather than a process number for, so it is kept beside the id;
|
||||
// a terminal listed without one is still a task.
|
||||
assert_eq!(
|
||||
background_tasks(&inner.subagents, &inner.background_processes),
|
||||
vec![
|
||||
BackgroundTask {
|
||||
id: "process-a".to_string(),
|
||||
description: None,
|
||||
kind: BackgroundTaskKind::Command,
|
||||
call: Some("item-a".to_string()),
|
||||
},
|
||||
BackgroundTask {
|
||||
id: "process-b".to_string(),
|
||||
description: None,
|
||||
kind: BackgroundTaskKind::Command,
|
||||
call: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
events.try_recv().expect("count"),
|
||||
Event::BackgroundTasks { count: 2 }
|
||||
@@ -1691,7 +1730,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
});
|
||||
@@ -1748,7 +1787,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
});
|
||||
@@ -1815,7 +1854,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
});
|
||||
@@ -1880,7 +1919,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
});
|
||||
@@ -1951,7 +1990,7 @@ mod tests {
|
||||
transport: Transport::Here,
|
||||
session_dir: dir.path().to_path_buf(),
|
||||
subagents: Arc::new(Subagents::new(dir.path().to_path_buf())),
|
||||
background_processes: Arc::new(Mutex::new(HashSet::new())),
|
||||
background_processes: Arc::new(Mutex::new(BTreeMap::new())),
|
||||
background_query: Mutex::new(BackgroundQuery::default()),
|
||||
reading: AtomicBool::new(true),
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user