From 2e8fbe999de78702046a5cf7063bef930f3121e6 Mon Sep 17 00:00:00 2001 From: Andrey Bruhanov Date: Fri, 16 Oct 2026 12:46:19 +0200 Subject: [PATCH 1/3] fix(acp): release completed macOS Codex worker slots Signed-off-by: Andrey Bruhanov --- FORK_PATCHES.md | 26 ++- crates/buzz-acp/TESTING.md | 21 +++ crates/buzz-acp/src/acp.rs | 9 +- .../src/pool/background_work_tests.rs | 4 + .../src/pool/resident_helper_tests.rs | 164 ++++++++++++++++++ crates/buzz-acp/src/process_tree.rs | 42 +++-- .../src/process_tree/macos_helpers.rs | 83 +++++++++ scripts/fork/patches.json | 51 +++++- 8 files changed, 376 insertions(+), 24 deletions(-) create mode 100644 crates/buzz-acp/src/pool/resident_helper_tests.rs create mode 100644 crates/buzz-acp/src/process_tree/macos_helpers.rs diff --git a/FORK_PATCHES.md b/FORK_PATCHES.md index 3d98511a9dd..4ac1f351187 100644 --- a/FORK_PATCHES.md +++ b/FORK_PATCHES.md @@ -5459,7 +5459,7 @@ New module: `false`. ## `crates/buzz-acp/src/process_tree.rs` -Inherited integration inventory: bounded process snapshots and resident-helper classification keep live task children out of idle-pool teardown. +Bounded process snapshots and resident-helper classification; macOS executable confirmation releases completed Codex turns while preserving actual child work. New module: `true`. @@ -5467,6 +5467,8 @@ New module: `true`. - Required symbol: `pub fn blocking_pids(` - Required symbol: `pub fn is_resident_helper(` - Invocation: `crates/buzz-acp/src/acp.rs` → `snapshot`; exact call `let tree = crate::process_tree::snapshot()?;`; behavior test `prompt_work_is_tracked_under_worker_and_resident_provider`; 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 prompt_work_is_tracked_under_worker_and_resident_provider -- --test-threads=1` +- Invocation: `crates/buzz-acp/src/acp.rs` → `blocking_pids`; exact call `blocking.extend(tree.blocking_pids(parent, &self.resident_pids));`; behavior test `work_under_late_codex_host_holds_slot_until_child_exits`; 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/acp.rs` → `is_resident_helper`; exact call `self.resident_pids.contains(&child.pid) || tree.is_resident_helper(child)`; behavior test `work_under_late_codex_host_holds_slot_until_child_exits`; 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 process_tree -- --test-threads=1` ## `crates/buzz-acp/src/process_tree/snapshot_tests.rs` @@ -5808,3 +5810,25 @@ New module: `false`. - Required symbol: `tokio::pin!(receive);` - 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` + +Confirm late Codex code-mode host executable identity on macOS with one bounded ps call; arguments, missing identity, and truncated names 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` +- 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. + +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` +- 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` diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index ce6ee0b5d45..b779d111d28 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -94,6 +94,27 @@ 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 + +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. + +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. + +For native acceptance, run the patched harness with Codex, `--agents 5` and +`--session-policy thread` (or the equivalent desktop settings). +Complete a short task using code-mode tools in each of six distinct threads. +The sixth must start without restarting the agent. Then leave a real background +child running under code mode: its original worker must remain protected until +exit, and collection must still return to the original thread. This check needs +the rebuilt harness; source tests do not update the installed desktop app. + ## Git bootstrap The credential fixture requires Git 2.46 or newer for the `authtype` credential diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index b8583e8bafc..d6969408738 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -677,14 +677,9 @@ impl AcpClient { continue; } let children = tree.children(parent); - blocking.extend(crate::process_tree::blocking_pids( - children, - &self.resident_pids, - )); + blocking.extend(tree.blocking_pids(parent, &self.resident_pids)); for child in children { - if self.resident_pids.contains(&child.pid) - || crate::process_tree::is_resident_helper(&child.command) - { + if self.resident_pids.contains(&child.pid) || tree.is_resident_helper(child) { self.resident_pids.insert(child.pid); pending.push(child.pid); } diff --git a/crates/buzz-acp/src/pool/background_work_tests.rs b/crates/buzz-acp/src/pool/background_work_tests.rs index 24fe02b56fd..d4daa994c21 100644 --- a/crates/buzz-acp/src/pool/background_work_tests.rs +++ b/crates/buzz-acp/src/pool/background_work_tests.rs @@ -3,6 +3,10 @@ use crate::queue::{EventQueue, QueuedEvent}; use nostr::{EventBuilder, Keys, Kind}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; +#[cfg(target_os = "macos")] +#[path = "resident_helper_tests.rs"] +mod resident_helper_tests; + async fn worker(index: usize) -> OwnedAgent { OwnedAgent { index, diff --git a/crates/buzz-acp/src/pool/resident_helper_tests.rs b/crates/buzz-acp/src/pool/resident_helper_tests.rs new file mode 100644 index 00000000000..128831bea30 --- /dev/null +++ b/crates/buzz-acp/src/pool/resident_helper_tests.rs @@ -0,0 +1,164 @@ +//! Reproduce late Codex service residency through real ACP and pool return. +use super::*; + +const ADAPTER: &str = r#" +import json, subprocess, sys +host = None +child = None +for line in sys.stdin: + req = json.loads(line) + method = req['method'] + if method == 'initialize': + result = {'protocolVersion': 1, 'agentCapabilities': {}} + elif method == 'session/new': + result = {'sessionId': 'helper-session'} + elif method == 'session/prompt': + if host is None: + host = subprocess.Popen([sys.argv[1]], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True) + host.stdin.write('echo ready\n') + host.stdin.flush() + assert host.stdout.readline().strip() == 'ready' + text = req['params']['prompt'][0]['text'] + if text == 'work': + host.stdin.write('sleep 60 & echo $!\n') + host.stdin.flush() + child = int(host.stdout.readline()) + elif text == 'argument': + child = subprocess.Popen(['/bin/bash', '-c', 'read -r line', sys.argv[1]], stdin=subprocess.PIPE) + elif text == 'stop': + host.stdin.write('kill %d; wait %d; echo stopped\n' % (child, child)) + host.stdin.flush() + assert host.stdout.readline().strip() == 'stopped' + result = {'stopReason': 'end_turn'} + else: + continue + print(json.dumps({'jsonrpc': '2.0', 'id': req['id'], 'result': result}), flush=True) +"#; + +async fn helper_worker(index: usize, executable: &std::path::Path) -> OwnedAgent { + let mut agent = worker(index).await; + agent.acp = AcpClient::spawn( + "python3", + &[ + "-u".into(), + "-c".into(), + ADAPTER.into(), + executable.to_string_lossy().into_owned(), + ], + &[], + false, + ) + .await + .unwrap(); + agent.acp.initialize().await.unwrap(); + agent.acp.capture_startup_children(); + agent + .acp + .session_new("/tmp", vec![], None, None) + .await + .unwrap(); + agent +} + +fn helper_executable(dir: &tempfile::TempDir) -> std::path::PathBuf { + let path = dir + .path() + .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. + std::os::unix::fs::symlink("/bin/bash", &path).unwrap(); + path +} + +async fn helper_prompt(agent: &mut OwnedAgent, text: &str) { + assert_eq!( + agent + .acp + .session_prompt_with_idle_timeout( + "helper-session", + text, + Duration::from_secs(2), + Duration::from_secs(5) + ) + .await + .unwrap(), + StopReason::EndTurn, + ); +} + +fn return_after_turn(pool: &mut AgentPool, mut agent: OwnedAgent, scope: SessionScope) { + agent.background_context = Some(RuntimeHold { + scope, + blocking_pids: HashSet::new(), + continuation: None, + collection_pending: false, + }); + pool.return_agent(agent); +} + +#[tokio::test] +async fn six_finished_threads_reuse_five_slots_with_late_codex_hosts() { + let dir = tempfile::tempdir().unwrap(); + let executable = helper_executable(&dir); + let mut slots = Vec::new(); + for index in 0..5 { + slots.push(Some(helper_worker(index, &executable).await)); + } + let mut pool = AgentPool::from_slots(slots); + for turn in 0..6 { + let scope = scope(); + let mut agent = pool.try_claim(Some(&scope)).unwrap_or_else(|| { + panic!( + "finished threads exhausted the pool before turn {}", + turn + 1 + ) + }); + helper_prompt(&mut agent, "idle").await; + return_after_turn(&mut pool, agent, scope); + } + assert!( + !pool.has_runtime_hold(), + "service processes must not retain completed turns" + ); +} + +#[tokio::test] +async fn work_under_late_codex_host_holds_slot_until_child_exits() { + 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(); + let mut agent = pool.try_claim(Some(&original)).unwrap(); + helper_prompt(&mut agent, "work").await; + return_after_turn(&mut pool, agent, original); + assert!( + pool.try_claim(Some(&scope())).is_none(), + "real child must retain the slot" + ); + let agent = pool.agents[0].as_mut().unwrap(); + helper_prompt(agent, "stop").await; + pool.queue_finished_background_work(&mut EventQueue::new(DedupMode::Queue)); + assert!( + pool.try_claim(Some(&scope())).is_some(), + "child exit must release the slot while host stays alive" + ); +} + +#[tokio::test] +async fn work_mentioning_codex_host_in_arguments_still_holds_slot() { + let dir = tempfile::tempdir().unwrap(); + let executable = helper_executable(&dir); + let mut agent = helper_worker(0, &executable).await; + helper_prompt(&mut agent, "argument").await; + let blocking = agent.acp.background_child_pids().unwrap(); + assert_eq!( + blocking.len(), + 1, + "ignore the host, but retain shell work mentioning its path" + ); + let mut pool = AgentPool::from_slots(vec![None]); + return_after_turn(&mut pool, agent, scope()); + assert!(pool.try_claim(Some(&scope())).is_none()); +} diff --git a/crates/buzz-acp/src/process_tree.rs b/crates/buzz-acp/src/process_tree.rs index 21b8bfc8748..eac549bdcba 100644 --- a/crates/buzz-acp/src/process_tree.rs +++ b/crates/buzz-acp/src/process_tree.rs @@ -63,26 +63,28 @@ pub fn parse_ps_line(line: &str) -> Option<(u32, u32, String)> { Some((pid, ppid, command)) } -/// Pids that should keep the worker out of idle-pool sleep. -/// -/// `resident` holds children captured at worker start and helpers adopted -/// since (MCP). Everything else still parented by the worker is background -/// work: a subagent or a shell the turn left running. -pub fn blocking_pids(children: &[ChildProcess], resident: &HashSet) -> HashSet { - children - .iter() - .filter(|child| !resident.contains(&child.pid) && !is_resident_helper(&child.command)) - .map(|child| child.pid) - .collect() -} - /// A single process snapshot, shared across a complete classification pass. #[derive(Default)] pub struct ProcessTree { children: HashMap>, + resident_helpers: HashSet, } impl ProcessTree { + /// Direct children that are work, excluding startup residents and helpers. + pub fn blocking_pids(&self, parent: u32, resident: &HashSet) -> HashSet { + self.children(parent) + .iter() + .filter(|child| !resident.contains(&child.pid) && !self.is_resident_helper(child)) + .map(|child| child.pid) + .collect() + } + + /// Helpers confirmed by executable identity, or the existing MCP classifier. + pub fn is_resident_helper(&self, child: &ChildProcess) -> bool { + self.resident_helpers.contains(&child.pid) || is_resident_helper(&child.command) + } + /// Direct children in this snapshot. pub fn children(&self, parent: u32) -> &[ChildProcess] { self.children.get(&parent).map(Vec::as_slice).unwrap_or(&[]) @@ -116,9 +118,15 @@ pub fn snapshot() -> Option { std::time::Duration::from_millis(250), 4 * 1024 * 1024, )?; - parse_snapshot(&output) + let tree = parse_snapshot(&output)?; + #[cfg(target_os = "macos")] + let tree = macos_helpers::classify(tree); + Some(tree) } +#[cfg(target_os = "macos")] +mod macos_helpers; + #[cfg(unix)] fn parse_snapshot(output: &str) -> Option { let mut tree = ProcessTree::default(); @@ -259,7 +267,11 @@ mod tests { ]; let mut resident = HashSet::new(); resident.insert(4); - let blocking = blocking_pids(&children, &resident); + let tree = ProcessTree { + children: HashMap::from([(99, children)]), + ..Default::default() + }; + let blocking = tree.blocking_pids(99, &resident); assert_eq!(blocking.len(), 2); assert!(blocking.contains(&2)); assert!(blocking.contains(&3)); diff --git a/crates/buzz-acp/src/process_tree/macos_helpers.rs b/crates/buzz-acp/src/process_tree/macos_helpers.rs new file mode 100644 index 00000000000..bd71b96b123 --- /dev/null +++ b/crates/buzz-acp/src/process_tree/macos_helpers.rs @@ -0,0 +1,83 @@ +//! 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. +use super::*; + +const CODE_MODE_HOST: &str = "codex-code-mode-host"; + +pub(super) fn classify(mut tree: ProcessTree) -> ProcessTree { + let candidates: HashSet = tree + .children + .values() + .flatten() + .filter(|child| { + child + .command + .split_whitespace() + .any(|word| word.rsplit('/').next() == Some(CODE_MODE_HOST)) + }) + .map(|child| child.pid) + .collect(); + // Bound argv as well as time/output. Uncertain enumeration keeps work held. + if candidates.is_empty() || candidates.len() > 128 { + return tree; + } + let pids = candidates + .iter() + .map(u32::to_string) + .collect::>() + .join(","); + let mut command = tokio::process::Command::new("/bin/ps"); + command.args(["-p", &pids, "-o", "pid=,comm="]); + let output = bounded_output(command, std::time::Duration::from_millis(250), 256 * 1024); + tree.resident_helpers = confirmed_hosts(output.as_deref(), &candidates); + tree +} + +fn confirmed_hosts(output: Option<&str>, candidates: &HashSet) -> HashSet { + let Some(output) = output else { + return HashSet::new(); + }; + output + .lines() + .filter_map(|line| { + let (pid, executable) = line.trim().split_once(char::is_whitespace)?; + let pid = pid.parse().ok()?; + // Do not collapse whitespace inside an executable path. No filesystem + // lookup here: a mounted filesystem must not stall the pool poll. + let name = std::path::Path::new(executable.trim()) + .file_name()? + .to_str()?; + (candidates.contains(&pid) && name == CODE_MODE_HOST).then_some(pid) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn executable_identity_handles_spaces_and_rejects_work_arguments() { + let candidates = HashSet::from([10, 11, 12, 13]); + assert_eq!( + confirmed_hosts( + Some( + "10 /Applications/Application Support/bin/codex-code-mode-host\n\ + 11 /bin/cat\n12 /bin/bash\n13 /tmp/codex-code-mode-host-worker\n\ + 14 /tmp/codex-code-mode-host\n" + ), + &candidates + ), + HashSet::from([10]) + ); + } + + #[test] + fn failed_or_missing_executable_confirmation_keeps_work_blocking() { + let candidates = HashSet::from([10]); + for output in [None, Some(""), Some("invalid"), Some("10 codex-code-mode")] { + assert!(confirmed_hosts(output, &candidates).is_empty()); + } + } +} diff --git a/scripts/fork/patches.json b/scripts/fork/patches.json index 68c077e4407..cf555ac303e 100644 --- a/scripts/fork/patches.json +++ b/scripts/fork/patches.json @@ -7977,7 +7977,7 @@ }, { "path": "crates/buzz-acp/src/process_tree.rs", - "responsibility": "Inherited integration inventory: bounded process snapshots and resident-helper classification keep live task children out of idle-pool teardown.", + "responsibility": "Bounded process snapshots and resident-helper classification; macOS executable confirmation releases completed Codex turns while preserving actual child work.", "is_new": true, "required_symbols": [ "pub fn snapshot()", @@ -7992,6 +7992,20 @@ "behavior_test": "prompt_work_is_tracked_under_worker_and_resident_provider", "verification_command": "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 prompt_work_is_tracked_under_worker_and_resident_provider -- --test-threads=1", "coverage": "Inherited production process classification exercised with real local ACP children; no new runtime mutation claim." + }, + { + "caller_path": "crates/buzz-acp/src/acp.rs", + "callee_symbol": "blocking_pids", + "invocation": "blocking.extend(tree.blocking_pids(parent, &self.resident_pids));", + "behavior_test": "work_under_late_codex_host_holds_slot_until_child_exits", + "verification_command": "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" + }, + { + "caller_path": "crates/buzz-acp/src/acp.rs", + "callee_symbol": "is_resident_helper", + "invocation": "self.resident_pids.contains(&child.pid) || tree.is_resident_helper(child)", + "behavior_test": "work_under_late_codex_host_holds_slot_until_child_exits", + "verification_command": "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" } ], "verification_commands": [ @@ -8467,6 +8481,41 @@ "verification_commands": [ "cargo test -p buzz-relay -- --test-threads=1" ] + }, + { + "path": "crates/buzz-acp/src/process_tree/macos_helpers.rs", + "responsibility": "Confirm late Codex code-mode host executable identity on macOS with one bounded ps call; arguments, missing identity, and truncated names cannot free slots.", + "is_new": true, + "required_symbols": [ + "pub(super) fn classify(", + "fn confirmed_hosts(" + ], + "invocation_seams": [ + { + "caller_path": "crates/buzz-acp/src/process_tree.rs", + "callee_symbol": "classify", + "invocation": "let tree = macos_helpers::classify(tree);", + "behavior_test": "six_finished_threads_reuse_five_slots_with_late_codex_hosts", + "verification_command": "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" + } + ], + "verification_commands": [ + "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" + ] + }, + { + "path": "crates/buzz-acp/src/pool/resident_helper_tests.rs", + "responsibility": "Real local ACP/process regressions for sixth-thread admission, late resident host, nested child exit and work mentioning host in arguments on macOS.", + "is_new": true, + "required_symbols": [ + "six_finished_threads_reuse_five_slots_with_late_codex_hosts", + "work_under_late_codex_host_holds_slot_until_child_exits", + "work_mentioning_codex_host_in_arguments_still_holds_slot" + ], + "invocation_seams": [], + "verification_commands": [ + "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" + ] } ] } From e4d73a3223d9e91767f5ce45f0792014a81dd616 Mon Sep 17 00:00:00 2001 From: Andrey Bruhanov Date: Fri, 16 Oct 2026 13:45:02 +0200 Subject: [PATCH 2/3] fix(desktop): preserve open dialog focus during composer autofocus Signed-off-by: Andrey Bruhanov --- FORK_PATCHES.md | 23 +++++ .../lib/useComposerAutofocus.test.mjs | 89 +++++++++++++++++++ .../messages/lib/useComposerAutofocus.ts | 7 +- scripts/fork/patches.json | 37 ++++++++ 4 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 desktop/src/features/messages/lib/useComposerAutofocus.test.mjs diff --git a/FORK_PATCHES.md b/FORK_PATCHES.md index 4ac1f351187..fea2bc3b309 100644 --- a/FORK_PATCHES.md +++ b/FORK_PATCHES.md @@ -5832,3 +5832,26 @@ New module: `true`. - Required symbol: `work_under_late_codex_host_holds_slot_until_child_exits` - Required symbol: `work_mentioning_codex_host_in_arguments_still_holds_slot` - 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` + +Preserve focus inside an open dialog/popover when late composer readiness or draft changes trigger autofocus; fixes the reproduced huddle smoke CI blocker. + +New module: `false`. + +- Required symbol: `useComposerAutofocus` +- Required symbol: `active.closest('[role="dialog"]')` +- Invocation: `desktop/src/features/messages/ui/MessageComposer.tsx` → `useComposerAutofocus`; exact call ` useComposerAutofocus(richText.focus, effectiveDraftKey, composerDisabled);`; behavior test `late composer readiness preserves focus on a dialog switch; existing huddle assigns distinct agent voices E2E`; verify `cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs` +- Verify: `cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs` +- Verify: `just desktop-test desktop-typecheck` + +## `desktop/src/features/messages/lib/useComposerAutofocus.test.mjs` + +Production-hook regression for late readiness and draft-switch focus retention in dialogs, preserving ordinary navigation autofocus. + +New module: `true`. + +- Required symbol: `late composer readiness preserves focus on a dialog switch` +- Required symbol: `a draft change preserves focus on an open dialog button` +- Required symbol: `initial mount and navigation from an ordinary button still focus the editor` +- Verify: `cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs` diff --git a/desktop/src/features/messages/lib/useComposerAutofocus.test.mjs b/desktop/src/features/messages/lib/useComposerAutofocus.test.mjs new file mode 100644 index 00000000000..c0fd5d20fa5 --- /dev/null +++ b/desktop/src/features/messages/lib/useComposerAutofocus.test.mjs @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import { after, afterEach, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); +}); + +afterEach(async () => { + const { cleanup } = await import("@testing-library/react"); + cleanup(); + document.body.replaceChildren(); +}); + +after(() => dom.window.close()); + +async function mountAutofocus(focus, draftKey = "huddle") { + const React = await import("react"); + const { render } = await import("@testing-library/react"); + const { useComposerAutofocus } = await import("./useComposerAutofocus.ts"); + function Harness(props) { + useComposerAutofocus(props.focus, props.draftKey, false); + return null; + } + const view = render(React.createElement(Harness, { focus, draftKey })); + return (nextFocus, nextKey = draftKey) => + view.rerender( + React.createElement(Harness, { focus: nextFocus, draftKey: nextKey }), + ); +} + +function editorInput() { + const editor = document.createElement("input"); + document.body.append(editor); + return editor; +} + +test("late composer readiness preserves focus on a dialog switch", async () => { + const editor = editorInput(); + const dialog = document.createElement("div"); + dialog.setAttribute("role", "dialog"); + const control = document.createElement("button"); + control.setAttribute("role", "switch"); + dialog.append(control); + document.body.append(dialog); + + const rerender = await mountAutofocus(() => {}); + control.focus(); + rerender(() => editor.focus()); + assert.equal(document.activeElement, control); +}); + +test("a draft change preserves focus on an open dialog button", async () => { + const editor = editorInput(); + const dialog = document.createElement("div"); + dialog.setAttribute("role", "dialog"); + const control = document.createElement("button"); + dialog.append(control); + document.body.append(dialog); + + const focus = () => editor.focus(); + const rerender = await mountAutofocus(focus, "before"); + control.focus(); + rerender(focus, "huddle"); + assert.equal(document.activeElement, control); +}); + +test("initial mount and navigation from an ordinary button still focus the editor", async () => { + const editor = editorInput(); + const navigation = document.createElement("button"); + document.body.append(navigation); + const focus = () => editor.focus(); + const rerender = await mountAutofocus(focus, "before"); + assert.equal(document.activeElement, editor); + + navigation.focus(); + rerender(focus, "huddle"); + assert.equal(document.activeElement, editor); +}); diff --git a/desktop/src/features/messages/lib/useComposerAutofocus.ts b/desktop/src/features/messages/lib/useComposerAutofocus.ts index 1b6bbac79ef..6ba27c0a8cc 100644 --- a/desktop/src/features/messages/lib/useComposerAutofocus.ts +++ b/desktop/src/features/messages/lib/useComposerAutofocus.ts @@ -20,8 +20,8 @@ import * as React from "react"; * Guards: * - Skip if the composer is currently disabled (archived channel, no * channel, or in-flight send at the moment of mount). - * - Skip if focus already lives in another text-entry surface (open - * dialog input, search box, etc.) so we don't yank focus from the user. + * - Skip if focus already lives in another text-entry surface or an open + * dialog/popover so we don't yank focus from the user. */ export function useComposerAutofocus( focus: () => void, @@ -44,7 +44,8 @@ export function useComposerAutofocus( tag === "INPUT" || tag === "TEXTAREA" || tag === "SELECT" || - active.isContentEditable + active.isContentEditable || + active.closest('[role="dialog"]') ) { return; } diff --git a/scripts/fork/patches.json b/scripts/fork/patches.json index cf555ac303e..3c1eb38af31 100644 --- a/scripts/fork/patches.json +++ b/scripts/fork/patches.json @@ -8516,6 +8516,43 @@ "verification_commands": [ "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" ] + }, + { + "path": "desktop/src/features/messages/lib/useComposerAutofocus.ts", + "responsibility": "Preserve focus inside an open dialog/popover when late composer readiness or draft changes trigger autofocus; fixes the reproduced huddle smoke CI blocker.", + "is_new": false, + "required_symbols": [ + "useComposerAutofocus", + "active.closest('[role=\"dialog\"]')" + ], + "invocation_seams": [ + { + "caller_path": "desktop/src/features/messages/ui/MessageComposer.tsx", + "callee_symbol": "useComposerAutofocus", + "invocation": " useComposerAutofocus(richText.focus, effectiveDraftKey, composerDisabled);", + "behavior_test": "late composer readiness preserves focus on a dialog switch; existing huddle assigns distinct agent voices E2E", + "verification_command": "cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs", + "coverage": "Real React hook/DOM focus behavior and existing browser workflow RED/GREEN; removing the dialog guard fails two hook tests." + } + ], + "verification_commands": [ + "cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs", + "just desktop-test desktop-typecheck" + ] + }, + { + "path": "desktop/src/features/messages/lib/useComposerAutofocus.test.mjs", + "responsibility": "Production-hook regression for late readiness and draft-switch focus retention in dialogs, preserving ordinary navigation autofocus.", + "is_new": true, + "required_symbols": [ + "late composer readiness preserves focus on a dialog switch", + "a draft change preserves focus on an open dialog button", + "initial mount and navigation from an ordinary button still focus the editor" + ], + "invocation_seams": [], + "verification_commands": [ + "cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs" + ] } ] } From 7dcb952a7b8130a260a6516c781a50e8b1f32ced Mon Sep 17 00:00:00 2001 From: Andrey Bruhanov Date: Fri, 16 Oct 2026 14:26:27 +0200 Subject: [PATCH 3/3] test(desktop): track voice-note players across message reordering Signed-off-by: Andrey Bruhanov --- FORK_PATCHES.md | 11 +++++++++++ desktop/tests/e2e/voice-note.spec.ts | 22 ++++++++++++++-------- scripts/fork/patches.json | 14 ++++++++++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/FORK_PATCHES.md b/FORK_PATCHES.md index fea2bc3b309..958941d1e98 100644 --- a/FORK_PATCHES.md +++ b/FORK_PATCHES.md @@ -5855,3 +5855,14 @@ New module: `true`. - Required symbol: `a draft change preserves focus on an open dialog button` - Required symbol: `initial mount and navigation from an ordinary button still focus the editor` - Verify: `cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs` + +## `desktop/tests/e2e/voice-note.spec.ts` + +Bind duplicate voice-note playback assertions to stable emitted message IDs through live reordering; loop the short media fixture so natural completion cannot impersonate mutual pause. + +New module: `false`. + +- Required symbol: `const messageIds = await page.evaluate(` +- Required symbol: `const firstCard = page.locator(` +- Required symbol: `(player as HTMLAudioElement).loop = true;` +- Verify: `cd desktop && pnpm exec playwright test --project=smoke tests/e2e/voice-note.spec.ts --retries=0` diff --git a/desktop/tests/e2e/voice-note.spec.ts b/desktop/tests/e2e/voice-note.spec.ts index eadab504f55..f72283debf2 100644 --- a/desktop/tests/e2e/voice-note.spec.ts +++ b/desktop/tests/e2e/voice-note.spec.ts @@ -910,7 +910,7 @@ test("starting a duplicate voice-note player pauses the other instance", async ( await page.getByTestId("channel-general").click(); await waitForMockLiveSubscription(page, "general"); - await page.evaluate( + const messageIds = await page.evaluate( ({ audioUrl }) => { const emit = ( window as Window & { @@ -918,7 +918,7 @@ test("starting a duplicate voice-note player pauses the other instance", async ( channelName: string; content: string; extraTags: string[][]; - }) => unknown; + }) => { id: string }; } ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; if (!emit) throw new Error("Mock message emitter is unavailable."); @@ -935,20 +935,26 @@ test("starting a duplicate voice-note player pauses the other instance", async ( ], ], }; - emit(input); - emit(input); + return [emit(input).id, emit(input).id]; }, { audioUrl: AUDIO_URL }, ); const cards = page.getByTestId("audio-message-attachment"); await expect(cards).toHaveCount(2); - const firstAudio = cards.nth(0).locator("audio"); - const secondAudio = cards.nth(1).locator("audio"); + // Live messages can reorder after delivery; retain the identity we click. + const firstCard = page.locator(`[data-message-id="${messageIds[0]}"]`); + const secondCard = page.locator(`[data-message-id="${messageIds[1]}"]`); + const firstAudio = firstCard.locator("audio"); + const secondAudio = secondCard.locator("audio"); + // The one-second fixture must not end naturally and impersonate a pause. + await cards.locator("audio").evaluateAll((players) => { + for (const player of players) (player as HTMLAudioElement).loop = true; + }); - await cards.nth(0).getByRole("button", { name: "Play voice note" }).click(); + await firstCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", false); - await cards.nth(1).getByRole("button", { name: "Play voice note" }).click(); + await secondCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", true); await expect(secondAudio).toHaveJSProperty("paused", false); }); diff --git a/scripts/fork/patches.json b/scripts/fork/patches.json index 3c1eb38af31..8cdfd87de48 100644 --- a/scripts/fork/patches.json +++ b/scripts/fork/patches.json @@ -8553,6 +8553,20 @@ "verification_commands": [ "cd desktop && node --import ./test-loader.mjs --experimental-strip-types --test src/features/messages/lib/useComposerAutofocus.test.mjs" ] + }, + { + "path": "desktop/tests/e2e/voice-note.spec.ts", + "responsibility": "Bind duplicate voice-note playback assertions to stable emitted message IDs through live reordering; loop the short media fixture so natural completion cannot impersonate mutual pause.", + "is_new": false, + "required_symbols": [ + "const messageIds = await page.evaluate(", + "const firstCard = page.locator(", + "(player as HTMLAudioElement).loop = true;" + ], + "invocation_seams": [], + "verification_commands": [ + "cd desktop && pnpm exec playwright test --project=smoke tests/e2e/voice-note.spec.ts --retries=0" + ] } ] }