diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d04f41f..86b3e99 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,12 +5,23 @@ on: branches: [main, "redesign/**"] pull_request: -# The GPUI app links Metal and macOS APIs, so the build runs on macOS. -# Windows/Linux support is out of scope until the macOS app ships (INTENT.md D5). +# GPUI renders through each platform's native graphics API (Metal on macOS, +# DXGI on Windows, X11/Wayland on Linux), so every change is built on all three +# shipped platforms. Linux needs the same system libraries the release bundle +# installs (`scripts/bundle-linux.sh`). +# +# The "Protect Main Branch" ruleset requires one status check named `build`. +# The matrix legs report `build ()`, so the `build` fan-in job below is +# what reports the required context — and it succeeds only when every +# platform leg did. jobs: check: - name: build - runs-on: macos-latest + name: build (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [macos-latest, ubuntu-22.04, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -19,6 +30,16 @@ jobs: with: components: clippy + - name: Install Linux dependencies + if: runner.os == 'Linux' + run: | + sudo apt-get update + sudo apt-get install --yes \ + clang cmake pkg-config \ + libfontconfig-dev libvulkan1 \ + libwayland-dev libx11-xcb-dev libxi-dev libxtst-dev \ + libxkbcommon-x11-dev libssl-dev + - name: Cache cargo uses: Swatinem/rust-cache@v2 @@ -29,6 +50,27 @@ jobs: run: cargo build --workspace # Informational: the repo carries pre-existing clippy lints, so this - # step reports rather than gates. + # step reports rather than gates. One all-targets pass is enough — the + # per-platform builds above are the check that the app ships everywhere. - name: Clippy + if: runner.os == 'macOS' run: cargo clippy --workspace --all-targets + + # The required `build` status check. `always()` keeps this context + # reporting even when a matrix leg fails or the matrix is cancelled, so + # branch protection can never hang on "Expected — Waiting for status to be + # reported"; the first step then turns a non-success into a red `build`. + build: + name: build + needs: check + if: always() + runs-on: ubuntu-latest + steps: + - name: Require every platform build + if: needs.check.result != 'success' + run: | + echo "::error::the platform build matrix did not pass: ${{ needs.check.result }}" + exit 1 + + - name: All platforms built + run: echo "macOS, Linux, and Windows builds passed" diff --git a/AGENT.md b/AGENT.md index ed3626a..1a5d648 100644 --- a/AGENT.md +++ b/AGENT.md @@ -387,7 +387,7 @@ Every feature in `PRODUCT.md` → `Capabilities` runs natively in the GPUI app a ## Verification - `cargo build --workspace` must stay clean (zero warnings) after every change. -- `cargo test --workspace` — unit tests + live pi integration tests. `live_pi.rs`, `live_catalog.rs`, and the other live tests skip when `pi` is missing; `orbit-pi` session-store tests skip when `~/.pi/agent/sessions` is empty. CI runs build + clippy on macOS (`.github/workflows/ci.yml`); tests run locally. +- `cargo test --workspace` — unit tests + live pi integration tests. `live_pi.rs`, `live_catalog.rs`, and the other live tests skip when `pi` is missing; `orbit-pi` session-store tests skip when `~/.pi/agent/sessions` is empty. CI builds every change on macOS, Linux (ubuntu-22.04), and Windows, plus clippy on macOS (`.github/workflows/ci.yml`); tests run locally. - Run the app: `cargo run -p orbit-pi`. Check: sessions list from disk, new session (`cmd-n`), prompt streams into the transcript, model/thinking pickers, abort (`escape`), settings (`cmd-,`), sidebar toggle. Notifications need an app bundle — `scripts/run-bundled.sh` builds the debug binary into the ad-hoc-signed `Orbit Pi Alpha` (bundle id `dev.orbit.pi.alpha`, Alpha icon) and opens it. - Windows: the same `cargo run -p orbit-pi` builds and runs (GPUI renders through DXGI here). `gpui`'s default `windows-manifest` feature is off in `crates/orbit-pi/Cargo.toml` — it links a second `RT_MANIFEST` resource beside the one `build.rs` embeds, and CVTRES fails the link with `CVT1100: duplicate resource`. `scripts/bundle-windows.ps1` builds the Inno Setup installer (`scripts/installer/orbit-pi.iss`); it needs Inno Setup 6 (`ISCC.exe`) and, per-user under `%LocalAppData%\Programs`, no elevation. Desktop notifications and the alert sound are still macOS-only stubs. Window chrome is the app's own: the header row carries minimize / maximize / close, drawing a restore glyph when `window.is_maximized()`, and the header strips drag the window through `platform::start_window_drag`. Check them by clicking — `IsIconic`/`IsZoomed` flip, closing exits the process — and check dragging by pressing a header strip and moving the pointer; both were broken while the presses were routed through GPUI's `WindowControlArea` path, which this app's focusable root makes unusable (see the Shell row). - No `unsafe` without a comment; no new dependencies without a stated reason. @@ -523,7 +523,7 @@ Every feature in `PRODUCT.md` → `Capabilities` runs natively in the GPUI app a ## Verification - `cargo build --workspace` must stay clean (zero warnings) after every change. -- `cargo test --workspace` — unit tests + live pi integration tests. `live_pi.rs`, `live_catalog.rs`, and the other live tests skip when `pi` is missing; `orbit-pi` session-store tests skip when `~/.pi/agent/sessions` is empty. CI runs build + clippy on macOS (`.github/workflows/ci.yml`); tests run locally. +- `cargo test --workspace` — unit tests + live pi integration tests. `live_pi.rs`, `live_catalog.rs`, and the other live tests skip when `pi` is missing; `orbit-pi` session-store tests skip when `~/.pi/agent/sessions` is empty. CI builds every change on macOS, Linux (ubuntu-22.04), and Windows, plus clippy on macOS (`.github/workflows/ci.yml`); tests run locally. - Run the app: `cargo run -p orbit-pi`. Check: sessions list from disk, new session (`cmd-n`), prompt streams into the transcript, model/thinking pickers, abort (`escape`), settings (`cmd-,`), sidebar toggle. Notifications need an app bundle — `scripts/run-bundled.sh` builds the debug binary into the ad-hoc-signed `Orbit Pi Alpha` (bundle id `dev.orbit.pi.alpha`, Alpha icon) and opens it. - Windows: the same `cargo run -p orbit-pi` builds and runs (GPUI renders through DXGI here). `gpui`'s default `windows-manifest` feature is off in `crates/orbit-pi/Cargo.toml` — it links a second `RT_MANIFEST` resource beside the one `build.rs` embeds, and CVTRES fails the link with `CVT1100: duplicate resource`. `scripts/bundle-windows.ps1` builds the Inno Setup installer (`scripts/installer/orbit-pi.iss`); it needs Inno Setup 6 (`ISCC.exe`) and, per-user under `%LocalAppData%\Programs`, no elevation. Desktop notifications and the alert sound are still macOS-only stubs. Window chrome is the app's own: the header row carries minimize / maximize / close, drawing a restore glyph when `window.is_maximized()`, and the header strips drag the window through `platform::start_window_drag`. Check them by clicking — `IsIconic`/`IsZoomed` flip, closing exits the process — and check dragging by pressing a header strip and moving the pointer; both were broken while the presses were routed through GPUI's `WindowControlArea` path, which this app's focusable root makes unusable (see the Shell row). - No `unsafe` without a comment; no new dependencies without a stated reason. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d55d3e..b144bb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **New Task** now starts a fresh session when pi has exited instead of doing + nothing. With no turn in flight the button sent `new_session` to the live + process, so while the reconnect banner was up the command went into the dead + process's stdin and was never answered; it now spawns a replacement whenever + the process is not alive, dropping the corpse and its stale exit banner. The + sidebar's workspace **+** gets the same fix. + ## [0.2.0] - 2026-09-28 ### Added diff --git a/crates/orbit-pi/src/app.rs b/crates/orbit-pi/src/app.rs index 3b24d96..8e7b4b7 100644 --- a/crates/orbit-pi/src/app.rs +++ b/crates/orbit-pi/src/app.rs @@ -2224,6 +2224,8 @@ mod devicons_tests; #[cfg(test)] mod error_label_tests; #[cfg(test)] +mod new_task_reconnect_tests; +#[cfg(test)] mod popup_layout_tests; #[cfg(test)] mod session_default_apply_tests; diff --git a/crates/orbit-pi/src/app/new_task_reconnect_tests.rs b/crates/orbit-pi/src/app/new_task_reconnect_tests.rs new file mode 100644 index 0000000..37d0ec0 --- /dev/null +++ b/crates/orbit-pi/src/app/new_task_reconnect_tests.rs @@ -0,0 +1,204 @@ +//! Regression tests for issue #30: with pi disconnected (the process exited, +//! the reconnect banner is up), the sidebar's New Task button did nothing. +//! `on_new_session` only checked whether a run was in flight, so it sent +//! `new_session` into the dead process's stdin — a write nobody answers — +//! instead of spawning a fresh process. The decision is `can_reuse_session`: +//! reuse only when the process is alive and idle; otherwise start a task on a +//! fresh process. +#![cfg(unix)] + +use super::*; +use crate::theme::{Theme, ThemeId}; +use std::os::unix::fs::PermissionsExt as _; + +/// A fake pi that swallows stdin into `stdin.log` and blocks, so the +/// transport sees a live child without spawning the real CLI. Deleted when +/// the test ends; the app's `PiClient` kills the child first. +struct FakePi { + dir: PathBuf, + bin: PathBuf, + log: PathBuf, +} + +impl FakePi { + fn start() -> Self { + let dir = std::env::temp_dir().join(format!( + "orbit-new-task-{}-{:?}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + )); + std::fs::create_dir_all(&dir).expect("create fake-pi dir"); + let bin = dir.join("fake-pi"); + let log = dir.join("stdin.log"); + std::fs::write( + &bin, + format!("#!/bin/sh\nexec cat >> \"{}\"\n", log.display()), + ) + .expect("write fake pi"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) + .expect("chmod fake pi"); + Self { dir, bin, log } + } + + /// The lines the app wrote to pi's stdin, once the writer thread flushes. + fn stdin(&self) -> String { + let deadline = Instant::now() + Duration::from_secs(2); + loop { + let text = std::fs::read_to_string(&self.log).unwrap_or_default(); + if !text.is_empty() || Instant::now() > deadline { + return text; + } + std::thread::sleep(Duration::from_millis(10)); + } + } +} + +impl Drop for FakePi { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +fn test_app(cx: &mut gpui::TestAppContext) -> Entity { + cx.update(|cx| { + cx.set_global(Theme::for_id(ThemeId::Orbit)); + cx.new(OrbitApp::new) + }) +} + +/// Install `fake` as the active pi process, as a live spawn would. +fn adopt_fake_pi(app: &mut OrbitApp, fake: &FakePi) { + let workspace = app + .current_workspace + .clone() + .or_else(|| std::env::current_dir().ok()) + .unwrap_or_else(|| PathBuf::from(".")); + let client = PiClient::spawn_with_bin(fake.bin.to_str().expect("utf-8 path"), &workspace, None) + .expect("spawn fake pi"); + app.adopt_client(client); +} + +/// What `ProcessExited` leaves behind: the client is still held while the +/// runtime records the death. New Task must not `send` into that stdin. +#[gpui::test] +fn an_exited_pi_process_never_reuses_the_session(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + cx.update(|cx| { + app.update(cx, |app, _| { + let workspace = app + .current_workspace + .clone() + .or_else(|| std::env::current_dir().ok()) + .unwrap_or_else(|| PathBuf::from(".")); + let dead = PiClient::spawn_with_bin("/usr/bin/false", &workspace, None) + .expect("spawn /usr/bin/false"); + app.client = Some(dead); + app.runtime.alive = false; + app.runtime.exited = true; + + assert!(!app.runtime_is_live()); + assert!( + !app.can_reuse_session(), + "a dead pi process cannot honor new_session" + ); + }); + }); +} + +/// A runtime that was never started (or was stopped): no process to reuse. +#[gpui::test] +fn a_missing_pi_process_never_reuses_the_session(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + cx.update(|cx| { + app.update(cx, |app, _| { + app.drop_client(); + assert!(!app.can_reuse_session()); + }); + }); +} + +/// A run in flight: `new_session` would abort the live turn, so the task +/// must start on its own parked-and-replaced process. +#[gpui::test] +fn a_running_turn_never_reuses_the_session(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + let fake = FakePi::start(); + cx.update(|cx| { + app.update(cx, |app, _| { + adopt_fake_pi(app, &fake); + assert!(app.can_reuse_session(), "an idle live process is reusable"); + app.busy = true; + assert!(!app.can_reuse_session(), "a live turn is not reusable"); + }); + }); +} + +/// The warm path stays warm: an idle live process takes `new_session` in +/// place — no second spawn, and the command reaches pi's stdin. +#[gpui::test] +fn an_idle_live_session_reuses_the_process_for_the_new_task(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + let fake = FakePi::start(); + let started_at = cx.update(|cx| { + app.update(cx, |app, _| { + adopt_fake_pi(app, &fake); + app.runtime.started_at + }) + }); + let cx = cx.add_empty_window(); + cx.update(|window, cx| { + app.update(cx, |app, cx| { + assert!(app.can_reuse_session()); + app.on_new_session(&crate::NewSession, window, cx); + }); + }); + let stdin = fake.stdin(); + assert!( + stdin.contains("\"type\":\"new_session\""), + "expected new_session on pi's stdin, got: {stdin:?}" + ); + cx.update(|_, cx| { + app.update(cx, |app, _| { + assert_eq!( + app.runtime.started_at, started_at, + "reusing the live process must not spawn another" + ); + }); + }); +} + +/// Dropping the dead process supersedes the exit banner it left behind, so +/// starting a fresh task does not keep a stale "pi process exited" on screen. +#[gpui::test] +fn dropping_an_exited_process_clears_its_exit_banner(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + cx.update(|cx| { + app.update(cx, |app, _| { + app.runtime.exited = true; + app.set_error(tr!("events.process_exited")); + app.drop_client(); + assert!(app.client.is_none()); + assert!( + app.error.is_none(), + "the banner described the process that was just dropped" + ); + }); + }); +} + +/// The clear is narrow: a healthy process dropping for any reason must not +/// take an unrelated error banner with it. +#[gpui::test] +fn dropping_a_healthy_process_keeps_unrelated_errors(cx: &mut gpui::TestAppContext) { + let app = test_app(cx); + cx.update(|cx| { + app.update(cx, |app, _| { + app.set_error("agent error: no API key"); + app.drop_client(); + assert_eq!(app.error.as_deref(), Some("agent error: no API key")); + }); + }); +} diff --git a/crates/orbit-pi/src/app/runtime.rs b/crates/orbit-pi/src/app/runtime.rs index d972b7c..3e4c30b 100644 --- a/crates/orbit-pi/src/app/runtime.rs +++ b/crates/orbit-pi/src/app/runtime.rs @@ -133,6 +133,12 @@ impl OrbitApp { /// Tear down the active pi process (dropping `PiClient` kills the child). pub(super) fn drop_client(&mut self) { + // A process that exited on its own left its exit banner up; dropping + // it supersedes that failure — a fresh process follows in the task / + // restart paths, and an explicit stop acknowledges it. + if self.runtime.exited { + self.error = None; + } self.client = None; self.runtime = RuntimeStatus::default(); // Extension widgets belonged to the departing process. @@ -460,6 +466,13 @@ impl OrbitApp { } } + /// Whether the active pi process is alive and has not exited — the only + /// state in which commands sent over its stdin can still be answered. A + /// never-started, stopped, or dead process all read as not live. + pub(super) fn runtime_is_live(&self) -> bool { + matches!(self.runtime_state(), RuntimeState::Running) + } + /// Spawn the pi process (Start button). No-op while one is running. pub(super) fn runtime_start(&mut self, cx: &mut Context) { if self.client.is_some() { diff --git a/crates/orbit-pi/src/app/session.rs b/crates/orbit-pi/src/app/session.rs index 3fe75a5..d65684d 100644 --- a/crates/orbit-pi/src/app/session.rs +++ b/crates/orbit-pi/src/app/session.rs @@ -40,6 +40,15 @@ impl OrbitApp { self.busy || self.transcript.is_streaming() } + /// Whether New Task can reuse the active process in place with + /// `new_session`: only when no turn is in flight and the process is + /// alive. A run in flight must be parked so `new_session` cannot abort + /// it; a dead process must be replaced by a fresh spawn instead of + /// sending the command into a stdin nobody reads (issue #30). + pub(super) fn can_reuse_session(&self) -> bool { + !self.is_running() && self.runtime_is_live() + } + pub(super) fn submit(&mut self, text: String, cx: &mut Context) { self.submit_as(text, theme::get(cx).ui.composer_send_mode, cx); } @@ -487,8 +496,10 @@ impl OrbitApp { // turn (`agent-session-runtime.teardownCurrent`), which surfaces as // "This operation was aborted". Park the running session instead — its // process keeps going in the background — and start the new task on a - // fresh pi process. An idle session is cheap to reuse in place. - if self.is_running() { + // fresh pi process. An idle session is cheap to reuse in place, but + // only while its process is alive: a dead one (see issue #30) gets a + // fresh process too rather than a command into the void. + if !self.can_reuse_session() { let cwd = self .current_workspace .clone() @@ -517,13 +528,10 @@ impl OrbitApp { window: &mut Window, cx: &mut Context, ) { - // Same workspace with an idle process: reuse it in place. A run in - // flight is parked instead, so `new_session` never aborts it; a - // different workspace always needs its own process anyway. - if self.current_workspace.as_ref() == Some(&cwd) - && self.client.is_some() - && !self.is_running() - { + // Same workspace with a live idle process: reuse it in place. A run + // in flight is parked instead, so `new_session` never aborts it; a + // dead process (or a different workspace) always needs its own. + if self.current_workspace.as_ref() == Some(&cwd) && self.can_reuse_session() { self.send(CommandBody::NewSession, "new_session"); self.input.read(cx).focus(window); cx.notify(); @@ -532,9 +540,10 @@ impl OrbitApp { self.begin_new_task(cwd, window, cx); } - /// Start a fresh task rooted at `cwd` on its own pi process, parking the - /// active session first so a running one keeps going in the background. - /// Shared by New Task and a workspace group's "+". + /// Start a fresh task rooted at `cwd` on its own pi process, parking a + /// live active session first so a running one keeps going in the + /// background (a dead one is dropped). Shared by New Task and a + /// workspace group's "+". pub(super) fn begin_new_task( &mut self, cwd: PathBuf, @@ -544,7 +553,14 @@ impl OrbitApp { // Leaving this session: cancel any open blocking dialog first, so a // parked run never waits on a modal tied to the previous session. self.cancel_open_dialog(cx); - self.park_active_session(); + // A live session is parked so a run in flight keeps going in the + // background. A dead one holds nothing worth keeping: drop it (its + // exit banner goes with it) instead of parking a corpse. + if self.runtime_is_live() { + self.park_active_session(); + } else { + self.drop_client(); + } self.busy = false; self.transcript.clear();