diff --git a/FORK_PATCHES.md b/FORK_PATCHES.md index 958941d1e98..3209cb6d301 100644 --- a/FORK_PATCHES.md +++ b/FORK_PATCHES.md @@ -5459,7 +5459,7 @@ New module: `false`. ## `crates/buzz-acp/src/process_tree.rs` -Bounded process snapshots and resident-helper classification; macOS executable confirmation releases completed Codex turns while preserving actual child work. +Bounded process snapshots and resident-helper classification; macOS/Linux executable confirmation releases completed Codex turns while preserving actual child work. New module: `true`. @@ -5811,26 +5811,29 @@ New module: `false`. - Required symbol: `demo_echo_keeps_inflight_read_across_ticks_and_still_drains` - Verify: `cargo test -p buzz-relay -- --test-threads=1` -## `crates/buzz-acp/src/process_tree/macos_helpers.rs` +## `crates/buzz-acp/src/process_tree/codex_helpers.rs` -Confirm late Codex code-mode host executable identity on macOS with one bounded ps call; arguments, missing identity, and truncated names cannot free slots. +Confirm late Codex code-mode host identity with one bounded ps call: macOS comm and Linux exe; missing identity and work arguments cannot free slots. New module: `true`. - Required symbol: `pub(super) fn classify(` - Required symbol: `fn confirmed_hosts(` -- Invocation: `crates/buzz-acp/src/process_tree.rs` → `classify`; exact call `let tree = macos_helpers::classify(tree);`; behavior test `six_finished_threads_reuse_five_slots_with_late_codex_hosts`; verify `env -u BUZZ_ACP_SESSION_POLICY -u BUZZ_ACP_ALLOWED_RESPOND_TO -u BUZZ_ACP_LAZY_POOL -u BUZZ_ACP_IDLE_POOL_SLEEP cargo test -p buzz-acp resident_helper_tests -- --test-threads=1` +- Invocation: `crates/buzz-acp/src/process_tree.rs` → `classify`; exact call `let tree = codex_helpers::classify(tree);`; behavior test `six_finished_threads_reuse_five_slots_with_late_codex_hosts`; verify `env -u BUZZ_ACP_SESSION_POLICY -u BUZZ_ACP_ALLOWED_RESPOND_TO -u BUZZ_ACP_LAZY_POOL -u BUZZ_ACP_IDLE_POOL_SLEEP cargo test -p buzz-acp resident_helper_tests -- --test-threads=1` - Verify: `env -u BUZZ_ACP_SESSION_POLICY -u BUZZ_ACP_ALLOWED_RESPOND_TO -u BUZZ_ACP_LAZY_POOL -u BUZZ_ACP_IDLE_POOL_SLEEP cargo test -p buzz-acp resident_helper_tests -- --test-threads=1` ## `crates/buzz-acp/src/pool/resident_helper_tests.rs` -Real local ACP/process regressions for sixth-thread admission, late resident host, nested child exit and work mentioning host in arguments on macOS. +Real local ACP/process regressions for sixth-thread admission, nested child exit and original-session collection on macOS/Linux; Linux executable unlink and symlink boundaries. New module: `true`. - Required symbol: `six_finished_threads_reuse_five_slots_with_late_codex_hosts` - Required symbol: `work_under_late_codex_host_holds_slot_until_child_exits` - Required symbol: `work_mentioning_codex_host_in_arguments_still_holds_slot` +- Required symbol: `child_of_codex_host_queues_original_collection_before_releasing_slot` +- Required symbol: `symlink_named_codex_host_does_not_hide_ordinary_work` +- Required symbol: `unlinked_codex_host_does_not_retain_finished_turn` - Verify: `env -u BUZZ_ACP_SESSION_POLICY -u BUZZ_ACP_ALLOWED_RESPOND_TO -u BUZZ_ACP_LAZY_POOL -u BUZZ_ACP_IDLE_POOL_SLEEP cargo test -p buzz-acp resident_helper_tests -- --test-threads=1` ## `desktop/src/features/messages/lib/useComposerAutofocus.ts` diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index b779d111d28..3efe33a66ef 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -94,18 +94,26 @@ collection guarantee; successful ACP completion alone does not confirm publicati Process tracking follows children under the worker and resident providers; detached/reparented processes are outside that topology. -### macOS Codex service processes and completed slots +### macOS/Linux Codex service processes and completed slots The Codex code-mode host may start after `session/new` and stay alive after a -turn. On macOS, a bounded executable-name lookup recognizes this service, -including installation paths containing spaces. Its real child processes still -protect the worker. Other platforms retain their existing classification; Linux's -truncated `comm` value is not accepted as executable identity. +turn. A bounded executable-name lookup recognizes this service, including +installation paths containing spaces. macOS uses `ps comm`; Linux uses `ps exe` +and accepts the kernel's ` (deleted)` suffix for a still-running unlinked binary. +Linux's truncated `comm`, argv-only impersonation and symlinks to unrelated +executables do not establish identity. Failed/unsupported lookups keep the work +held. Its real child processes still protect the worker; other platforms retain +their existing classification. The `resident_helper_tests` regressions run real local ACP adapters and shell processes through pool return/claim. They cover six finished threads with five slots, child exit beneath a live host, and ordinary shell work mentioning the -host path in arguments. The fixtures require no provider credentials or relay. +host path in arguments. They also verify that child exit queues collection in +the original session and retains the slot until collection finishes. Linux-only +cases cover an unlinked helper executable and a symlink to ordinary work. The +fixtures require no provider credentials or relay. Run them on native Linux: +Rosetta may expose its own executable in `/proc//exe`, so emulated x86_64 +containers on ARM do not attest Linux helper identity. For native acceptance, run the patched harness with Codex, `--agents 5` and `--session-policy thread` (or the equivalent desktop settings). diff --git a/crates/buzz-acp/src/pool/background_work_tests.rs b/crates/buzz-acp/src/pool/background_work_tests.rs index d4daa994c21..97b183e6878 100644 --- a/crates/buzz-acp/src/pool/background_work_tests.rs +++ b/crates/buzz-acp/src/pool/background_work_tests.rs @@ -3,7 +3,7 @@ use crate::queue::{EventQueue, QueuedEvent}; use nostr::{EventBuilder, Keys, Kind}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; -#[cfg(target_os = "macos")] +#[cfg(any(target_os = "macos", target_os = "linux"))] #[path = "resident_helper_tests.rs"] mod resident_helper_tests; diff --git a/crates/buzz-acp/src/pool/resident_helper_tests.rs b/crates/buzz-acp/src/pool/resident_helper_tests.rs index 128831bea30..de114b8b8d1 100644 --- a/crates/buzz-acp/src/pool/resident_helper_tests.rs +++ b/crates/buzz-acp/src/pool/resident_helper_tests.rs @@ -20,9 +20,13 @@ for line in sys.stdin: assert host.stdout.readline().strip() == 'ready' text = req['params']['prompt'][0]['text'] if text == 'work': - host.stdin.write('sleep 60 & echo $!\n') + host.stdin.write('python3 -c \'import time; print("work-ready", flush=True); time.sleep(60)\' & echo $!\n') host.stdin.flush() - child = int(host.stdout.readline()) + # Ack from the child after exec: a freshly forked shell can still + # have the helper executable identity before becoming real work. + started = {host.stdout.readline().strip(), host.stdout.readline().strip()} + assert 'work-ready' in started + child = int(next(value for value in started if value != 'work-ready')) elif text == 'argument': child = subprocess.Popen(['/bin/bash', '-c', 'read -r line', sys.argv[1]], stdin=subprocess.PIPE) elif text == 'stop': @@ -66,7 +70,11 @@ fn helper_executable(dir: &tempfile::TempDir) -> std::path::PathBuf { .join("Application Support") .join("codex-code-mode-host"); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - // Launch a real executable under this name; no argv-only impersonation. + // Linux confirms the kernel executable path, so a symlink to bash would + // correctly remain ordinary work. Copy the executable for that fixture. + #[cfg(target_os = "linux")] + std::fs::copy("/bin/bash", &path).unwrap(); + #[cfg(target_os = "macos")] std::os::unix::fs::symlink("/bin/bash", &path).unwrap(); path } @@ -162,3 +170,89 @@ async fn work_mentioning_codex_host_in_arguments_still_holds_slot() { return_after_turn(&mut pool, agent, scope()); assert!(pool.try_claim(Some(&scope())).is_none()); } + +#[cfg(target_os = "linux")] +#[tokio::test] +async fn symlink_named_codex_host_does_not_hide_ordinary_work() { + let dir = tempfile::tempdir().unwrap(); + let executable = dir.path().join("codex-code-mode-host"); + std::os::unix::fs::symlink("/bin/bash", &executable).unwrap(); + let mut agent = helper_worker(0, &executable).await; + helper_prompt(&mut agent, "idle").await; + assert_eq!(agent.acp.background_child_pids().unwrap().len(), 1); + let mut pool = AgentPool::from_slots(vec![None]); + return_after_turn(&mut pool, agent, scope()); + assert!(pool.try_claim(Some(&scope())).is_none()); +} + +#[cfg(target_os = "linux")] +#[tokio::test] +async fn unlinked_codex_host_does_not_retain_finished_turn() { + let dir = tempfile::tempdir().unwrap(); + let executable = helper_executable(&dir); + let mut agent = helper_worker(0, &executable).await; + helper_prompt(&mut agent, "idle").await; + std::fs::remove_file(&executable).unwrap(); + let mut pool = AgentPool::from_slots(vec![None]); + return_after_turn(&mut pool, agent, scope()); + assert!(pool.try_claim(Some(&scope())).is_some()); +} + +#[tokio::test] +async fn child_of_codex_host_queues_original_collection_before_releasing_slot() { + let dir = tempfile::tempdir().unwrap(); + let executable = helper_executable(&dir); + let agent = helper_worker(0, &executable).await; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + let original = scope(); + bind(&mut pool, &original, 0); + let mut agent = pool.try_claim(Some(&original)).unwrap(); + agent + .state + .sessions + .insert(original.clone(), "helper-session".into()); + helper_prompt(&mut agent, "work").await; + let generation = agent.state.scope_owner_generations[&original]; + let delivery_key = Uuid::new_v4(); + agent.background_context = Some(RuntimeHold { + scope: original.clone(), + blocking_pids: HashSet::new(), + collection_pending: false, + continuation: Some(crate::queue::RuntimeContinuation { + delivery_key, + ordinary_preempted: false, + worker_index: 0, + session_id: "helper-session".into(), + generation, + thread_tags: ThreadTags { + root_event_id: Some("a".repeat(64)), + parent_event_id: Some("b".repeat(64)), + ..Default::default() + }, + received_at: std::time::Instant::now(), + }), + }); + pool.return_agent(agent); + let mut queue = EventQueue::new(DedupMode::Queue); + pool.queue_finished_background_work(&mut queue); + assert!(!queue.has_pending_continuation(&original)); + assert!(pool.try_claim(Some(&scope())).is_none()); + helper_prompt(pool.agents[0].as_mut().unwrap(), "stop").await; + pool.queue_finished_background_work(&mut queue); + let batch = queue.flush_next().expect("child exit queues collection"); + assert_eq!(batch.scope, original); + let continuation = batch.continuation.unwrap(); + assert_eq!(continuation.delivery_key, delivery_key); + assert_eq!(continuation.session_id, "helper-session"); + assert_eq!(continuation.generation, generation); + assert_eq!( + continuation.thread_tags.parent_event_id, + Some("b".repeat(64)) + ); + assert!(pool.try_claim(Some(&scope())).is_none()); + let mut agent = pool.try_claim(Some(&original)).unwrap(); + agent.finish_background_collection(); + queue.mark_complete(&original); + pool.return_agent(agent); + assert!(pool.try_claim(Some(&scope())).is_some()); +} diff --git a/crates/buzz-acp/src/process_tree.rs b/crates/buzz-acp/src/process_tree.rs index eac549bdcba..2ff4e020968 100644 --- a/crates/buzz-acp/src/process_tree.rs +++ b/crates/buzz-acp/src/process_tree.rs @@ -119,13 +119,13 @@ pub fn snapshot() -> Option { 4 * 1024 * 1024, )?; let tree = parse_snapshot(&output)?; - #[cfg(target_os = "macos")] - let tree = macos_helpers::classify(tree); + #[cfg(any(target_os = "macos", target_os = "linux"))] + let tree = codex_helpers::classify(tree); Some(tree) } -#[cfg(target_os = "macos")] -mod macos_helpers; +#[cfg(any(target_os = "macos", target_os = "linux"))] +mod codex_helpers; #[cfg(unix)] fn parse_snapshot(output: &str) -> Option { diff --git a/crates/buzz-acp/src/process_tree/macos_helpers.rs b/crates/buzz-acp/src/process_tree/codex_helpers.rs similarity index 66% rename from crates/buzz-acp/src/process_tree/macos_helpers.rs rename to crates/buzz-acp/src/process_tree/codex_helpers.rs index bd71b96b123..3c2e95416b8 100644 --- a/crates/buzz-acp/src/process_tree/macos_helpers.rs +++ b/crates/buzz-acp/src/process_tree/codex_helpers.rs @@ -1,6 +1,7 @@ //! Codex starts its code-mode service lazily, after session setup. Its lifetime -//! is not a background task. macOS `ps comm` preserves its executable path, -//! including spaces, without confusing arguments of other work for helpers. +//! is not a background task. Confirm the full executable path without treating +//! arguments of ordinary work as identity: macOS uses `comm`, Linux uses `exe`. +//! Linux `comm` truncates long names; `exe` comes from the kernel executable link. use super::*; const CODE_MODE_HOST: &str = "codex-code-mode-host"; @@ -28,7 +29,10 @@ pub(super) fn classify(mut tree: ProcessTree) -> ProcessTree { .collect::>() .join(","); let mut command = tokio::process::Command::new("/bin/ps"); + #[cfg(target_os = "macos")] command.args(["-p", &pids, "-o", "pid=,comm="]); + #[cfg(target_os = "linux")] + command.args(["-ww", "-p", &pids, "-o", "pid=,exe="]); let output = bounded_output(command, std::time::Duration::from_millis(250), 256 * 1024); tree.resident_helpers = confirmed_hosts(output.as_deref(), &candidates); tree @@ -45,9 +49,11 @@ fn confirmed_hosts(output: Option<&str>, candidates: &HashSet) -> HashSet