From b89ed55499eb83c9c8d0f07109b6e23fea706860 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Wed, 26 Aug 2026 16:41:23 -0400 Subject: [PATCH 01/22] feat(agents): bundle native monitoring tools Signed-off-by: John Tennant --- distro/skills/berd-monitor/SKILL.md | 52 + distro/skills/berd-orchestrator/SKILL.md | 28 + justfile | 8 +- scripts/prepare-berdctl-sidecar.sh | 27 +- scripts/windows/Dev-Windows.ps1 | 7 + scripts/windows/Stage-Sidecar-Windows.ps1 | 11 +- scripts/windows/Test-WindowsDev.ps1 | 2 + src-tauri/Cargo.lock | 33 + src-tauri/Cargo.toml | 7 +- src-tauri/crates/berd-monitor/Cargo.toml | 22 + src-tauri/crates/berd-monitor/src/main.rs | 1102 +++++++++++++++++ .../crates/berdctl/api-surface-feedback.json | 17 +- src-tauri/crates/berdctl/api-surface.json | 17 +- .../crates/berdctl/cli-surface-feedback.json | 2 +- src-tauri/crates/berdctl/cli-surface.json | 2 +- src-tauri/src/services/acp/goose_serve.rs | 77 +- src-tauri/tauri.conf.json | 7 +- src-tauri/tauri.windows.conf.json | 6 +- .../__tests__/commands/commands.test.ts | 53 + .../bridge/useBerdctlQueuedMessageDrain.ts | 8 +- .../berdctl/commands/impl/sendSession.ts | 36 +- .../berdctl/commands/runtime/sessionSend.ts | 16 +- src/features/chat/ui/ChatInput.tsx | 6 +- src/features/chat/ui/MessageBubble.tsx | 9 +- .../chat/ui/__tests__/MessageBubble.test.tsx | 16 + .../api/__tests__/acpReplayMetadata.test.ts | 16 + src/shared/api/acpReplayMetadata.ts | 19 + src/shared/i18n/locales/en/chat.json | 1 + src/shared/i18n/locales/es/chat.json | 1 + src/shared/types/messages.ts | 1 + 30 files changed, 1564 insertions(+), 45 deletions(-) create mode 100644 distro/skills/berd-monitor/SKILL.md create mode 100644 distro/skills/berd-orchestrator/SKILL.md create mode 100644 src-tauri/crates/berd-monitor/Cargo.toml create mode 100644 src-tauri/crates/berd-monitor/src/main.rs diff --git a/distro/skills/berd-monitor/SKILL.md b/distro/skills/berd-monitor/SKILL.md new file mode 100644 index 000000000..fc199b26a --- /dev/null +++ b/distro/skills/berd-monitor/SKILL.md @@ -0,0 +1,52 @@ +--- +name: berd-monitor +description: >- + Run a long-lived command outside the current Berd turn and wake the owning + session with actionable stdout. Use for builds, tests, reviews, deployments, + polling, watchers, and other external waits. +metadata: + berdBundled: true +--- + +# Berd Monitor + +Use `berd-monitor` when a command may outlive the current turn. It detaches the +producer from Berd and the agent harness, buffers output across delivery +failures, and sends complete stdout lines to the exact originating session. + +Run short, bounded commands normally. Do not hold a turn open with `sleep`, +polling, log tailing, or a foreground timeout. + +## Start + +```bash +berd-monitor run \ + --state-key \ + --label '' \ + --instructions '' \ + -- [args...] +``` + +`AGENT_SESSION_ID` selects the current session. Use `--session-id` only for +another positively identified session; never infer a session from the working +directory. The command prints the detached monitor PID. After checking the +monitor's `watcher.log` under the printed state directory when diagnosis is +needed, continue other work or end the turn—never wait on the detached PID. + +The producer's stdout is the event API: + +- emit concise, newline-terminated milestones and flush promptly; +- put verbose output and diagnostics in durable logs or stderr; +- avoid NUL bytes and emit a final summary when practical. + +Default `--if-running steer` adds an event to an active run or starts a new +turn. Use `--if-running queue` only when the active run must not be steered. +Pending events retry without being dropped, and a trailing partial line is +delivered when the producer exits. + +Delivered messages are visibly labeled as coming from `berd-monitor`. Stop a +monitor with: + +```bash +berd-monitor stop --state-key +``` diff --git a/distro/skills/berd-orchestrator/SKILL.md b/distro/skills/berd-orchestrator/SKILL.md new file mode 100644 index 000000000..b8135266c --- /dev/null +++ b/distro/skills/berd-orchestrator/SKILL.md @@ -0,0 +1,28 @@ +--- +name: berd-orchestrator +description: >- + Coordinate work across Berd sessions while keeping one conversation + available to the user. Use for a long-lived orchestration session. +metadata: + berdBundled: true +--- + +# Berd Orchestrator + +Keep this session available for conversation. Use `berdctl session list`, +`get`, `create`, and `send` to delegate substantial work to other sessions +instead of doing it here. + +Prefer an existing session that owns the relevant context. Create one when +work has no owner, and give it a complete task. Do not approve, merge, or +archive work without the user's direction. + +When sending between sessions, pass `--from ''` +so the receiving transcript explains where the message came from. + +Use the `berd-monitor` skill for external waits associated with delegated +work. End the turn after starting a monitor; do not poll in the foreground. + +Stay quiet while work is progressing. When attention is needed, either +continue an obvious next step or bring the user one result or decision with a +`[session link](berd://session/)`. diff --git a/justfile b/justfile index ca853fac1..350803bac 100644 --- a/justfile +++ b/justfile @@ -498,16 +498,18 @@ dev: export VITE_APP_VERSION="$BERD_APP_VERSION_RICH" echo "Using app version: ${BERD_APP_VERSION} (${BERD_APP_VERSION_RICH})" - # tauri dev only builds the root package; the berdctl CLI workspace - # member needs an explicit build, resolved at runtime via BERDCTL_BIN - # because tauri.dev.conf.json blanks externalBin. + # tauri dev only builds the root package; the agent-facing CLI workspace + # members need explicit builds because tauri.dev.conf.json blanks externalBin. BERDCTL_FEATURES=() [[ "${VITE_FEEDBACK:-0}" == "1" ]] && BERDCTL_FEATURES+=(--features block-feedback) # ${arr[@]+...} guards the empty-array expansion, which bash 3.2 (stock # macOS) treats as an unbound variable under `set -u`. (cd src-tauri && cargo build -p berdctl ${BERDCTL_FEATURES[@]+"${BERDCTL_FEATURES[@]}"}) + (cd src-tauri && cargo build -p berd-monitor) export BERDCTL_BIN="${CARGO_TARGET_DIR}/debug/berdctl" + export BERD_MONITOR_BIN="${CARGO_TARGET_DIR}/debug/berd-monitor" echo "Using berdctl CLI: ${BERDCTL_BIN}" + echo "Using berd-monitor CLI: ${BERD_MONITOR_BIN}" if [[ "${VITE_AGENT_TOOLS:-0}" == "1" ]]; then ./scripts/prepare-bb-cli-resource.sh diff --git a/scripts/prepare-berdctl-sidecar.sh b/scripts/prepare-berdctl-sidecar.sh index e91e8a447..0f9e4838e 100755 --- a/scripts/prepare-berdctl-sidecar.sh +++ b/scripts/prepare-berdctl-sidecar.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Build and stage the berdctl CLI for Tauri's externalBin bundling. +# Build and stage the berdctl and berd-monitor CLIs for Tauri externalBin bundling. # # Tauri expects external binaries to be present at build time with the target # triple appended to the configured stem. For config @@ -13,8 +13,9 @@ usage() { cat <<'USAGE' Usage: scripts/prepare-berdctl-sidecar.sh [target-triple] -Builds the berdctl workspace crate in release mode and copies the binary -into src-tauri/binaries with the target triple suffix required by Tauri. +Builds the berdctl and berd-monitor workspace crates in release mode and +copies both binaries into src-tauri/binaries with the target triple suffix +required by Tauri. The triple defaults to the rustc host. Pass it explicitly (or set BERDCTL_TRIPLE) when the Tauri build itself uses an explicit --target, so @@ -29,9 +30,9 @@ if [[ "${1:-}" == "-h" || "${1:-}" == "--help" ]]; then fi EXPLICIT_TRIPLE="${1:-${BERDCTL_TRIPLE:-}}" -CARGO_ARGS=(build -p berdctl --release) +CARGO_ARGS=(build -p berdctl -p berd-monitor --release) if [[ "${VITE_FEEDBACK:-0}" == "1" ]]; then - CARGO_ARGS+=(--features block-feedback) + CARGO_ARGS+=(--features berdctl/block-feedback) fi if [[ -n "$EXPLICIT_TRIPLE" ]]; then TRIPLE="$EXPLICIT_TRIPLE" @@ -75,3 +76,19 @@ mkdir -p "$OUT_DIR" cp "$BUILT" "$OUT" chmod +x "$OUT" echo "Staged berdctl sidecar: $OUT" + +if [[ -n "$EXPLICIT_TRIPLE" ]]; then + MONITOR_BUILT="$TARGET_DIR/$TRIPLE/release/berd-monitor" +else + MONITOR_BUILT="$TARGET_DIR/release/berd-monitor" +fi + +if [[ ! -x "$MONITOR_BUILT" ]]; then + echo "Built berd-monitor binary not found at: $MONITOR_BUILT" >&2 + exit 1 +fi + +MONITOR_OUT="$OUT_DIR/berd-monitor-$TRIPLE" +cp "$MONITOR_BUILT" "$MONITOR_OUT" +chmod +x "$MONITOR_OUT" +echo "Staged berd-monitor sidecar: $MONITOR_OUT" diff --git a/scripts/windows/Dev-Windows.ps1 b/scripts/windows/Dev-Windows.ps1 index 63a4d0149..12edb6db4 100644 --- a/scripts/windows/Dev-Windows.ps1 +++ b/scripts/windows/Dev-Windows.ps1 @@ -124,6 +124,13 @@ if (-not (Test-Path $env:BERDCTL_BIN -PathType Leaf)) { } Write-WindowsDevInfo "Using berdctl CLI: $env:BERDCTL_BIN" +Invoke-CheckedCommand -FilePath "cargo" -ArgumentList @("build", "-p", "berd-monitor") -WorkingDirectory (Join-Path (Get-BerdRepoRoot) "src-tauri") -Label "cargo build berd-monitor" +$env:BERD_MONITOR_BIN = Join-Path (Join-Path $env:CARGO_TARGET_DIR "debug") "berd-monitor.exe" +if (-not (Test-Path $env:BERD_MONITOR_BIN -PathType Leaf)) { + throw "Expected berd-monitor.exe at $env:BERD_MONITOR_BIN after cargo build." +} +Write-WindowsDevInfo "Using berd-monitor CLI: $env:BERD_MONITOR_BIN" + if ([string]::IsNullOrWhiteSpace($env:GOOSE_BIN)) { $env:GOOSE_BUILD_PROFILE = "debug" $result = Invoke-EnsureLocalGoose -Action Check diff --git a/scripts/windows/Stage-Sidecar-Windows.ps1 b/scripts/windows/Stage-Sidecar-Windows.ps1 index e7542af02..bf8225564 100644 --- a/scripts/windows/Stage-Sidecar-Windows.ps1 +++ b/scripts/windows/Stage-Sidecar-Windows.ps1 @@ -71,9 +71,9 @@ Write-WindowsDevInfo "Staged Goose sidecar: $staged" $tauriTargetDir = Get-TauriCargoTargetDir $env:CARGO_TARGET_DIR = $tauriTargetDir $hostTriple = Get-RustHostTriple -$cargoArgs = @("build", "-p", "berdctl", "--release") +$cargoArgs = @("build", "-p", "berdctl", "-p", "berd-monitor", "--release") if ($env:VITE_FEEDBACK -eq "1") { - $cargoArgs += @("--features", "block-feedback") + $cargoArgs += @("--features", "berdctl/block-feedback") } if (-not [string]::IsNullOrWhiteSpace($hostTriple) -and $Triple -ne $hostTriple) { $cargoArgs += @("--target", $Triple) @@ -82,10 +82,15 @@ if (-not [string]::IsNullOrWhiteSpace($hostTriple) -and $Triple -ne $hostTriple) $berdctlReleaseDir = Join-Path $tauriTargetDir "release" } Invoke-CheckedCommand -FilePath "cargo" -ArgumentList $cargoArgs ` - -WorkingDirectory (Join-Path (Get-BerdRepoRoot) "src-tauri") -Label "cargo build -p berdctl --release" + -WorkingDirectory (Join-Path (Get-BerdRepoRoot) "src-tauri") -Label "cargo build -p berdctl -p berd-monitor --release" $berdctlSource = Join-Path $berdctlReleaseDir (Get-WindowsExeName "berdctl") $staged = Stage-WindowsSidecar -SourcePath $berdctlSource -Triple $Triple -Stem "berdctl" -BinDir $binDir Write-WindowsDevInfo "Staged berdctl sidecar: $staged" +# ── berd-monitor ───────────────────────────────────────────── +$monitorSource = Join-Path $berdctlReleaseDir (Get-WindowsExeName "berd-monitor") +$staged = Stage-WindowsSidecar -SourcePath $monitorSource -Triple $Triple -Stem "berd-monitor" -BinDir $binDir +Write-WindowsDevInfo "Staged berd-monitor sidecar: $staged" + # Catch is deliberately not staged on Windows (see header). Write-WindowsDevInfo "Skipping Catch sidecar: unsupported on Windows (excluded from externalBin)." diff --git a/scripts/windows/Test-WindowsDev.ps1 b/scripts/windows/Test-WindowsDev.ps1 index 5f6c0982b..e58f0b7c7 100644 --- a/scripts/windows/Test-WindowsDev.ps1 +++ b/scripts/windows/Test-WindowsDev.ps1 @@ -439,6 +439,7 @@ try { $windowsExternalBin = @(Get-ObjectValue (Get-ObjectValue $windowsConf "bundle") "externalBin") Assert-Equal "Windows externalBin stages goosed" ($windowsExternalBin -contains "binaries/goosed") $true Assert-Equal "Windows externalBin stages berdctl" ($windowsExternalBin -contains "binaries/berdctl") $true + Assert-Equal "Windows externalBin stages berd-monitor" ($windowsExternalBin -contains "binaries/berd-monitor") $true Assert-Equal "Windows externalBin excludes catch" ($windowsExternalBin -contains "binaries/catch") $false # Tauri merges platform overlays into the base config with json_patch (RFC @@ -454,6 +455,7 @@ try { $mergedExternalBin = if ($null -ne $windowsExternalBin) { $windowsExternalBin } else { $baseExternalBin } Assert-Equal "merged Windows externalBin stages goosed" ($mergedExternalBin -contains "binaries/goosed") $true Assert-Equal "merged Windows externalBin stages berdctl" ($mergedExternalBin -contains "binaries/berdctl") $true + Assert-Equal "merged Windows externalBin stages berd-monitor" ($mergedExternalBin -contains "binaries/berd-monitor") $true Assert-Equal "merged Windows externalBin drops catch" ($mergedExternalBin -contains "binaries/catch") $false # ── Windows bundle recipes route through native staging ────── diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 11ef77893..be42f1984 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -583,6 +583,16 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a8241f3ebb85c056b509d4327ad0358fbbba6ffb340bf388f26350aeda225b1" +[[package]] +name = "berd-monitor" +version = "0.6.2" +dependencies = [ + "clap", + "fs2", + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "berd-voice" version = "0.1.0" @@ -951,6 +961,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca" dependencies = [ "clap_builder", + "clap_derive", ] [[package]] @@ -966,6 +977,18 @@ dependencies = [ "terminal_size", ] +[[package]] +name = "clap_derive" +version = "4.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d012d2b9d65aca7f18f4d9878a045bc17899bba951561ba5ec3c2ba1eed9a061" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "clap_lex" version = "1.1.0" @@ -2038,6 +2061,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fs_extra" version = "1.3.0" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 0d87e6415..3e72803f1 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -14,7 +14,12 @@ crate-type = ["staticlib", "cdylib", "rlib"] # --features ...` only works for workspace members. app-test-driver # stays excluded (a plain path dependency, as before this workspace existed). [workspace] -members = ["crates/berd-voice", "crates/berdctl", "plugins/berdctl"] +members = [ + "crates/berd-monitor", + "crates/berd-voice", + "crates/berdctl", + "plugins/berdctl", +] exclude = ["plugins/app-test-driver"] [build-dependencies] diff --git a/src-tauri/crates/berd-monitor/Cargo.toml b/src-tauri/crates/berd-monitor/Cargo.toml new file mode 100644 index 000000000..3ae520ea2 --- /dev/null +++ b/src-tauri/crates/berd-monitor/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "berd-monitor" +version = "0.6.2" +description = "Detached command monitor for Berd sessions" +authors = ["Block, Inc."] +edition = "2021" + +[dependencies] +clap = { version = "4", features = ["derive", "env"] } +fs2 = "0.4" + +[target.'cfg(unix)'.dependencies] +libc = "0.2" + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.59", features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_System_Diagnostics_ToolHelp", + "Win32_System_JobObjects", + "Win32_System_Threading", +] } diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs new file mode 100644 index 000000000..35db70258 --- /dev/null +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -0,0 +1,1102 @@ +use clap::{Parser, Subcommand, ValueEnum}; +use fs2::FileExt; +use std::env; +use std::ffi::OsString; +use std::fs::{self, File, OpenOptions}; +use std::io::{self, BufRead, BufReader, Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitCode, Stdio}; +use std::sync::mpsc::{self, RecvTimeoutError}; +use std::thread; +use std::time::{Duration, Instant, SystemTime}; + +const FOREGROUND_ENV: &str = "BERD_MONITOR_FOREGROUND"; +const LAUNCH_TOKEN_ENV: &str = "BERD_MONITOR_LAUNCH_TOKEN"; +const RETRY_INTERVAL: Duration = Duration::from_secs(15); +const BATCH_WINDOW: Duration = Duration::from_millis(250); +const MAX_DELIVERY_BYTES: usize = 40_000; +const MAX_PROMPT_CODE_UNITS: usize = 49_000; +const MAX_INSTRUCTIONS_CODE_UNITS: usize = 4_000; +const MAX_LABEL_CODE_UNITS: usize = 120; +const LAUNCH_TIMEOUT: Duration = Duration::from_secs(10); + +#[derive(Clone, Copy, Debug, ValueEnum)] +enum RunningMode { + Steer, + Queue, +} + +impl RunningMode { + fn as_str(self) -> &'static str { + match self { + Self::Steer => "steer", + Self::Queue => "queue", + } + } +} + +#[derive(Debug, Parser)] +#[command( + name = "berd-monitor", + about = "Run a long-lived command and wake its owning Berd session" +)] +struct Cli { + #[command(subcommand)] + command: MonitorCommand, +} + +#[derive(Debug, Subcommand)] +enum MonitorCommand { + /// Start a detached line-oriented command monitor. + Run { + /// Stable key used to identify and stop this monitor. + #[arg(long)] + state_key: String, + /// Concise source name included with delivered events. + #[arg(long)] + label: String, + /// Guidance appended to every delivered event. + #[arg(long, default_value = "")] + instructions: String, + /// Berd session that owns the monitor. + #[arg(long, env = "AGENT_SESSION_ID")] + session_id: String, + /// Whether events steer a running turn or queue behind it. + #[arg(long, value_enum, default_value_t = RunningMode::Steer)] + if_running: RunningMode, + /// Producer command and arguments, following `--`. + #[arg(last = true, required = true, allow_hyphen_values = true)] + command: Vec, + }, + /// Request that a running monitor stop its producer. + Stop { + /// Stable key passed when the monitor was started. + #[arg(long)] + state_key: String, + /// Berd session that owns the monitor. + #[arg(long, env = "AGENT_SESSION_ID")] + session_id: String, + }, +} + +struct StatePaths { + root: PathBuf, + log: PathBuf, + pending: PathBuf, + owner: PathBuf, + stop: PathBuf, +} + +impl StatePaths { + fn for_key(key: &str, session_id: &str) -> Self { + let identity = format!("{session_id}\0{key}"); + let suffix = stable_hash(identity.as_bytes()); + let safe = key + .chars() + .take(80) + .map(|character| { + if character.is_ascii_alphanumeric() || matches!(character, '.' | '_' | '-') { + character + } else { + '-' + } + }) + .collect::(); + let root = state_base_dir().join(format!("{safe}-{suffix:016x}")); + Self { + log: root.join("watcher.log"), + pending: root.join("pending.txt"), + owner: root.join("owner.pid"), + stop: root.join("stop-requested"), + root, + } + } + + fn launch_status(&self, token: &str) -> PathBuf { + self.root.join(format!("launch-{token}.status")) + } +} + +fn state_base_dir() -> PathBuf { + if let Some(explicit) = env::var_os("BERD_MONITOR_STATE_DIR") { + return PathBuf::from(explicit); + } + #[cfg(target_os = "windows")] + if let Some(local) = env::var_os("LOCALAPPDATA") { + return PathBuf::from(local).join("Berd").join("monitor"); + } + #[cfg(target_os = "macos")] + if let Some(home) = env::var_os("HOME") { + return PathBuf::from(home) + .join("Library") + .join("Caches") + .join("Berd") + .join("monitor"); + } + #[cfg(all(unix, not(target_os = "macos")))] + { + if let Some(runtime) = env::var_os("XDG_RUNTIME_DIR") { + return PathBuf::from(runtime).join("berd-monitor"); + } + if let Some(state) = env::var_os("XDG_STATE_HOME") { + return PathBuf::from(state).join("berd").join("monitor"); + } + if let Some(home) = env::var_os("HOME") { + return PathBuf::from(home) + .join(".local") + .join("state") + .join("berd") + .join("monitor"); + } + } + env::temp_dir().join(format!("berd-monitor-{}", current_user_suffix())) +} + +#[cfg(unix)] +fn current_user_suffix() -> u32 { + unsafe { libc::geteuid() } +} + +#[cfg(windows)] +fn current_user_suffix() -> u32 { + std::process::id() +} + +fn stable_hash(bytes: &[u8]) -> u64 { + bytes.iter().fold(0xcbf29ce484222325, |hash, byte| { + (hash ^ u64::from(*byte)).wrapping_mul(0x100000001b3) + }) +} + +fn main() -> ExitCode { + match run_cli() { + Ok(()) => ExitCode::SUCCESS, + Err(error) => { + eprintln!("berd-monitor: {error}"); + ExitCode::FAILURE + } + } +} + +fn run_cli() -> Result<(), String> { + let cli = Cli::parse(); + match cli.command { + MonitorCommand::Run { + state_key, + label, + instructions, + session_id, + if_running, + command, + } => { + validate_single_line("--label", &label)?; + if label.encode_utf16().count() > MAX_LABEL_CODE_UNITS { + return Err(format!( + "--label must not exceed {MAX_LABEL_CODE_UNITS} characters" + )); + } + if instructions.encode_utf16().count() > MAX_INSTRUCTIONS_CODE_UNITS { + return Err(format!( + "--instructions must not exceed {MAX_INSTRUCTIONS_CODE_UNITS} characters" + )); + } + if env::var_os(FOREGROUND_ENV).is_some() { + run_foreground( + &state_key, + &label, + &instructions, + &session_id, + if_running, + &command, + env::var(LAUNCH_TOKEN_ENV).ok().as_deref(), + ) + .map_err(|error| error.to_string()) + } else { + spawn_detached(&state_key, &session_id) + } + } + MonitorCommand::Stop { + state_key, + session_id, + } => request_stop(&state_key, &session_id).map_err(|error| error.to_string()), + } +} + +fn validate_single_line(flag: &str, value: &str) -> Result<(), String> { + if value.trim().is_empty() { + return Err(format!("{flag} must not be empty")); + } + if value.contains('\n') || value.contains('\r') { + return Err(format!("{flag} must be a single line")); + } + Ok(()) +} + +fn spawn_detached(state_key: &str, session_id: &str) -> Result<(), String> { + let executable = + env::current_exe().map_err(|error| format!("resolve current executable: {error}"))?; + let token = format!( + "{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + ); + let paths = StatePaths::for_key(state_key, session_id); + let status_path = paths.launch_status(&token); + let mut child = Command::new(executable); + child + .args(env::args_os().skip(1)) + .env(FOREGROUND_ENV, "1") + .env(LAUNCH_TOKEN_ENV, &token) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + configure_detached(&mut child); + let mut process = child + .spawn() + .map_err(|error| format!("start detached monitor: {error}"))?; + let deadline = Instant::now() + LAUNCH_TIMEOUT; + loop { + if let Ok(status) = fs::read_to_string(&status_path) { + let _ = fs::remove_file(&status_path); + if status.trim() == "ready" { + println!("{} {}", process.id(), paths.root.display()); + return Ok(()); + } + return Err(status + .strip_prefix("error: ") + .unwrap_or(status.trim()) + .to_owned()); + } + if Instant::now() >= deadline { + return Err(format!( + "monitor did not become ready within {} seconds; inspect {}", + LAUNCH_TIMEOUT.as_secs(), + paths.log.display() + )); + } + if let Ok(Some(status)) = process.try_wait() { + return Err(format!("monitor exited before becoming ready ({status})")); + } + thread::sleep(Duration::from_millis(50)); + } +} + +#[cfg(unix)] +fn configure_detached(command: &mut Command) { + use std::os::unix::process::CommandExt; + unsafe { + command.pre_exec(|| { + if libc::setsid() == -1 { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } +} + +#[cfg(unix)] +fn configure_producer(command: &mut Command) { + use std::os::unix::process::CommandExt; + unsafe { + command.pre_exec(|| { + if libc::setpgid(0, 0) == -1 { + return Err(io::Error::last_os_error()); + } + Ok(()) + }); + } +} + +#[cfg(windows)] +fn configure_producer(command: &mut Command) { + use std::os::windows::process::CommandExt; + const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; + const CREATE_SUSPENDED: u32 = 0x0000_0004; + command.creation_flags(CREATE_NEW_PROCESS_GROUP | CREATE_SUSPENDED); +} + +#[cfg(unix)] +fn resume_producer(_child: &std::process::Child) -> io::Result<()> { + Ok(()) +} + +#[cfg(windows)] +fn resume_producer(child: &std::process::Child) -> io::Result<()> { + use std::mem::{size_of, zeroed}; + use windows_sys::Win32::Foundation::{CloseHandle, INVALID_HANDLE_VALUE}; + use windows_sys::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, Thread32First, Thread32Next, TH32CS_SNAPTHREAD, THREADENTRY32, + }; + use windows_sys::Win32::System::Threading::{OpenThread, ResumeThread, THREAD_SUSPEND_RESUME}; + + let snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0) }; + if snapshot == INVALID_HANDLE_VALUE { + return Err(io::Error::last_os_error()); + } + let mut entry: THREADENTRY32 = unsafe { zeroed() }; + entry.dwSize = size_of::() as u32; + let mut found = unsafe { Thread32First(snapshot, &mut entry) } != 0; + while found && entry.th32OwnerProcessID != child.id() { + found = unsafe { Thread32Next(snapshot, &mut entry) } != 0; + } + unsafe { + CloseHandle(snapshot); + } + if !found { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "could not find the suspended producer thread", + )); + } + let thread = unsafe { OpenThread(THREAD_SUSPEND_RESUME, 0, entry.th32ThreadID) }; + if thread.is_null() { + return Err(io::Error::last_os_error()); + } + let resumed = unsafe { ResumeThread(thread) }; + let resume_error = (resumed == u32::MAX).then(io::Error::last_os_error); + unsafe { + CloseHandle(thread); + } + if let Some(error) = resume_error { + return Err(error); + } + Ok(()) +} + +#[cfg(unix)] +struct ProducerTree { + process_group: i32, +} + +#[cfg(unix)] +fn attach_producer_tree(child: &std::process::Child) -> io::Result { + Ok(ProducerTree { + process_group: child.id() as i32, + }) +} + +#[cfg(unix)] +fn terminate_producer_tree(child: &mut std::process::Child, tree: &ProducerTree) { + unsafe { + libc::kill(-tree.process_group, libc::SIGKILL); + } + let _ = child.kill(); +} + +#[cfg(windows)] +struct ProducerTree { + job: windows_sys::Win32::Foundation::HANDLE, +} + +#[cfg(windows)] +fn attach_producer_tree(child: &std::process::Child) -> io::Result { + use std::mem::{size_of, zeroed}; + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JobObjectExtendedLimitInformation, + SetInformationJobObject, JOBOBJECT_EXTENDED_LIMIT_INFORMATION, + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }; + + let job = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + if job.is_null() { + return Err(io::Error::last_os_error()); + } + let mut limits: JOBOBJECT_EXTENDED_LIMIT_INFORMATION = unsafe { zeroed() }; + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + let configured = unsafe { + SetInformationJobObject( + job, + JobObjectExtendedLimitInformation, + (&raw const limits).cast(), + size_of::() as u32, + ) + }; + let assigned = configured != 0 + && unsafe { AssignProcessToJobObject(job, child.as_raw_handle().cast()) } != 0; + if !assigned { + let error = io::Error::last_os_error(); + unsafe { + windows_sys::Win32::Foundation::CloseHandle(job); + } + return Err(error); + } + Ok(ProducerTree { job }) +} + +#[cfg(windows)] +fn terminate_producer_tree(child: &mut std::process::Child, tree: &ProducerTree) { + unsafe { + windows_sys::Win32::System::JobObjects::TerminateJobObject(tree.job, 1); + } + let _ = child.kill(); +} + +#[cfg(windows)] +impl Drop for ProducerTree { + fn drop(&mut self) { + unsafe { + windows_sys::Win32::Foundation::CloseHandle(self.job); + } + } +} + +fn ensure_private_directory(path: &Path) -> io::Result<()> { + fs::create_dir_all(path)?; + #[cfg(unix)] + { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + let metadata = fs::symlink_metadata(path)?; + if metadata.file_type().is_symlink() || metadata.uid() != unsafe { libc::geteuid() } { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + format!("refusing unsafe monitor state directory {}", path.display()), + )); + } + fs::set_permissions(path, fs::Permissions::from_mode(0o700))?; + } + Ok(()) +} + +fn claim_owner(paths: &StatePaths) -> io::Result { + let mut owner = OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(&paths.owner)?; + owner.try_lock_exclusive().map_err(|error| { + io::Error::new( + error.kind(), + format!("a monitor already owns {}: {error}", paths.root.display()), + ) + })?; + owner.set_len(0)?; + writeln!(owner, "{}", std::process::id())?; + owner.flush()?; + Ok(owner) +} + +fn write_launch_status(paths: &StatePaths, token: Option<&str>, status: &str) { + if let Some(token) = token { + let _ = atomic_write(&paths.launch_status(token), status.as_bytes()); + } +} + +#[cfg(windows)] +fn configure_detached(command: &mut Command) { + use std::os::windows::process::CommandExt; + const CREATE_NEW_PROCESS_GROUP: u32 = 0x0000_0200; + const DETACHED_PROCESS: u32 = 0x0000_0008; + command.creation_flags(CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS); +} + +fn request_stop(state_key: &str, session_id: &str) -> io::Result<()> { + let paths = StatePaths::for_key(state_key, session_id); + let owner = OpenOptions::new() + .read(true) + .write(true) + .open(&paths.owner) + .map_err(|error| { + if error.kind() == io::ErrorKind::NotFound { + io::Error::new( + io::ErrorKind::NotFound, + format!("no monitor found for state key {state_key:?}"), + ) + } else { + error + } + })?; + if owner.try_lock_exclusive().is_ok() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("no monitor found for state key {state_key:?}"), + )); + } + fs::write(&paths.stop, b"stop\n")?; + println!("stop requested for {}", paths.root.display()); + Ok(()) +} + +fn run_foreground( + state_key: &str, + label: &str, + instructions: &str, + session_id: &str, + if_running: RunningMode, + producer_command: &[OsString], + launch_token: Option<&str>, +) -> io::Result<()> { + let paths = StatePaths::for_key(state_key, session_id); + ensure_private_directory(&paths.root)?; + let _owner = claim_owner(&paths).inspect_err(|error| { + write_launch_status(&paths, launch_token, &format!("error: {error}")); + })?; + let result = (|| { + paths + .stop + .exists() + .then(|| fs::remove_file(&paths.stop)) + .transpose()?; + run_producer( + &paths, + label, + instructions, + session_id, + if_running, + producer_command, + launch_token, + ) + })(); + if let Err(error) = &result { + let _ = log_line(&paths, &format!("monitor failed: {error}")); + write_launch_status(&paths, launch_token, &format!("error: {error}")); + } + result +} + +fn run_producer( + paths: &StatePaths, + label: &str, + instructions: &str, + session_id: &str, + if_running: RunningMode, + producer_command: &[OsString], + launch_token: Option<&str>, +) -> io::Result<()> { + let diagnostics = OpenOptions::new() + .create(true) + .append(true) + .open(&paths.log)?; + log_line( + paths, + &format!("starting producer: {}", render_command(producer_command)), + )?; + let mut pending = read_optional(&paths.pending)?; + + let mut producer = Command::new(&producer_command[0]); + producer + .args(&producer_command[1..]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::from(diagnostics.try_clone()?)); + configure_producer(&mut producer); + let mut child = producer.spawn()?; + let producer_tree = match attach_producer_tree(&child) { + Ok(tree) => tree, + Err(error) => { + let _ = child.kill(); + return Err(io::Error::new( + error.kind(), + format!("attach producer process tree: {error}"), + )); + } + }; + if let Err(error) = resume_producer(&child) { + terminate_producer_tree(&mut child, &producer_tree); + return Err(io::Error::new( + error.kind(), + format!("resume producer process: {error}"), + )); + } + write_launch_status(paths, launch_token, "ready"); + let stdout = child.stdout.take().expect("stdout was piped"); + let (sender, receiver) = mpsc::sync_channel::>(1024); + thread::spawn(move || { + let mut reader = BufReader::new(stdout); + loop { + let mut line = Vec::new(); + match reader.read_until(b'\n', &mut line) { + Ok(0) => break, + Ok(_) => { + if !line.ends_with(b"\n") { + line.push(b'\n'); + } + if sender.send(line).is_err() { + break; + } + } + Err(_) => break, + } + } + }); + + let mut batch = Vec::new(); + let mut batch_deadline: Option = None; + let mut retry_deadline = Instant::now(); + let mut producer_status = None; + let mut receiver_closed = false; + let mut termination_requested = false; + + loop { + if paths.stop.exists() && !termination_requested { + log_line(paths, "stop requested")?; + terminate_producer_tree(&mut child, &producer_tree); + termination_requested = true; + } + + let now = Instant::now(); + if batch_deadline.is_some_and(|deadline| now >= deadline) { + append_batch(paths, &mut pending, &mut batch)?; + batch_deadline = None; + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + retry_deadline = now + RETRY_INTERVAL; + } + if now >= retry_deadline { + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + retry_deadline = now + RETRY_INTERVAL; + } + + if producer_status.is_none() { + producer_status = child.try_wait()?; + } + if producer_status.is_some() && receiver_closed { + break; + } + + let timeout = batch_deadline + .map(|deadline| deadline.saturating_duration_since(Instant::now())) + .unwrap_or(Duration::from_millis(100)) + .min(Duration::from_millis(100)); + match receiver.recv_timeout(timeout) { + Ok(line) => { + batch.extend_from_slice(&line); + batch_deadline.get_or_insert(Instant::now() + BATCH_WINDOW); + } + Err(RecvTimeoutError::Timeout) => {} + Err(RecvTimeoutError::Disconnected) => receiver_closed = true, + } + } + + terminate_producer_tree(&mut child, &producer_tree); + append_batch(paths, &mut pending, &mut batch)?; + let status = producer_status.expect("producer has exited"); + let summary = format!("[monitor] producer exited with status {status}\n"); + pending.extend_from_slice(summary.as_bytes()); + append_file(&paths.pending, summary.as_bytes())?; + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + + while !pending.is_empty() && !paths.stop.exists() { + thread::sleep(RETRY_INTERVAL); + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + } + if !pending.is_empty() && paths.stop.exists() { + pending.clear(); + atomic_write(&paths.pending, &pending)?; + log_line(paths, "discarded undelivered output after explicit stop")?; + } + log_line(paths, &format!("producer exited with status {status}"))?; + Ok(()) +} + +fn append_batch(paths: &StatePaths, pending: &mut Vec, batch: &mut Vec) -> io::Result<()> { + if batch.is_empty() { + return Ok(()); + } + pending.extend_from_slice(batch); + append_file(&paths.pending, batch)?; + batch.clear(); + Ok(()) +} + +fn flush_pending( + paths: &StatePaths, + pending: &mut Vec, + label: &str, + instructions: &str, + session_id: &str, + if_running: RunningMode, +) -> io::Result<()> { + while !pending.is_empty() { + let end = pending_chunk_end(pending, MAX_DELIVERY_BYTES); + let text = String::from_utf8_lossy(&pending[..end]); + let mut prompt = format!( + "[monitor: {label} | pid {}]\n{}", + std::process::id(), + text.trim_end() + ); + if !instructions.is_empty() { + prompt.push_str("\n\n"); + prompt.push_str(instructions); + } + if prompt.encode_utf16().count() > MAX_PROMPT_CODE_UNITS { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "monitor delivery prompt exceeded its internal size bound", + )); + } + if !deliver(session_id, &prompt, if_running) { + log_line(paths, "delivery failed; buffered output will be retried")?; + return Ok(()); + } + pending.drain(..end); + atomic_write(&paths.pending, pending)?; + log_line(paths, "delivered one event batch")?; + } + Ok(()) +} + +fn deliver(session_id: &str, prompt: &str, if_running: RunningMode) -> bool { + for lock in lock_candidates() { + for binary in berdctl_candidates() { + let status = Command::new(&binary) + .arg("--lock-path") + .arg(&lock) + .arg("--timeout-ms") + .arg("10000") + .arg("session") + .arg("send") + .arg("--session-id") + .arg(session_id) + .arg("--prompt") + .arg(prompt) + .arg("--if-running") + .arg(if_running.as_str()) + .arg("--from") + .arg("berd-monitor") + .arg("--json") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + if status.is_ok_and(|status| status.success()) { + return true; + } + } + } + false +} + +fn lock_candidates() -> Vec { + let Some(explicit) = env::var_os("BERDCTL_LOCK").map(PathBuf::from) else { + return Vec::new(); + }; + let mut candidates = vec![explicit.clone()]; + if let Some(parent) = explicit.parent() { + let mut siblings = fs::read_dir(parent) + .into_iter() + .flatten() + .flatten() + .filter_map(|entry| { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if name.starts_with("control-") && name.ends_with(".json") { + let modified = entry + .metadata() + .ok()? + .modified() + .ok() + .unwrap_or(SystemTime::UNIX_EPOCH); + Some((modified, entry.path())) + } else { + None + } + }) + .collect::>(); + siblings.sort_by(|left, right| right.0.cmp(&left.0)); + for (_, sibling) in siblings { + if sibling != explicit { + candidates.push(sibling); + } + } + } + candidates +} + +fn berdctl_candidates() -> Vec { + let mut candidates = Vec::new(); + if let Some(explicit) = env::var_os("BERDCTL_BIN") { + candidates.push(explicit); + } + candidates.push(OsString::from(if cfg!(windows) { + "berdctl.exe" + } else { + "berdctl" + })); + candidates +} + +fn pending_chunk_end(pending: &[u8], limit: usize) -> usize { + if pending.len() <= limit { + return pending.len(); + } + let mut hard_end = limit; + if let Err(error) = std::str::from_utf8(&pending[..hard_end]) { + if error.error_len().is_none() { + hard_end = error.valid_up_to(); + } + } + let end = pending[..hard_end] + .iter() + .rposition(|byte| *byte == b'\n') + .map(|index| index + 1) + .unwrap_or(hard_end); + end.max(1) +} + +fn read_optional(path: &Path) -> io::Result> { + match File::open(path) { + Ok(mut file) => { + let mut contents = Vec::new(); + file.read_to_end(&mut contents)?; + Ok(contents) + } + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(Vec::new()), + Err(error) => Err(error), + } +} + +fn append_file(path: &Path, data: &[u8]) -> io::Result<()> { + OpenOptions::new() + .create(true) + .append(true) + .open(path)? + .write_all(data) +} + +fn atomic_write(path: &Path, data: &[u8]) -> io::Result<()> { + let temporary = path.with_extension(format!("tmp.{}", std::process::id())); + fs::write(&temporary, data)?; + if path.exists() { + fs::remove_file(path)?; + } + fs::rename(temporary, path) +} + +fn log_line(paths: &StatePaths, message: &str) -> io::Result<()> { + writeln!( + OpenOptions::new() + .create(true) + .append(true) + .open(&paths.log)?, + "{message}" + ) +} + +fn render_command(command: &[OsString]) -> String { + command + .iter() + .map(|part| part.to_string_lossy()) + .collect::>() + .join(" ") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(unix)] + fn test_process_exists(pid: u32) -> bool { + let result = unsafe { libc::kill(pid as i32, 0) }; + result == 0 || io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) + } + + #[test] + fn chunk_prefers_complete_lines() { + let input = b"first\nsecond\nthird\n"; + assert_eq!(pending_chunk_end(input, 13), 13); + assert_eq!(&input[..pending_chunk_end(input, 10)], b"first\n"); + } + + #[test] + fn chunk_never_exceeds_limit_for_a_long_line() { + let input = vec![b'x'; MAX_DELIVERY_BYTES + 10_000]; + assert_eq!( + pending_chunk_end(&input, MAX_DELIVERY_BYTES), + MAX_DELIVERY_BYTES + ); + } + + #[test] + fn state_key_path_is_stable_and_safe() { + let first = StatePaths::for_key("pr/123 checks", "session-a"); + let second = StatePaths::for_key("pr/123 checks", "session-a"); + assert_eq!(first.root, second.root); + assert!(first.root.to_string_lossy().contains("pr-123-checks-")); + assert_ne!( + first.root, + StatePaths::for_key("pr/123 checks", "session-b").root + ); + } + + #[test] + fn active_owner_rejects_a_duplicate_monitor() { + let key = format!( + "owner-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + ensure_private_directory(&paths.root).unwrap(); + let owner = claim_owner(&paths).unwrap(); + assert_eq!( + claim_owner(&paths).unwrap_err().kind(), + io::ErrorKind::WouldBlock + ); + drop(owner); + claim_owner(&paths).unwrap(); + fs::remove_dir_all(&paths.root).unwrap(); + } + + #[test] + fn simultaneous_owner_claims_have_one_winner() { + use std::sync::{Arc, Barrier}; + + let key = format!( + "owner-race-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + ensure_private_directory(&paths.root).unwrap(); + let barrier = Arc::new(Barrier::new(3)); + let mut workers = Vec::new(); + for _ in 0..2 { + let barrier = Arc::clone(&barrier); + let key = key.clone(); + workers.push(thread::spawn(move || { + barrier.wait(); + let owner = claim_owner(&StatePaths::for_key(&key, "test-session")); + if owner.is_ok() { + thread::sleep(Duration::from_millis(100)); + } + owner.is_ok() + })); + } + barrier.wait(); + let winners = workers + .into_iter() + .map(|worker| worker.join().unwrap()) + .filter(|won| *won) + .count(); + assert_eq!(winners, 1); + fs::remove_dir_all(&paths.root).unwrap(); + } + + #[test] + fn missing_producer_is_recorded_as_a_launch_failure() { + let key = format!( + "missing-producer-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + let error = run_foreground( + &key, + "test", + "", + "test-session", + RunningMode::Steer, + &[OsString::from("berd-monitor-command-that-does-not-exist")], + None, + ) + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::NotFound); + assert!(fs::read_to_string(&paths.log) + .unwrap() + .contains("monitor failed")); + fs::remove_dir_all(&paths.root).unwrap(); + } + + #[cfg(unix)] + #[test] + fn stop_terminates_the_producer_process_group_and_discards_pending() { + let key = format!( + "process-tree-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let session_id = "test-session"; + let paths = StatePaths::for_key(&key, session_id); + let worker_key = key.clone(); + let worker = thread::spawn(move || { + run_foreground( + &worker_key, + "test", + "", + session_id, + RunningMode::Steer, + &[ + OsString::from("sh"), + OsString::from("-c"), + OsString::from("sleep 30 & child=$!; echo $child; wait"), + ], + None, + ) + }); + let deadline = Instant::now() + Duration::from_secs(5); + while !paths.pending.is_file() && Instant::now() < deadline { + thread::sleep(Duration::from_millis(25)); + } + let child_pid = loop { + if let Ok(value) = fs::read_to_string(&paths.pending) { + if let Ok(pid) = value.trim().parse::() { + break pid; + } + } + assert!( + Instant::now() < deadline, + "producer output was not buffered" + ); + thread::sleep(Duration::from_millis(25)); + }; + request_stop(&key, session_id).unwrap(); + worker.join().unwrap().unwrap(); + assert!(fs::read(&paths.pending).unwrap().is_empty()); + let child_gone_deadline = Instant::now() + Duration::from_secs(2); + while test_process_exists(child_pid as u32) && Instant::now() < child_gone_deadline { + thread::sleep(Duration::from_millis(25)); + } + assert!(!test_process_exists(child_pid as u32)); + fs::remove_dir_all(&paths.root).unwrap(); + } + + #[test] + fn running_mode_matches_berdctl_values() { + assert_eq!(RunningMode::Steer.as_str(), "steer"); + assert_eq!(RunningMode::Queue.as_str(), "queue"); + } +} diff --git a/src-tauri/crates/berdctl/api-surface-feedback.json b/src-tauri/crates/berdctl/api-surface-feedback.json index e3e404ac5..3e820fe99 100644 --- a/src-tauri/crates/berdctl/api-surface-feedback.json +++ b/src-tauri/crates/berdctl/api-surface-feedback.json @@ -87,7 +87,7 @@ } }, "send": { - "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes.", + "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript.", "fields": [ { "name": "session_id", @@ -117,6 +117,14 @@ "kind": "string", "description": "What to do if the target session is running: refuse, steer, or queue.", "values": ["refuse", "steer", "queue"] + }, + { + "name": "from", + "required": false, + "kind": "string", + "description": "Optional visible sender label for this message (1-120 chars).", + "min": 1, + "max": 120 } ], "schema": { @@ -144,6 +152,13 @@ "description": "What to do if the target session is running: refuse, steer, or queue.", "type": "string", "enum": ["refuse", "steer", "queue"] + }, + "from": { + "description": "Optional visible sender label for this message (1-120 chars).", + "type": "string", + "minLength": 1, + "maxLength": 120, + "pattern": "^[^\\r\\n]*$" } }, "required": ["session_id", "prompt"], diff --git a/src-tauri/crates/berdctl/api-surface.json b/src-tauri/crates/berdctl/api-surface.json index 18c5163b0..1e4d56930 100644 --- a/src-tauri/crates/berdctl/api-surface.json +++ b/src-tauri/crates/berdctl/api-surface.json @@ -87,7 +87,7 @@ } }, "send": { - "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes.", + "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript.", "fields": [ { "name": "session_id", @@ -117,6 +117,14 @@ "kind": "string", "description": "What to do if the target session is running: refuse, steer, or queue.", "values": ["refuse", "steer", "queue"] + }, + { + "name": "from", + "required": false, + "kind": "string", + "description": "Optional visible sender label for this message (1-120 chars).", + "min": 1, + "max": 120 } ], "schema": { @@ -144,6 +152,13 @@ "description": "What to do if the target session is running: refuse, steer, or queue.", "type": "string", "enum": ["refuse", "steer", "queue"] + }, + "from": { + "description": "Optional visible sender label for this message (1-120 chars).", + "type": "string", + "minLength": 1, + "maxLength": 120, + "pattern": "^[^\\r\\n]*$" } }, "required": ["session_id", "prompt"], diff --git a/src-tauri/crates/berdctl/cli-surface-feedback.json b/src-tauri/crates/berdctl/cli-surface-feedback.json index 1bd8962b5..34666f45a 100644 --- a/src-tauri/crates/berdctl/cli-surface-feedback.json +++ b/src-tauri/crates/berdctl/cli-surface-feedback.json @@ -13,7 +13,7 @@ "send": { "action": "send", "about": "Send a prompt into an existing chat session", - "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." + "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." }, "open": { "action": "open", diff --git a/src-tauri/crates/berdctl/cli-surface.json b/src-tauri/crates/berdctl/cli-surface.json index c15c55bbe..c5093ff21 100644 --- a/src-tauri/crates/berdctl/cli-surface.json +++ b/src-tauri/crates/berdctl/cli-surface.json @@ -13,7 +13,7 @@ "send": { "action": "send", "about": "Send a prompt into an existing chat session", - "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." + "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." }, "open": { "action": "open", diff --git a/src-tauri/src/services/acp/goose_serve.rs b/src-tauri/src/services/acp/goose_serve.rs index fb34f649a..15d0ce137 100644 --- a/src-tauri/src/services/acp/goose_serve.rs +++ b/src-tauri/src/services/acp/goose_serve.rs @@ -830,6 +830,7 @@ fn resolve_berdctl_spawn_paths( prepend_dirs: &mut Vec, ) -> BerdctlSpawnPaths { let berdctl_bin = resolve_berdctl_bin(); + let berd_monitor_bin = resolve_berd_monitor_bin(); let app_data_dir = match app_handle.path().app_data_dir() { Ok(app_data_dir) => Some(app_data_dir), Err(error) => { @@ -839,14 +840,25 @@ fn resolve_berdctl_spawn_paths( None } }; - if let (Some(cli_path), Some(app_data_dir)) = (berdctl_bin.as_deref(), app_data_dir.as_deref()) - { + if let Some(app_data_dir) = app_data_dir.as_deref() { let shim_dir = app_data_dir.join("bin"); - match create_berdctl_shim(&shim_dir, cli_path) { - // After the distro bin dir so that dir keeps its - // pinned PATH-front position. - Ok(()) => prepend_dirs.push(shim_dir), - Err(error) => log::warn!("Skipping berdctl PATH shim: {error}"), + let mut installed_any = false; + if let Some(cli_path) = berdctl_bin.as_deref() { + match create_berdctl_shim(&shim_dir, cli_path) { + Ok(()) => installed_any = true, + Err(error) => log::warn!("Skipping berdctl PATH shim: {error}"), + } + } + if let Some(cli_path) = berd_monitor_bin.as_deref() { + match create_berd_monitor_shim(&shim_dir, cli_path) { + Ok(()) => installed_any = true, + Err(error) => log::warn!("Skipping berd-monitor PATH shim: {error}"), + } + } + if installed_any { + // After the distro bin dir so that dir keeps its pinned + // PATH-front position. + prepend_dirs.push(shim_dir); } } BerdctlSpawnPaths { @@ -879,6 +891,7 @@ fn apply_berdctl_env( } else { log::warn!("Skipping BERDCTL_BIN: could not resolve the berdctl binary path"); } + } /// Create or refresh the PATH shim that lets harness children run a bare @@ -890,9 +903,19 @@ fn apply_berdctl_env( /// replacing an in-use copied `.exe` can fail. #[cfg(feature = "berdctl")] fn create_berdctl_shim(shim_dir: &Path, cli_path: &Path) -> Result<(), String> { + create_cli_shim(shim_dir, cli_path, berdctl_shim_name()) +} + +#[cfg(feature = "berdctl")] +fn create_berd_monitor_shim(shim_dir: &Path, cli_path: &Path) -> Result<(), String> { + create_cli_shim(shim_dir, cli_path, berd_monitor_shim_name()) +} + +#[cfg(feature = "berdctl")] +fn create_cli_shim(shim_dir: &Path, cli_path: &Path, shim_name: &str) -> Result<(), String> { if !cli_path.exists() { return Err(format!( - "berdctl binary not found at {}", + "agent CLI binary not found at {}", cli_path.display() )); } @@ -900,7 +923,7 @@ fn create_berdctl_shim(shim_dir: &Path, cli_path: &Path) -> Result<(), String> { std::fs::create_dir_all(shim_dir) .map_err(|error| format!("failed to create {}: {error}", shim_dir.display()))?; - let link = shim_dir.join(berdctl_shim_name()); + let link = shim_dir.join(shim_name); // `remove_file` deletes a symlink itself rather than its target. match std::fs::remove_file(&link) { Ok(()) => {} @@ -912,7 +935,7 @@ fn create_berdctl_shim(shim_dir: &Path, cli_path: &Path) -> Result<(), String> { )); } } - create_berdctl_shim_file(cli_path, &link) + create_cli_shim_file(cli_path, &link) } /// Mirrors `get_goose_command`: explicit env override (exported by `just @@ -930,6 +953,18 @@ fn resolve_berdctl_bin() -> Option { Some(exe.parent()?.join(berdctl_binary_name())) } +#[cfg(feature = "berdctl")] +fn resolve_berd_monitor_bin() -> Option { + if let Ok(override_path) = std::env::var("BERD_MONITOR_BIN") { + if !override_path.is_empty() { + return Some(PathBuf::from(override_path)); + } + } + + let exe = std::env::current_exe().ok()?; + Some(exe.parent()?.join(berd_monitor_binary_name())) +} + #[cfg(feature = "berdctl")] fn berdctl_binary_name() -> &'static str { if cfg!(windows) { @@ -939,6 +974,15 @@ fn berdctl_binary_name() -> &'static str { } } +#[cfg(feature = "berdctl")] +fn berd_monitor_binary_name() -> &'static str { + if cfg!(windows) { + "berd-monitor.exe" + } else { + "berd-monitor" + } +} + #[cfg(feature = "berdctl")] fn berdctl_shim_name() -> &'static str { if cfg!(windows) { @@ -948,8 +992,17 @@ fn berdctl_shim_name() -> &'static str { } } +#[cfg(feature = "berdctl")] +fn berd_monitor_shim_name() -> &'static str { + if cfg!(windows) { + "berd-monitor.cmd" + } else { + berd_monitor_binary_name() + } +} + #[cfg(all(feature = "berdctl", unix))] -fn create_berdctl_shim_file(cli_path: &Path, link: &Path) -> Result<(), String> { +fn create_cli_shim_file(cli_path: &Path, link: &Path) -> Result<(), String> { std::os::unix::fs::symlink(cli_path, link).map_err(|error| { format!( "failed to symlink {} -> {}: {error}", @@ -960,7 +1013,7 @@ fn create_berdctl_shim_file(cli_path: &Path, link: &Path) -> Result<(), String> } #[cfg(all(feature = "berdctl", windows))] -fn create_berdctl_shim_file(cli_path: &Path, link: &Path) -> Result<(), String> { +fn create_cli_shim_file(cli_path: &Path, link: &Path) -> Result<(), String> { let content = format!("@echo off\r\n\"{}\" %*\r\n", cli_path.to_string_lossy()); std::fs::write(link, content).map_err(|error| { format!( diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 335c6ecf7..5ddf6940a 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -83,7 +83,12 @@ "../resources/berd-sounds-5.mp3": "berd-sounds-5.mp3", "../resources/berd-sounds-6.mp3": "berd-sounds-6.mp3" }, - "externalBin": ["binaries/goosed", "binaries/berdctl", "binaries/catch"], + "externalBin": [ + "binaries/goosed", + "binaries/berdctl", + "binaries/berd-monitor", + "binaries/catch" + ], "linux": { "deb": { "depends": ["libvulkan1"] diff --git a/src-tauri/tauri.windows.conf.json b/src-tauri/tauri.windows.conf.json index 16f16c208..ae47a37b4 100644 --- a/src-tauri/tauri.windows.conf.json +++ b/src-tauri/tauri.windows.conf.json @@ -17,7 +17,11 @@ }, "bundle": { "targets": ["nsis"], - "externalBin": ["binaries/goosed", "binaries/berdctl"], + "externalBin": [ + "binaries/goosed", + "binaries/berdctl", + "binaries/berd-monitor" + ], "windows": { "webviewInstallMode": { "type": "downloadBootstrapper", diff --git a/src/features/berdctl/__tests__/commands/commands.test.ts b/src/features/berdctl/__tests__/commands/commands.test.ts index 6f6333232..49832dddc 100644 --- a/src/features/berdctl/__tests__/commands/commands.test.ts +++ b/src/features/berdctl/__tests__/commands/commands.test.ts @@ -1683,6 +1683,59 @@ describe("sessions.send", () => { ), ).toEqual(["next prompt", "another prompt"]); }); + + it("preserves a visible sender label on queued prompts", async () => { + mockSessionFound(); + useChatStore.getState().setChatState("session-1", "streaming"); + + const result = await dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "[monitor: checks] complete", + if_running: "queue", + from: "the Berd session handling berd-monitor implementation", + }, + ctx, + ); + + expect(result).toEqual({ session_id: "session-1", send_status: "queued" }); + expect( + useChatStore.getState().queuedMessageBySession["session-1"]?.[0]?.payload + .sendOptions, + ).toEqual({ + userMessageMetadata: { + origin: "berdctl_cross_session", + berdSenderLabel: + "the Berd session handling berd-monitor implementation", + }, + acpGooseMetadata: { + origin: "berdctl_cross_session", + berdSenderLabel: + "the Berd session handling berd-monitor implementation", + }, + }); + }); + + it("rejects multiline sender labels before dispatch", async () => { + const error = await expectCommandError( + dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "monitor update", + if_running: "queue", + from: "first line\nsecond line", + }, + ctx, + ), + "invalid_args", + ); + + expect(error.message).toContain("single line"); + }); }); describe("sessions.open", () => { diff --git a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts index 739e5dffc..50d087dc2 100644 --- a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts +++ b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts @@ -110,7 +110,13 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { ); assertQueuedSessionReady(state.getSessionRuntime(queuedSessionId)); }, - { returnOnDispatch: true }, + { + returnOnDispatch: true, + ...(queuedMessage.payload.sendOptions?.userMessageMetadata + ?.berdSenderLabel + ? { sendOptions: queuedMessage.payload.sendOptions } + : {}), + }, ); let sendSucceeded = false; let shouldResumeDrain = false; diff --git a/src/features/berdctl/commands/impl/sendSession.ts b/src/features/berdctl/commands/impl/sendSession.ts index e48755cea..55d9d1526 100644 --- a/src/features/berdctl/commands/impl/sendSession.ts +++ b/src/features/berdctl/commands/impl/sendSession.ts @@ -39,6 +39,16 @@ const sendSessionSchema = z .describe( "What to do if the target session is running: refuse, steer, or queue.", ), + from: z + .string() + .trim() + .min(1) + .max(120) + .regex(/^[^\r\n]*$/, "Sender label must be a single line.") + .optional() + .describe( + "Optional visible sender label for this message (1-120 chars).", + ), }) .strict(); @@ -61,10 +71,12 @@ export const sendSessionCommand = defineCommand({ "Idle sends are fire-and-forget and visibly add a user message marked as sent " + "by Berd from another session. Running sessions are refused by default; use " + "--if-running steer to add context to the active run, or --if-running queue " + - "to send one follow-up after the current run finishes.", + "to send one follow-up after the current run finishes. Use --from to give " + + "the sending session or tool a concise visible label in the transcript.", helpFooter: `Example: berdctl session send --session-id \\ - --prompt "Check the latest CI failure" --if-running queue --json + --prompt "Check the latest CI failure" --if-running queue \\ + --from "the Berd session handling CI" --json Result: {"session_id": "...", "send_status": "dispatched"|"steered"|"queued"} @@ -88,6 +100,9 @@ Result: import("../runtime/projects"), import("../runtime/sessionSend"), ]); + const sendOptions = berdctlCrossSessionSendOptions({ + senderLabel: args.from, + }); await loadSessionForBerdctl(args.session_id); const session = requireSession(args.session_id); @@ -130,7 +145,7 @@ Result: args.session_id, args.prompt, undefined, - berdctlCrossSessionSendOptions(), + sendOptions, { throwOnError: true }, ); return { session_id: session.id, send_status: "steered" }; @@ -140,7 +155,7 @@ Result: args.session_id, admitSystemInheritedQueuedMessage({ text: args.prompt, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions, }), ); return { session_id: session.id, send_status: "queued" }; @@ -160,7 +175,7 @@ Result: createDeferredQueuedMessagePayload({ text: args.prompt, persona: { kind: "inherit" }, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions, }), { startupName: args.startup_name, project }, ); @@ -178,7 +193,7 @@ Result: args.session_id, admitSystemInheritedQueuedMessage({ text: args.prompt, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions, }), ); return { session_id: session.id, send_status: "queued" }; @@ -196,7 +211,10 @@ Result: 0) === 0, ); }, - { returnOnDispatch: true }, + { + returnOnDispatch: true, + sendOptions, + }, ); } catch (error) { if (error instanceof SessionDispatchContentionError) { @@ -205,7 +223,7 @@ Result: args.session_id, admitSystemInheritedQueuedMessage({ text: args.prompt, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions, }), ); return { session_id: session.id, send_status: "queued" }; @@ -224,7 +242,7 @@ Result: args.session_id, admitSystemInheritedQueuedMessage({ text: args.prompt, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions, }), ); return { session_id: session.id, send_status: "queued" }; diff --git a/src/features/berdctl/commands/runtime/sessionSend.ts b/src/features/berdctl/commands/runtime/sessionSend.ts index 2121c3a24..1524b3688 100644 --- a/src/features/berdctl/commands/runtime/sessionSend.ts +++ b/src/features/berdctl/commands/runtime/sessionSend.ts @@ -23,13 +23,20 @@ export { isBerdctlCrossSessionQueuedMessage } from "@/features/chat/lib/queuedMe export const BERDCTL_CROSS_SESSION_ORIGIN = "berdctl_cross_session" satisfies NonNullable; -export function berdctlCrossSessionSendOptions(): ChatSendOptions { +export function berdctlCrossSessionSendOptions( + options: { senderLabel?: string } = {}, +): ChatSendOptions { + const senderMetadata = options.senderLabel + ? { berdSenderLabel: options.senderLabel } + : {}; return { userMessageMetadata: { origin: BERDCTL_CROSS_SESSION_ORIGIN, + ...senderMetadata, }, acpGooseMetadata: { origin: BERDCTL_CROSS_SESSION_ORIGIN, + ...senderMetadata, }, }; } @@ -38,7 +45,10 @@ export async function sendPromptToExistingSessionInBackground( sessionId: string, prompt: string, beforeUserMessageCommitted?: () => void, - options: { returnOnDispatch?: boolean } = {}, + options: { + returnOnDispatch?: boolean; + sendOptions?: ChatSendOptions; + } = {}, ): Promise { const acquisition = await acquireExistingSessionForBackgroundSend(sessionId); if (acquisition.status === "contended") { @@ -77,7 +87,7 @@ export async function sendPromptToExistingSessionInBackground( providerId, persona, { - ...berdctlCrossSessionSendOptions(), + ...(options.sendOptions ?? berdctlCrossSessionSendOptions()), systemPrompt: session ? formatIncludedWorkspacesPrompt(session) : undefined, diff --git a/src/features/chat/ui/ChatInput.tsx b/src/features/chat/ui/ChatInput.tsx index dba6c0dc6..34c980b79 100644 --- a/src/features/chat/ui/ChatInput.tsx +++ b/src/features/chat/ui/ChatInput.tsx @@ -107,7 +107,11 @@ function stripCrossSessionOrigin>( return metadata; } - const { origin: _origin, ...rest } = metadata; + const { + origin: _origin, + berdSenderLabel: _berdSenderLabel, + ...rest + } = metadata; return Object.keys(rest).length > 0 ? (rest as T) : undefined; } diff --git a/src/features/chat/ui/MessageBubble.tsx b/src/features/chat/ui/MessageBubble.tsx index 3882ae4ec..788b341db 100644 --- a/src/features/chat/ui/MessageBubble.tsx +++ b/src/features/chat/ui/MessageBubble.tsx @@ -900,6 +900,9 @@ export const MessageBubble = memo(function MessageBubble({ const isSteeredMessage = isUser && message.metadata?.delivery === "steer"; const isBerdctlCrossSessionMessage = isUser && message.metadata?.origin === "berdctl_cross_session"; + const berdSenderLabel = isBerdctlCrossSessionMessage + ? message.metadata?.berdSenderLabel + : undefined; const timestamp = ( - {t("message.berdctlCrossSessionLabel")} + {berdSenderLabel + ? t("message.berdctlCrossSessionNamedLabel", { + sender: berdSenderLabel, + }) + : t("message.berdctlCrossSessionLabel")} ) : null} {isSteeredMessage ? ( diff --git a/src/features/chat/ui/__tests__/MessageBubble.test.tsx b/src/features/chat/ui/__tests__/MessageBubble.test.tsx index 5e258e613..c20768887 100644 --- a/src/features/chat/ui/__tests__/MessageBubble.test.tsx +++ b/src/features/chat/ui/__tests__/MessageBubble.test.tsx @@ -617,6 +617,22 @@ describe("MessageBubble", () => { ); }); + it("shows a named sender for attributed cross-session messages", () => { + const message = userMessage("[monitor: PR checks] complete"); + message.metadata = { + ...message.metadata, + origin: "berdctl_cross_session", + berdSenderLabel: "berd-monitor", + }; + + render(); + + expect(screen.getByText("Sent by berd-monitor")).toBeInTheDocument(); + expect( + screen.queryByText("Sent by Berd from another session"), + ).not.toBeInTheDocument(); + }); + it("renders provenance and steer labels together", () => { const message = userMessage("steered from another session"); message.metadata = { diff --git a/src/shared/api/__tests__/acpReplayMetadata.test.ts b/src/shared/api/__tests__/acpReplayMetadata.test.ts index b33da9e2c..543d6b539 100644 --- a/src/shared/api/__tests__/acpReplayMetadata.test.ts +++ b/src/shared/api/__tests__/acpReplayMetadata.test.ts @@ -137,6 +137,22 @@ describe("getReplayUserMetadata", () => { ).toEqual({ origin: "berdctl_cross_session" }); }); + it("restores sender attribution on cross-session messages", () => { + expect( + getReplayUserMetadata({ + _meta: { + goose: { + origin: "berdctl_cross_session", + berdSenderLabel: "berd-monitor", + }, + }, + }), + ).toEqual({ + origin: "berdctl_cross_session", + berdSenderLabel: "berd-monitor", + }); + }); + it("restores voice conversation origin metadata", () => { expect( getReplayUserMetadata({ diff --git a/src/shared/api/acpReplayMetadata.ts b/src/shared/api/acpReplayMetadata.ts index 6c26091b7..17be06f60 100644 --- a/src/shared/api/acpReplayMetadata.ts +++ b/src/shared/api/acpReplayMetadata.ts @@ -13,6 +13,7 @@ export type ReplayUserMetadata = Pick< MessageMetadata, | "delivery" | "origin" + | "berdSenderLabel" | "voiceUtteranceId" | "voiceConversationLifecycleId" | "voiceConversationRevision" @@ -75,6 +76,10 @@ export function getReplayUserMetadata( : goose.origin === "voice_conversation" ? "voice_conversation" : undefined; + const berdSenderLabel = + origin === "berdctl_cross_session" + ? boundedSingleLineString(goose.berdSenderLabel, 120) + : undefined; const voiceUtteranceId = origin === "voice_conversation" ? nonEmptyString(goose.voiceUtteranceId) @@ -97,6 +102,7 @@ export function getReplayUserMetadata( return { ...(delivery ? { delivery } : {}), ...(origin ? { origin } : {}), + ...(berdSenderLabel ? { berdSenderLabel } : {}), ...(voiceUtteranceId ? { voiceUtteranceId } : {}), ...(voiceConversationLifecycleId ? { voiceConversationLifecycleId } : {}), ...(voiceConversationRevision !== undefined @@ -105,6 +111,19 @@ export function getReplayUserMetadata( }; } +function boundedSingleLineString( + value: unknown, + maxLength: number, +): string | undefined { + const normalized = nonEmptyString(value); + return normalized && + normalized.length <= maxLength && + !normalized.includes("\n") && + !normalized.includes("\r") + ? normalized + : undefined; +} + function getGooseReplayMeta( source: ReplayMetadataSource, ): Record | null { diff --git a/src/shared/i18n/locales/en/chat.json b/src/shared/i18n/locales/en/chat.json index aa5775f70..aaa42cb21 100644 --- a/src/shared/i18n/locales/en/chat.json +++ b/src/shared/i18n/locales/en/chat.json @@ -406,6 +406,7 @@ "anthropicThinkingHistory": "This chat can't continue with a Claude model because its earlier reasoning history is no longer in a form Claude will accept. Start a new chat, or switch this chat to a non-Claude model to keep going." }, "berdctlCrossSessionLabel": "Sent by Berd from another session", + "berdctlCrossSessionNamedLabel": "Sent by {{sender}}", "steerLabel": "Steered", "viewMore": "View more", "viewLess": "View less", diff --git a/src/shared/i18n/locales/es/chat.json b/src/shared/i18n/locales/es/chat.json index da05f2ef1..93716f040 100644 --- a/src/shared/i18n/locales/es/chat.json +++ b/src/shared/i18n/locales/es/chat.json @@ -405,6 +405,7 @@ "anthropicThinkingHistory": "Este chat no puede continuar con un modelo Claude porque su historial de razonamiento previo ya no tiene una forma que Claude acepte. Inicia un chat nuevo o cambia este chat a un modelo que no sea Claude para continuar." }, "berdctlCrossSessionLabel": "Enviado por Berd desde otra sesión", + "berdctlCrossSessionNamedLabel": "Enviado por {{sender}}", "steerLabel": "Guiado", "viewMore": "Ver más", "viewLess": "Ver menos", diff --git a/src/shared/types/messages.ts b/src/shared/types/messages.ts index b0aedb915..b7dcb2924 100644 --- a/src/shared/types/messages.ts +++ b/src/shared/types/messages.ts @@ -255,6 +255,7 @@ export interface MessageMetadata { delivery?: "steering" | "steer"; steeringRequestId?: string; origin?: "berdctl_cross_session" | "voice_conversation"; + berdSenderLabel?: string; voiceUtteranceId?: string; voiceConversationLifecycleId?: string; voiceConversationRevision?: number; From e74b4ed2d69274bc13263ad20b79ff025485bce6 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Wed, 26 Aug 2026 16:42:31 -0400 Subject: [PATCH 02/22] style: format native monitor changes Signed-off-by: John Tennant --- src-tauri/src/services/acp/goose_serve.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src-tauri/src/services/acp/goose_serve.rs b/src-tauri/src/services/acp/goose_serve.rs index 15d0ce137..0905b457b 100644 --- a/src-tauri/src/services/acp/goose_serve.rs +++ b/src-tauri/src/services/acp/goose_serve.rs @@ -891,7 +891,6 @@ fn apply_berdctl_env( } else { log::warn!("Skipping BERDCTL_BIN: could not resolve the berdctl binary path"); } - } /// Create or refresh the PATH shim that lets harness children run a bare From f009d404952b5cb8a595c45db870e70e98b8854f Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 12:58:41 -0400 Subject: [PATCH 03/22] fix(berdctl): attribute created sessions Signed-off-by: John Tennant --- distro/skills/berd-orchestrator/SKILL.md | 5 +++-- .../crates/berdctl/api-surface-feedback.json | 17 ++++++++++++++++- src-tauri/crates/berdctl/api-surface.json | 17 ++++++++++++++++- .../crates/berdctl/cli-surface-feedback.json | 2 +- src-tauri/crates/berdctl/cli-surface.json | 2 +- src-tauri/crates/berdctl/src/main.rs | 9 +++++++-- .../__tests__/commands/commands.test.ts | 15 ++++++++++++++- .../berdctl/commands/impl/createSession.ts | 18 ++++++++++++++++-- 8 files changed, 74 insertions(+), 11 deletions(-) diff --git a/distro/skills/berd-orchestrator/SKILL.md b/distro/skills/berd-orchestrator/SKILL.md index b8135266c..38d4f07bf 100644 --- a/distro/skills/berd-orchestrator/SKILL.md +++ b/distro/skills/berd-orchestrator/SKILL.md @@ -17,8 +17,9 @@ Prefer an existing session that owns the relevant context. Create one when work has no owner, and give it a complete task. Do not approve, merge, or archive work without the user's direction. -When sending between sessions, pass `--from ''` -so the receiving transcript explains where the message came from. +When creating or sending to another session, pass +`--from ''` so the receiving transcript explains +where the message came from. Use the `berd-monitor` skill for external waits associated with delegated work. End the turn after starting a monitor; do not poll in the foreground. diff --git a/src-tauri/crates/berdctl/api-surface-feedback.json b/src-tauri/crates/berdctl/api-surface-feedback.json index 3e820fe99..19f86dd5b 100644 --- a/src-tauri/crates/berdctl/api-surface-feedback.json +++ b/src-tauri/crates/berdctl/api-surface-feedback.json @@ -6,7 +6,7 @@ "description": "Manage the user's chat sessions: create (fire-and-forget, on any installed agent harness), send, open, list, get, rename, move, move to group, clear project, fork, archive.", "actions": { "create": { - "description": "Create a new chat session on any installed agent harness and send the prompt in it. Fire-and-forget: returns the session id immediately and the session runs in the background without changing what the user sees; the user can open it themselves. Only check on it later (action \"get\") if the user asks.", + "description": "Create a new chat session on any installed agent harness and send the prompt in it. Fire-and-forget: returns the session id immediately and the session runs in the background without changing what the user sees; the user can open it themselves. Use --from to give the delegating session or tool a concise visible label on the initial message. Only check on it later (action \"get\") if the user asks.", "fields": [ { "name": "prompt", @@ -47,6 +47,14 @@ "description": "Branch/worktree name when the project's startup mode is branch or worktree; required for those modes.", "min": 1, "max": 200 + }, + { + "name": "from", + "required": false, + "kind": "string", + "description": "Optional visible sender label for the initial message (1-120 chars).", + "min": 1, + "max": 120 } ], "schema": { @@ -80,6 +88,13 @@ "type": "string", "minLength": 1, "maxLength": 200 + }, + "from": { + "description": "Optional visible sender label for the initial message (1-120 chars).", + "type": "string", + "minLength": 1, + "maxLength": 120, + "pattern": "^[^\\r\\n]*$" } }, "required": ["prompt"], diff --git a/src-tauri/crates/berdctl/api-surface.json b/src-tauri/crates/berdctl/api-surface.json index 1e4d56930..1bef48807 100644 --- a/src-tauri/crates/berdctl/api-surface.json +++ b/src-tauri/crates/berdctl/api-surface.json @@ -6,7 +6,7 @@ "description": "Manage the user's chat sessions: create (fire-and-forget, on any installed agent harness), send, open, list, get, rename, move, move to group, clear project, fork, archive.", "actions": { "create": { - "description": "Create a new chat session on any installed agent harness and send the prompt in it. Fire-and-forget: returns the session id immediately and the session runs in the background without changing what the user sees; the user can open it themselves. Only check on it later (action \"get\") if the user asks.", + "description": "Create a new chat session on any installed agent harness and send the prompt in it. Fire-and-forget: returns the session id immediately and the session runs in the background without changing what the user sees; the user can open it themselves. Use --from to give the delegating session or tool a concise visible label on the initial message. Only check on it later (action \"get\") if the user asks.", "fields": [ { "name": "prompt", @@ -47,6 +47,14 @@ "description": "Branch/worktree name when the project's startup mode is branch or worktree; required for those modes.", "min": 1, "max": 200 + }, + { + "name": "from", + "required": false, + "kind": "string", + "description": "Optional visible sender label for the initial message (1-120 chars).", + "min": 1, + "max": 120 } ], "schema": { @@ -80,6 +88,13 @@ "type": "string", "minLength": 1, "maxLength": 200 + }, + "from": { + "description": "Optional visible sender label for the initial message (1-120 chars).", + "type": "string", + "minLength": 1, + "maxLength": 120, + "pattern": "^[^\\r\\n]*$" } }, "required": ["prompt"], diff --git a/src-tauri/crates/berdctl/cli-surface-feedback.json b/src-tauri/crates/berdctl/cli-surface-feedback.json index 34666f45a..833ae19f4 100644 --- a/src-tauri/crates/berdctl/cli-surface-feedback.json +++ b/src-tauri/crates/berdctl/cli-surface-feedback.json @@ -8,7 +8,7 @@ "create": { "action": "create", "about": "Create a new chat session and send a prompt in it (fire-and-forget)", - "afterHelp": "Examples:\n berdctl session create --prompt \"Triage the failing nightly build\" \\\n --harness-id claude-acp --json\n berdctl session create --prompt \"Implement the fix\" \\\n --project-id --startup-name my-feature\n\nResult:\n {\"session_id\": \"...\", \"title\": \"...\", \"harness_id\": \"...\",\n \"send_status\": \"dispatched\"}\n The session runs in the background; the user's view does not change. Check\n progress later with `berdctl session get --session-id `." + "afterHelp": "Examples:\n berdctl session create --prompt \"Triage the failing nightly build\" \\\n --harness-id claude-acp --from \"the release orchestrator\" --json\n berdctl session create --prompt \"Implement the fix\" \\\n --project-id --startup-name my-feature\n\nResult:\n {\"session_id\": \"...\", \"title\": \"...\", \"harness_id\": \"...\",\n \"send_status\": \"dispatched\"}\n The session runs in the background; the user's view does not change. Check\n progress later with `berdctl session get --session-id `." }, "send": { "action": "send", diff --git a/src-tauri/crates/berdctl/cli-surface.json b/src-tauri/crates/berdctl/cli-surface.json index c5093ff21..9f2acbeb9 100644 --- a/src-tauri/crates/berdctl/cli-surface.json +++ b/src-tauri/crates/berdctl/cli-surface.json @@ -8,7 +8,7 @@ "create": { "action": "create", "about": "Create a new chat session and send a prompt in it (fire-and-forget)", - "afterHelp": "Examples:\n berdctl session create --prompt \"Triage the failing nightly build\" \\\n --harness-id claude-acp --json\n berdctl session create --prompt \"Implement the fix\" \\\n --project-id --startup-name my-feature\n\nResult:\n {\"session_id\": \"...\", \"title\": \"...\", \"harness_id\": \"...\",\n \"send_status\": \"dispatched\"}\n The session runs in the background; the user's view does not change. Check\n progress later with `berdctl session get --session-id `." + "afterHelp": "Examples:\n berdctl session create --prompt \"Triage the failing nightly build\" \\\n --harness-id claude-acp --from \"the release orchestrator\" --json\n berdctl session create --prompt \"Implement the fix\" \\\n --project-id --startup-name my-feature\n\nResult:\n {\"session_id\": \"...\", \"title\": \"...\", \"harness_id\": \"...\",\n \"send_status\": \"dispatched\"}\n The session runs in the background; the user's view does not change. Check\n progress later with `berdctl session get --session-id `." }, "send": { "action": "send", diff --git a/src-tauri/crates/berdctl/src/main.rs b/src-tauri/crates/berdctl/src/main.rs index 5cc943846..da8e9a981 100644 --- a/src-tauri/crates/berdctl/src/main.rs +++ b/src-tauri/crates/berdctl/src/main.rs @@ -622,7 +622,9 @@ mod tests { const EXPECTED_SESSION_CREATE_HELP: &str = r#"Create a new chat session on any installed agent harness and send the prompt in it. Fire-and-forget: returns the session id immediately and the session runs in the background without changing what the user sees; the user can open it -themselves. Only check on it later (action "get") if the user asks. +themselves. Use --from to give the delegating session or tool a concise visible +label on the initial message. Only check on it later (action "get") if the user +asks. Usage: berdctl session create [OPTIONS] --prompt @@ -647,6 +649,9 @@ Options: Branch/worktree name when the project's startup mode is branch or worktree; required for those modes. + --from + Optional visible sender label for the initial message (1-120 chars). + --json Print the raw JSON result on a single line (default: pretty-printed JSON) @@ -660,7 +665,7 @@ Options: Examples: berdctl session create --prompt "Triage the failing nightly build" \ - --harness-id claude-acp --json + --harness-id claude-acp --from "the release orchestrator" --json berdctl session create --prompt "Implement the fix" \ --project-id --startup-name my-feature diff --git a/src/features/berdctl/__tests__/commands/commands.test.ts b/src/features/berdctl/__tests__/commands/commands.test.ts index 49832dddc..7b9b91e3e 100644 --- a/src/features/berdctl/__tests__/commands/commands.test.ts +++ b/src/features/berdctl/__tests__/commands/commands.test.ts @@ -707,6 +707,7 @@ describe("sessions.create", () => { prompt: "what is 1+1", agent_id: "agent-7", model_id: "model-9", + from: "the test orchestrator", }, ctx, ); @@ -733,7 +734,19 @@ describe("sessions.create", () => { useChatStore.getState().queuedMessageBySession["session-new"]; expect(queued?.[0]).toMatchObject({ kind: "transport-ready", - payload: { text: "what is 1+1" }, + payload: { + text: "what is 1+1", + sendOptions: { + userMessageMetadata: { + origin: "berdctl_cross_session", + berdSenderLabel: "the test orchestrator", + }, + acpGooseMetadata: { + origin: "berdctl_cross_session", + berdSenderLabel: "the test orchestrator", + }, + }, + }, }); expect(controller.openSession).not.toHaveBeenCalled(); }); diff --git a/src/features/berdctl/commands/impl/createSession.ts b/src/features/berdctl/commands/impl/createSession.ts index ffdee39e9..dfebbf343 100644 --- a/src/features/berdctl/commands/impl/createSession.ts +++ b/src/features/berdctl/commands/impl/createSession.ts @@ -41,6 +41,16 @@ const createSessionSchema = z .describe( "Branch/worktree name when the project's startup mode is branch or worktree; required for those modes.", ), + from: z + .string() + .trim() + .min(1) + .max(120) + .regex(/^[^\r\n]*$/, "Sender label must be a single line.") + .optional() + .describe( + "Optional visible sender label for the initial message (1-120 chars).", + ), }) .strict(); @@ -65,10 +75,12 @@ export const createSessionCommand = defineCommand({ "Create a new chat session on any installed agent harness and send the prompt in it. " + "Fire-and-forget: returns the session id immediately and the session runs in the " + "background without changing what the user sees; the user can open it themselves. " + + "Use --from to give the delegating session or tool a concise visible label on " + + "the initial message. " + 'Only check on it later (action "get") if the user asks.', helpFooter: `Examples: berdctl session create --prompt "Triage the failing nightly build" \\ - --harness-id claude-acp --json + --harness-id claude-acp --from "the release orchestrator" --json berdctl session create --prompt "Implement the fix" \\ --project-id --startup-name my-feature @@ -211,7 +223,9 @@ Result: persona: persona ? { kind: "persona", id: persona.id, name: persona.displayName } : { kind: "inherit" }, - sendOptions: berdctlCrossSessionSendOptions(), + sendOptions: berdctlCrossSessionSendOptions({ + senderLabel: args.from, + }), }), { project, queueReady: true }, ); From f982eb7ee9b1830bcb41d821e48fff8013334aa3 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 13:39:14 -0400 Subject: [PATCH 04/22] fix(monitor): bound unterminated output records Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 93 ++++++++++++++++++----- 1 file changed, 73 insertions(+), 20 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 35db70258..65b77b653 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -3,7 +3,7 @@ use fs2::FileExt; use std::env; use std::ffi::OsString; use std::fs::{self, File, OpenOptions}; -use std::io::{self, BufRead, BufReader, Read, Write}; +use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, ExitCode, Stdio}; use std::sync::mpsc::{self, RecvTimeoutError}; @@ -605,24 +605,7 @@ fn run_producer( write_launch_status(paths, launch_token, "ready"); let stdout = child.stdout.take().expect("stdout was piped"); let (sender, receiver) = mpsc::sync_channel::>(1024); - thread::spawn(move || { - let mut reader = BufReader::new(stdout); - loop { - let mut line = Vec::new(); - match reader.read_until(b'\n', &mut line) { - Ok(0) => break, - Ok(_) => { - if !line.ends_with(b"\n") { - line.push(b'\n'); - } - if sender.send(line).is_err() { - break; - } - } - Err(_) => break, - } - } - }); + thread::spawn(move || forward_producer_output(stdout, sender)); let mut batch = Vec::new(); let mut batch_deadline: Option = None; @@ -677,8 +660,34 @@ fn run_producer( .min(Duration::from_millis(100)); match receiver.recv_timeout(timeout) { Ok(line) => { + if !batch.is_empty() && batch.len() + line.len() > MAX_DELIVERY_BYTES { + append_batch(paths, &mut pending, &mut batch)?; + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + retry_deadline = Instant::now() + RETRY_INTERVAL; + } batch.extend_from_slice(&line); - batch_deadline.get_or_insert(Instant::now() + BATCH_WINDOW); + if batch.len() >= MAX_DELIVERY_BYTES { + append_batch(paths, &mut pending, &mut batch)?; + flush_pending( + paths, + &mut pending, + label, + instructions, + session_id, + if_running, + )?; + retry_deadline = Instant::now() + RETRY_INTERVAL; + batch_deadline = None; + } else { + batch_deadline.get_or_insert(Instant::now() + BATCH_WINDOW); + } } Err(RecvTimeoutError::Timeout) => {} Err(RecvTimeoutError::Disconnected) => receiver_closed = true, @@ -720,6 +729,33 @@ fn run_producer( Ok(()) } +fn forward_producer_output(mut reader: R, sender: mpsc::SyncSender>) { + let mut read_buffer = [0_u8; 8 * 1024]; + let mut record = Vec::with_capacity(MAX_DELIVERY_BYTES); + loop { + let read = match reader.read(&mut read_buffer) { + Ok(0) => break, + Ok(read) => read, + Err(_) => return, + }; + for byte in &read_buffer[..read] { + record.push(*byte); + if *byte == b'\n' || record.len() == MAX_DELIVERY_BYTES { + if sender.send(std::mem::take(&mut record)).is_err() { + return; + } + record = Vec::with_capacity(MAX_DELIVERY_BYTES); + } + } + } + if !record.is_empty() { + if !record.ends_with(b"\n") { + record.push(b'\n'); + } + let _ = sender.send(record); + } +} + fn append_batch(paths: &StatePaths, pending: &mut Vec, batch: &mut Vec) -> io::Result<()> { if batch.is_empty() { return Ok(()); @@ -938,6 +974,23 @@ mod tests { ); } + #[test] + fn producer_output_splits_unterminated_records_before_they_can_grow_unbounded() { + let input = vec![b'x'; MAX_DELIVERY_BYTES * 3 + 17]; + let (sender, receiver) = mpsc::sync_channel(8); + + forward_producer_output(io::Cursor::new(&input), sender); + let records = receiver.into_iter().collect::>(); + + assert_eq!(records.len(), 4); + assert!(records + .iter() + .all(|record| record.len() <= MAX_DELIVERY_BYTES)); + let mut output = records.concat(); + assert_eq!(output.pop(), Some(b'\n')); + assert_eq!(output, input); + } + #[test] fn state_key_path_is_stable_and_safe() { let first = StatePaths::for_key("pr/123 checks", "session-a"); From 1201f012a24faf312c0d4ef1209fa7e47de288e4 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 13:40:18 -0400 Subject: [PATCH 05/22] fix(chat): retain Berd message provenance Signed-off-by: John Tennant --- src/features/chat/ui/__tests__/MessageBubble.test.tsx | 9 ++++----- src/shared/i18n/locales/en/chat.json | 2 +- src/shared/i18n/locales/es/chat.json | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/features/chat/ui/__tests__/MessageBubble.test.tsx b/src/features/chat/ui/__tests__/MessageBubble.test.tsx index c20768887..ab0e28dca 100644 --- a/src/features/chat/ui/__tests__/MessageBubble.test.tsx +++ b/src/features/chat/ui/__tests__/MessageBubble.test.tsx @@ -617,20 +617,19 @@ describe("MessageBubble", () => { ); }); - it("shows a named sender for attributed cross-session messages", () => { + it("shows a sender descriptor without replacing trusted Berd provenance", () => { const message = userMessage("[monitor: PR checks] complete"); message.metadata = { ...message.metadata, origin: "berdctl_cross_session", - berdSenderLabel: "berd-monitor", + berdSenderLabel: "Morgan", }; render(); - expect(screen.getByText("Sent by berd-monitor")).toBeInTheDocument(); expect( - screen.queryByText("Sent by Berd from another session"), - ).not.toBeInTheDocument(); + screen.getByText("Sent by Berd from another session · source: Morgan"), + ).toBeInTheDocument(); }); it("renders provenance and steer labels together", () => { diff --git a/src/shared/i18n/locales/en/chat.json b/src/shared/i18n/locales/en/chat.json index aaa42cb21..2111b9a26 100644 --- a/src/shared/i18n/locales/en/chat.json +++ b/src/shared/i18n/locales/en/chat.json @@ -406,7 +406,7 @@ "anthropicThinkingHistory": "This chat can't continue with a Claude model because its earlier reasoning history is no longer in a form Claude will accept. Start a new chat, or switch this chat to a non-Claude model to keep going." }, "berdctlCrossSessionLabel": "Sent by Berd from another session", - "berdctlCrossSessionNamedLabel": "Sent by {{sender}}", + "berdctlCrossSessionNamedLabel": "Sent by Berd from another session · source: {{sender}}", "steerLabel": "Steered", "viewMore": "View more", "viewLess": "View less", diff --git a/src/shared/i18n/locales/es/chat.json b/src/shared/i18n/locales/es/chat.json index 93716f040..23669d44a 100644 --- a/src/shared/i18n/locales/es/chat.json +++ b/src/shared/i18n/locales/es/chat.json @@ -405,7 +405,7 @@ "anthropicThinkingHistory": "Este chat no puede continuar con un modelo Claude porque su historial de razonamiento previo ya no tiene una forma que Claude acepte. Inicia un chat nuevo o cambia este chat a un modelo que no sea Claude para continuar." }, "berdctlCrossSessionLabel": "Enviado por Berd desde otra sesión", - "berdctlCrossSessionNamedLabel": "Enviado por {{sender}}", + "berdctlCrossSessionNamedLabel": "Enviado por Berd desde otra sesión · fuente: {{sender}}", "steerLabel": "Guiado", "viewMore": "Ver más", "viewLess": "Ver menos", From 006cd245acbf73722f9988466d8f4009c5bf6cca Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 13:45:16 -0400 Subject: [PATCH 06/22] fix(monitor): keep stop requests responsive Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 143 ++++++++++++++++++++-- 1 file changed, 131 insertions(+), 12 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 65b77b653..129e2ef4b 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -18,6 +18,8 @@ const MAX_DELIVERY_BYTES: usize = 40_000; const MAX_PROMPT_CODE_UNITS: usize = 49_000; const MAX_INSTRUCTIONS_CODE_UNITS: usize = 4_000; const MAX_LABEL_CODE_UNITS: usize = 120; +const MAX_LOCK_CANDIDATES: usize = 8; +const DELIVERY_POLL_INTERVAL: Duration = Duration::from_millis(50); const LAUNCH_TIMEOUT: Duration = Duration::from_secs(10); #[derive(Clone, Copy, Debug, ValueEnum)] @@ -792,7 +794,7 @@ fn flush_pending( "monitor delivery prompt exceeded its internal size bound", )); } - if !deliver(session_id, &prompt, if_running) { + if !deliver(paths, session_id, &prompt, if_running) { log_line(paths, "delivery failed; buffered output will be retried")?; return Ok(()); } @@ -803,10 +805,36 @@ fn flush_pending( Ok(()) } -fn deliver(session_id: &str, prompt: &str, if_running: RunningMode) -> bool { - for lock in lock_candidates() { - for binary in berdctl_candidates() { - let status = Command::new(&binary) +fn deliver( + paths: &StatePaths, + session_id: &str, + prompt: &str, + if_running: RunningMode, +) -> bool { + deliver_with_candidates( + paths, + session_id, + prompt, + if_running, + lock_candidates(), + berdctl_candidates(), + ) +} + +fn deliver_with_candidates( + paths: &StatePaths, + session_id: &str, + prompt: &str, + if_running: RunningMode, + locks: Vec, + binaries: Vec, +) -> bool { + for lock in locks { + for binary in &binaries { + if paths.stop.exists() { + return false; + } + let mut child = match Command::new(binary) .arg("--lock-path") .arg(&lock) .arg("--timeout-ms") @@ -825,9 +853,22 @@ fn deliver(session_id: &str, prompt: &str, if_running: RunningMode) -> bool { .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) - .status(); - if status.is_ok_and(|status| status.success()) { - return true; + .spawn() + { + Ok(child) => child, + Err(_) => continue, + }; + loop { + if paths.stop.exists() { + let _ = child.kill(); + let _ = child.wait(); + return false; + } + match child.try_wait() { + Ok(Some(status)) if status.success() => return true, + Ok(Some(_)) | Err(_) => break, + Ok(None) => thread::sleep(DELIVERY_POLL_INTERVAL), + } } } } @@ -838,7 +879,11 @@ fn lock_candidates() -> Vec { let Some(explicit) = env::var_os("BERDCTL_LOCK").map(PathBuf::from) else { return Vec::new(); }; - let mut candidates = vec![explicit.clone()]; + lock_candidates_for(&explicit) +} + +fn lock_candidates_for(explicit: &Path) -> Vec { + let mut candidates = vec![explicit.to_path_buf()]; if let Some(parent) = explicit.parent() { let mut siblings = fs::read_dir(parent) .into_iter() @@ -861,7 +906,10 @@ fn lock_candidates() -> Vec { }) .collect::>(); siblings.sort_by(|left, right| right.0.cmp(&left.0)); - for (_, sibling) in siblings { + for (_, sibling) in siblings + .into_iter() + .take(MAX_LOCK_CANDIDATES.saturating_sub(1)) + { if sibling != explicit { candidates.push(sibling); } @@ -875,11 +923,14 @@ fn berdctl_candidates() -> Vec { if let Some(explicit) = env::var_os("BERDCTL_BIN") { candidates.push(explicit); } - candidates.push(OsString::from(if cfg!(windows) { + let default = OsString::from(if cfg!(windows) { "berdctl.exe" } else { "berdctl" - })); + }); + if !candidates.contains(&default) { + candidates.push(default); + } candidates } @@ -991,6 +1042,74 @@ mod tests { assert_eq!(output, input); } + #[cfg(unix)] + #[test] + fn stop_interrupts_a_nonresponsive_delivery_probe() { + use std::os::unix::fs::PermissionsExt; + + let key = format!( + "delivery-stop-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let session_id = "test-session"; + let paths = StatePaths::for_key(&key, session_id); + ensure_private_directory(&paths.root).unwrap(); + let owner = claim_owner(&paths).unwrap(); + let fake_berdctl = paths.root.join("fake-berdctl"); + fs::write(&fake_berdctl, "#!/bin/sh\nexec sleep 30\n").unwrap(); + fs::set_permissions(&fake_berdctl, fs::Permissions::from_mode(0o700)).unwrap(); + + let worker_key = key.clone(); + let worker_binary = fake_berdctl.into_os_string(); + let started = Instant::now(); + let worker = thread::spawn(move || { + let worker_paths = StatePaths::for_key(&worker_key, session_id); + deliver_with_candidates( + &worker_paths, + session_id, + "test", + RunningMode::Steer, + vec![PathBuf::from("stale-a"), PathBuf::from("stale-b")], + vec![worker_binary], + ) + }); + thread::sleep(Duration::from_millis(150)); + request_stop(&key, session_id).unwrap(); + + assert!(!worker.join().unwrap()); + assert!(started.elapsed() < Duration::from_secs(2)); + drop(owner); + fs::remove_dir_all(&paths.root).unwrap(); + } + + #[test] + fn lock_candidates_keep_the_explicit_lock_and_bound_siblings() { + let root = env::temp_dir().join(format!( + "berd-monitor-locks-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&root).unwrap(); + let explicit = root.join("control-explicit.json"); + fs::write(&explicit, "{}").unwrap(); + for index in 0..(MAX_LOCK_CANDIDATES * 2) { + fs::write(root.join(format!("control-{index}.json")), "{}").unwrap(); + } + + let candidates = lock_candidates_for(&explicit); + + assert_eq!(candidates.first(), Some(&explicit)); + assert_eq!(candidates.len(), MAX_LOCK_CANDIDATES); + fs::remove_dir_all(root).unwrap(); + } + #[test] fn state_key_path_is_stable_and_safe() { let first = StatePaths::for_key("pr/123 checks", "session-a"); From dc22c923071ce0c36c30c2fa79f949a36adde2b8 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 13:48:40 -0400 Subject: [PATCH 07/22] fix(monitor): replace pending output atomically Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/Cargo.toml | 1 + src-tauri/crates/berd-monitor/src/main.rs | 89 ++++++++++++++++++++++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/src-tauri/crates/berd-monitor/Cargo.toml b/src-tauri/crates/berd-monitor/Cargo.toml index 3ae520ea2..a15db1b44 100644 --- a/src-tauri/crates/berd-monitor/Cargo.toml +++ b/src-tauri/crates/berd-monitor/Cargo.toml @@ -16,6 +16,7 @@ libc = "0.2" windows-sys = { version = "0.59", features = [ "Win32_Foundation", "Win32_Security", + "Win32_Storage_FileSystem", "Win32_System_Diagnostics_ToolHelp", "Win32_System_JobObjects", "Win32_System_Threading", diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 129e2ef4b..7526a80b6 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -973,12 +973,70 @@ fn append_file(path: &Path, data: &[u8]) -> io::Result<()> { } fn atomic_write(path: &Path, data: &[u8]) -> io::Result<()> { + atomic_write_with(path, data, replace_file_atomically) +} + +fn atomic_write_with( + path: &Path, + data: &[u8], + replace: impl FnOnce(&Path, &Path) -> io::Result<()>, +) -> io::Result<()> { let temporary = path.with_extension(format!("tmp.{}", std::process::id())); - fs::write(&temporary, data)?; - if path.exists() { - fs::remove_file(path)?; + let mut file = OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&temporary)?; + file.write_all(data)?; + file.sync_all()?; + drop(file); + + if let Err(error) = replace(&temporary, path) { + let _ = fs::remove_file(&temporary); + return Err(error); + } + + #[cfg(unix)] + if let Some(parent) = path.parent() { + File::open(parent)?.sync_all()?; + } + Ok(()) +} + +#[cfg(unix)] +fn replace_file_atomically(source: &Path, destination: &Path) -> io::Result<()> { + fs::rename(source, destination) +} + +#[cfg(windows)] +fn replace_file_atomically(source: &Path, destination: &Path) -> io::Result<()> { + use std::os::windows::ffi::OsStrExt; + use windows_sys::Win32::Storage::FileSystem::{ + MoveFileExW, MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, + }; + + let source = source + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + let destination = destination + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + let result = unsafe { + MoveFileExW( + source.as_ptr(), + destination.as_ptr(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + if result == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) } - fs::rename(temporary, path) } fn log_line(paths: &StatePaths, message: &str) -> io::Result<()> { @@ -1003,6 +1061,29 @@ fn render_command(command: &[OsString]) -> String { mod tests { use super::*; + #[test] + fn failed_atomic_replace_preserves_pending_output() { + let root = env::temp_dir().join(format!( + "berd-monitor-atomic-write-test-{}", + std::process::id() + )); + fs::create_dir_all(&root).unwrap(); + let pending = root.join("pending"); + fs::write(&pending, b"still pending").unwrap(); + + let error = atomic_write_with(&pending, b"replacement", |_, _| { + Err(io::Error::other("injected replacement failure")) + }) + .unwrap_err(); + + assert_eq!(error.kind(), io::ErrorKind::Other); + assert_eq!(fs::read(&pending).unwrap(), b"still pending"); + assert!(!pending + .with_extension(format!("tmp.{}", std::process::id())) + .exists()); + fs::remove_dir_all(root).unwrap(); + } + #[cfg(unix)] fn test_process_exists(pid: u32) -> bool { let result = unsafe { libc::kill(pid as i32, 0) }; From 8b2fc6372c1d398a5e48abd1f7b19ad1252ae111 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:01:56 -0400 Subject: [PATCH 08/22] fix(monitor): stabilize cross-platform formatting Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 7526a80b6..a10254dd7 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -794,7 +794,7 @@ fn flush_pending( "monitor delivery prompt exceeded its internal size bound", )); } - if !deliver(paths, session_id, &prompt, if_running) { + if !deliver_to_session(paths, session_id, &prompt, if_running) { log_line(paths, "delivery failed; buffered output will be retried")?; return Ok(()); } @@ -805,7 +805,7 @@ fn flush_pending( Ok(()) } -fn deliver( +fn deliver_to_session( paths: &StatePaths, session_id: &str, prompt: &str, From abf9cf4d5b3afa03e1b22722ef46fcd997252224 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:31:00 -0400 Subject: [PATCH 09/22] fix(monitor): terminate timed-out launches Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 77 +++++++++++++++++++++-- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index a10254dd7..eb6a86b80 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -21,6 +21,7 @@ const MAX_LABEL_CODE_UNITS: usize = 120; const MAX_LOCK_CANDIDATES: usize = 8; const DELIVERY_POLL_INTERVAL: Duration = Duration::from_millis(50); const LAUNCH_TIMEOUT: Duration = Duration::from_secs(10); +const LAUNCH_TERMINATION_GRACE: Duration = Duration::from_secs(1); #[derive(Clone, Copy, Debug, ValueEnum)] enum RunningMode { @@ -259,10 +260,19 @@ fn spawn_detached(state_key: &str, session_id: &str) -> Result<(), String> { let mut process = child .spawn() .map_err(|error| format!("start detached monitor: {error}"))?; - let deadline = Instant::now() + LAUNCH_TIMEOUT; + wait_for_launch(&mut process, &paths, &status_path, LAUNCH_TIMEOUT) +} + +fn wait_for_launch( + process: &mut std::process::Child, + paths: &StatePaths, + status_path: &Path, + timeout: Duration, +) -> Result<(), String> { + let deadline = Instant::now() + timeout; loop { - if let Ok(status) = fs::read_to_string(&status_path) { - let _ = fs::remove_file(&status_path); + if let Ok(status) = fs::read_to_string(status_path) { + let _ = fs::remove_file(status_path); if status.trim() == "ready" { println!("{} {}", process.id(), paths.root.display()); return Ok(()); @@ -273,19 +283,43 @@ fn spawn_detached(state_key: &str, session_id: &str) -> Result<(), String> { .to_owned()); } if Instant::now() >= deadline { + terminate_timed_out_launch(process, paths, status_path); return Err(format!( "monitor did not become ready within {} seconds; inspect {}", - LAUNCH_TIMEOUT.as_secs(), + timeout.as_secs_f64(), paths.log.display() )); } if let Ok(Some(status)) = process.try_wait() { + let _ = fs::remove_file(status_path); return Err(format!("monitor exited before becoming ready ({status})")); } thread::sleep(Duration::from_millis(50)); } } +fn terminate_timed_out_launch( + process: &mut std::process::Child, + paths: &StatePaths, + status_path: &Path, +) { + let _ = fs::write(&paths.stop, b"stop\n"); + let deadline = Instant::now() + LAUNCH_TERMINATION_GRACE; + while Instant::now() < deadline { + match process.try_wait() { + Ok(Some(_)) => break, + Ok(None) => thread::sleep(Duration::from_millis(50)), + Err(_) => break, + } + } + if process.try_wait().ok().flatten().is_none() { + let _ = process.kill(); + let _ = process.wait(); + } + let _ = fs::remove_file(status_path); + let _ = fs::remove_file(&paths.stop); +} + #[cfg(unix)] fn configure_detached(command: &mut Command) { use std::os::unix::process::CommandExt; @@ -1061,6 +1095,41 @@ fn render_command(command: &[OsString]) -> String { mod tests { use super::*; + #[cfg(unix)] + #[test] + fn launch_timeout_terminates_and_reaps_the_child() { + let key = format!( + "launch-timeout-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + ensure_private_directory(&paths.root).unwrap(); + let status_path = paths.launch_status("never-ready"); + let mut child = Command::new("sh") + .args(["-c", "exec sleep 30"]) + .spawn() + .unwrap(); + let pid = child.id(); + + let error = wait_for_launch( + &mut child, + &paths, + &status_path, + Duration::from_millis(100), + ) + .unwrap_err(); + + assert!(error.contains("did not become ready")); + assert!(!test_process_exists(pid)); + assert!(!status_path.exists()); + assert!(!paths.stop.exists()); + fs::remove_dir_all(paths.root).unwrap(); + } + #[test] fn failed_atomic_replace_preserves_pending_output() { let root = env::temp_dir().join(format!( From e698da2feadf91803f8fb95f441996703ab8eaba Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:32:50 -0400 Subject: [PATCH 10/22] fix(monitor): clean up producers on errors Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 78 ++++++++++++++++++++--- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index eb6a86b80..e5fd40294 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -480,6 +480,24 @@ impl Drop for ProducerTree { } } +struct ProducerProcess { + child: std::process::Child, + tree: ProducerTree, +} + +impl ProducerProcess { + fn terminate_and_wait(&mut self) { + terminate_producer_tree(&mut self.child, &self.tree); + let _ = self.child.wait(); + } +} + +impl Drop for ProducerProcess { + fn drop(&mut self) { + self.terminate_and_wait(); + } +} + fn ensure_private_directory(path: &Path) -> io::Result<()> { fs::create_dir_all(path)?; #[cfg(unix)] @@ -613,33 +631,37 @@ fn run_producer( )?; let mut pending = read_optional(&paths.pending)?; - let mut producer = Command::new(&producer_command[0]); - producer + let mut command = Command::new(&producer_command[0]); + command .args(&producer_command[1..]) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::from(diagnostics.try_clone()?)); - configure_producer(&mut producer); - let mut child = producer.spawn()?; + configure_producer(&mut command); + let mut child = command.spawn()?; let producer_tree = match attach_producer_tree(&child) { Ok(tree) => tree, Err(error) => { let _ = child.kill(); + let _ = child.wait(); return Err(io::Error::new( error.kind(), format!("attach producer process tree: {error}"), )); } }; - if let Err(error) = resume_producer(&child) { - terminate_producer_tree(&mut child, &producer_tree); + let mut producer = ProducerProcess { + child, + tree: producer_tree, + }; + if let Err(error) = resume_producer(&producer.child) { return Err(io::Error::new( error.kind(), format!("resume producer process: {error}"), )); } write_launch_status(paths, launch_token, "ready"); - let stdout = child.stdout.take().expect("stdout was piped"); + let stdout = producer.child.stdout.take().expect("stdout was piped"); let (sender, receiver) = mpsc::sync_channel::>(1024); thread::spawn(move || forward_producer_output(stdout, sender)); @@ -653,7 +675,7 @@ fn run_producer( loop { if paths.stop.exists() && !termination_requested { log_line(paths, "stop requested")?; - terminate_producer_tree(&mut child, &producer_tree); + producer.terminate_and_wait(); termination_requested = true; } @@ -684,7 +706,7 @@ fn run_producer( } if producer_status.is_none() { - producer_status = child.try_wait()?; + producer_status = producer.child.try_wait()?; } if producer_status.is_some() && receiver_closed { break; @@ -730,7 +752,7 @@ fn run_producer( } } - terminate_producer_tree(&mut child, &producer_tree); + producer.terminate_and_wait(); append_batch(paths, &mut pending, &mut batch)?; let status = producer_status.expect("producer has exited"); let summary = format!("[monitor] producer exited with status {status}\n"); @@ -1159,6 +1181,42 @@ mod tests { result == 0 || io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) } + #[cfg(unix)] + #[test] + fn producer_guard_cleans_up_the_process_group_on_error() { + use std::io::{BufRead, BufReader}; + + let mut observed_pids = None; + let result: io::Result<()> = (|| { + let mut command = Command::new("sh"); + command + .args(["-c", "sleep 30 & child=$!; echo $child; wait"]) + .stdout(Stdio::piped()); + configure_producer(&mut command); + let child = command.spawn()?; + let tree = attach_producer_tree(&child)?; + let mut producer = ProducerProcess { child, tree }; + resume_producer(&producer.child)?; + let direct_pid = producer.child.id(); + let mut descendant = String::new(); + BufReader::new(producer.child.stdout.take().unwrap()) + .read_line(&mut descendant)?; + observed_pids = Some((direct_pid, descendant.trim().parse::().unwrap())); + Err(io::Error::other("injected post-spawn failure")) + })(); + + assert_eq!(result.unwrap_err().kind(), io::ErrorKind::Other); + let (direct_pid, descendant_pid) = observed_pids.unwrap(); + let deadline = Instant::now() + Duration::from_secs(2); + while (test_process_exists(direct_pid) || test_process_exists(descendant_pid)) + && Instant::now() < deadline + { + thread::sleep(Duration::from_millis(25)); + } + assert!(!test_process_exists(direct_pid)); + assert!(!test_process_exists(descendant_pid)); + } + #[test] fn chunk_prefers_complete_lines() { let input = b"first\nsecond\nthird\n"; From 9ac883fe01dfcdead484477f744680260b851ccd Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:35:06 -0400 Subject: [PATCH 11/22] fix(monitor): bound delivery subprocesses Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 59 +++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index e5fd40294..37035cba7 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -20,6 +20,7 @@ const MAX_INSTRUCTIONS_CODE_UNITS: usize = 4_000; const MAX_LABEL_CODE_UNITS: usize = 120; const MAX_LOCK_CANDIDATES: usize = 8; const DELIVERY_POLL_INTERVAL: Duration = Duration::from_millis(50); +const DELIVERY_TIMEOUT: Duration = Duration::from_secs(15); const LAUNCH_TIMEOUT: Duration = Duration::from_secs(10); const LAUNCH_TERMINATION_GRACE: Duration = Duration::from_secs(1); @@ -874,6 +875,7 @@ fn deliver_to_session( if_running, lock_candidates(), berdctl_candidates(), + DELIVERY_TIMEOUT, ) } @@ -884,6 +886,7 @@ fn deliver_with_candidates( if_running: RunningMode, locks: Vec, binaries: Vec, + timeout: Duration, ) -> bool { for lock in locks { for binary in &binaries { @@ -914,12 +917,18 @@ fn deliver_with_candidates( Ok(child) => child, Err(_) => continue, }; + let deadline = Instant::now() + timeout; loop { if paths.stop.exists() { let _ = child.kill(); let _ = child.wait(); return false; } + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + break; + } match child.try_wait() { Ok(Some(status)) if status.success() => return true, Ok(Some(_)) | Err(_) => break, @@ -1283,6 +1292,7 @@ mod tests { RunningMode::Steer, vec![PathBuf::from("stale-a"), PathBuf::from("stale-b")], vec![worker_binary], + DELIVERY_TIMEOUT, ) }); thread::sleep(Duration::from_millis(150)); @@ -1294,6 +1304,55 @@ mod tests { fs::remove_dir_all(&paths.root).unwrap(); } + #[cfg(unix)] + #[test] + fn delivery_timeout_terminates_and_reaps_the_probe() { + use std::os::unix::fs::PermissionsExt; + + let key = format!( + "delivery-timeout-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let session_id = "test-session"; + let paths = StatePaths::for_key(&key, session_id); + ensure_private_directory(&paths.root).unwrap(); + let pid_file = paths.root.join("probe.pid"); + let fake_berdctl = paths.root.join("fake-berdctl"); + fs::write( + &fake_berdctl, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$$\" > '{}'\nexec sleep 30\n", + pid_file.display() + ), + ) + .unwrap(); + fs::set_permissions(&fake_berdctl, fs::Permissions::from_mode(0o700)).unwrap(); + + let started = Instant::now(); + assert!(!deliver_with_candidates( + &paths, + session_id, + "test", + RunningMode::Steer, + vec![PathBuf::from("stale")], + vec![fake_berdctl.into_os_string()], + Duration::from_millis(500), + )); + + let pid = fs::read_to_string(pid_file) + .unwrap() + .trim() + .parse::() + .unwrap(); + assert!(started.elapsed() < Duration::from_secs(2)); + assert!(!test_process_exists(pid)); + fs::remove_dir_all(paths.root).unwrap(); + } + #[test] fn lock_candidates_keep_the_explicit_lock_and_bound_siblings() { let root = env::temp_dir().join(format!( From 92ff33c32f81a514a37517376b10038967febabc Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:45:01 -0400 Subject: [PATCH 12/22] fix(monitor): deduplicate retried deliveries Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 196 ++++++++++++++++-- .../crates/berdctl/api-surface-feedback.json | 17 +- src-tauri/crates/berdctl/api-surface.json | 17 +- .../crates/berdctl/cli-surface-feedback.json | 2 +- src-tauri/crates/berdctl/cli-surface.json | 2 +- .../__tests__/commands/commands.test.ts | 72 +++++++ .../berdctl/commands/impl/sendSession.ts | 28 ++- .../berdctl/commands/runtime/sessionSend.ts | 25 ++- .../api/__tests__/acpReplayMetadata.test.ts | 2 + src/shared/api/acpReplayMetadata.ts | 6 + src/shared/types/messages.ts | 1 + 11 files changed, 339 insertions(+), 29 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 37035cba7..2c1823c07 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -6,6 +6,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, ExitCode, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc::{self, RecvTimeoutError}; use std::thread; use std::time::{Duration, Instant, SystemTime}; @@ -23,6 +24,9 @@ const DELIVERY_POLL_INTERVAL: Duration = Duration::from_millis(50); const DELIVERY_TIMEOUT: Duration = Duration::from_secs(15); const LAUNCH_TIMEOUT: Duration = Duration::from_secs(10); const LAUNCH_TERMINATION_GRACE: Duration = Duration::from_secs(1); +const PENDING_HEADER_PREFIX: &str = "BERD_MONITOR_PENDING_V1 "; + +static PENDING_GENERATION_COUNTER: AtomicU64 = AtomicU64::new(0); #[derive(Clone, Copy, Debug, ValueEnum)] enum RunningMode { @@ -91,6 +95,41 @@ struct StatePaths { stop: PathBuf, } +struct PendingState { + generation: String, + active_len: usize, + bytes: Vec, +} + +impl PendingState { + fn empty() -> Self { + Self { + generation: next_pending_generation(), + active_len: 0, + bytes: Vec::new(), + } + } + + fn delivery_id(&self, paths: &StatePaths) -> String { + let state_hash = stable_hash(paths.root.to_string_lossy().as_bytes()); + format!("berd-monitor-{state_hash:016x}-{}", self.generation) + } + + fn rotate(&mut self) { + self.generation = next_pending_generation(); + self.active_len = 0; + } +} + +fn next_pending_generation() -> String { + let timestamp = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + let sequence = PENDING_GENERATION_COUNTER.fetch_add(1, Ordering::Relaxed); + format!("{}-{timestamp}-{sequence}", std::process::id()) +} + impl StatePaths { fn for_key(key: &str, session_id: &str) -> Self { let identity = format!("{session_id}\0{key}"); @@ -630,7 +669,7 @@ fn run_producer( paths, &format!("starting producer: {}", render_command(producer_command)), )?; - let mut pending = read_optional(&paths.pending)?; + let mut pending = read_pending(&paths.pending)?; let mut command = Command::new(&producer_command[0]); command @@ -757,8 +796,7 @@ fn run_producer( append_batch(paths, &mut pending, &mut batch)?; let status = producer_status.expect("producer has exited"); let summary = format!("[monitor] producer exited with status {status}\n"); - pending.extend_from_slice(summary.as_bytes()); - append_file(&paths.pending, summary.as_bytes())?; + append_pending(&paths.pending, &mut pending, summary.as_bytes())?; flush_pending( paths, &mut pending, @@ -768,7 +806,7 @@ fn run_producer( if_running, )?; - while !pending.is_empty() && !paths.stop.exists() { + while !pending.bytes.is_empty() && !paths.stop.exists() { thread::sleep(RETRY_INTERVAL); flush_pending( paths, @@ -779,9 +817,10 @@ fn run_producer( if_running, )?; } - if !pending.is_empty() && paths.stop.exists() { - pending.clear(); - atomic_write(&paths.pending, &pending)?; + if !pending.bytes.is_empty() && paths.stop.exists() { + pending.bytes.clear(); + pending.rotate(); + persist_pending(&paths.pending, &pending)?; log_line(paths, "discarded undelivered output after explicit stop")?; } log_line(paths, &format!("producer exited with status {status}"))?; @@ -815,27 +854,34 @@ fn forward_producer_output(mut reader: R, sender: mpsc::SyncSender, batch: &mut Vec) -> io::Result<()> { +fn append_batch( + paths: &StatePaths, + pending: &mut PendingState, + batch: &mut Vec, +) -> io::Result<()> { if batch.is_empty() { return Ok(()); } - pending.extend_from_slice(batch); - append_file(&paths.pending, batch)?; + append_pending(&paths.pending, pending, batch)?; batch.clear(); Ok(()) } fn flush_pending( paths: &StatePaths, - pending: &mut Vec, + pending: &mut PendingState, label: &str, instructions: &str, session_id: &str, if_running: RunningMode, ) -> io::Result<()> { - while !pending.is_empty() { - let end = pending_chunk_end(pending, MAX_DELIVERY_BYTES); - let text = String::from_utf8_lossy(&pending[..end]); + while !pending.bytes.is_empty() { + if pending.active_len == 0 { + pending.active_len = pending_chunk_end(&pending.bytes, MAX_DELIVERY_BYTES); + persist_pending(&paths.pending, pending)?; + } + let end = pending.active_len; + let text = String::from_utf8_lossy(&pending.bytes[..end]); let mut prompt = format!( "[monitor: {label} | pid {}]\n{}", std::process::id(), @@ -851,12 +897,14 @@ fn flush_pending( "monitor delivery prompt exceeded its internal size bound", )); } - if !deliver_to_session(paths, session_id, &prompt, if_running) { + let delivery_id = pending.delivery_id(paths); + if !deliver_to_session(paths, session_id, &prompt, if_running, &delivery_id) { log_line(paths, "delivery failed; buffered output will be retried")?; return Ok(()); } - pending.drain(..end); - atomic_write(&paths.pending, pending)?; + pending.bytes.drain(..end); + pending.rotate(); + persist_pending(&paths.pending, pending)?; log_line(paths, "delivered one event batch")?; } Ok(()) @@ -867,12 +915,14 @@ fn deliver_to_session( session_id: &str, prompt: &str, if_running: RunningMode, + delivery_id: &str, ) -> bool { deliver_with_candidates( paths, session_id, prompt, if_running, + delivery_id, lock_candidates(), berdctl_candidates(), DELIVERY_TIMEOUT, @@ -884,6 +934,7 @@ fn deliver_with_candidates( session_id: &str, prompt: &str, if_running: RunningMode, + delivery_id: &str, locks: Vec, binaries: Vec, timeout: Duration, @@ -906,6 +957,8 @@ fn deliver_with_candidates( .arg(prompt) .arg("--if-running") .arg(if_running.as_str()) + .arg("--delivery-id") + .arg(delivery_id) .arg("--from") .arg("berd-monitor") .arg("--json") @@ -1029,6 +1082,74 @@ fn read_optional(path: &Path) -> io::Result> { } } +fn read_pending(path: &Path) -> io::Result { + let contents = read_optional(path)?; + if contents.is_empty() { + return Ok(PendingState::empty()); + } + if !contents.starts_with(PENDING_HEADER_PREFIX.as_bytes()) { + return Ok(PendingState { + generation: next_pending_generation(), + active_len: 0, + bytes: contents, + }); + } + let header_end = contents + .iter() + .position(|byte| *byte == b'\n') + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid pending header"))?; + let header = std::str::from_utf8(&contents[PENDING_HEADER_PREFIX.len()..header_end]) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "invalid pending header"))?; + let (generation, active_len) = header + .split_once(' ') + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid pending header"))?; + let active_len = active_len + .parse::() + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "invalid pending length"))?; + let bytes = contents[(header_end + 1)..].to_vec(); + if generation.is_empty() || active_len > bytes.len() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid pending delivery state", + )); + } + Ok(PendingState { + generation: generation.to_owned(), + active_len, + bytes, + }) +} + +fn persist_pending(path: &Path, pending: &PendingState) -> io::Result<()> { + if pending.bytes.is_empty() { + return match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + }; + } + let mut contents = format!( + "{PENDING_HEADER_PREFIX}{} {}\n", + pending.generation, pending.active_len + ) + .into_bytes(); + contents.extend_from_slice(&pending.bytes); + atomic_write(path, &contents) +} + +fn append_pending(path: &Path, pending: &mut PendingState, data: &[u8]) -> io::Result<()> { + if data.is_empty() { + return Ok(()); + } + let existed = path.exists(); + pending.bytes.extend_from_slice(data); + if existed { + append_file(path, data) + } else { + persist_pending(path, pending) + } +} + fn append_file(path: &Path, data: &[u8]) -> io::Result<()> { OpenOptions::new() .create(true) @@ -1184,6 +1305,39 @@ mod tests { fs::remove_dir_all(root).unwrap(); } + #[test] + fn pending_delivery_id_survives_restart_and_rotates_after_ack() { + let key = format!( + "pending-id-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + ensure_private_directory(&paths.root).unwrap(); + let mut pending = PendingState::empty(); + append_pending(&paths.pending, &mut pending, b"same event\n").unwrap(); + pending.active_len = pending.bytes.len(); + persist_pending(&paths.pending, &pending).unwrap(); + let first_id = pending.delivery_id(&paths); + + let mut restarted = read_pending(&paths.pending).unwrap(); + assert_eq!(restarted.delivery_id(&paths), first_id); + assert_eq!(restarted.active_len, restarted.bytes.len()); + + restarted.bytes.clear(); + restarted.rotate(); + persist_pending(&paths.pending, &restarted).unwrap(); + append_pending(&paths.pending, &mut restarted, b"same event\n").unwrap(); + restarted.active_len = restarted.bytes.len(); + persist_pending(&paths.pending, &restarted).unwrap(); + + assert_ne!(restarted.delivery_id(&paths), first_id); + fs::remove_dir_all(paths.root).unwrap(); + } + #[cfg(unix)] fn test_process_exists(pid: u32) -> bool { let result = unsafe { libc::kill(pid as i32, 0) }; @@ -1290,6 +1444,7 @@ mod tests { session_id, "test", RunningMode::Steer, + "test-delivery", vec![PathBuf::from("stale-a"), PathBuf::from("stale-b")], vec![worker_binary], DELIVERY_TIMEOUT, @@ -1338,6 +1493,7 @@ mod tests { session_id, "test", RunningMode::Steer, + "test-delivery", vec![PathBuf::from("stale")], vec![fake_berdctl.into_os_string()], Duration::from_millis(500), @@ -1511,8 +1667,8 @@ mod tests { thread::sleep(Duration::from_millis(25)); } let child_pid = loop { - if let Ok(value) = fs::read_to_string(&paths.pending) { - if let Ok(pid) = value.trim().parse::() { + if let Ok(pending) = read_pending(&paths.pending) { + if let Ok(pid) = String::from_utf8_lossy(&pending.bytes).trim().parse::() { break pid; } } @@ -1524,7 +1680,7 @@ mod tests { }; request_stop(&key, session_id).unwrap(); worker.join().unwrap().unwrap(); - assert!(fs::read(&paths.pending).unwrap().is_empty()); + assert!(read_optional(&paths.pending).unwrap().is_empty()); let child_gone_deadline = Instant::now() + Duration::from_secs(2); while test_process_exists(child_pid as u32) && Instant::now() < child_gone_deadline { thread::sleep(Duration::from_millis(25)); diff --git a/src-tauri/crates/berdctl/api-surface-feedback.json b/src-tauri/crates/berdctl/api-surface-feedback.json index 19f86dd5b..e5807acb7 100644 --- a/src-tauri/crates/berdctl/api-surface-feedback.json +++ b/src-tauri/crates/berdctl/api-surface-feedback.json @@ -102,7 +102,7 @@ } }, "send": { - "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript.", + "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript. Use --delivery-id when retries must create at most one user turn.", "fields": [ { "name": "session_id", @@ -140,6 +140,14 @@ "description": "Optional visible sender label for this message (1-120 chars).", "min": 1, "max": 120 + }, + { + "name": "delivery_id", + "required": false, + "kind": "string", + "description": "Optional idempotency id; a repeated id for this session is accepted without creating another user turn (1-200 chars).", + "min": 1, + "max": 200 } ], "schema": { @@ -174,6 +182,13 @@ "minLength": 1, "maxLength": 120, "pattern": "^[^\\r\\n]*$" + }, + "delivery_id": { + "description": "Optional idempotency id; a repeated id for this session is accepted without creating another user turn (1-200 chars).", + "type": "string", + "minLength": 1, + "maxLength": 200, + "pattern": "^[^\\r\\n]*$" } }, "required": ["session_id", "prompt"], diff --git a/src-tauri/crates/berdctl/api-surface.json b/src-tauri/crates/berdctl/api-surface.json index 1bef48807..9aaf40f05 100644 --- a/src-tauri/crates/berdctl/api-surface.json +++ b/src-tauri/crates/berdctl/api-surface.json @@ -102,7 +102,7 @@ } }, "send": { - "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript.", + "description": "Send a prompt into an existing chat session without opening or focusing it. Idle sends are fire-and-forget and visibly add a user message marked as sent by Berd from another session. Running sessions are refused by default; use --if-running steer to add context to the active run, or --if-running queue to send one follow-up after the current run finishes. Use --from to give the sending session or tool a concise visible label in the transcript. Use --delivery-id when retries must create at most one user turn.", "fields": [ { "name": "session_id", @@ -140,6 +140,14 @@ "description": "Optional visible sender label for this message (1-120 chars).", "min": 1, "max": 120 + }, + { + "name": "delivery_id", + "required": false, + "kind": "string", + "description": "Optional idempotency id; a repeated id for this session is accepted without creating another user turn (1-200 chars).", + "min": 1, + "max": 200 } ], "schema": { @@ -174,6 +182,13 @@ "minLength": 1, "maxLength": 120, "pattern": "^[^\\r\\n]*$" + }, + "delivery_id": { + "description": "Optional idempotency id; a repeated id for this session is accepted without creating another user turn (1-200 chars).", + "type": "string", + "minLength": 1, + "maxLength": 200, + "pattern": "^[^\\r\\n]*$" } }, "required": ["session_id", "prompt"], diff --git a/src-tauri/crates/berdctl/cli-surface-feedback.json b/src-tauri/crates/berdctl/cli-surface-feedback.json index 833ae19f4..1f199f737 100644 --- a/src-tauri/crates/berdctl/cli-surface-feedback.json +++ b/src-tauri/crates/berdctl/cli-surface-feedback.json @@ -13,7 +13,7 @@ "send": { "action": "send", "about": "Send a prompt into an existing chat session", - "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." + "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --delivery-id --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"|\"deduplicated\"}\n The user's current view does not change." }, "open": { "action": "open", diff --git a/src-tauri/crates/berdctl/cli-surface.json b/src-tauri/crates/berdctl/cli-surface.json index 9f2acbeb9..a096970e9 100644 --- a/src-tauri/crates/berdctl/cli-surface.json +++ b/src-tauri/crates/berdctl/cli-surface.json @@ -13,7 +13,7 @@ "send": { "action": "send", "about": "Send a prompt into an existing chat session", - "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"}\n The user's current view does not change." + "afterHelp": "Example:\n berdctl session send --session-id \\\n --prompt \"Check the latest CI failure\" --if-running queue \\\n --from \"the Berd session handling CI\" --delivery-id --json\n\nResult:\n {\"session_id\": \"...\", \"send_status\": \"dispatched\"|\"steered\"|\"queued\"|\"deduplicated\"}\n The user's current view does not change." }, "open": { "action": "open", diff --git a/src/features/berdctl/__tests__/commands/commands.test.ts b/src/features/berdctl/__tests__/commands/commands.test.ts index 7b9b91e3e..114730fbc 100644 --- a/src/features/berdctl/__tests__/commands/commands.test.ts +++ b/src/features/berdctl/__tests__/commands/commands.test.ts @@ -1709,6 +1709,7 @@ describe("sessions.send", () => { prompt: "[monitor: checks] complete", if_running: "queue", from: "the Berd session handling berd-monitor implementation", + delivery_id: "monitor-event-1", }, ctx, ); @@ -1722,15 +1723,86 @@ describe("sessions.send", () => { origin: "berdctl_cross_session", berdSenderLabel: "the Berd session handling berd-monitor implementation", + berdDeliveryId: "monitor-event-1", }, acpGooseMetadata: { origin: "berdctl_cross_session", berdSenderLabel: "the Berd session handling berd-monitor implementation", + berdDeliveryId: "monitor-event-1", }, }); }); + it("accepts a repeated delivery id without queueing another user turn", async () => { + mockSessionFound(); + useChatStore.getState().setChatState("session-1", "streaming"); + + const first = await dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "monitor event", + if_running: "queue", + delivery_id: "monitor-event-1", + }, + ctx, + ); + const duplicate = await dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "monitor event retried", + if_running: "queue", + delivery_id: "monitor-event-1", + }, + ctx, + ); + + expect(first).toEqual({ session_id: "session-1", send_status: "queued" }); + expect(duplicate).toEqual({ + session_id: "session-1", + send_status: "deduplicated", + }); + expect( + useChatStore.getState().queuedMessageBySession["session-1"], + ).toHaveLength(1); + }); + + it("deduplicates a delivery id restored in the transcript before target guards", async () => { + mockSessionFound(); + const accepted = createUserMessage("monitor event"); + accepted.metadata = { + origin: "berdctl_cross_session", + berdDeliveryId: "monitor-event-1", + }; + useChatStore.getState().addMessage("session-1", accepted); + useChatStore.getState().setChatState("session-1", "streaming"); + useSessionWindowStore + .getState() + .setSnapshot([{ sessionId: "session-1", windowLabel: "session" }]); + + const duplicate = await dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "monitor event retried after restart", + delivery_id: "monitor-event-1", + }, + ctx, + ); + + expect(duplicate).toEqual({ + session_id: "session-1", + send_status: "deduplicated", + }); + expect(mocks.acpSendMessage).not.toHaveBeenCalled(); + expect(mocks.acpSteerMessage).not.toHaveBeenCalled(); + }); + it("rejects multiline sender labels before dispatch", async () => { const error = await expectCommandError( dispatchCommand( diff --git a/src/features/berdctl/commands/impl/sendSession.ts b/src/features/berdctl/commands/impl/sendSession.ts index 55d9d1526..902f89451 100644 --- a/src/features/berdctl/commands/impl/sendSession.ts +++ b/src/features/berdctl/commands/impl/sendSession.ts @@ -49,12 +49,22 @@ const sendSessionSchema = z .describe( "Optional visible sender label for this message (1-120 chars).", ), + delivery_id: z + .string() + .trim() + .min(1) + .max(200) + .regex(/^[^\r\n]*$/, "Delivery id must be a single line.") + .optional() + .describe( + "Optional idempotency id; a repeated id for this session is accepted without creating another user turn (1-200 chars).", + ), }) .strict(); interface SendSessionResult { session_id: string; - send_status: "dispatched" | "steered" | "queued"; + send_status: "dispatched" | "steered" | "queued" | "deduplicated"; } function runningTargetMessage(sessionId: string): string { @@ -72,14 +82,15 @@ export const sendSessionCommand = defineCommand({ "by Berd from another session. Running sessions are refused by default; use " + "--if-running steer to add context to the active run, or --if-running queue " + "to send one follow-up after the current run finishes. Use --from to give " + - "the sending session or tool a concise visible label in the transcript.", + "the sending session or tool a concise visible label in the transcript. " + + "Use --delivery-id when retries must create at most one user turn.", helpFooter: `Example: berdctl session send --session-id \\ --prompt "Check the latest CI failure" --if-running queue \\ - --from "the Berd session handling CI" --json + --from "the Berd session handling CI" --delivery-id --json Result: - {"session_id": "...", "send_status": "dispatched"|"steered"|"queued"} + {"session_id": "...", "send_status": "dispatched"|"steered"|"queued"|"deduplicated"} The user's current view does not change.`, schema: sendSessionSchema, bridgeTimeoutMs: 60_000, @@ -90,6 +101,7 @@ Result: { findProjectOrThrow }, { berdctlCrossSessionSendOptions, + hasAcceptedBerdctlDelivery, sendPromptToExistingSessionInBackground, SessionDispatchContentionError, SessionDispatchUnresolvedError, @@ -102,11 +114,19 @@ Result: ]); const sendOptions = berdctlCrossSessionSendOptions({ senderLabel: args.from, + deliveryId: args.delivery_id, }); await loadSessionForBerdctl(args.session_id); const session = requireSession(args.session_id); + if ( + args.delivery_id && + hasAcceptedBerdctlDelivery(args.session_id, args.delivery_id) + ) { + return { session_id: session.id, send_status: "deduplicated" }; + } + if (useSessionWindowStore.getState().isOpenInWindow(args.session_id)) { throw new CommandError( "target_session_running", diff --git a/src/features/berdctl/commands/runtime/sessionSend.ts b/src/features/berdctl/commands/runtime/sessionSend.ts index 1524b3688..7a37b533c 100644 --- a/src/features/berdctl/commands/runtime/sessionSend.ts +++ b/src/features/berdctl/commands/runtime/sessionSend.ts @@ -8,6 +8,7 @@ import { SessionDispatchUnresolvedError, } from "@/features/chat/lib/queuedSessionSend"; import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; +import { useChatStore } from "@/features/chat/stores/chatStore"; export { sendQueuedPromptToExistingSessionInBackground, SessionDispatchContentionError, @@ -24,23 +25,45 @@ export const BERDCTL_CROSS_SESSION_ORIGIN = "berdctl_cross_session" satisfies NonNullable; export function berdctlCrossSessionSendOptions( - options: { senderLabel?: string } = {}, + options: { senderLabel?: string; deliveryId?: string } = {}, ): ChatSendOptions { const senderMetadata = options.senderLabel ? { berdSenderLabel: options.senderLabel } : {}; + const deliveryMetadata = options.deliveryId + ? { berdDeliveryId: options.deliveryId } + : {}; return { userMessageMetadata: { origin: BERDCTL_CROSS_SESSION_ORIGIN, ...senderMetadata, + ...deliveryMetadata, }, acpGooseMetadata: { origin: BERDCTL_CROSS_SESSION_ORIGIN, ...senderMetadata, + ...deliveryMetadata, }, }; } +export function hasAcceptedBerdctlDelivery( + sessionId: string, + deliveryId: string, +): boolean { + const chatStore = useChatStore.getState(); + return ( + (chatStore.messagesBySession[sessionId] ?? []).some( + (message) => message.metadata?.berdDeliveryId === deliveryId, + ) || + (chatStore.queuedMessageBySession[sessionId] ?? []).some( + (record) => + record.payload.sendOptions?.userMessageMetadata?.berdDeliveryId === + deliveryId, + ) + ); +} + export async function sendPromptToExistingSessionInBackground( sessionId: string, prompt: string, diff --git a/src/shared/api/__tests__/acpReplayMetadata.test.ts b/src/shared/api/__tests__/acpReplayMetadata.test.ts index 543d6b539..d4e576468 100644 --- a/src/shared/api/__tests__/acpReplayMetadata.test.ts +++ b/src/shared/api/__tests__/acpReplayMetadata.test.ts @@ -144,12 +144,14 @@ describe("getReplayUserMetadata", () => { goose: { origin: "berdctl_cross_session", berdSenderLabel: "berd-monitor", + berdDeliveryId: "monitor-event-1", }, }, }), ).toEqual({ origin: "berdctl_cross_session", berdSenderLabel: "berd-monitor", + berdDeliveryId: "monitor-event-1", }); }); diff --git a/src/shared/api/acpReplayMetadata.ts b/src/shared/api/acpReplayMetadata.ts index 17be06f60..09eeb1e4d 100644 --- a/src/shared/api/acpReplayMetadata.ts +++ b/src/shared/api/acpReplayMetadata.ts @@ -14,6 +14,7 @@ export type ReplayUserMetadata = Pick< | "delivery" | "origin" | "berdSenderLabel" + | "berdDeliveryId" | "voiceUtteranceId" | "voiceConversationLifecycleId" | "voiceConversationRevision" @@ -80,6 +81,10 @@ export function getReplayUserMetadata( origin === "berdctl_cross_session" ? boundedSingleLineString(goose.berdSenderLabel, 120) : undefined; + const berdDeliveryId = + origin === "berdctl_cross_session" + ? boundedSingleLineString(goose.berdDeliveryId, 200) + : undefined; const voiceUtteranceId = origin === "voice_conversation" ? nonEmptyString(goose.voiceUtteranceId) @@ -103,6 +108,7 @@ export function getReplayUserMetadata( ...(delivery ? { delivery } : {}), ...(origin ? { origin } : {}), ...(berdSenderLabel ? { berdSenderLabel } : {}), + ...(berdDeliveryId ? { berdDeliveryId } : {}), ...(voiceUtteranceId ? { voiceUtteranceId } : {}), ...(voiceConversationLifecycleId ? { voiceConversationLifecycleId } : {}), ...(voiceConversationRevision !== undefined diff --git a/src/shared/types/messages.ts b/src/shared/types/messages.ts index b7dcb2924..a8539defc 100644 --- a/src/shared/types/messages.ts +++ b/src/shared/types/messages.ts @@ -256,6 +256,7 @@ export interface MessageMetadata { steeringRequestId?: string; origin?: "berdctl_cross_session" | "voice_conversation"; berdSenderLabel?: string; + berdDeliveryId?: string; voiceUtteranceId?: string; voiceConversationLifecycleId?: string; voiceConversationRevision?: number; From edda53e2e6db3728ac6b9c14863f65ccce6e822f Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:51:00 -0400 Subject: [PATCH 13/22] fix(monitor): stabilize CI test formatting Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 2c1823c07..719827126 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -1267,13 +1267,9 @@ mod tests { .unwrap(); let pid = child.id(); - let error = wait_for_launch( - &mut child, - &paths, - &status_path, - Duration::from_millis(100), - ) - .unwrap_err(); + let launch_timeout = Duration::from_millis(100); + let launch_result = wait_for_launch(&mut child, &paths, &status_path, launch_timeout); + let error = launch_result.unwrap_err(); assert!(error.contains("did not become ready")); assert!(!test_process_exists(pid)); @@ -1362,9 +1358,11 @@ mod tests { resume_producer(&producer.child)?; let direct_pid = producer.child.id(); let mut descendant = String::new(); - BufReader::new(producer.child.stdout.take().unwrap()) - .read_line(&mut descendant)?; - observed_pids = Some((direct_pid, descendant.trim().parse::().unwrap())); + let stdout = producer.child.stdout.take().unwrap(); + let mut reader = BufReader::new(stdout); + reader.read_line(&mut descendant)?; + let descendant_pid = descendant.trim().parse::().unwrap(); + observed_pids = Some((direct_pid, descendant_pid)); Err(io::Error::other("injected post-spawn failure")) })(); @@ -1668,7 +1666,9 @@ mod tests { } let child_pid = loop { if let Ok(pending) = read_pending(&paths.pending) { - if let Ok(pid) = String::from_utf8_lossy(&pending.bytes).trim().parse::() { + let pending_text = String::from_utf8_lossy(&pending.bytes); + let parsed_pid = pending_text.trim().parse::(); + if let Ok(pid) = parsed_pid { break pid; } } From 748a6b019a6503cfcbac31c1f2eff5d12be1cf7e Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:53:02 -0400 Subject: [PATCH 14/22] fix(berdctl): preserve queued delivery ids Signed-off-by: John Tennant --- .../useBerdctlQueuedMessageDrain.test.tsx | 36 +++++++++++++++++++ .../bridge/useBerdctlQueuedMessageDrain.ts | 3 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx b/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx index 96ae51ada..9142ba047 100644 --- a/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx +++ b/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx @@ -558,6 +558,42 @@ describe("useBerdctlQueuedMessageDrain", () => { ).toBeUndefined(); }); + it("preserves a queued delivery id without a sender label", async () => { + const sendOptions = { + userMessageMetadata: { + origin: "berdctl_cross_session" as const, + berdDeliveryId: "monitor-event-1", + }, + acpGooseMetadata: { + origin: "berdctl_cross_session" as const, + berdDeliveryId: "monitor-event-1", + }, + }; + const chatStore = useChatStore.getState(); + chatStore.setChatState("session-1", "streaming"); + chatStore.enqueueTransportReadyMessage("session-1", { + persona: { kind: "inherit" }, + text: "queued delivery", + sendOptions, + }); + render(); + + act(() => { + useChatStore.getState().setChatState("session-1", "idle"); + }); + + await waitFor(() => { + expect( + mocks.sendPromptToExistingSessionInBackground, + ).toHaveBeenCalledWith( + "session-1", + "queued delivery", + expect.any(Function), + { returnOnDispatch: true, sendOptions }, + ); + }); + }); + it("drains consecutive berdctl records in FIFO order while idle", async () => { const chatStore = useChatStore.getState(); chatStore.enqueueTransportReadyMessage("session-1", { diff --git a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts index 50d087dc2..576f1c5cf 100644 --- a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts +++ b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts @@ -113,7 +113,8 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { { returnOnDispatch: true, ...(queuedMessage.payload.sendOptions?.userMessageMetadata - ?.berdSenderLabel + ?.berdSenderLabel || + queuedMessage.payload.sendOptions?.userMessageMetadata?.berdDeliveryId ? { sendOptions: queuedMessage.payload.sendOptions } : {}), }, From c9295c894902ab5e8586ff338f1c5047cf2795b8 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:55:37 -0400 Subject: [PATCH 15/22] fix(berdctl): serialize delivery admission Signed-off-by: John Tennant --- .../__tests__/commands/commands.test.ts | 44 +++ .../berdctl/commands/impl/sendSession.ts | 274 +++++++++--------- .../berdctl/commands/runtime/sessionSend.ts | 22 ++ 3 files changed, 206 insertions(+), 134 deletions(-) diff --git a/src/features/berdctl/__tests__/commands/commands.test.ts b/src/features/berdctl/__tests__/commands/commands.test.ts index 114730fbc..fce1b9a71 100644 --- a/src/features/berdctl/__tests__/commands/commands.test.ts +++ b/src/features/berdctl/__tests__/commands/commands.test.ts @@ -1771,6 +1771,50 @@ describe("sessions.send", () => { ).toHaveLength(1); }); + it("serializes concurrent admission of the same delivery id", async () => { + const session = makeAcpSession({ + sessionId: "session-1", + providerId: "codex-acp", + }); + let releaseLoads: (() => void) | undefined; + const loadsReleased = new Promise((resolve) => { + releaseLoads = resolve; + }); + let loadCount = 0; + mocks.acpGetSessionInfo.mockImplementation(async () => { + loadCount += 1; + await loadsReleased; + return session; + }); + + const send = (prompt: string) => + dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt, + if_running: "queue", + delivery_id: "monitor-event-1", + }, + ctx, + ); + const first = send("monitor event"); + const retry = send("monitor event retried concurrently"); + await vi.waitFor(() => expect(loadCount).toBe(2)); + releaseLoads?.(); + + await expect(Promise.all([first, retry])).resolves.toEqual([ + { session_id: "session-1", send_status: "dispatched" }, + { session_id: "session-1", send_status: "deduplicated" }, + ]); + expect( + (useChatStore.getState().messagesBySession["session-1"] ?? []).length + + (useChatStore.getState().queuedMessageBySession["session-1"]?.length ?? + 0), + ).toBe(1); + }); + it("deduplicates a delivery id restored in the transcript before target guards", async () => { mockSessionFound(); const accepted = createUserMessage("monitor event"); diff --git a/src/features/berdctl/commands/impl/sendSession.ts b/src/features/berdctl/commands/impl/sendSession.ts index 902f89451..a45b4608b 100644 --- a/src/features/berdctl/commands/impl/sendSession.ts +++ b/src/features/berdctl/commands/impl/sendSession.ts @@ -101,7 +101,7 @@ Result: { findProjectOrThrow }, { berdctlCrossSessionSendOptions, - hasAcceptedBerdctlDelivery, + reserveBerdctlDelivery, sendPromptToExistingSessionInBackground, SessionDispatchContentionError, SessionDispatchUnresolvedError, @@ -120,163 +120,169 @@ Result: await loadSessionForBerdctl(args.session_id); const session = requireSession(args.session_id); - if ( - args.delivery_id && - hasAcceptedBerdctlDelivery(args.session_id, args.delivery_id) - ) { + const releaseDeliveryReservation = args.delivery_id + ? reserveBerdctlDelivery(args.session_id, args.delivery_id) + : undefined; + if (args.delivery_id && !releaseDeliveryReservation) { return { session_id: session.id, send_status: "deduplicated" }; } - if (useSessionWindowStore.getState().isOpenInWindow(args.session_id)) { - throw new CommandError( - "target_session_running", - `Refusing to send to session "${args.session_id}" while it is open in a separate window; close that window first or ask the user.`, - ); - } - - const chatStore = useChatStore.getState(); - const runtime = chatStore.getSessionRuntime(args.session_id); - if ( - args.startup_name && - ((chatStore.queuedMessageBySession[args.session_id]?.length ?? 0) > 0 || - session.messageCount > 0) - ) { - throw new CommandError( - "invalid_args", - "startup_name is only valid for the first send when workspace setup is available.", - ); - } - if (!isQueuedSessionReady(runtime)) { - switch (args.if_running) { - case "refuse": - throw new CommandError( - "target_session_running", - runningTargetMessage(args.session_id), - ); + try { + if (useSessionWindowStore.getState().isOpenInWindow(args.session_id)) { + throw new CommandError( + "target_session_running", + `Refusing to send to session "${args.session_id}" while it is open in a separate window; close that window first or ask the user.`, + ); + } - case "steer": - if (runtime.isRunCancellationPending) { + const chatStore = useChatStore.getState(); + const runtime = chatStore.getSessionRuntime(args.session_id); + if ( + args.startup_name && + ((chatStore.queuedMessageBySession[args.session_id]?.length ?? 0) > 0 || + session.messageCount > 0) + ) { + throw new CommandError( + "invalid_args", + "startup_name is only valid for the first send when workspace setup is available.", + ); + } + if (!isQueuedSessionReady(runtime)) { + switch (args.if_running) { + case "refuse": throw new CommandError( "target_session_running", - `Refusing to steer session "${args.session_id}" while cancellation is pending; use --if-running queue or wait for cancellation to finish.`, + runningTargetMessage(args.session_id), ); - } - await steerPromptInSession( - args.session_id, - args.prompt, - undefined, - sendOptions, - { throwOnError: true }, - ); - return { session_id: session.id, send_status: "steered" }; - case "queue": - chatStore.enqueueTransportReadyMessage( - args.session_id, - admitSystemInheritedQueuedMessage({ - text: args.prompt, + case "steer": + if (runtime.isRunCancellationPending) { + throw new CommandError( + "target_session_running", + `Refusing to steer session "${args.session_id}" while cancellation is pending; use --if-running queue or wait for cancellation to finish.`, + ); + } + await steerPromptInSession( + args.session_id, + args.prompt, + undefined, sendOptions, - }), - ); - return { session_id: session.id, send_status: "queued" }; + { throwOnError: true }, + ); + return { session_id: session.id, send_status: "steered" }; - default: - throw new Error( - `Unhandled if_running mode: ${String(args.if_running satisfies never)}`, - ); + case "queue": + chatStore.enqueueTransportReadyMessage( + args.session_id, + admitSystemInheritedQueuedMessage({ + text: args.prompt, + sendOptions, + }), + ); + return { session_id: session.id, send_status: "queued" }; + + default: + throw new Error( + `Unhandled if_running mode: ${String(args.if_running satisfies never)}`, + ); + } } - } - const project = session.projectId - ? await findProjectOrThrow(session.projectId) - : null; - const firstSend = acceptFirstSend( - args.session_id, - createDeferredQueuedMessagePayload({ - text: args.prompt, - persona: { kind: "inherit" }, - sendOptions, - }), - { startupName: args.startup_name, project }, - ); - if (firstSend.needsName) { - throw new CommandError( - "workspace_name_required", - "This session needs a workspace startup name before its first send.", - ); - } - if (firstSend.accepted) { - return { session_id: session.id, send_status: "queued" }; - } - if ((chatStore.queuedMessageBySession[args.session_id]?.length ?? 0) > 0) { - chatStore.enqueueTransportReadyMessage( + const project = session.projectId + ? await findProjectOrThrow(session.projectId) + : null; + const firstSend = acceptFirstSend( args.session_id, - admitSystemInheritedQueuedMessage({ + createDeferredQueuedMessagePayload({ text: args.prompt, + persona: { kind: "inherit" }, sendOptions, }), + { startupName: args.startup_name, project }, ); - return { session_id: session.id, send_status: "queued" }; - } - - try { - await sendPromptToExistingSessionInBackground( - args.session_id, - args.prompt, - () => { - const liveChatStore = useChatStore.getState(); - assertQueuedSessionReady( - liveChatStore.getSessionRuntime(args.session_id), - (liveChatStore.queuedMessageBySession[args.session_id]?.length ?? - 0) === 0, - ); - }, - { - returnOnDispatch: true, - sendOptions, - }, - ); - } catch (error) { - if (error instanceof SessionDispatchContentionError) { - if (args.if_running === "queue") { - useChatStore.getState().enqueueTransportReadyMessage( - args.session_id, - admitSystemInheritedQueuedMessage({ - text: args.prompt, - sendOptions, - }), - ); - return { session_id: session.id, send_status: "queued" }; - } + if (firstSend.needsName) { throw new CommandError( - "target_session_running", - runningTargetMessage(args.session_id), + "workspace_name_required", + "This session needs a workspace startup name before its first send.", ); } - if (error instanceof SessionDispatchUnresolvedError) { - throw new CommandError("invalid_args", error.message); + if (firstSend.accepted) { + return { session_id: session.id, send_status: "queued" }; } - if (error instanceof PreCommitSendRejectedError) { - if (args.if_running === "queue") { - useChatStore.getState().enqueueTransportReadyMessage( - args.session_id, - admitSystemInheritedQueuedMessage({ - text: args.prompt, - sendOptions, - }), - ); - return { session_id: session.id, send_status: "queued" }; - } - throw new CommandError( - "target_session_running", - runningTargetMessage(args.session_id), + if ( + (chatStore.queuedMessageBySession[args.session_id]?.length ?? 0) > 0 + ) { + chatStore.enqueueTransportReadyMessage( + args.session_id, + admitSystemInheritedQueuedMessage({ + text: args.prompt, + sendOptions, + }), ); + return { session_id: session.id, send_status: "queued" }; } - if (error instanceof CommandError) { - throw error; + + try { + await sendPromptToExistingSessionInBackground( + args.session_id, + args.prompt, + () => { + const liveChatStore = useChatStore.getState(); + assertQueuedSessionReady( + liveChatStore.getSessionRuntime(args.session_id), + (liveChatStore.queuedMessageBySession[args.session_id]?.length ?? + 0) === 0, + ); + }, + { + returnOnDispatch: true, + sendOptions, + }, + ); + } catch (error) { + if (error instanceof SessionDispatchContentionError) { + if (args.if_running === "queue") { + useChatStore.getState().enqueueTransportReadyMessage( + args.session_id, + admitSystemInheritedQueuedMessage({ + text: args.prompt, + sendOptions, + }), + ); + return { session_id: session.id, send_status: "queued" }; + } + throw new CommandError( + "target_session_running", + runningTargetMessage(args.session_id), + ); + } + if (error instanceof SessionDispatchUnresolvedError) { + throw new CommandError("invalid_args", error.message); + } + if (error instanceof PreCommitSendRejectedError) { + if (args.if_running === "queue") { + useChatStore.getState().enqueueTransportReadyMessage( + args.session_id, + admitSystemInheritedQueuedMessage({ + text: args.prompt, + sendOptions, + }), + ); + return { session_id: session.id, send_status: "queued" }; + } + throw new CommandError( + "target_session_running", + runningTargetMessage(args.session_id), + ); + } + if (error instanceof CommandError) { + throw error; + } + throw new Error(formatAcpErrorMessage(error)); } - throw new Error(formatAcpErrorMessage(error)); + return { session_id: session.id, send_status: "dispatched" }; + } finally { + releaseDeliveryReservation?.(); } - return { session_id: session.id, send_status: "dispatched" }; }, }); diff --git a/src/features/berdctl/commands/runtime/sessionSend.ts b/src/features/berdctl/commands/runtime/sessionSend.ts index 7a37b533c..6bc7e508e 100644 --- a/src/features/berdctl/commands/runtime/sessionSend.ts +++ b/src/features/berdctl/commands/runtime/sessionSend.ts @@ -24,6 +24,8 @@ export { isBerdctlCrossSessionQueuedMessage } from "@/features/chat/lib/queuedMe export const BERDCTL_CROSS_SESSION_ORIGIN = "berdctl_cross_session" satisfies NonNullable; +const reservedDeliveryIds = new Set(); + export function berdctlCrossSessionSendOptions( options: { senderLabel?: string; deliveryId?: string } = {}, ): ChatSendOptions { @@ -64,6 +66,26 @@ export function hasAcceptedBerdctlDelivery( ); } +export function reserveBerdctlDelivery( + sessionId: string, + deliveryId: string, +): (() => void) | null { + const key = JSON.stringify([sessionId, deliveryId]); + if ( + reservedDeliveryIds.has(key) || + hasAcceptedBerdctlDelivery(sessionId, deliveryId) + ) { + return null; + } + reservedDeliveryIds.add(key); + let released = false; + return () => { + if (released) return; + released = true; + reservedDeliveryIds.delete(key); + }; +} + export async function sendPromptToExistingSessionInBackground( sessionId: string, prompt: string, From 571ae7da539c7d70bdd0ea52ca0b3e703ced4526 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 14:59:17 -0400 Subject: [PATCH 16/22] fix(monitor): sanitize nul output bytes Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 104 ++++++++++++++++++---- 1 file changed, 88 insertions(+), 16 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 719827126..2d42d9841 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -881,22 +881,7 @@ fn flush_pending( persist_pending(&paths.pending, pending)?; } let end = pending.active_len; - let text = String::from_utf8_lossy(&pending.bytes[..end]); - let mut prompt = format!( - "[monitor: {label} | pid {}]\n{}", - std::process::id(), - text.trim_end() - ); - if !instructions.is_empty() { - prompt.push_str("\n\n"); - prompt.push_str(instructions); - } - if prompt.encode_utf16().count() > MAX_PROMPT_CODE_UNITS { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "monitor delivery prompt exceeded its internal size bound", - )); - } + let prompt = build_delivery_prompt(label, &pending.bytes[..end], instructions)?; let delivery_id = pending.delivery_id(paths); if !deliver_to_session(paths, session_id, &prompt, if_running, &delivery_id) { log_line(paths, "delivery failed; buffered output will be retried")?; @@ -910,6 +895,26 @@ fn flush_pending( Ok(()) } +fn build_delivery_prompt(label: &str, bytes: &[u8], instructions: &str) -> io::Result { + let text = String::from_utf8_lossy(bytes).replace('\0', "\u{fffd}"); + let mut prompt = format!( + "[monitor: {label} | pid {}]\n{}", + std::process::id(), + text.trim_end() + ); + if !instructions.is_empty() { + prompt.push_str("\n\n"); + prompt.push_str(instructions); + } + if prompt.encode_utf16().count() > MAX_PROMPT_CODE_UNITS { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "monitor delivery prompt exceeded its internal size bound", + )); + } + Ok(prompt) +} + fn deliver_to_session( paths: &StatePaths, session_id: &str, @@ -1411,6 +1416,73 @@ mod tests { assert_eq!(output, input); } + #[test] + fn delivery_prompt_replaces_embedded_nul_bytes_without_growing() { + let prompt = build_delivery_prompt("test", b"before\0after\n", "continue").unwrap(); + + assert!(!prompt.contains('\0')); + assert!(prompt.contains("before\u{fffd}after")); + assert!(prompt.contains("continue")); + } + + #[cfg(unix)] + #[test] + fn nul_output_is_delivered_once_and_does_not_block_later_output() { + use std::os::unix::fs::PermissionsExt; + + let key = format!( + "delivery-nul-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let session_id = "test-session"; + let paths = StatePaths::for_key(&key, session_id); + ensure_private_directory(&paths.root).unwrap(); + let capture = paths.root.join("delivered.txt"); + let fake_berdctl = paths.root.join("fake-berdctl"); + fs::write( + &fake_berdctl, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" >> '{}'\n", + capture.display() + ), + ) + .unwrap(); + fs::set_permissions(&fake_berdctl, fs::Permissions::from_mode(0o700)).unwrap(); + let binary = fake_berdctl.into_os_string(); + + let first = build_delivery_prompt("test", b"before\0after\n", "").unwrap(); + assert!(deliver_with_candidates( + &paths, + session_id, + &first, + RunningMode::Steer, + "nul-delivery", + vec![PathBuf::from("stale")], + vec![binary.clone()], + Duration::from_secs(2), + )); + let later = build_delivery_prompt("test", b"later output\n", "").unwrap(); + assert!(deliver_with_candidates( + &paths, + session_id, + &later, + RunningMode::Steer, + "later-delivery", + vec![PathBuf::from("stale")], + vec![binary], + Duration::from_secs(2), + )); + + let delivered = fs::read_to_string(capture).unwrap(); + assert_eq!(delivered.matches("before\u{fffd}after").count(), 1); + assert!(delivered.contains("later output")); + fs::remove_dir_all(paths.root).unwrap(); + } + #[cfg(unix)] #[test] fn stop_interrupts_a_nonresponsive_delivery_probe() { From 8271666a6b1b9d6189d95b666ab550e7aec45a80 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 15:19:58 -0400 Subject: [PATCH 17/22] fix(monitor): preserve fresh stop requests Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 166 ++++++++++++++++++---- 1 file changed, 138 insertions(+), 28 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 2d42d9841..03c357e09 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -92,7 +92,6 @@ struct StatePaths { log: PathBuf, pending: PathBuf, owner: PathBuf, - stop: PathBuf, } struct PendingState { @@ -150,7 +149,6 @@ impl StatePaths { log: root.join("watcher.log"), pending: root.join("pending.txt"), owner: root.join("owner.pid"), - stop: root.join("stop-requested"), root, } } @@ -158,6 +156,13 @@ impl StatePaths { fn launch_status(&self, token: &str) -> PathBuf { self.root.join(format!("launch-{token}.status")) } + + fn stop_for(&self, owner_token: &str) -> PathBuf { + self.root.join(format!( + "stop-{:016x}.requested", + stable_hash(owner_token.as_bytes()) + )) + } } fn state_base_dir() -> PathBuf { @@ -300,13 +305,21 @@ fn spawn_detached(state_key: &str, session_id: &str) -> Result<(), String> { let mut process = child .spawn() .map_err(|error| format!("start detached monitor: {error}"))?; - wait_for_launch(&mut process, &paths, &status_path, LAUNCH_TIMEOUT) + let stop_path = paths.stop_for(&token); + wait_for_launch( + &mut process, + &paths, + &status_path, + &stop_path, + LAUNCH_TIMEOUT, + ) } fn wait_for_launch( process: &mut std::process::Child, paths: &StatePaths, status_path: &Path, + stop_path: &Path, timeout: Duration, ) -> Result<(), String> { let deadline = Instant::now() + timeout; @@ -323,7 +336,7 @@ fn wait_for_launch( .to_owned()); } if Instant::now() >= deadline { - terminate_timed_out_launch(process, paths, status_path); + terminate_timed_out_launch(process, status_path, stop_path); return Err(format!( "monitor did not become ready within {} seconds; inspect {}", timeout.as_secs_f64(), @@ -340,10 +353,10 @@ fn wait_for_launch( fn terminate_timed_out_launch( process: &mut std::process::Child, - paths: &StatePaths, status_path: &Path, + stop_path: &Path, ) { - let _ = fs::write(&paths.stop, b"stop\n"); + let _ = fs::write(stop_path, b"stop\n"); let deadline = Instant::now() + LAUNCH_TERMINATION_GRACE; while Instant::now() < deadline { match process.try_wait() { @@ -357,7 +370,7 @@ fn terminate_timed_out_launch( let _ = process.wait(); } let _ = fs::remove_file(status_path); - let _ = fs::remove_file(&paths.stop); + let _ = fs::remove_file(stop_path); } #[cfg(unix)] @@ -555,7 +568,7 @@ fn ensure_private_directory(path: &Path) -> io::Result<()> { Ok(()) } -fn claim_owner(paths: &StatePaths) -> io::Result { +fn claim_owner(paths: &StatePaths, owner_token: &str) -> io::Result { let mut owner = OpenOptions::new() .create(true) .truncate(false) @@ -569,11 +582,37 @@ fn claim_owner(paths: &StatePaths) -> io::Result { ) })?; owner.set_len(0)?; - writeln!(owner, "{}", std::process::id())?; + writeln!(owner, "{owner_token}")?; owner.flush()?; Ok(owner) } +fn owner_token(paths: &StatePaths) -> io::Result { + let token = fs::read_to_string(&paths.owner)?.trim().to_owned(); + if token.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "monitor owner token is empty", + )); + } + Ok(token) +} + +fn stop_requested(paths: &StatePaths) -> bool { + owner_token(paths) + .map(|token| paths.stop_for(&token).exists()) + .unwrap_or(false) +} + +fn remove_active_stop(paths: &StatePaths) -> io::Result<()> { + let stop_path = paths.stop_for(&owner_token(paths)?); + match fs::remove_file(stop_path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + } +} + fn write_launch_status(paths: &StatePaths, token: Option<&str>, status: &str) { if let Some(token) = token { let _ = atomic_write(&paths.launch_status(token), status.as_bytes()); @@ -610,7 +649,8 @@ fn request_stop(state_key: &str, session_id: &str) -> io::Result<()> { format!("no monitor found for state key {state_key:?}"), )); } - fs::write(&paths.stop, b"stop\n")?; + let token = owner_token(&paths)?; + fs::write(paths.stop_for(&token), b"stop\n")?; println!("stop requested for {}", paths.root.display()); Ok(()) } @@ -623,18 +663,45 @@ fn run_foreground( if_running: RunningMode, producer_command: &[OsString], launch_token: Option<&str>, +) -> io::Result<()> { + run_foreground_after_claim( + state_key, + label, + instructions, + session_id, + if_running, + producer_command, + launch_token, + || {}, + ) +} + +fn run_foreground_after_claim( + state_key: &str, + label: &str, + instructions: &str, + session_id: &str, + if_running: RunningMode, + producer_command: &[OsString], + launch_token: Option<&str>, + after_claim: impl FnOnce(), ) -> io::Result<()> { let paths = StatePaths::for_key(state_key, session_id); ensure_private_directory(&paths.root)?; - let _owner = claim_owner(&paths).inspect_err(|error| { + let owner_token = launch_token + .map(str::to_owned) + .unwrap_or_else(next_pending_generation); + let _owner = claim_owner(&paths, &owner_token).inspect_err(|error| { write_launch_status(&paths, launch_token, &format!("error: {error}")); })?; + after_claim(); + if paths.stop_for(&owner_token).exists() { + log_line(&paths, "stop requested before producer start")?; + write_launch_status(&paths, launch_token, "ready"); + remove_active_stop(&paths)?; + return Ok(()); + } let result = (|| { - paths - .stop - .exists() - .then(|| fs::remove_file(&paths.stop)) - .transpose()?; run_producer( &paths, label, @@ -713,7 +780,7 @@ fn run_producer( let mut termination_requested = false; loop { - if paths.stop.exists() && !termination_requested { + if stop_requested(paths) && !termination_requested { log_line(paths, "stop requested")?; producer.terminate_and_wait(); termination_requested = true; @@ -806,7 +873,7 @@ fn run_producer( if_running, )?; - while !pending.bytes.is_empty() && !paths.stop.exists() { + while !pending.bytes.is_empty() && !stop_requested(paths) { thread::sleep(RETRY_INTERVAL); flush_pending( paths, @@ -817,12 +884,13 @@ fn run_producer( if_running, )?; } - if !pending.bytes.is_empty() && paths.stop.exists() { + if !pending.bytes.is_empty() && stop_requested(paths) { pending.bytes.clear(); pending.rotate(); persist_pending(&paths.pending, &pending)?; log_line(paths, "discarded undelivered output after explicit stop")?; } + remove_active_stop(paths)?; log_line(paths, &format!("producer exited with status {status}"))?; Ok(()) } @@ -946,7 +1014,7 @@ fn deliver_with_candidates( ) -> bool { for lock in locks { for binary in &binaries { - if paths.stop.exists() { + if stop_requested(paths) { return false; } let mut child = match Command::new(binary) @@ -977,7 +1045,7 @@ fn deliver_with_candidates( }; let deadline = Instant::now() + timeout; loop { - if paths.stop.exists() { + if stop_requested(paths) { let _ = child.kill(); let _ = child.wait(); return false; @@ -1266,6 +1334,7 @@ mod tests { let paths = StatePaths::for_key(&key, "test-session"); ensure_private_directory(&paths.root).unwrap(); let status_path = paths.launch_status("never-ready"); + let stop_path = paths.stop_for("never-ready"); let mut child = Command::new("sh") .args(["-c", "exec sleep 30"]) .spawn() @@ -1273,13 +1342,14 @@ mod tests { let pid = child.id(); let launch_timeout = Duration::from_millis(100); - let launch_result = wait_for_launch(&mut child, &paths, &status_path, launch_timeout); + let launch_result = + wait_for_launch(&mut child, &paths, &status_path, &stop_path, launch_timeout); let error = launch_result.unwrap_err(); assert!(error.contains("did not become ready")); assert!(!test_process_exists(pid)); assert!(!status_path.exists()); - assert!(!paths.stop.exists()); + assert!(!stop_path.exists()); fs::remove_dir_all(paths.root).unwrap(); } @@ -1499,7 +1569,7 @@ mod tests { let session_id = "test-session"; let paths = StatePaths::for_key(&key, session_id); ensure_private_directory(&paths.root).unwrap(); - let owner = claim_owner(&paths).unwrap(); + let owner = claim_owner(&paths, "delivery-stop-owner").unwrap(); let fake_berdctl = paths.root.join("fake-berdctl"); fs::write(&fake_berdctl, "#!/bin/sh\nexec sleep 30\n").unwrap(); fs::set_permissions(&fake_berdctl, fs::Permissions::from_mode(0o700)).unwrap(); @@ -1627,13 +1697,13 @@ mod tests { ); let paths = StatePaths::for_key(&key, "test-session"); ensure_private_directory(&paths.root).unwrap(); - let owner = claim_owner(&paths).unwrap(); + let owner = claim_owner(&paths, "owner-a").unwrap(); assert_eq!( - claim_owner(&paths).unwrap_err().kind(), + claim_owner(&paths, "owner-b").unwrap_err().kind(), io::ErrorKind::WouldBlock ); drop(owner); - claim_owner(&paths).unwrap(); + claim_owner(&paths, "owner-b").unwrap(); fs::remove_dir_all(&paths.root).unwrap(); } @@ -1658,7 +1728,7 @@ mod tests { let key = key.clone(); workers.push(thread::spawn(move || { barrier.wait(); - let owner = claim_owner(&StatePaths::for_key(&key, "test-session")); + let owner = claim_owner(&StatePaths::for_key(&key, "test-session"), "race-owner"); if owner.is_ok() { thread::sleep(Duration::from_millis(100)); } @@ -1703,6 +1773,46 @@ mod tests { fs::remove_dir_all(&paths.root).unwrap(); } + #[cfg(unix)] + #[test] + fn stop_after_owner_claim_prevents_producer_start() { + let key = format!( + "startup-stop-race-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let session_id = "test-session"; + let paths = StatePaths::for_key(&key, session_id); + let producer_started = paths.root.join("producer-started"); + let command = format!("printf started > '{}'", producer_started.display()); + + run_foreground_after_claim( + &key, + "test", + "", + session_id, + RunningMode::Steer, + &[ + OsString::from("sh"), + OsString::from("-c"), + OsString::from(command), + ], + Some("startup-stop-owner"), + || request_stop(&key, session_id).unwrap(), + ) + .unwrap(); + + assert!(!producer_started.exists()); + assert!(!paths.stop_for("startup-stop-owner").exists()); + assert!(fs::read_to_string(&paths.log) + .unwrap() + .contains("stop requested before producer start")); + fs::remove_dir_all(&paths.root).unwrap(); + } + #[cfg(unix)] #[test] fn stop_terminates_the_producer_process_group_and_discards_pending() { From 2935acd3fbab0bf65146a28bd177378d972cc23f Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 15:21:00 -0400 Subject: [PATCH 18/22] fix(chat): clear delivery metadata on manual edits Signed-off-by: John Tennant --- src/features/chat/ui/ChatInput.tsx | 1 + src/features/chat/ui/__tests__/ChatInput.test.tsx | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/features/chat/ui/ChatInput.tsx b/src/features/chat/ui/ChatInput.tsx index 34c980b79..b9489bf39 100644 --- a/src/features/chat/ui/ChatInput.tsx +++ b/src/features/chat/ui/ChatInput.tsx @@ -110,6 +110,7 @@ function stripCrossSessionOrigin>( const { origin: _origin, berdSenderLabel: _berdSenderLabel, + berdDeliveryId: _berdDeliveryId, ...rest } = metadata; return Object.keys(rest).length > 0 ? (rest as T) : undefined; diff --git a/src/features/chat/ui/__tests__/ChatInput.test.tsx b/src/features/chat/ui/__tests__/ChatInput.test.tsx index cfab01afd..ee54a378a 100644 --- a/src/features/chat/ui/__tests__/ChatInput.test.tsx +++ b/src/features/chat/ui/__tests__/ChatInput.test.tsx @@ -3455,7 +3455,7 @@ describe("ChatInput", () => { ); }); - it("strips cross-session origin metadata when resending an edited queued message", async () => { + it("strips cross-session delivery metadata when resending an edited queued message", async () => { const onSend = vi.fn(() => true); const user = userEvent.setup(); @@ -3468,10 +3468,14 @@ describe("ChatInput", () => { sendOptions: { acpGooseMetadata: { origin: "berdctl_cross_session", + berdSenderLabel: "berd-monitor", + berdDeliveryId: "event-1", threadId: "thread-1", }, userMessageMetadata: { origin: "berdctl_cross_session", + berdSenderLabel: "berd-monitor", + berdDeliveryId: "event-1", }, }, }); From e3f20252530ec4fa53c1735d17241f9a4a57af26 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 15:38:44 -0400 Subject: [PATCH 19/22] fix(monitor): surface stdout capture failures Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 156 ++++++++++++++++++++-- 1 file changed, 148 insertions(+), 8 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index 03c357e09..b5275801b 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -100,6 +100,11 @@ struct PendingState { bytes: Vec, } +enum ProducerOutputEvent { + Data(Vec), + ReadError(io::Error), +} + impl PendingState { fn empty() -> Self { Self { @@ -769,7 +774,7 @@ fn run_producer( } write_launch_status(paths, launch_token, "ready"); let stdout = producer.child.stdout.take().expect("stdout was piped"); - let (sender, receiver) = mpsc::sync_channel::>(1024); + let (sender, receiver) = mpsc::sync_channel::(1024); thread::spawn(move || forward_producer_output(stdout, sender)); let mut batch = Vec::new(); @@ -778,6 +783,7 @@ fn run_producer( let mut producer_status = None; let mut receiver_closed = false; let mut termination_requested = false; + let mut capture_failure = None; loop { if stop_requested(paths) && !termination_requested { @@ -824,7 +830,7 @@ fn run_producer( .unwrap_or(Duration::from_millis(100)) .min(Duration::from_millis(100)); match receiver.recv_timeout(timeout) { - Ok(line) => { + Ok(ProducerOutputEvent::Data(line)) => { if !batch.is_empty() && batch.len() + line.len() > MAX_DELIVERY_BYTES { append_batch(paths, &mut pending, &mut batch)?; flush_pending( @@ -854,6 +860,10 @@ fn run_producer( batch_deadline.get_or_insert(Instant::now() + BATCH_WINDOW); } } + Ok(ProducerOutputEvent::ReadError(error)) => { + capture_failure = Some(record_capture_failure(paths, &mut producer, error)?); + termination_requested = true; + } Err(RecvTimeoutError::Timeout) => {} Err(RecvTimeoutError::Disconnected) => receiver_closed = true, } @@ -862,7 +872,10 @@ fn run_producer( producer.terminate_and_wait(); append_batch(paths, &mut pending, &mut batch)?; let status = producer_status.expect("producer has exited"); - let summary = format!("[monitor] producer exited with status {status}\n"); + let summary = capture_failure + .as_ref() + .map(|(_, message)| format!("[monitor] {message}\n")) + .unwrap_or_else(|| format!("[monitor] producer exited with status {status}\n")); append_pending(&paths.pending, &mut pending, summary.as_bytes())?; flush_pending( paths, @@ -891,35 +904,68 @@ fn run_producer( log_line(paths, "discarded undelivered output after explicit stop")?; } remove_active_stop(paths)?; + if let Some((kind, message)) = capture_failure { + return Err(io::Error::new(kind, message)); + } log_line(paths, &format!("producer exited with status {status}"))?; Ok(()) } -fn forward_producer_output(mut reader: R, sender: mpsc::SyncSender>) { +fn record_capture_failure( + paths: &StatePaths, + producer: &mut ProducerProcess, + error: io::Error, +) -> io::Result<(io::ErrorKind, String)> { + let message = format!("producer stdout capture failed: {error}"); + log_line(paths, &message)?; + producer.terminate_and_wait(); + Ok((error.kind(), message)) +} + +fn forward_producer_output(mut reader: R, sender: mpsc::SyncSender) { let mut read_buffer = [0_u8; 8 * 1024]; let mut record = Vec::with_capacity(MAX_DELIVERY_BYTES); loop { let read = match reader.read(&mut read_buffer) { Ok(0) => break, Ok(read) => read, - Err(_) => return, + Err(error) => { + if !send_output_record(&sender, &mut record) { + return; + } + let _ = sender.send(ProducerOutputEvent::ReadError(error)); + return; + } }; for byte in &read_buffer[..read] { record.push(*byte); if *byte == b'\n' || record.len() == MAX_DELIVERY_BYTES { - if sender.send(std::mem::take(&mut record)).is_err() { + if sender + .send(ProducerOutputEvent::Data(std::mem::take(&mut record))) + .is_err() + { return; } record = Vec::with_capacity(MAX_DELIVERY_BYTES); } } } + let _ = send_output_record(&sender, &mut record); +} + +fn send_output_record( + sender: &mpsc::SyncSender, + record: &mut Vec, +) -> bool { if !record.is_empty() { if !record.ends_with(b"\n") { record.push(b'\n'); } - let _ = sender.send(record); + return sender + .send(ProducerOutputEvent::Data(std::mem::take(record))) + .is_ok(); } + true } fn append_batch( @@ -1475,7 +1521,15 @@ mod tests { let (sender, receiver) = mpsc::sync_channel(8); forward_producer_output(io::Cursor::new(&input), sender); - let records = receiver.into_iter().collect::>(); + let records = receiver + .into_iter() + .map(|event| match event { + ProducerOutputEvent::Data(record) => record, + ProducerOutputEvent::ReadError(error) => { + panic!("unexpected capture failure: {error}") + } + }) + .collect::>(); assert_eq!(records.len(), 4); assert!(records @@ -1486,6 +1540,92 @@ mod tests { assert_eq!(output, input); } + #[test] + fn producer_output_preserves_partial_record_before_read_error() { + struct BytesThenError(bool); + + impl Read for BytesThenError { + fn read(&mut self, buffer: &mut [u8]) -> io::Result { + if self.0 { + return Err(io::Error::other("injected stdout read failure")); + } + self.0 = true; + let bytes = b"partial output"; + buffer[..bytes.len()].copy_from_slice(bytes); + Ok(bytes.len()) + } + } + + let (sender, receiver) = mpsc::sync_channel(2); + forward_producer_output(BytesThenError(false), sender); + let events = receiver.into_iter().collect::>(); + + assert_eq!(events.len(), 2); + assert!(matches!( + &events[0], + ProducerOutputEvent::Data(bytes) if bytes == b"partial output\n" + )); + assert!(matches!( + &events[1], + ProducerOutputEvent::ReadError(error) + if error.to_string() == "injected stdout read failure" + )); + } + + #[cfg(unix)] + #[test] + fn capture_failure_is_recorded_and_cleans_up_the_process_group() { + use std::io::{BufRead, BufReader}; + + let key = format!( + "capture-failure-test-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + let paths = StatePaths::for_key(&key, "test-session"); + ensure_private_directory(&paths.root).unwrap(); + let mut command = Command::new("sh"); + command + .args(["-c", "sleep 30 & child=$!; echo $child; wait"]) + .stdout(Stdio::piped()); + configure_producer(&mut command); + let child = command.spawn().unwrap(); + let tree = attach_producer_tree(&child).unwrap(); + let mut producer = ProducerProcess { child, tree }; + resume_producer(&producer.child).unwrap(); + let direct_pid = producer.child.id(); + let mut descendant = String::new(); + BufReader::new(producer.child.stdout.take().unwrap()) + .read_line(&mut descendant) + .unwrap(); + let descendant_pid = descendant.trim().parse::().unwrap(); + + let failure = record_capture_failure( + &paths, + &mut producer, + io::Error::other("injected stdout read failure"), + ) + .unwrap(); + + assert_eq!(failure.0, io::ErrorKind::Other); + assert!(failure.1.contains("injected stdout read failure")); + assert!(fs::read_to_string(&paths.log) + .unwrap() + .contains("producer stdout capture failed")); + let deadline = Instant::now() + Duration::from_secs(2); + while (test_process_exists(direct_pid) || test_process_exists(descendant_pid)) + && Instant::now() < deadline + { + thread::sleep(Duration::from_millis(25)); + } + assert!(!test_process_exists(direct_pid)); + assert!(!test_process_exists(descendant_pid)); + fs::remove_dir_all(paths.root).unwrap(); + } + #[test] fn delivery_prompt_replaces_embedded_nul_bytes_without_growing() { let prompt = build_delivery_prompt("test", b"before\0after\n", "continue").unwrap(); From 9744ca21ad57b27080122260936c6fdb99258e14 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 15:40:52 -0400 Subject: [PATCH 20/22] fix(berdctl): deduplicate hydrated deliveries Signed-off-by: John Tennant --- .../__tests__/commands/commands.test.ts | 37 +++++++++++++++++++ .../berdctl/commands/impl/sendSession.ts | 13 +++++++ .../berdctl/commands/runtime/sessionSend.ts | 9 +++++ 3 files changed, 59 insertions(+) diff --git a/src/features/berdctl/__tests__/commands/commands.test.ts b/src/features/berdctl/__tests__/commands/commands.test.ts index fce1b9a71..8d1246d90 100644 --- a/src/features/berdctl/__tests__/commands/commands.test.ts +++ b/src/features/berdctl/__tests__/commands/commands.test.ts @@ -1847,6 +1847,43 @@ describe("sessions.send", () => { expect(mocks.acpSteerMessage).not.toHaveBeenCalled(); }); + it("deduplicates a delivery id restored while hydrating a cold transcript", async () => { + mockSessionFound({ providerId: "codex-acp" }); + mocks.loadSessionMessages.mockImplementationOnce(async () => { + const accepted = createUserMessage("monitor event"); + accepted.metadata = { + origin: "berdctl_cross_session", + berdDeliveryId: "monitor-event-1", + }; + useChatStore.getState().addMessage("session-1", accepted); + return true; + }); + + const duplicate = await dispatchCommand( + "sessions", + { + action: "send", + session_id: "session-1", + prompt: "monitor event retried after restart", + delivery_id: "monitor-event-1", + }, + ctx, + ); + + expect(duplicate).toEqual({ + session_id: "session-1", + send_status: "deduplicated", + }); + expect(useChatStore.getState().messagesBySession["session-1"]).toHaveLength( + 1, + ); + expect( + useChatStore.getState().queuedMessageBySession["session-1"], + ).toBeUndefined(); + expect(mocks.acpSendMessage).not.toHaveBeenCalled(); + expect(mocks.acpSteerMessage).not.toHaveBeenCalled(); + }); + it("rejects multiline sender labels before dispatch", async () => { const error = await expectCommandError( dispatchCommand( diff --git a/src/features/berdctl/commands/impl/sendSession.ts b/src/features/berdctl/commands/impl/sendSession.ts index a45b4608b..998bac0e0 100644 --- a/src/features/berdctl/commands/impl/sendSession.ts +++ b/src/features/berdctl/commands/impl/sendSession.ts @@ -101,6 +101,8 @@ Result: { findProjectOrThrow }, { berdctlCrossSessionSendOptions, + BerdctlDeliveryAlreadyAcceptedError, + hasAcceptedBerdctlDelivery, reserveBerdctlDelivery, sendPromptToExistingSessionInBackground, SessionDispatchContentionError, @@ -237,9 +239,20 @@ Result: { returnOnDispatch: true, sendOptions, + validateHydratedTranscript: () => { + if ( + args.delivery_id && + hasAcceptedBerdctlDelivery(args.session_id, args.delivery_id) + ) { + throw new BerdctlDeliveryAlreadyAcceptedError(); + } + }, }, ); } catch (error) { + if (error instanceof BerdctlDeliveryAlreadyAcceptedError) { + return { session_id: session.id, send_status: "deduplicated" }; + } if (error instanceof SessionDispatchContentionError) { if (args.if_running === "queue") { useChatStore.getState().enqueueTransportReadyMessage( diff --git a/src/features/berdctl/commands/runtime/sessionSend.ts b/src/features/berdctl/commands/runtime/sessionSend.ts index 6bc7e508e..539d67d92 100644 --- a/src/features/berdctl/commands/runtime/sessionSend.ts +++ b/src/features/berdctl/commands/runtime/sessionSend.ts @@ -26,6 +26,13 @@ export const BERDCTL_CROSS_SESSION_ORIGIN = const reservedDeliveryIds = new Set(); +export class BerdctlDeliveryAlreadyAcceptedError extends Error { + constructor() { + super("The Berd delivery was already accepted."); + this.name = "BerdctlDeliveryAlreadyAcceptedError"; + } +} + export function berdctlCrossSessionSendOptions( options: { senderLabel?: string; deliveryId?: string } = {}, ): ChatSendOptions { @@ -93,6 +100,7 @@ export async function sendPromptToExistingSessionInBackground( options: { returnOnDispatch?: boolean; sendOptions?: ChatSendOptions; + validateHydratedTranscript?: () => void; } = {}, ): Promise { const acquisition = await acquireExistingSessionForBackgroundSend(sessionId); @@ -126,6 +134,7 @@ export async function sendPromptToExistingSessionInBackground( dispatchToken: targetLease.token, }); const session = useChatSessionStore.getState().getSession(sessionId); + options.validateHydratedTranscript?.(); await sendPromptInBackground( sessionId, prompt, From 52c271dac72f1eca13f8fa6379b28a2a7b2437b9 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 15:58:21 -0400 Subject: [PATCH 21/22] fix(berdctl): deduplicate recovered queue deliveries Signed-off-by: John Tennant --- .../useBerdctlQueuedMessageDrain.test.tsx | 84 ++++++++++++++++++- .../bridge/useBerdctlQueuedMessageDrain.ts | 63 +++++++++++--- .../berdctl/commands/runtime/sessionSend.ts | 13 ++- 3 files changed, 145 insertions(+), 15 deletions(-) diff --git a/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx b/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx index 9142ba047..520f3c16b 100644 --- a/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx +++ b/src/features/berdctl/__tests__/bridge/useBerdctlQueuedMessageDrain.test.tsx @@ -13,6 +13,7 @@ import { useSessionWindowStore } from "@/features/chat/stores/sessionWindowStore import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; import * as queuePersistence from "@/features/chat/stores/queuePersistence"; import { useBerdctlQueuedMessageDrain } from "@/features/berdctl/bridge/useBerdctlQueuedMessageDrain"; +import { createUserMessage } from "@/shared/types/messages"; const mocks = vi.hoisted(() => ({ sendPromptToExistingSessionInBackground: vi.fn(), @@ -589,11 +590,92 @@ describe("useBerdctlQueuedMessageDrain", () => { "session-1", "queued delivery", expect.any(Function), - { returnOnDispatch: true, sendOptions }, + { + returnOnDispatch: true, + sendOptions, + validateHydratedTranscript: expect.any(Function), + }, ); }); }); + it("dismisses a stale queued delivery already accepted in the transcript", async () => { + const accepted = createUserMessage("queued delivery"); + accepted.metadata = { + origin: "berdctl_cross_session", + berdDeliveryId: "monitor-event-1", + }; + const chatStore = useChatStore.getState(); + chatStore.addMessage("session-1", accepted); + chatStore.enqueueTransportReadyMessage("session-1", { + persona: { kind: "inherit" }, + text: "queued delivery", + sendOptions: { + userMessageMetadata: { + origin: "berdctl_cross_session" as const, + berdDeliveryId: "monitor-event-1", + }, + }, + }); + + render(); + + await waitFor(() => { + expect( + useChatStore.getState().queuedMessageBySession["session-1"], + ).toBeUndefined(); + }); + expect( + mocks.sendPromptToExistingSessionInBackground, + ).not.toHaveBeenCalled(); + expect(useChatStore.getState().messagesBySession["session-1"]).toHaveLength( + 1, + ); + }); + + it("dismisses a stale queued delivery accepted during cold hydration", async () => { + mocks.sendPromptToExistingSessionInBackground.mockImplementationOnce( + async ( + _sessionId: string, + _prompt: string, + _beforeUserMessageCommitted: () => void, + options?: { validateHydratedTranscript?: () => void }, + ) => { + const accepted = createUserMessage("queued delivery"); + accepted.metadata = { + origin: "berdctl_cross_session", + berdDeliveryId: "monitor-event-1", + }; + useChatStore.getState().addMessage("session-1", accepted); + options?.validateHydratedTranscript?.(); + }, + ); + useChatStore.getState().enqueueTransportReadyMessage("session-1", { + persona: { kind: "inherit" }, + text: "queued delivery", + sendOptions: { + userMessageMetadata: { + origin: "berdctl_cross_session" as const, + berdDeliveryId: "monitor-event-1", + }, + }, + }); + + render(); + + await waitFor(() => { + expect( + useChatStore.getState().queuedMessageBySession["session-1"], + ).toBeUndefined(); + }); + expect( + mocks.sendPromptToExistingSessionInBackground, + ).toHaveBeenCalledOnce(); + expect(useChatStore.getState().messagesBySession["session-1"]).toHaveLength( + 1, + ); + }); + it("drains consecutive berdctl records in FIFO order while idle", async () => { const chatStore = useChatStore.getState(); chatStore.enqueueTransportReadyMessage("session-1", { diff --git a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts index 576f1c5cf..468a54b16 100644 --- a/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts +++ b/src/features/berdctl/bridge/useBerdctlQueuedMessageDrain.ts @@ -17,6 +17,8 @@ import { import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; import { useSessionWindowStore } from "@/features/chat/stores/sessionWindowStore"; import { + BerdctlDeliveryAlreadyAcceptedError, + hasAcceptedBerdctlDeliveryInTranscript, isBerdctlCrossSessionQueuedMessage, sendPromptToExistingSessionInBackground, } from "@/features/berdctl/commands/runtime/sessionSend"; @@ -98,6 +100,17 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { return; } + const deliveryId = + queuedMessage.payload.sendOptions?.userMessageMetadata?.berdDeliveryId; + if ( + deliveryId && + hasAcceptedBerdctlDeliveryInTranscript(queuedSessionId, deliveryId) + ) { + dismissQueuedMessageIfCurrent(queuedSessionId, queuedMessage); + queueMicrotask(() => drainQueuedMessage(queuedSessionId, ownerId)); + return; + } + drainingSessionIds.add(queuedSessionId); const send = sendPromptToExistingSessionInBackground( queuedSessionId, @@ -117,6 +130,20 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { queuedMessage.payload.sendOptions?.userMessageMetadata?.berdDeliveryId ? { sendOptions: queuedMessage.payload.sendOptions } : {}), + ...(deliveryId + ? { + validateHydratedTranscript: () => { + if ( + hasAcceptedBerdctlDeliveryInTranscript( + queuedSessionId, + deliveryId, + ) + ) { + throw new BerdctlDeliveryAlreadyAcceptedError(); + } + }, + } + : {}), }, ); let sendSucceeded = false; @@ -125,19 +152,14 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { void send .then(() => { sendSucceeded = true; - const latestQueuedMessage = - useChatStore.getState().queuedMessageBySession[queuedSessionId]?.[0]; - if ( - latestQueuedMessage?.recordId === queuedMessage.recordId && - latestQueuedMessage.payload === queuedMessage.payload && - !latestQueuedMessage.editing - ) { - useChatStore - .getState() - .dismissQueuedMessage(queuedSessionId, queuedMessage.recordId); - } + dismissQueuedMessageIfCurrent(queuedSessionId, queuedMessage); }) .catch((error) => { + if (error instanceof BerdctlDeliveryAlreadyAcceptedError) { + dismissQueuedMessageIfCurrent(queuedSessionId, queuedMessage); + shouldResumeDrain = true; + return; + } if (error instanceof SessionDispatchContentionError) { waitingForContention = true; const waiter: ContentionWaiter = { @@ -181,6 +203,25 @@ function drainQueuedMessage(queuedSessionId: string, ownerId: string): void { }); } +function dismissQueuedMessageIfCurrent( + sessionId: string, + queuedMessage: QueuedMessageRecord, +): boolean { + const latestQueuedMessage = + useChatStore.getState().queuedMessageBySession[sessionId]?.[0]; + if ( + latestQueuedMessage?.recordId !== queuedMessage.recordId || + latestQueuedMessage.payload !== queuedMessage.payload || + latestQueuedMessage.editing + ) { + return false; + } + useChatStore + .getState() + .dismissQueuedMessage(sessionId, queuedMessage.recordId); + return true; +} + function getQueuedSessionIds( queuedMessageBySession: Record, queuedSessionId?: string, diff --git a/src/features/berdctl/commands/runtime/sessionSend.ts b/src/features/berdctl/commands/runtime/sessionSend.ts index 539d67d92..e7a34cfb5 100644 --- a/src/features/berdctl/commands/runtime/sessionSend.ts +++ b/src/features/berdctl/commands/runtime/sessionSend.ts @@ -62,9 +62,7 @@ export function hasAcceptedBerdctlDelivery( ): boolean { const chatStore = useChatStore.getState(); return ( - (chatStore.messagesBySession[sessionId] ?? []).some( - (message) => message.metadata?.berdDeliveryId === deliveryId, - ) || + hasAcceptedBerdctlDeliveryInTranscript(sessionId, deliveryId) || (chatStore.queuedMessageBySession[sessionId] ?? []).some( (record) => record.payload.sendOptions?.userMessageMetadata?.berdDeliveryId === @@ -73,6 +71,15 @@ export function hasAcceptedBerdctlDelivery( ); } +export function hasAcceptedBerdctlDeliveryInTranscript( + sessionId: string, + deliveryId: string, +): boolean { + return (useChatStore.getState().messagesBySession[sessionId] ?? []).some( + (message) => message.metadata?.berdDeliveryId === deliveryId, + ); +} + export function reserveBerdctlDelivery( sessionId: string, deliveryId: string, From 0ddd1953792d9814f795827e28f0191d07ab63aa Mon Sep 17 00:00:00 2001 From: John Tennant Date: Fri, 28 Aug 2026 16:20:49 -0400 Subject: [PATCH 22/22] fix(monitor): disarm producer cleanup Signed-off-by: John Tennant --- src-tauri/crates/berd-monitor/src/main.rs | 51 ++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/src-tauri/crates/berd-monitor/src/main.rs b/src-tauri/crates/berd-monitor/src/main.rs index b5275801b..73094c758 100644 --- a/src-tauri/crates/berd-monitor/src/main.rs +++ b/src-tauri/crates/berd-monitor/src/main.rs @@ -540,12 +540,22 @@ impl Drop for ProducerTree { struct ProducerProcess { child: std::process::Child, - tree: ProducerTree, + tree: Option, } impl ProducerProcess { fn terminate_and_wait(&mut self) { - terminate_producer_tree(&mut self.child, &self.tree); + self.terminate_and_wait_with(terminate_producer_tree); + } + + fn terminate_and_wait_with( + &mut self, + terminate: impl FnOnce(&mut std::process::Child, &ProducerTree), + ) { + let Some(tree) = self.tree.take() else { + return; + }; + terminate(&mut self.child, &tree); let _ = self.child.wait(); } } @@ -764,7 +774,7 @@ fn run_producer( }; let mut producer = ProducerProcess { child, - tree: producer_tree, + tree: Some(producer_tree), }; if let Err(error) = resume_producer(&producer.child) { return Err(io::Error::new( @@ -1475,7 +1485,10 @@ mod tests { configure_producer(&mut command); let child = command.spawn()?; let tree = attach_producer_tree(&child)?; - let mut producer = ProducerProcess { child, tree }; + let mut producer = ProducerProcess { + child, + tree: Some(tree), + }; resume_producer(&producer.child)?; let direct_pid = producer.child.id(); let mut descendant = String::new(); @@ -1499,6 +1512,31 @@ mod tests { assert!(!test_process_exists(descendant_pid)); } + #[cfg(unix)] + #[test] + fn explicit_producer_cleanup_disarms_drop() { + let mut command = Command::new("sh"); + command.args(["-c", "exec sleep 30"]); + configure_producer(&mut command); + let child = command.spawn().unwrap(); + let tree = attach_producer_tree(&child).unwrap(); + let mut producer = ProducerProcess { + child, + tree: Some(tree), + }; + resume_producer(&producer.child).unwrap(); + let mut termination_calls = 0; + + producer.terminate_and_wait_with(|child, tree| { + termination_calls += 1; + terminate_producer_tree(child, tree); + }); + producer.terminate_and_wait_with(|_, _| termination_calls += 1); + drop(producer); + + assert_eq!(termination_calls, 1); + } + #[test] fn chunk_prefers_complete_lines() { let input = b"first\nsecond\nthird\n"; @@ -1594,7 +1632,10 @@ mod tests { configure_producer(&mut command); let child = command.spawn().unwrap(); let tree = attach_producer_tree(&child).unwrap(); - let mut producer = ProducerProcess { child, tree }; + let mut producer = ProducerProcess { + child, + tree: Some(tree), + }; resume_producer(&producer.child).unwrap(); let direct_pid = producer.child.id(); let mut descendant = String::new();