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
61 changes: 42 additions & 19 deletions docs/features/55-paste-file-paths.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,45 @@ The web layer cannot close it alone. `DataTransfer` deliberately withholds files

## Key Decisions

1. **Read the paths in Rust, not in the webview.** A new command returns the current pasteboard's file URLs as POSIX paths, read from `NSPasteboard.generalPasteboard`. `commands/session.rs:144-165` already carries the NSPasteboard plumbing and `objc2-app-kit` is already a dependency, so this is an addition to established code rather than new machinery. The frontend never sees a path it could have fabricated from `File.name`.
2. **Branch inside the existing paste handler.** Extend `onPaste` (`RunnerTerminal.tsx:749`): after the image scan fails, if any item is `kind === "file"`, call the new command. On a non-empty result, `preventDefault` and insert; on an empty result, fall through untouched so today's behavior is preserved for any clipboard shape the native read doesn't recognize.
3. **Prefer the native read over `text/uri-list`.** WKWebView sometimes exposes a `text/uri-list` flavor with `file://` URLs, which would avoid an IPC round-trip, but its presence is inconsistent across source applications. One authoritative path (the pasteboard) beats two code paths that disagree.
4. **Quote for the shell, always.** Wrap each path in single quotes with embedded single quotes escaped (`'` → `'\''`). Unquoted paths break on spaces, which are common in real project trees. Quoting unconditionally keeps the rule simple and matches what iTerm2 does.
5. **Insert as text, never submit.** Write the quoted string through the existing raw-stdin injection. Do **not** route through `inject_paste`, which appends Enter — a pasted path is the middle of a sentence the user is still composing. This also means the draft-gate state (spec 54) sees it as ordinary local input, which is correct: the user now has a pending draft.
6. **No cwd-relative rewriting.** Absolute paths always resolve regardless of where the agent has `cd`'d, and the agent can shorten them itself. Relativizing would need the session's live cwd, which Runner only knows at spawn time.
1. **Read the paths in Rust, not in the webview.** A new command returns the current pasteboard's file URLs as POSIX paths. The frontend never sees a path it could have fabricated from `File.name`.

This is **new machinery**, contrary to an earlier draft of this spec. `commands/session.rs:144-165` is a MIME→OSType lookup table, not NSPasteboard plumbing: the image write shells out to `osascript -e "set the clipboard to (read POSIX file … as «class PNGf»)"` (`:225`), and the repo contains no NSPasteboard binding anywhere. `objc2-app-kit` is a dependency but is declared `default-features = false` with `["std", "NSButton", "NSControl", "NSResponder", "NSView", "NSWindow", "NSWorkspace"]`, and objc2 gates every class behind its own feature — so `NSPasteboard` does not compile today. The work is one Cargo feature plus a first direct binding. Take the binding rather than matching the `osascript` precedent: reads happen on every candidate paste, and a process spawn per paste is not worth the symmetry.

2. **Decide synchronously, act asynchronously.** `preventDefault()` after an `await` does nothing — the browser has already run the default action by the time an IPC round-trip resolves. So the handler cannot "call the command, then decide", which an earlier draft of this spec asked for. It must commit before it knows the answer, exactly as the image branch does (`RunnerTerminal.tsx:767`: decide from `clipboardData`, *then* go async).

Gate on **absence of usable text**, not on `kind === "file"`. If `clipboardData.getData("text/plain")` is non-empty, return immediately and let xterm paste it — that is every ordinary paste, unchanged and with no IPC. Otherwise `preventDefault` eagerly and consult the pasteboard; an empty result then swallows a paste that had nothing to insert anyway, which is a no-op.

This also removes the spec's riskiest assumption. Whether WKWebView exposes an arbitrary non-image file as `kind === "file"` is unverified; if it does not, a `kind`-based trigger never fires and the feature silently does nothing. Text-absence does not depend on that behavior.

3. **A file reference beats image bytes.** `inferPasteImageMime` falls back to the filename extension (`:100-103`) and WKWebView sets `DataTransferItem.type` from the file's UTI, so copying `shot.png` in Finder currently takes the image branch and attaches it — the path branch would never run. Native terminals paste the path.

Resolve by clipboard *flavor*, not MIME sniff: if the pasteboard carries `public.file-url`, the user copied a reference and gets a path; otherwise they copied content and get today's attach flow. This preserves [#79](https://github.com/yicheng47/runner/issues/79) exactly — screenshots and copies from Preview or a browser put bytes on the pasteboard with no file-url — and changes only the Finder/GoLand image-*file* case, which is the case the user is asking about. Requires reading the file-url flavor before the image scan.

