diff --git a/docs/impls/0035-auto-resume-width-and-opt-in.md b/docs/impls/0035-auto-resume-width-and-opt-in.md index 16fb5cb..eb669c6 100644 --- a/docs/impls/0035-auto-resume-width-and-opt-in.md +++ b/docs/impls/0035-auto-resume-width-and-opt-in.md @@ -30,10 +30,10 @@ Note this is a genuine race, not a fixed ordering: `consumeResumeOnLaunch` fires Stopped and transitional panes deliberately differ: stopped panes keep pushing every laid-out geometry measurement so later zoom, sidebar, split-layout, and window changes can correct the persisted size, while transitional panes suppress resize until the fork completes. Stopped pushes are plain RPCs that skip both the local TUI clear and the forced resize dance, and they preserve `replayJustDrainedRef` so a geometry-only push cannot consume the next clear-capable push's replay protection. 3. **Do not fix this with ordering.** The obvious alternative — defer `consumeResumeOnLaunch` until layout settles, or have it await a "surfaces ready" signal — makes correctness depend on a race we'd have to keep winning as the boot sequence evolves. Decisions 1 and 2 make the outcome correct whichever side wins: if the mount push lands first, the fork is already right; if the resume lands first, the correction fires the moment the pane is live and measurable. No new timing dependency. - **Corrected by [0036](0036-launch-resume-fork-width.md) / [#363](https://github.com/yicheng47/runner/issues/363).** The concern was sound but stated too broadly. Both decisions here act *after* the PTY forks, so they fix the eventual state and leave the agent's resume banner wrapped at the fork width — permanently, for runtimes that keep their ring across a resume. Awaiting window-restore settle is not the race this warned about: restore lands asynchronously, so *any* geometry read before it is wrong regardless of who wins. 0036 gates the queue on that settle. + **Corrected by [0038](0038-launch-resume-fork-width.md) / [#363](https://github.com/yicheng47/runner/issues/363).** The concern was sound but stated too broadly. Both decisions here act *after* the PTY forks, so they fix the eventual state and leave the agent's resume banner wrapped at the fork width — permanently, for runtimes that keep their ring across a resume. Awaiting window-restore settle is not the race this warned about: restore lands asynchronously, so *any* geometry read before it is wrong regardless of who wins. 0038 gates the queue on that settle. 4. **Accept that background sessions still fork at their last known width.** Most auto-resumed sessions have no mounted pane at all — only the active tab and the persistent surfaces are mounted. There is no correct width to give them, and the persisted one (now trustworthy per decision 1) is the best available guess. When the user later opens such a tab, activation pushes real dims and the agent repaints. Explicitly not solved here: history a background agent hard-wrapped before you ever looked at it. That's the same irreducible limit 0032 recorded. - **Corrected by [0036](0036-launch-resume-fork-width.md) / [#363](https://github.com/yicheng47/runner/issues/363).** "No correct width" was too pessimistic. `terminalSizing.ts` already derives a grid with nothing mounted, and the persisted tab layouts already say which pane a session returns into — so a background session can be estimated from *this* launch's window and its own share of the split. That beats a value persisted at the previous quit, which is stale by construction whenever the window moved, resized, changed display scale, or the session now renders in a split. The persisted value stays as the rung below the estimate. + **Corrected by [0038](0038-launch-resume-fork-width.md) / [#363](https://github.com/yicheng47/runner/issues/363).** "No correct width" was too pessimistic. `terminalSizing.ts` already derives a grid with nothing mounted, and the persisted tab layouts already say which pane a session returns into — so a background session can be estimated from *this* launch's window and its own share of the split. That beats a value persisted at the previous quit, which is stale by construction whenever the window moved, resized, changed display scale, or the session now renders in a split. The persisted value stays as the rung below the estimate. 5. **Auto-resume becomes opt-in, default off.** Export one `DEFAULT_RESUME_ON_LAUNCH = false` constant and use it in both the `App.tsx` consumer and the `GeneralPane.tsx` toggle. Sharing the constant structurally prevents the two defaults from drifting and rendering a toggle state the consumer does not act on. 6. **No migration for existing users, but the behavior change is real and must be stated.** The setting is stored under `settings.resumeOnLaunch` and read with a default; anyone who never touched the toggle has no stored key, so flipping the default silently turns auto-resume off for them too. That is the intent — spawning agents unprompted at launch should be something you choose, not something you discover. It shipped only two releases ago (0.4.2), so the blast radius is small, and writing a one-time "preserve previous default" key would leave a permanent wart to explain. Call it out in the release notes instead. 7. **The quit-side stamp stays unconditional.** The backend continues to mark sessions at graceful quit regardless of the toggle, and a launch with the toggle off still *clears* pending stamps without resuming (the edge case #320 built deliberately). This keeps enabling the setting a forward-looking act rather than something that resurrects a session set from an old quit. diff --git a/docs/impls/0036-launch-resume-fork-width.md b/docs/impls/0038-launch-resume-fork-width.md similarity index 97% rename from docs/impls/0036-launch-resume-fork-width.md rename to docs/impls/0038-launch-resume-fork-width.md index 8147a26..158b857 100644 --- a/docs/impls/0036-launch-resume-fork-width.md +++ b/docs/impls/0038-launch-resume-fork-width.md @@ -23,7 +23,7 @@ So 0035 made the *eventual* state correct and left the *visible* damage in place 2. **Supply dims at the call site; stop passing `null`.** `resumeOnLaunch` gains cols/rows. `session_resume` already threads them (`commands/session.rs:620-649`), so this is caller-side only. Precedence for what to send, best first: **measured** from the session's laid-out container when it has one → **estimated** from current window geometry via the `terminalSizing.ts` helpers → **persisted** `last_cols`/`last_rows` → `DEFAULT_PTY_SIZE`. Today's chain starts at step three; this prepends the two rungs that reflect reality. 3. **Prefer deferring over guessing for panes that are about to exist.** A session whose pane is mounting should fork against that pane's real measurement rather than an estimate. Where that can be awaited cheaply without stalling the queue, do it; where it cannot, fall to the estimate. Do not stall the whole queue on one pane — a slow or never-mounting surface must not block the rest (0035's failure-tolerance rule from #320 still holds). 4. **0035's decisions 1 and 2 stay, as the safety net.** Persisting geometry while stopped and re-asserting on going live remain correct and still catch anything dropped in flight (session exiting mid-resize, crash-and-resume, window handoff). This impl removes the need for them to be the *primary* mechanism, not the mechanisms themselves. -5. **Record the correction in 0035.** Its decisions 3 and 4 currently read as considered acceptances, and someone will hit this again if they stand unqualified. Add a short note pointing at 0036 and this issue. +5. **Record the correction in 0035.** Its decisions 3 and 4 currently read as considered acceptances, and someone will hit this again if they stand unqualified. Add a short note pointing at 0038 and this issue. ## Open questions — resolved @@ -54,7 +54,7 @@ So 0035 made the *eventual* state correct and left the *visible* damage in place ### Phase 1 — settle gate - `src/lib/windowSettle.ts`: `awaitWindowGeometrySettle` resolves once the webview viewport agrees with the native frame, or at `WINDOW_SETTLE_CEILING_MS`, or immediately when there is no native window to read. Deps are injected so the gate is testable without a window. -- `src/App.tsx`: awaited before `consumeResumeOnLaunch`, together with `hydratePaneLayoutsFromDb()` — the other input the dims computation depends on. Only when the toggle is on; the default-off path still clears stamps immediately. +- `src/App.tsx`: awaited before `consumeResumeOnLaunch`, together with `hydratePaneLayoutsFromDb()` — the other input the dims computation depends on. Originally gated on the resume toggle; since #367 the settle gate runs on every launch, because the mission grid-hint push (`estimateMissionTerminalGrid()` → `mission_grid_hint_set`) needs settled geometry regardless of the toggle. Only the pane-layout hydration remains toggle-gated; the default-off path still clears stamps right after the gate. - Tests (`windowSettle.test.ts`): settles with no wait when the viewport already agrees; waits out a lagging viewport; accepts rounding within tolerance; returns `timeout` at the ceiling instead of hanging; returns `unavailable` when the native size cannot be read. ### Phase 2 — dims at the call site diff --git a/src-tauri/src/commands/app.rs b/src-tauri/src/commands/app.rs index d3883e5..ef5ceac 100644 --- a/src-tauri/src/commands/app.rs +++ b/src-tauri/src/commands/app.rs @@ -34,6 +34,19 @@ pub(crate) fn show_main_window(app: &AppHandle) -> crate::error::Result<()> { Ok(()) } +/// Route a frontend diagnostic line into the same rotating file log the +/// backend writes. Packaged builds have no webview console, and the +/// launch-resume width bug (#366) only reproduces there — this is how the +/// `[launch-dims]` lines reach `~/Library/Logs/…/runner.log`. +#[tauri::command] +pub fn frontend_log(level: String, message: String) { + match level.as_str() { + "error" => log::error!(target: "frontend", "{message}"), + "warn" => log::warn!(target: "frontend", "{message}"), + _ => log::info!(target: "frontend", "{message}"), + } +} + /// Reveal the app log directory in the system file browser. /// /// On macOS this is `~/Library/Logs/com.wycstudios.runner/` for diff --git a/src-tauri/src/commands/mission.rs b/src-tauri/src/commands/mission.rs index 4133f28..7547f75 100644 --- a/src-tauri/src/commands/mission.rs +++ b/src-tauri/src/commands/mission.rs @@ -493,6 +493,39 @@ pub(crate) async fn mission_start_impl( mission_start_impl_with_size(state, app, input, None).await } +/// Grid a mission fork will use, plus the source tag the fork log line +/// reports. Caller-measured dims win; with none, the frontend's cached +/// mission pane grid fills in so backend-initiated starts (MCP +/// `mission_start` / `mission_reset` pass no size) don't fork at 80×24 +/// and lose their scrollback to the first slot-tab visit's cols-gate +/// purge (#367). `None` means no hint was ever recorded and +/// `register_mission_session` falls to `DEFAULT_PTY_SIZE`. +pub(crate) fn mission_fork_size( + caller: Option<(u16, u16)>, + hint: Option<(u16, u16)>, +) -> (Option<(u16, u16)>, &'static str) { + match (caller, hint) { + (Some(size), _) => (Some(size), "caller-supplied"), + (None, Some(size)) => (Some(size), "mission-hint"), + (None, None) => (None, "DEFAULT_PTY_SIZE"), + } +} + +/// Record the frontend's most recent measured/estimated mission pane grid. +/// Pushed once per launch after the window-geometry settle gate and again +/// whenever the mission workspace measures real slot dims; consumed by +/// `mission_fork_size` above. +#[tauri::command] +pub fn mission_grid_hint_set(state: State<'_, AppState>, cols: u16, rows: u16) -> Result<()> { + if cols == 0 || rows == 0 { + return Err(Error::msg(format!( + "mission grid hint must be positive; got {cols}x{rows}" + ))); + } + *state.mission_grid_hint.lock().unwrap() = Some((cols, rows)); + Ok(()) +} + async fn mission_start_impl_with_size( state: &AppState, app: &tauri::AppHandle, @@ -507,6 +540,9 @@ async fn mission_start_impl_with_size( use crate::session::manager::{SessionEvents, TauriSessionEvents}; use std::sync::Arc; + let (initial_size, size_source) = + mission_fork_size(initial_size, *state.mission_grid_hint.lock().unwrap()); + let out = { let mut conn = state.db.get()?; start(&mut conn, &state.app_data_dir, input)? @@ -694,6 +730,7 @@ async fn mission_start_impl_with_size( state.db.clone(), first_turn, initial_size, + size_source, ); match register_res { Ok(pending) => { @@ -1200,6 +1237,11 @@ pub(crate) async fn mission_reset_impl( use crate::session::manager::{SessionEvents, TauriSessionEvents}; use std::sync::Arc; + // Same hint fallback as mission_start: the MCP `mission_reset` tool + // passes no size, and an unsized respawn re-arms the cols-gate purge. + let (initial_size, size_source) = + mission_fork_size(initial_size, *state.mission_grid_hint.lock().unwrap()); + // 1. Snapshot the mission + crew + roster up front. let mission_snap = { let conn = state.db.get()?; @@ -1424,6 +1466,7 @@ pub(crate) async fn mission_reset_impl( state.db.clone(), first_turn, initial_size, + size_source, ); match register_res { Ok(pending) => { @@ -1952,6 +1995,35 @@ mod tests { .unwrap(); } + // #367: MCP mission_start/mission_reset pass no size and the recorded + // frontend grid hint must fill in. These cover the resolver's ordering + // only; the resolver-to-fork seam (resolved size reaching the SpawnSpec + // the runtime actually forks) is covered by + // `hinted_mission_start_forks_slots_at_the_hint` / + // `unhinted_mission_start_still_forks_at_default` in + // `session::manager::tests`. + #[test] + fn mission_fork_size_uses_hint_when_caller_absent() { + assert_eq!( + mission_fork_size(None, Some((161, 45))), + (Some((161, 45)), "mission-hint"), + ); + } + + #[test] + fn mission_fork_size_abstains_without_caller_or_hint() { + assert_eq!(mission_fork_size(None, None), (None, "DEFAULT_PTY_SIZE")); + } + + #[test] + fn mission_fork_size_prefers_caller_over_hint() { + // The UI path measures a real pane; a stale hint must not override it. + assert_eq!( + mission_fork_size(Some((120, 30)), Some((161, 45))), + (Some((120, 30)), "caller-supplied"), + ); + } + #[test] fn ensure_first_turn_fits_guards_the_argv_ceiling() { // #247: the composed body must stay under the argv ceiling even diff --git a/src-tauri/src/commands/node.rs b/src-tauri/src/commands/node.rs index 67fcec0..8b0dbb0 100644 --- a/src-tauri/src/commands/node.rs +++ b/src-tauri/src/commands/node.rs @@ -616,6 +616,7 @@ mod tests { routers: RouterRegistry::new(), mcp: Arc::new(McpHandle::new()), windows: Arc::new(WindowRegistry::new()), + mission_grid_hint: Arc::new(std::sync::Mutex::new(None)), }); app } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index fc8eb1d..5b6aee0 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -17,7 +17,7 @@ mod window_state; mod windows; use std::path::{Path, PathBuf}; -use std::sync::{Arc, RwLock}; +use std::sync::{Arc, Mutex, RwLock}; #[cfg(target_os = "macos")] use tauri::menu::{AboutMetadataBuilder, PredefinedMenuItem}; @@ -63,6 +63,14 @@ pub struct AppState { /// was last focused, so exactly one window owns a duplicated subject's /// PTY. `main` is registered in `setup`. pub windows: Arc, + /// Most recent known-good mission pane grid (cols, rows), pushed by + /// the frontend: once per launch after the window-geometry settle + /// gate, and whenever the mission workspace measures real slot dims. + /// Backend-initiated mission spawns (MCP `mission_start` / + /// `mission_reset`) consume it when the caller supplies no size, so + /// their slots don't fork at 80×24 and lose their scrollback to the + /// first slot-tab visit's cols-gate purge (#367). + pub mission_grid_hint: Arc>>, } #[cfg_attr(mobile, tauri::mobile_entry_point)] @@ -228,6 +236,7 @@ pub fn run() { // MCP-reconstructed `AppState` sees the same map. let window_registry = Arc::new(windows::WindowRegistry::new()); window_registry.register("main"); + let mission_grid_hint = Arc::new(Mutex::new(None)); let mcp_state = mcp::state::McpState { db: Arc::clone(&pool), app_data_dir: app_data_dir.clone(), @@ -238,6 +247,7 @@ pub fn run() { routers: Arc::clone(&routers), mcp: Arc::clone(&mcp_handle), windows: Arc::clone(&window_registry), + mission_grid_hint: Arc::clone(&mission_grid_hint), app_handle: app.handle().clone(), }; if let Err(e) = mcp_handle.start(&app_data_dir.join("mcp.sock"), mcp_state) { @@ -254,6 +264,7 @@ pub fn run() { routers, mcp: mcp_handle, windows: window_registry, + mission_grid_hint, }; // Mount router + bus for every `running` mission before @@ -358,6 +369,7 @@ pub fn run() { .invoke_handler(tauri::generate_handler![ commands::app::app_ready, commands::app::runner_logs_reveal, + commands::app::frontend_log, commands::crew::crew_list, commands::crew::crew_get, commands::crew::crew_create, @@ -399,6 +411,7 @@ pub fn run() { commands::slot::slot_set_lead, commands::slot::slot_reorder, commands::mission::mission_start, + commands::mission::mission_grid_hint_set, commands::mission::mission_attach, commands::mission::mission_stop, commands::mission::mission_archive, diff --git a/src-tauri/src/mcp/state.rs b/src-tauri/src/mcp/state.rs index 01ce40a..f046703 100644 --- a/src-tauri/src/mcp/state.rs +++ b/src-tauri/src/mcp/state.rs @@ -1,5 +1,5 @@ use std::path::PathBuf; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use tauri::AppHandle; @@ -25,6 +25,7 @@ pub(crate) struct McpState { pub routers: Arc, pub mcp: Arc, pub windows: Arc, + pub mission_grid_hint: Arc>>, pub app_handle: AppHandle, } @@ -40,6 +41,7 @@ impl McpState { routers: Arc::clone(&self.routers), mcp: Arc::clone(&self.mcp), windows: Arc::clone(&self.windows), + mission_grid_hint: Arc::clone(&self.mission_grid_hint), } } } diff --git a/src-tauri/src/session/manager/mod.rs b/src-tauri/src/session/manager/mod.rs index 372c3fe..edeafcd 100644 --- a/src-tauri/src/session/manager/mod.rs +++ b/src-tauri/src/session/manager/mod.rs @@ -49,7 +49,7 @@ mod tests; const MAX_OUTPUT_BUFFER_CHUNKS: usize = 4096; const RECENT_LOCAL_INPUT_WINDOW: Duration = Duration::from_secs(2); -const DEFAULT_PTY_SIZE: (u16, u16) = (80, 24); +pub(crate) const DEFAULT_PTY_SIZE: (u16, u16) = (80, 24); /// Minimum spacing between consecutive `claude-code` PTY launches. /// Long enough for one claude's OAuth refresh round-trip (network @@ -682,6 +682,9 @@ pub struct PendingMissionSpawn { mission: Mission, runner: Runner, slot_handle: String, + /// Where `spec.initial_size` came from (caller-supplied / mission-hint / + /// DEFAULT_PTY_SIZE), for the post-spawn fork log line (#366). + size_source: &'static str, plan: router::runtime::ResumePlan, first_turn_delivered_via_argv: bool, resolved_cwd: Option, diff --git a/src-tauri/src/session/manager/output.rs b/src-tauri/src/session/manager/output.rs index cf494b1..92f0588 100644 --- a/src-tauri/src/session/manager/output.rs +++ b/src-tauri/src/session/manager/output.rs @@ -535,13 +535,21 @@ impl SessionManager { // restored (the #306 symptom: remount shows only the latest // frame). Shells keep their buffer unconditionally — no repaint // would arrive, and their history is meaningful. - let cols_changed = { + let (cols_changed, prev_cols) = { let mut state = state.lock().unwrap(); - let changed = state.last_pty_cols != Some(cols); + let prev = state.last_pty_cols; + let changed = prev != Some(cols); state.last_pty_cols = Some(cols); - changed + (changed, prev) }; if cols_changed && runtime_clears_on_resize(session_id, pool) { + // Estimate-vs-reality mismatches show up here: a session + // forked at a wrong width loses its ring to this purge on + // the pane's first real-cols push (#366 diagnostics). + log::info!( + "cols-gate purge: session={session_id} cols {} -> {cols}", + prev_cols.map_or_else(|| "none".to_string(), |c| c.to_string()), + ); self.purge_output_buffer_keep_modes(session_id); } Ok(()) diff --git a/src-tauri/src/session/manager/spawn.rs b/src-tauri/src/session/manager/spawn.rs index 2a24ffd..05c4645 100644 --- a/src-tauri/src/session/manager/spawn.rs +++ b/src-tauri/src/session/manager/spawn.rs @@ -229,6 +229,7 @@ impl SessionManager { pool: Arc, first_turn: Option, initial_size: Option<(u16, u16)>, + size_source: &'static str, ) -> Result { let initial_size = Some(initial_size.unwrap_or(DEFAULT_PTY_SIZE)); @@ -354,6 +355,7 @@ impl SessionManager { mission: mission.clone(), runner: runner.clone(), slot_handle: slot.slot_handle.clone(), + size_source, plan, first_turn_delivered_via_argv, resolved_cwd, @@ -397,6 +399,7 @@ impl SessionManager { mission, runner, slot_handle, + size_source, plan, first_turn_delivered_via_argv, resolved_cwd, @@ -477,6 +480,18 @@ impl SessionManager { return Ok(CompleteSpawnOutcome::Cancelled); } + // The PTY exists and survived both cancellation windows — this + // line reports a fork that actually happened. One per slot so a + // production log shows the width every mission session started + // at and where it came from (#366). + if let Some((cols, rows)) = initial_size { + log::info!( + "mission slot fork: session={session_id} runtime={} \ + size={cols}x{rows} source={size_source}", + runner.runtime, + ); + } + let spawn_pid = self.runtime_pid(&rt_session); // Persist the runtime-side identity for diagnostics and for @@ -626,6 +641,7 @@ impl SessionManager { Arc::clone(&pool), first_turn, None, + "DEFAULT_PTY_SIZE", )?; let session_id = pending.session_id.clone(); let mission_id = pending.mission.id.clone(); @@ -1074,10 +1090,12 @@ impl SessionManager { } row }; - let initial_size = cols - .zip(rows) - .or(snap.last_cols.zip(snap.last_rows)) - .unwrap_or(DEFAULT_PTY_SIZE); + let (initial_size, size_source) = match (cols.zip(rows), snap.last_cols.zip(snap.last_rows)) + { + (Some(size), _) => (size, "caller-supplied"), + (None, Some(size)) => (size, "persisted-last-size"), + (None, None) => (DEFAULT_PTY_SIZE, "DEFAULT_PTY_SIZE"), + }; // Stamp the resume watermark (and, for full-frame-repaint // runtimes, purge the prior output buffer) up front. Two @@ -1344,6 +1362,22 @@ impl SessionManager { } }; + // The PTY exists — this line reports a fork that actually + // happened, not an attempt. One per resume so a production log + // shows the width every resumed session started at and where it + // came from (#366). + log::info!( + "{} fork: session={session_id} runtime={} size={}x{} source={size_source}", + if allow_fresh_fallback { + "resume" + } else { + "resume-on-launch" + }, + runner.runtime, + initial_size.0, + initial_size.1, + ); + let spawn_pid = self.runtime_pid(&rt_session); if let Ok(conn) = pool.get() { diff --git a/src-tauri/src/session/manager/tests.rs b/src-tauri/src/session/manager/tests.rs index 2135e73..2b01ac9 100644 --- a/src-tauri/src/session/manager/tests.rs +++ b/src-tauri/src/session/manager/tests.rs @@ -1454,12 +1454,170 @@ fn mission_registration_preserves_initial_terminal_size() { Arc::clone(&pool), None, Some((132, 41)), + "caller-supplied", ) .unwrap(); assert_eq!(pending.spec.initial_size, Some((132, 41))); } +#[test] +fn hinted_mission_start_forks_slots_at_the_hint() { + // #367 across the whole seam: the size the resolver derives from the + // frontend grid hint (mission_fork_size — exactly what mission_start + // feeds register when the caller passes no size) must reach the PTY + // fork itself. FakeRuntime records the SpawnSpec actually forked. + let (size, source) = crate::commands::mission::mission_fork_size(None, Some((161, 45))); + let pool = pool_with_schema(); + let mission_base = Mission { + crew_id: "c".into(), + ..mission() + }; + let runner = runner("/bin/cat", &[]); + let slot_id = insert_crew_runner(&pool, &mission_base.id, &runner.id); + let fresh_mission_id: String = { + let conn = pool.get().unwrap(); + conn.query_row("SELECT id FROM missions LIMIT 1", [], |r| r.get(0)) + .unwrap() + }; + let mission = Mission { + id: fresh_mission_id, + ..mission_base + }; + let mut slot = slot_for(&runner); + slot.id = slot_id; + + let fake = fake_runtime(); + let mgr = mgr_with_fake(None, Arc::clone(&fake)); + let cap = capture(); + let pending = mgr + .register_mission_session( + &mission, + &runner, + &slot, + std::path::Path::new("/tmp"), + PathBuf::from("/dev/null"), + Arc::clone(&pool), + None, + size, + source, + ) + .unwrap(); + let session_id = pending.session_id.clone(); + let outcome = mgr + .complete_mission_session_spawn( + pending, + Arc::clone(&cap) as Arc, + Arc::new(std::sync::atomic::AtomicBool::new(false)), + ) + .unwrap(); + + assert!(matches!(outcome, CompleteSpawnOutcome::Spawned)); + assert_eq!( + fake.last_spawn_spec().unwrap().initial_size, + Some((161, 45)) + ); + mgr.kill(&session_id).unwrap(); +} + +#[test] +fn unhinted_mission_start_still_forks_at_default() { + // Same seam with no caller size and no recorded hint: the fork still + // happens, at DEFAULT_PTY_SIZE — the pre-#367 behavior. + let (size, source) = crate::commands::mission::mission_fork_size(None, None); + let pool = pool_with_schema(); + let mission_base = Mission { + crew_id: "c".into(), + ..mission() + }; + let runner = runner("/bin/cat", &[]); + let slot_id = insert_crew_runner(&pool, &mission_base.id, &runner.id); + let fresh_mission_id: String = { + let conn = pool.get().unwrap(); + conn.query_row("SELECT id FROM missions LIMIT 1", [], |r| r.get(0)) + .unwrap() + }; + let mission = Mission { + id: fresh_mission_id, + ..mission_base + }; + let mut slot = slot_for(&runner); + slot.id = slot_id; + + let fake = fake_runtime(); + let mgr = mgr_with_fake(None, Arc::clone(&fake)); + let cap = capture(); + let pending = mgr + .register_mission_session( + &mission, + &runner, + &slot, + std::path::Path::new("/tmp"), + PathBuf::from("/dev/null"), + Arc::clone(&pool), + None, + size, + source, + ) + .unwrap(); + let session_id = pending.session_id.clone(); + let outcome = mgr + .complete_mission_session_spawn( + pending, + Arc::clone(&cap) as Arc, + Arc::new(std::sync::atomic::AtomicBool::new(false)), + ) + .unwrap(); + + assert!(matches!(outcome, CompleteSpawnOutcome::Spawned)); + assert_eq!( + fake.last_spawn_spec().unwrap().initial_size, + Some(DEFAULT_PTY_SIZE) + ); + mgr.kill(&session_id).unwrap(); +} + +#[test] +fn mission_registration_defaults_to_80x24_when_unsized() { + // The last rung of the #367 chain: no caller size and no recorded + // grid hint must still fork — at DEFAULT_PTY_SIZE, as before. + let pool = pool_with_schema(); + let mission_base = Mission { + crew_id: "c".into(), + ..mission() + }; + let runner = runner("/bin/cat", &[]); + let slot_id = insert_crew_runner(&pool, &mission_base.id, &runner.id); + let fresh_mission_id: String = { + let conn = pool.get().unwrap(); + conn.query_row("SELECT id FROM missions LIMIT 1", [], |r| r.get(0)) + .unwrap() + }; + let mission = Mission { + id: fresh_mission_id, + ..mission_base + }; + let mut slot = slot_for(&runner); + slot.id = slot_id; + + let mgr = mgr_with_fake(None, fake_runtime()); + let pending = mgr + .register_mission_session( + &mission, + &runner, + &slot, + std::path::Path::new("/tmp"), + PathBuf::from("/dev/null"), + Arc::clone(&pool), + None, + None, + "DEFAULT_PTY_SIZE", + ) + .unwrap(); + + assert_eq!(pending.spec.initial_size, Some(DEFAULT_PTY_SIZE)); +} + #[test] fn mission_spawn_cwd_prefers_mission_over_runner_working_dir() { // Regression guard for #101: the per-mission cwd typed into the diff --git a/src/App.tsx b/src/App.tsx index 4d90d86..7ac39fa 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -16,9 +16,11 @@ import { UpdateProvider, useUpdate } from "./contexts/UpdateContext"; import { consumeResumeOnLaunch } from "./lib/autoResume"; import { api } from "./lib/api"; import { nudgeAppZoom, syncTitlebarZoom } from "./lib/appZoom"; +import { logLaunchDims } from "./lib/frontendLog"; import { eventMatchesShortcut } from "./lib/keymap"; import { launchDimsFor } from "./lib/launchDims"; import { hydratePaneLayoutsFromDb } from "./lib/paneLayout"; +import { estimateMissionTerminalGrid } from "./lib/terminalSizing"; import { awaitWindowGeometrySettle } from "./lib/windowSettle"; import { DEFAULT_RESUME_ON_LAUNCH, @@ -60,17 +62,32 @@ export default function App() { STORAGE_RESUME_ON_LAUNCH, DEFAULT_RESUME_ON_LAUNCH, ); + // The settle gate runs on every launch, not just resume-enabled + // ones (impl 0038, decision 1): the restored frame has to have + // reached the webview before any geometry read — both the + // resume-dims chain below and the mission grid-hint push. It has + // a ceiling, so it can't hang the queue. + const settleStart = performance.now(); + const settle = await awaitWindowGeometrySettle(); + logLaunchDims( + `settle=${settle} elapsedMs=${Math.round( + performance.now() - settleStart, + )}`, + ); + if (cancelled) return; + // Seed the backend's mission pane grid hint so an MCP-started + // mission forks its slots at the destination width instead of + // 80×24 (#367). Unconditional: MCP starts don't care whether + // resume-on-launch is enabled. + const hint = estimateMissionTerminalGrid(); + if (hint) { + logLaunchDims(`mission-grid-hint ${hint.cols}x${hint.rows}`); + void api.mission.setGridHint(hint).catch(console.error); + } if (enabled) { - // Both inputs to every dims computation below, awaited before - // the first fork (impl 0036, decision 1): the restored frame has - // to have reached the webview, and the tab layouts have to be - // hydrated for a split pane's share of the width to be known. - // Neither can hang the queue — the settle gate has a ceiling and - // a failed hydration just costs the split divisor. - const settle = await awaitWindowGeometrySettle(); - if (settle !== "settled") { - console.info(`[auto-resume] window geometry settle: ${settle}`); - } + // Second input to the resume dims chain: tab layouts have to + // be hydrated for a split pane's share of the width to be + // known. A failed hydration just costs the split divisor. await hydratePaneLayoutsFromDb().catch(console.error); if (cancelled) return; } diff --git a/src/components/AppShell.tsx b/src/components/AppShell.tsx index c6792b5..4db18a1 100644 --- a/src/components/AppShell.tsx +++ b/src/components/AppShell.tsx @@ -117,7 +117,10 @@ export function AppShell({ children }: { children?: ReactNode }) { previewOpen={sidebarPreviewOpen} onPreviewOpenChange={setSidebarPreviewOpen} /> -
+
void; } /** @@ -185,6 +193,7 @@ export const RunnerTerminal = forwardRef< autoFocus, disabled, resizeDisabled, + onSizePushed, }, ref, ) { @@ -221,6 +230,21 @@ export const RunnerTerminal = forwardRef< const lastPushedColsRef = useRef(0); const lastPushedRowsRef = useRef(0); const reassertSizeRef = useRef<(() => void) | null>(null); + // Mirrors `onSizePushed` so the long-lived push closures below see the + // current callback without re-creating the terminal. + const onSizePushedRef = useRef(onSizePushed); + useEffect(() => { + onSizePushedRef.current = onSizePushed; + }, [onSizePushed]); + // One `[launch-dims] first-fit` line per LAUNCH-RESUMED session (#366): + // `takeLaunchResumed` consumes the mark autoResume set when this + // session's launch resume succeeded, so fresh chats, UI-started + // missions, manual resumes, and remounts never log. + const noteFirstRealFit = useCallback((cols: number, rows: number) => { + const sid = sessionIdRef.current; + if (!sid || !takeLaunchResumed(sid)) return; + logLaunchDims(`first-fit session=${sid} ${cols}x${rows}`); + }, []); // Snapshot replay is deferred until the pane is both active and // measurable. Mission workspaces mount every slot's RunnerTerminal // at once with `activeTab="feed"` by default — every slot pane is @@ -377,6 +401,7 @@ export const RunnerTerminal = forwardRef< const beforeRows = t.rows; ensureWebglRenderer(); fit.fit(); + noteFirstRealFit(t.cols, t.rows); if (t.cols !== beforeCols || t.rows !== beforeRows) { console.info( `[terminal] refresh-fit session=${sessionIdRef.current} ` + @@ -432,6 +457,7 @@ export const RunnerTerminal = forwardRef< ); lastPushedColsRef.current = cols; lastPushedRowsRef.current = rows; + onSizePushedRef.current?.({ cols, rows }); void api.session.resize(sid, cols, rows).catch(() => { rejectSizePush(cols, rows); }); @@ -485,6 +511,7 @@ export const RunnerTerminal = forwardRef< ); lastPushedColsRef.current = cols; lastPushedRowsRef.current = rows; + onSizePushedRef.current?.({ cols, rows }); void api.session.resize(sid, cols, rows).catch(() => { rejectSizePush(cols, rows); }); @@ -517,6 +544,7 @@ export const RunnerTerminal = forwardRef< replayJustDrainedRef.current = false; lastPushedColsRef.current = cols; lastPushedRowsRef.current = rows; + onSizePushedRef.current?.({ cols, rows }); const nudgedRows = rows > 1 ? rows - 1 : rows + 1; void api.session .resize(sid, cols, nudgedRows) @@ -530,7 +558,7 @@ export const RunnerTerminal = forwardRef< return false; } }, - [ensureWebglRenderer, rejectSizePush, blankGate], + [ensureWebglRenderer, rejectSizePush, blankGate, noteFirstRealFit], ); useEffect(() => { @@ -584,6 +612,8 @@ export const RunnerTerminal = forwardRef< ); lastPushedColsRef.current = current.cols; lastPushedRowsRef.current = current.rows; + onSizePushedRef.current?.({ cols: current.cols, rows: current.rows }); + noteFirstRealFit(current.cols, current.rows); void api.session.resize(sid, current.cols, current.rows).catch(() => { rejectSizePush(current.cols, current.rows); }); @@ -605,6 +635,7 @@ export const RunnerTerminal = forwardRef< const initialRect = containerRef.current.getBoundingClientRect(); if (initialRect.width > 0 && initialRect.height > 0) { fit.fit(); + noteFirstRealFit(term.cols, term.rows); // Push the freshly-fitted dims to the backend right here, before // the snapshot effect below fires its outputSnapshot RPC. The // backend's buffered bytes were emitted by the agent at whatever @@ -1128,7 +1159,7 @@ export const RunnerTerminal = forwardRef< termRef.current = null; fitRef.current = null; }; - }, [refreshActiveTerminal, rejectSizePush]); + }, [refreshActiveTerminal, rejectSizePush, noteFirstRealFit]); // A mounted hidden terminal still owns its CPU-side xterm buffer, but it // must not hold a WebGL context: RunnerChat deliberately keeps every @@ -1462,7 +1493,7 @@ export const RunnerTerminal = forwardRef< return ( // Keep this attribute in sync with terminalSizing's // TERMINAL_HOST_SESSION_ATTR: it lets launch auto-resume find this host - // and measure the box the terminal will fit to (impl 0036, rung 1) + // and measure the box the terminal will fit to (impl 0038, rung 1) // without a surface-local terminal registry.
+ invoke("mission_grid_hint_set", { + cols: size.cols, + rows: size.rows, + }), /** Re-mount router/bus on workspace mount; idempotent. After app restart * the in-memory router/bus need to be rebuilt from the persisted log * before stdin pushes can land on resumed slot PTYs. */ @@ -369,7 +378,7 @@ export const api = { clearResumeOnLaunch: () => invoke("session_clear_resume_on_launch"), /** Auto-resume-on-launch. Dims come from the caller's precedence chain - * (impl 0036) — passing null forks at the row's persisted geometry, or + * (impl 0038) — passing null forks at the row's persisted geometry, or * 80×24 when it has none. */ resumeOnLaunch: ( sessionId: string, diff --git a/src/lib/autoResume.test.ts b/src/lib/autoResume.test.ts index 5b11f69..3362e51 100644 --- a/src/lib/autoResume.test.ts +++ b/src/lib/autoResume.test.ts @@ -5,6 +5,10 @@ import { consumeResumeOnLaunch, resolveLaunchDims, } from "./autoResume"; +import { + resetLaunchResumeTraceForTest, + takeLaunchResumed, +} from "./launchResumeTrace"; import { DEFAULT_RESUME_ON_LAUNCH, readStoredBool, @@ -14,6 +18,7 @@ import type { TerminalGridSize } from "./terminalSizing"; afterEach(() => { vi.unstubAllGlobals(); + resetLaunchResumeTraceForTest(); }); const noDims = () => null; @@ -137,6 +142,46 @@ describe("consumeResumeOnLaunch", () => { expect(onError).toHaveBeenCalledOnce(); }); + // #366: RunnerTerminal emits one [launch-dims] first-fit line per + // LAUNCH-resumed session by consuming this mark. Ordinary sessions + // (fresh chats, manual resumes) never enter the set, a failed resume + // must not be marked, and a mark consumes exactly once so remounts + // can't re-log. + it("marks only successfully resumed sessions for the first-fit trace", async () => { + const takeResumeOnLaunch = vi + .fn<() => Promise>() + .mockResolvedValueOnce("session-a") + .mockResolvedValueOnce("session-b") + .mockResolvedValueOnce(null); + const resumeOnLaunch = vi + .fn< + ( + sessionId: string, + cols: number | null, + rows: number | null, + ) => Promise + >() + .mockRejectedValueOnce(new Error("spawn failed")) + .mockResolvedValueOnce(); + + await consumeResumeOnLaunch( + true, + { + takeResumeOnLaunch, + clearResumeOnLaunch: vi.fn<() => Promise>(), + resumeOnLaunch, + }, + noDims, + vi.fn<(ms: number) => Promise>().mockResolvedValue(), + vi.fn(), + ); + + expect(takeLaunchResumed("session-a")).toBe(false); // resume failed + expect(takeLaunchResumed("session-b")).toBe(true); // resumed + expect(takeLaunchResumed("session-b")).toBe(false); // consumed once + expect(takeLaunchResumed("fresh-chat")).toBe(false); // never launched + }); + it("clears pending stamps without resuming when disabled", async () => { const takeResumeOnLaunch = vi.fn<() => Promise>(); const resumeOnLaunch = diff --git a/src/lib/autoResume.ts b/src/lib/autoResume.ts index b114a38..9de18fc 100644 --- a/src/lib/autoResume.ts +++ b/src/lib/autoResume.ts @@ -1,3 +1,4 @@ +import { markLaunchResumed } from "./launchResumeTrace"; import type { TerminalGridSize } from "./terminalSizing"; export const AUTO_RESUME_STAGGER_MS = 300; @@ -13,7 +14,7 @@ interface AutoResumeApi { } /** - * Launch-resume dims precedence (impl 0036, decision 2). A real measurement + * Launch-resume dims precedence (impl 0038, decision 2). A real measurement * from a laid-out pane beats an estimate derived from this launch's window * geometry; null hands the remaining two rungs to the backend, which resolves * `last_cols`/`last_rows` and then `DEFAULT_PTY_SIZE` (`spawn.rs`). @@ -74,6 +75,10 @@ export async function consumeResumeOnLaunch( dims?.cols ?? null, dims?.rows ?? null, ); + // Only sessions that actually resumed get a pending first-fit + // mark — RunnerTerminal consumes it to log the estimate-vs-actual + // grid delta for this launch (#366). + markLaunchResumed(sessionId); } catch (error) { onError(error); } diff --git a/src/lib/frontendLog.ts b/src/lib/frontendLog.ts new file mode 100644 index 0000000..bc082ba --- /dev/null +++ b/src/lib/frontendLog.ts @@ -0,0 +1,23 @@ +import { invoke } from "@tauri-apps/api/core"; + +/** + * Mirror a `[launch-dims]` diagnostic line into the backend's rotating file + * log via the `frontend_log` command. Packaged builds have no webview + * console, and the launch-resume width bug (#366) only reproduces there — + * this is the only route these lines have into + * `~/Library/Logs/…/runner.log`. Best-effort: a missing Tauri runtime + * (browser preview) degrades to the console line alone. + */ +export function logLaunchDims(message: string): void { + const line = `[launch-dims] ${message}`; + console.info(line); + try { + void invoke("frontend_log", { level: "info", message: line }).catch( + () => { + // Browser preview has no Tauri runtime. + }, + ); + } catch { + // invoke can throw synchronously outside Tauri. + } +} diff --git a/src/lib/launchDims.test.ts b/src/lib/launchDims.test.ts index 6dfa37b..de161b3 100644 --- a/src/lib/launchDims.test.ts +++ b/src/lib/launchDims.test.ts @@ -1,6 +1,6 @@ /** @vitest-environment jsdom */ -// Which source a launch resume's dims come from (impl 0036, decision 2). The +// Which source a launch resume's dims come from (impl 0038, decision 2). The // sizing helpers are stubbed — their pixel math is terminalSizing's own // concern, and stubbing keeps xterm out of jsdom. What matters here is that a // laid-out pane wins over an estimate, that a chat session is estimated at its @@ -12,8 +12,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { applyPresetPure, resetPaneLayoutsForTest } from "./paneLayout"; type Grid = { cols: number; rows: number } | null; +type Box = { width: number; height: number } | null; -const chatPaneAreaBox = vi.fn(() => ({ width: 1000, height: 800 })); +const chatPaneAreaBox = vi.fn<() => Box>(() => ({ width: 1000, height: 800 })); +const missionPaneAreaBox = vi.fn<() => Box>(() => ({ + width: 952, + height: 762, +})); +const shellContentBox = vi.fn<() => Box>(() => ({ width: 1240, height: 844 })); const estimateMissionTerminalGrid = vi.fn<() => Grid>(() => ({ cols: 90, rows: 30, @@ -30,6 +36,8 @@ const terminalGridFromPixels = vi.fn<(w: number, h: number) => Grid>(() => ({ vi.mock("./terminalSizing", () => ({ TERMINAL_HOST_SESSION_ATTR: "data-terminal-session", chatPaneAreaBox: () => chatPaneAreaBox(), + missionPaneAreaBox: () => missionPaneAreaBox(), + shellContentBox: () => shellContentBox(), estimateMissionTerminalGrid: () => estimateMissionTerminalGrid(), terminalGridFromHostElement: (host: HTMLElement) => terminalGridFromHostElement(host), @@ -37,6 +45,10 @@ vi.mock("./terminalSizing", () => ({ terminalGridFromPixels(width, height), })); +// The [launch-dims] mirror invokes a Tauri command; keep tests silent and +// runtime-free. +vi.mock("./frontendLog", () => ({ logLaunchDims: () => {} })); + const { launchDimsFor } = await import("./launchDims"); beforeEach(() => { @@ -91,4 +103,13 @@ describe("launchDimsFor", () => { expect(launchDimsFor("mission-slot")).toEqual({ cols: 90, rows: 30 }); expect(chatPaneAreaBox).not.toHaveBeenCalled(); }); + + it("abstains when the chat pane area cannot be measured", () => { + // #371: a failed
measurement makes the estimate rung return + // null so the backend's persisted rung takes over — no window-derived + // guess. + chatPaneAreaBox.mockReturnValueOnce(null); + expect(launchDimsFor("chat-a")).toBeNull(); + expect(terminalGridFromPixels).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/launchDims.ts b/src/lib/launchDims.ts index 64a92f7..fd6bc96 100644 --- a/src/lib/launchDims.ts +++ b/src/lib/launchDims.ts @@ -1,4 +1,4 @@ -// Dims for a session about to be auto-resumed at launch (impl 0036, decision +// Dims for a session about to be auto-resumed at launch (impl 0038, decision // 2). Wires the two frontend rungs of the precedence chain to real sources; // `resolveLaunchDims` (autoResume.ts) owns the ordering, and returning null // from here is what hands the persisted/default rungs to the backend. @@ -21,9 +21,12 @@ import { type PaneLayout, } from "./paneLayout"; import { resolveLaunchDims } from "./autoResume"; +import { logLaunchDims } from "./frontendLog"; import { chatPaneAreaBox, estimateMissionTerminalGrid, + missionPaneAreaBox, + shellContentBox, TERMINAL_HOST_SESSION_ATTR, terminalGridFromHostElement, terminalGridFromPixels, @@ -47,16 +50,53 @@ function tabLayoutFor(sessionId: string): PaneLayout | null { ); } -function estimateForSession(sessionId: string): TerminalGridSize | null { +function estimateForSession( + sessionId: string, + detail: string[], +): TerminalGridSize | null { + const shell = shellContentBox(); + detail.push( + shell + ? `main=${Math.round(shell.width)}x${Math.round(shell.height)}` + : "main=null", + ); const layout = tabLayoutFor(sessionId); - if (!layout) return estimateMissionTerminalGrid(); - const box = paneBoxForSession(layout, sessionId, chatPaneAreaBox()); - return box ? terminalGridFromPixels(box.width, box.height) : null; + if (!layout) { + detail.push("surface=mission"); + const area = missionPaneAreaBox(); + if (shell && area) { + detail.push(`rail=${Math.round(shell.width - area.width)}px`); + } + return estimateMissionTerminalGrid(); + } + detail.push("surface=chat"); + const area = chatPaneAreaBox(); + if (!area) return null; + if (shell) detail.push(`panel=${Math.round(shell.width - area.width)}px`); + const box = paneBoxForSession(layout, sessionId, area); + if (!box) return null; + detail.push(`paneShare=${(box.width / area.width).toFixed(2)}`); + return terminalGridFromPixels(box.width, box.height); } export function launchDimsFor(sessionId: string): TerminalGridSize | null { - return resolveLaunchDims({ - measure: () => measureMountedTerminal(sessionId), - estimate: () => estimateForSession(sessionId), + // The detail trail exists for the #366 file log: one line per resumed + // session showing which rung answered and the inputs the estimate saw. + const detail: string[] = []; + const dims = resolveLaunchDims({ + measure: () => { + const measured = measureMountedTerminal(sessionId); + if (measured) detail.push("rung=measured"); + return measured; + }, + estimate: () => { + detail.push("rung=estimate"); + return estimateForSession(sessionId, detail); + }, }); + logLaunchDims( + `session=${sessionId} ${detail.join(" ")} -> ` + + (dims ? `${dims.cols}x${dims.rows}` : "null (backend persisted/default rung)"), + ); + return dims; } diff --git a/src/lib/launchResumeTrace.ts b/src/lib/launchResumeTrace.ts new file mode 100644 index 0000000..275f85b --- /dev/null +++ b/src/lib/launchResumeTrace.ts @@ -0,0 +1,21 @@ +// Session ids auto-resumed by THIS launch (#366 diagnostics). autoResume +// marks an id once its resume RPC succeeds; RunnerTerminal consumes the +// mark on the session's next real fit to emit exactly one +// `[launch-dims] first-fit` line per launch-resumed session. Fresh chats, +// UI-started missions, manual resumes, and navigation remounts never +// enter the set, so they never log. + +const pendingFirstFit = new Set(); + +export function markLaunchResumed(sessionId: string): void { + pendingFirstFit.add(sessionId); +} + +/** True exactly once per marked session — the caller owns the log line. */ +export function takeLaunchResumed(sessionId: string): boolean { + return pendingFirstFit.delete(sessionId); +} + +export function resetLaunchResumeTraceForTest(): void { + pendingFirstFit.clear(); +} diff --git a/src/lib/paneLayout.test.ts b/src/lib/paneLayout.test.ts index 2f5f7dd..0146d3e 100644 --- a/src/lib/paneLayout.test.ts +++ b/src/lib/paneLayout.test.ts @@ -803,7 +803,7 @@ describe("cold-start hydration", () => { }); }); -// The split divisor behind the launch-resume estimate (impl 0036, open +// The split divisor behind the launch-resume estimate (impl 0038, open // question 3). A session returning into a 2–3 pane tab must be sized at its // own pane's share of the area, not the tab's — the layout is known from // persisted state before any pane mounts, so no measurement is needed. diff --git a/src/lib/paneLayout.ts b/src/lib/paneLayout.ts index 834a5fd..e9b4207 100644 --- a/src/lib/paneLayout.ts +++ b/src/lib/paneLayout.ts @@ -138,7 +138,7 @@ const PANE_MIN_SIZE_PX = 120; * pane area), mirroring ChatPaneGroup's render. Null when the session is not * in this layout. * - * This is the split divisor the launch-resume estimate needs (impl 0036, open + * This is the split divisor the launch-resume estimate needs (impl 0038, open * question 3): the layout is known from persisted tab state before any pane * mounts, so a session returning into a 2–3 pane tab can be estimated at its * own width rather than the tab's. diff --git a/src/lib/terminalSizing.test.ts b/src/lib/terminalSizing.test.ts index 6c337de..50bd70f 100644 --- a/src/lib/terminalSizing.test.ts +++ b/src/lib/terminalSizing.test.ts @@ -33,9 +33,11 @@ beforeAll(async () => { )); }); -/** jsdom gives every element a zero rect; give
a real one. */ +/** jsdom gives every element a zero rect; give AppShell's
a real one. + * The attribute matches SHELL_MAIN_ATTR — shellContentBox targets it + * specifically (#371). */ function mountShell(width: number, height: number): void { - document.body.innerHTML = "
"; + document.body.innerHTML = "
"; const main = document.querySelector("main")!; main.getBoundingClientRect = () => ({ width, height }) as DOMRect; } @@ -92,7 +94,7 @@ describe("pickRespawnDims", () => { }); }); -// Surface chrome arithmetic (impl 0036). These feed the launch-resume +// Surface chrome arithmetic (impl 0038). These feed the launch-resume // estimate directly, so a stale constant here forks the PTY at the wrong // size — the failure #363 is about. Asserted on the pixel boxes rather than // through the grid helpers, which would need a real xterm fit. @@ -125,9 +127,9 @@ describe("pane area boxes", () => { it("clamps a dragged side panel to its range", () => { localStorage.setItem("runner.chat.panel.width", "9000"); - expect(chatPaneAreaBox().width).toBe(1440 - 480); + expect(chatPaneAreaBox()!.width).toBe(1440 - 480); localStorage.setItem("runner.chat.panel.width", "10"); - expect(chatPaneAreaBox().width).toBe(1440 - 200); + expect(chatPaneAreaBox()!.width).toBe(1440 - 200); }); it("takes the mission topbar, tab strip, and rail out of the shell box", () => { @@ -138,11 +140,32 @@ describe("pane area boxes", () => { }); }); - it("falls back to the window when the shell has not laid out", () => { + // #371: a failed
measurement must abstain (null), never fall back + // to window.innerWidth — the window includes the sidebar, so that + // fallback over-estimated by ~260px and beat the persisted rung with a + // confidently wrong value. + it("abstains when the shell has not laid out", () => { document.body.innerHTML = ""; - expect(chatPaneAreaBox()).toEqual({ - width: window.innerWidth - 320, - height: window.innerHeight - 44, - }); + expect(chatPaneAreaBox()).toBeNull(); + expect(missionPaneAreaBox()).toBeNull(); + }); + + it("abstains when AppShell's main is hidden (zero rect)", () => { + // The Settings takeover puts AppShell's
at display:none; jsdom's + // default zero rect models exactly that. + document.body.innerHTML = "
"; + expect(chatPaneAreaBox()).toBeNull(); + expect(missionPaneAreaBox()).toBeNull(); + }); + + it("ignores a foreign
even when it comes first in document order", () => { + // SettingsPage renders its own
(#371); a bare + // querySelector("main") would grab it. The measurement must key on + // AppShell's data-shell-main attribute instead. + document.body.innerHTML = "
"; + const [decoy, shell] = Array.from(document.querySelectorAll("main")); + decoy.getBoundingClientRect = () => ({ width: 999, height: 999 }) as DOMRect; + shell.getBoundingClientRect = () => ({ width: 1440, height: 900 }) as DOMRect; + expect(chatPaneAreaBox()).toEqual({ width: 1440 - 320, height: 900 - 44 }); }); }); diff --git a/src/lib/terminalSizing.ts b/src/lib/terminalSizing.ts index b2b6275..f71e479 100644 --- a/src/lib/terminalSizing.ts +++ b/src/lib/terminalSizing.ts @@ -22,6 +22,14 @@ export const TERMINAL_SCROLLBAR_WIDTH_PX = 8; * Must match the literal in that component's JSX. */ export const TERMINAL_HOST_SESSION_ATTR = "data-terminal-session"; +/** Attribute AppShell stamps on its `
` — the box every + * terminal-bearing surface renders into. `shellContentBox` queries for it + * specifically because the document can hold more than one `
`: + * SettingsPage renders its own while AppShell's sits display:none under + * the takeover, and a bare `querySelector("main")` can grab the zero-rect + * one (#371). Must match the literal in AppShell's JSX. */ +export const SHELL_MAIN_ATTR = "data-shell-main"; + // Mission-surface chrome above the pane container, mirroring // MissionWorkspace: an `h-11` topbar and an `h-[38px]` slot-tab strip. Both // are border-box, so each value is the full cost. @@ -141,14 +149,16 @@ export function pickRespawnDims(sources: { /** The shell's
box — the area every terminal-bearing surface renders * into. Mounted from boot, so this reads true even with no chat or mission - * surface on screen. */ -function shellContentBox(): { width: number; height: number } { - const main = document.querySelector("main"); + * surface on screen. Null when it can't be measured (not mounted yet, or + * display:none under the Settings takeover): a failed measurement must make + * the estimate rung abstain so `launchDimsFor` falls through to the + * persisted `last_cols` rung — `window.innerWidth` includes the sidebar and + * over-estimates by its width (#371). */ +export function shellContentBox(): { width: number; height: number } | null { + const main = document.querySelector(`main[${SHELL_MAIN_ATTR}]`); const rect = main?.getBoundingClientRect(); - return { - width: rect && rect.width > 0 ? rect.width : window.innerWidth, - height: rect && rect.height > 0 ? rect.height : window.innerHeight, - }; + if (!rect || rect.width <= 0 || rect.height <= 0) return null; + return { width: rect.width, height: rect.height }; } /** Width a collapsible side surface (mission rail, chat side panel) is taking @@ -175,10 +185,12 @@ function storedSideWidth( * Pixel box the mission workspace's pane container occupies right now: *
minus the topbar, the slot-tab strip, and the runners rail. The * workspace shows one slot terminal at a time, so unlike the chat surface - * there is no split to divide this by. + * there is no split to divide this by. Null when
is unmeasurable. */ -export function missionPaneAreaBox(): { width: number; height: number } { - const { width, height } = shellContentBox(); +export function missionPaneAreaBox(): { width: number; height: number } | null { + const shell = shellContentBox(); + if (!shell) return null; + const { width, height } = shell; const railWidth = storedSideWidth( "runner.mission.rail.open", "runner.mission.rail.width", @@ -196,8 +208,8 @@ export function missionPaneAreaBox(): { width: number; height: number } { } export function estimateMissionTerminalGrid(): TerminalGridSize | null { - const { width, height } = missionPaneAreaBox(); - return terminalGridFromPixels(width, height); + const box = missionPaneAreaBox(); + return box ? terminalGridFromPixels(box.width, box.height) : null; } /** @@ -205,10 +217,13 @@ export function estimateMissionTerminalGrid(): TerminalGridSize | null { * chat topbar and the side panel. This is the area ChatPaneGroup divides * between panes, so a session's own box is this run through * `paneBoxForSession` (paneLayout.ts) — the split divisor is not folded in - * here, because the same area serves every pane of a tab. + * here, because the same area serves every pane of a tab. Null when
+ * is unmeasurable. */ -export function chatPaneAreaBox(): { width: number; height: number } { - const { width, height } = shellContentBox(); +export function chatPaneAreaBox(): { width: number; height: number } | null { + const shell = shellContentBox(); + if (!shell) return null; + const { width, height } = shell; const panelWidth = storedSideWidth( "runner.chat.panel.open", "runner.chat.panel.width", diff --git a/src/lib/windowSettle.test.ts b/src/lib/windowSettle.test.ts index 07c2118..c14b20e 100644 --- a/src/lib/windowSettle.test.ts +++ b/src/lib/windowSettle.test.ts @@ -1,7 +1,7 @@ // The settle gate's contract: it resolves as soon as the webview viewport // agrees with the restored native frame, it never waits past the ceiling, and // it degrades to "proceed anyway" rather than hanging when there is nothing to -// compare against (impl 0036, phase 1). +// compare against (impl 0038, phase 1). import { describe, expect, it, vi } from "vitest"; diff --git a/src/lib/windowSettle.ts b/src/lib/windowSettle.ts index c0f587d..fdb8ada 100644 --- a/src/lib/windowSettle.ts +++ b/src/lib/windowSettle.ts @@ -1,4 +1,4 @@ -// Window-restore settle gate for launch auto-resume (impl 0036, decision 1). +// Window-restore settle gate for launch auto-resume (impl 0038, decision 1). // // `window_state::restore` (impl 0027) applies the saved frame during Tauri // `setup`, before the webview evaluates any script, and `app_ready` reveals diff --git a/src/pages/MissionWorkspace.tsx b/src/pages/MissionWorkspace.tsx index c881e1c..5754a91 100644 --- a/src/pages/MissionWorkspace.tsx +++ b/src/pages/MissionWorkspace.tsx @@ -595,27 +595,37 @@ export default function MissionWorkspace({ // probe reads the current rect, and a hidden terminal's cached // dims — stale after any rail/sidebar/window width change, which // would re-arm the ring purge — are the last resort only. - const measureSlotDims = useCallback( - (): TerminalGridSize | null => - pickRespawnDims({ - measureActiveSlot: () => - activeTab !== "feed" - ? (terminalsRef.current.get(activeTab)?.measure() ?? null) - : null, - probeContainer: () => - paneContainerRef.current - ? workspaceDimsFromContainer(paneContainerRef.current) - : null, - readHiddenCache: () => { - for (const s of sessions) { - const d = terminalsRef.current.get(s.id)?.measure(); - if (d) return d; - } - return null; - }, - }), - [activeTab, sessions], - ); + const measureSlotDims = useCallback((): TerminalGridSize | null => { + const dims = pickRespawnDims({ + measureActiveSlot: () => + activeTab !== "feed" + ? (terminalsRef.current.get(activeTab)?.measure() ?? null) + : null, + probeContainer: () => + paneContainerRef.current + ? workspaceDimsFromContainer(paneContainerRef.current) + : null, + readHiddenCache: () => { + for (const s of sessions) { + const d = terminalsRef.current.get(s.id)?.measure(); + if (d) return d; + } + return null; + }, + }); + // A real workspace measurement is the freshest grid hint a + // backend-initiated mission start can get (#367). + if (dims) void api.mission.setGridHint(dims).catch(() => {}); + return dims; + }, [activeTab, sessions]); + + // Same hint, driven by the terminals themselves: every real geometry + // push from a slot pane (window/rail/sidebar/zoom resize, tab + // activation) refreshes it, so an MCP mission started long after + // launch forks at the current width, not the launch estimate (#367). + const pushSlotGridHint = useCallback((dims: TerminalGridSize) => { + void api.mission.setGridHint(dims).catch(() => {}); + }, []); // Reset = wipe the run, respawn slots, keep the mission row. Used // for testing — you get the same mission back with a clean event @@ -1225,6 +1235,7 @@ export default function MissionWorkspace({ if (handle) terminalsRef.current.set(s.id, handle); else terminalsRef.current.delete(s.id); }} + onSizePushed={pushSlotGridHint} /> )) @@ -1375,6 +1386,7 @@ function SlotPtyPane({ onArchiveMission, hiddenByDisplayNone, registerTerminal, + onSizePushed, }: { session: SessionRow; deliveryBlockedUnreadCount: number | null; @@ -1402,6 +1414,9 @@ function SlotPtyPane({ /** Hand the parent a handle to this slot's xterm so it can measure * cols/rows before the resume RPC and avoid the 80×24 default. */ registerTerminal: (handle: RunnerTerminalHandle | null) => void; + /** Forwarded to RunnerTerminal: real geometry pushes refresh the + * backend's mission grid hint (#367). */ + onSizePushed: (size: TerminalGridSize) => void; }) { const resuming = !!forcedResuming; const dead = session.status !== "running"; @@ -1549,6 +1564,7 @@ function SlotPtyPane({ hiddenByDisplayNone={hiddenByDisplayNone} disabled={dead || resuming || starting} resizeDisabled={resuming || starting} + onSizePushed={onSizePushed} />
{!dead && deliveryBlockedUnreadCount !== null ? (