Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/impls/0035-auto-resume-width-and-opt-in.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions src-tauri/src/commands/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions src-tauri/src/commands/mission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)?
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src-tauri/src/commands/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 14 additions & 1 deletion src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<windows::WindowRegistry>,
/// 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<Mutex<Option<(u16, u16)>>>,
}

#[cfg_attr(mobile, tauri::mobile_entry_point)]
Expand Down Expand Up @@ -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(),
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src-tauri/src/mcp/state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::{Arc, Mutex};

use tauri::AppHandle;

Expand All @@ -25,6 +25,7 @@ pub(crate) struct McpState {
pub routers: Arc<RouterRegistry>,
pub mcp: Arc<McpHandle>,
pub windows: Arc<WindowRegistry>,
pub mission_grid_hint: Arc<Mutex<Option<(u16, u16)>>>,
pub app_handle: AppHandle,
}

Expand All @@ -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),
}
}
}
5 changes: 4 additions & 1 deletion src-tauri/src/session/manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String>,
Expand Down
14 changes: 11 additions & 3 deletions src-tauri/src/session/manager/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
Loading
Loading