4. **Prefer the native read over `text/uri-list`.** WKWebView sometimes exposes a `text/uri-list` flavor with `file://` URLs, which would avoid an IPC round-trip, but its presence is inconsistent across source applications. One authoritative path (the pasteboard) beats two code paths that disagree.

5. **Quote only when the path needs it.** iTerm2 quotes unconditionally because its panes are shell prompts. Runner's panes are usually *agent* prompts, where `'/Users/jason/foo.go'` is noise and defeats `@`-style file completion. Quote when the path contains whitespace or shell metacharacters, using single quotes with embedded quotes escaped (`'` → `'\''`); otherwise insert it bare.

6. **Insert as text, never submit.** Write through the existing raw-stdin injection (`api.session.injectStdin`). Do **not** route through `inject_paste`, which appends Enter — a pasted path is the middle of a sentence the user is still composing. This also means the draft-gate state (spec 54) sees it as ordinary local input, which is correct: the user now has a pending draft.

7. **No cwd-relative rewriting.** Absolute paths always resolve regardless of where the agent has `cd`'d, and the agent can shorten them itself. Relativizing would need the session's live cwd, which Runner only knows at spawn time.

## Implementation Phases

### Phase 1 — native pasteboard read

- Add a command (`commands/session.rs`, beside the image-paste helpers at `:144-165`) returning `Vec<String>` of POSIX paths for the pasteboard's file URLs, empty when the flavor is absent.
- Unit-test the quoting helper: plain path, path with spaces, path with an embedded single quote, multiple paths.
- Add the `NSPasteboard` feature to `objc2-app-kit` in `src-tauri/Cargo.toml` (decision 1).
- Add a command (`commands/session.rs`, beside the image-paste helpers at `:144-165`) returning `Vec<String>` of POSIX paths for the pasteboard's file URLs, empty when the flavor is absent. Non-macOS returns empty.
- Unit-test the quoting helper: plain path (unquoted), path with spaces, path with an embedded single quote, path with shell metacharacters, multiple paths.

### Phase 2 — frontend branch

- Extend `onPaste` (`RunnerTerminal.tsx:749-781`) per decision 2: file-kind items with no image mime → call the command → quote, join with spaces, `preventDefault` + `stopImmediatePropagation`, inject through `api.session.injectStdin`.
- Keep the existing early returns intact so a text paste never reaches the new branch.
- Component test: an image paste still takes the image path; a non-image file paste injects the quoted path; a text paste is untouched.
- Extend `onPaste` (`RunnerTerminal.tsx:749-781`) per decisions 2–3. Order matters:
1. Non-empty `text/plain` → return immediately, xterm handles it.
2. Otherwise `preventDefault` + `stopImmediatePropagation`, then call the pasteboard command.
3. Non-empty result → quote as needed, join with single spaces, inject through `api.session.injectStdin`.
4. Empty result → fall back to the existing image scan; if that also finds nothing, do nothing.
- Keep the `!sid || disabledRef.current` early return ahead of all of it.
- Component tests: text paste untouched and no IPC; image *bytes* paste still attaches; image *file* paste injects a path; non-image file paste injects a quoted path; empty pasteboard is a silent no-op.

### Phase 3 — validation

Expand All @@ -48,18 +68,21 @@ The web layer cannot close it alone. `DataTransfer` deliberately withholds files

## Verification

- [ ] Copy a `.go` file in GoLand, ⌘V over a terminal pane — the absolute path appears at the cursor, quoted, not submitted.
- [ ] Copy a file whose path contains a space — the pasted text is correctly quoted and the agent resolves it.
- [ ] Copy a `.go` file in GoLand, ⌘V over a terminal pane — the absolute path appears at the cursor, unquoted, not submitted.
- [ ] Copy a file whose path contains a space — the pasted text is quoted and the agent resolves it.
- [ ] Copy two files at once — both paths appear, space-separated.
- [ ] Copy a file in Finder — same result as GoLand.
- [ ] Copy an image — the existing `[Image #N]` attach flow is unchanged.
- [ ] Copy ordinary text — pastes exactly as before.
- [ ] Take a screenshot to the clipboard (⌘⇧4), paste — the existing `[Image #N]` attach flow is unchanged (#79).
- [ ] Copy an image *file* in Finder, paste — its **path** appears, not an attachment (decision 3, deliberate change).
- [ ] Copy ordinary text — pastes exactly as before, with no IPC round-trip.
- [ ] Paste with an empty clipboard — nothing happens, no error.
- [ ] Paste with the session stopped or the pane disabled — no injection, no error.
- [ ] The pasted path leaves the pane in a pending-draft state (spec 54), not a submitted one.

## Relevant Code

- `src/components/RunnerTerminal.tsx:749-783` — `onPaste`, the interception point; `:81-99` — `normalizePasteImageMime` / `inferPasteImageMime`, the mime filter that currently drops non-image files.
- `src-tauri/src/commands/session.rs:144-165` — existing NSPasteboard flavor plumbing for image paste.
- `src/lib/api.ts` — `session.pasteImage` / `session.injectStdin`, the call shapes to mirror.
- `docs/features/54-draft-aware-delivery-gate.md` — the draft model a pasted path feeds into (decision 5).
- `src/components/RunnerTerminal.tsx:749-781` — `onPaste`, the interception point; `:767` — the synchronous commit point the new branch must mirror; `:81-103` — `normalizePasteImageMime` / `inferPasteImageMime`, whose filename fallback creates the collision in decision 3.
- `src-tauri/src/commands/session.rs:143-160` — `paste_image_format`, a MIME→OSType table (**not** NSPasteboard plumbing); `:225` — the `osascript` shell-out that actually writes the pasteboard.
- `src-tauri/Cargo.toml:67` — `objc2-app-kit` with `default-features = false`; `NSPasteboard` must be added to its feature list.
- `src/lib/api.ts:396-397` — `session.injectStdin`; `:412` — `session.pasteImage`, the call shape to mirror.
- `docs/features/54-draft-aware-delivery-gate.md` — the draft model a pasted path feeds into (decision 6).
106 changes: 106 additions & 0 deletions docs/impls/0039-backend-initiated-spawn-width.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Backend-initiated spawn width

## Status

Planned. Tracking issue [#367](https://github.com/yicheng47/runner/issues/367).

## Problem

Spawns the frontend initiates carry a measured grid; spawns the backend initiates carry nothing and fall to `DEFAULT_PTY_SIZE` (`session/manager/mod.rs:52`, 80×24). Two paths reach that state today, both through MCP:

- `mission_start_impl` (`commands/mission.rs:488-494`) hardcodes `None` for `initial_size`. Its only caller is the MCP tool (`mcp/tools/mission.rs:436`); the UI's Start Mission modal goes through `mission_start_impl_with_size` (`:867`) with real dims.
- `session_start_direct` via MCP (`mcp/tools/session.rs:47-55`) passes `None, None` for `cols`/`rows`. `session_start_direct_impl` (`commands/session.rs:715-724`) accepts them; the UI supplies them, the MCP tool does not.

Forking at 80 also seeds `last_pty_cols = Some(80)`. When the pane is first opened and pushes its real width, the cols-gate fires (`session/manager/output.rs:538-546`):

```rust
let cols_changed = { … state.last_pty_cols != Some(cols) … };
if cols_changed && runtime_clears_on_resize(session_id, pool) {
self.purge_output_buffer_keep_modes(session_id);
}
```

`runtime_clears_on_resize` (`:796`) covers claude-code, codex, qoder, and trae — every agent runtime. So the entire transcript the agent produced before its tab was first visited is discarded, and snapshot replay has nothing to restore. The longer the session ran unobserved, the more is lost.

The tree already documents this failure, but only as fixed for the *respawn* paths — `commands/mission.rs:1413-1416` and `MissionWorkspace.tsx:629-630` describe exactly this mechanism. Initial spawn via MCP was left behind.

This is not [#366](https://github.com/yicheng47/runner/issues/366)/[#363](https://github.com/yicheng47/runner/issues/363). Those are about the *estimate* feeding launch auto-resume, where the resulting width is wrong. Here the width ends up correct and the history is gone. Shared mechanism, different cause, different fix site.

### Why an estimate cannot fix this

The gate is exact equality on cols. An estimate that lands one column off purges exactly as thoroughly as no estimate at all. There is no partial credit, so a computed approximation buys nothing — it must match the destination pane exactly or it is wasted work.

That rules out deriving the grid backend-side from window geometry (the issue's option 2). It would mean a second copy of the chrome constants in Rust — the duplication that already bites `runtimeClearsOnResize` (`output.rs:770` / `RunnerTerminal.tsx:168`) — in exchange for a number that is *almost* right, which is worth nothing here.

The only source of an exactly-correct destination width is a real measurement of a real pane. So: reuse the last one.

## Key Decisions

1. **Cache a measured grid; never compute one.** The frontend writes the mission pane grid and the chat pane grid to the backend as it measures them. Backend-initiated spawns read the cache when the caller supplied no size. Rust gains no knowledge of chrome constants, header heights, or panel widths.
2. **Persist it in `_app_state`, not in memory.** Follow the established shape in `db.rs`: private `app_state_get`/`app_state_set` plus a typed `pub fn` pair per key, exactly as `login_shell_env_lkg` (`:218-232`) and `runtime_overrides` (`:234-260`) do. Persisting is what covers the first MCP-started mission after a cold launch — an in-memory cache would be empty precisely when the app hasn't rendered a pane yet.
3. **Two keys, because they are two different boxes.** `missionPaneAreaBox` (`terminalSizing.ts:180`) subtracts the runners rail, the topbar, and the slot-tab strip. `chatPaneAreaBox` (`:210`) subtracts the side panel and is then divided by `paneBoxForSession`. A mission spawn must not read a chat measurement. Store `mission` and `chat` grids separately and have each spawn path read its own.
4. **Write from where the grid is already known.** The measurement is whatever the pane pushes to `session.resize` — same value, same moment. Do not add a separate measuring pass; do not measure on a timer.
5. **No cached grid means 80×24, unchanged.** A fresh install whose first action is an MCP `mission_start` still loses that first session's early scrollback. That is the honest residual: guessing would not avoid the purge (see above), and it self-heals as soon as any pane has rendered once. Do not add a fallback estimate to paper over it.
6. **Do not touch the purge gate.** It is correct given genuinely-80-col bytes: replaying absolute-positioned 80-col frames into a wide grid is the box-drawing shredding impl 0020 / [#306](https://github.com/yicheng47/runner/issues/306) removed. This fix removes the cause, not the safeguard.
7. **Optional explicit dims on the MCP tools are a non-goal.** A caller that knew the geometry could pass it (the issue's option 3), but MCP callers are agents with no view of the window, so the parameter would sit unused while widening the tool surface.

## Open questions

- **Staleness.** The cache is not invalidated when the window resizes with no pane mounted, so a spawn can read a grid that no longer matches. The cost of being wrong is exactly today's behavior — one purge — so this is not worth a watcher. Confirm the reasoning holds rather than adding invalidation.
- **Rows.** The gate is cols-only, so a stale rows value costs nothing. Both are stored because the spawn takes a pair; no accuracy claim is made about rows.
- **Multi-window** (impl 0018). Last writer wins across windows. Acceptable while windows are near-identical; note it rather than solve it.
- **Whether the mission grid should be per-project.** Different projects may sit at different rail widths. Probably not worth it — the rail width is global. Confirm.

## Goals

- A mission started through MCP and left unopened for several minutes shows its full transcript when its slot tab is first visited.
- A direct chat started through MCP behaves the same.
- Sessions started from the UI are byte-for-byte unaffected.

## Non-Goals

- Changing the cols-gate purge, `runtime_clears_on_resize`, or snapshot replay.
- Anything in #363/#366/impl 0038's launch auto-resume estimate.
- Recovering scrollback already purged by shipped versions.
- Adding `cols`/`rows` parameters to the MCP tool schemas.

## Implementation Phases

### Phase 1 — persisted pane-grid cache

- Add `_app_state` keys for the last measured mission grid and chat grid, with typed getters/setters in `db.rs` mirroring `login_shell_env_lkg`.
- Add a command the frontend calls with `{ surface, cols, rows }`.
- Unit-test the round trip, including the absent-key case returning `None`.

### Phase 2 — write side

- Have the mission workspace and the chat surface record their measured grid alongside the resize they already push.
- Do not introduce a new measurement path; reuse the value being sent to `session.resize`.

### Phase 3 — read side

- `mission_start_impl`: read the cached mission grid and pass it as `initial_size` instead of the hardcoded `None`.
- MCP `session_start_direct`: read the cached chat grid and pass it as `cols`/`rows`.
- Both fall back to today's behavior when the cache is empty.
- Test that an explicit caller-supplied size still wins over the cache.

### Phase 4 — validation

- `cargo fmt --check`, `cargo clippy --workspace`, `cargo test --workspace`, `pnpm exec tsc --noEmit`, `pnpm run lint`, `pnpm test`.

## Verification

Automated:

- [ ] Cache round-trips through `_app_state` and survives a pool reopen.
- [ ] Absent key yields `None`; spawn then uses `DEFAULT_PTY_SIZE`.
- [ ] `mission_start_impl` with a cached grid forks at that grid; with an explicit size the explicit value wins.
- [ ] Mission and chat caches do not read each other.
- [ ] A resize to the same cols the session forked at does not purge (the gate's no-op case is what this fix relies on).

Manual (Jason smoke-tests):

- [ ] Open a mission workspace once so a grid is cached. Start a mission through MCP, leave the slot tab closed for a few minutes, then open it — full transcript present, no truncation at the top.
- [ ] Same for an MCP-started direct chat.
- [ ] Start a mission from the UI modal — unchanged.
- [ ] Quit, relaunch, immediately start an MCP mission without opening any pane — still forks at the persisted grid.
Loading
Loading