From fa6d7e40075a4cdf48cf02da696960f51b7f1ca9 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 9 May 2026 07:34:18 -0400 Subject: [PATCH] chore: plans --- .../plans/2026-05-09-daytona-run-terminal.md | 73 ++++++++++++ .../2026-05-09-run-files-diff-scope-picker.md | 111 ++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-09-daytona-run-terminal.md create mode 100644 docs/superpowers/plans/2026-05-09-run-files-diff-scope-picker.md diff --git a/docs/superpowers/plans/2026-05-09-daytona-run-terminal.md b/docs/superpowers/plans/2026-05-09-daytona-run-terminal.md new file mode 100644 index 000000000..f54bf4333 --- /dev/null +++ b/docs/superpowers/plans/2026-05-09-daytona-run-terminal.md @@ -0,0 +1,73 @@ +# Run Terminal Plan + +## Summary + +Build an embedded run terminal for Daytona and Docker sandboxes. + +The run page gets a `Terminal` tab at `/runs/:id/terminal`. The browser uses `xterm.js`, connects to Fabro over WebSocket, and Fabro bridges bytes to a provider-specific terminal session for that run's sandbox. Daytona uses Daytona PTY. Docker uses an attached Docker exec with TTY. Keep the existing SSH access endpoint as a separate copyable external-access command for Daytona; do not build an SSH-over-WebSocket fallback. + +## Interfaces + +- Add WebSocket endpoint: `GET /api/v1/runs/{id}/terminal`. +- WebSocket auth uses existing Fabro web/session auth; reject invalid origins before upgrade. +- Browser protocol: + - Client binary message: raw PTY stdin bytes. + - Client text message: `{"type":"resize","cols":120,"rows":32}` or `{"type":"close"}`. + - Server binary message: raw PTY output bytes. + - Server text message: `{"type":"ready"}`, `{"type":"error","message":"..."}`, `{"type":"closed"}`. +- No OpenAPI/generated API client change for the WebSocket. Existing `POST /api/v1/runs/{id}/ssh` remains for "Copy SSH command". +- Add a small terminal-specific capability in `fabro-sandbox`, not full terminal support on the existing `Sandbox` trait: + - `TerminalSize { cols: u16, rows: u16 }` + - `TerminalSession` with `write_input`, `read_output`, `resize`, and `close` + - concrete `DaytonaTerminalSession` and `DockerTerminalSession` implementations + +## Key Changes + +- Backend: + - Enable Axum WebSocket support in `fabro-server`. + - Add a focused terminal handler that loads the run sandbox record, reconnects the concrete provider, starts/restores it, opens a terminal session, bridges browser input/output, handles resize, and closes/kills the terminal session when the browser disconnects. + - Add a Daytona PTY helper in `fabro-sandbox` that uses Daytona Toolbox APIs directly for create/connect/resize/kill because the pinned Rust SDK exposes PTY management but not a complete streaming handle. + - Add a Docker terminal helper in `fabro-sandbox` that creates an attached Docker exec with `tty=true`, `attach_stdin=true`, `attach_stdout=true`, `attach_stderr=true`, starts it attached, writes browser input into Bollard's exec input writer, forwards exec output to the browser, and calls `resize_exec` on resize. + - Docker disconnect cleanup should close the exec input/output and run the shell through a lightweight wrapper that records its PID so Fabro can terminate the shell process if the WebSocket drops. + - Use the run sandbox working directory as the PTY `cwd`; set `TERM=xterm-256color` and `LANG=C.UTF-8`. + - Keep provider credentials server-side only. Never send Daytona API keys, Daytona PTY URLs, Docker socket details, or provider connection handles to the browser. +- Frontend: + - Add `@xterm/xterm` and `@xterm/addon-fit`. + - Add `run-terminal.tsx`, mounted as `/runs/:id/terminal`, with full-height terminal layout. + - Add a `Terminal` tab when the run has a sandbox id. + - WebSocket URL uses `ws://` for `http://127.0.0.1` and `wss://` for HTTPS. + - Add header actions: reconnect terminal, copy existing SSH command when the provider supports SSH, and connection status. +- Behavior: + - Daytona and Docker are supported in v1. Local sandboxes show an unsupported-provider error. + - PTY sessions are not persistent in v1. Closing or refreshing the tab starts a fresh shell. + - Terminal input/output is not logged by Fabro. + +## Test Plan + +- Rust unit tests: + - WebSocket message parser accepts valid resize and rejects malformed/oversized control messages. + - Origin validation allows same-origin localhost and rejects cross-origin browser origins. + - Daytona PTY helper builds the expected Toolbox REST/WebSocket URLs and auth headers. + - Docker terminal helper creates exec options with TTY, stdin/stdout/stderr attached, workspace cwd, and terminal env. +- Server tests: + - Unauthenticated terminal WebSocket upgrade is rejected. + - Local or missing sandbox returns a clean unsupported/unavailable failure. + - Daytona runs use the Daytona terminal adapter; Docker runs use the Docker terminal adapter. + - On browser disconnect, the handler closes/kills the provider terminal session. + - Resize messages call the provider resize operation with the latest cols/rows. +- Web tests: + - Terminal tab appears for sandbox-backed runs. + - Route opens `ws://127.0.0.1:port/...` on local HTTP and `wss://` on HTTPS. + - Binary PTY output is written to xterm; keyboard input sends binary WebSocket messages. + - Unsupported/error/closed states render without crashing. +- Manual acceptance: + - Open Daytona-backed and Docker-backed runs, use `ls`, `pwd`, `vim`/`less`, Ctrl-C, resize the browser, refresh the tab, and confirm the old shell session is cleaned up. + - Confirm "Copy SSH command" appears for Daytona and is absent/disabled for Docker. + - Run `cargo nextest run -p fabro-server`, relevant `fabro-sandbox` tests, and `cd apps/fabro-web && bun test && bun run typecheck`. + +## Assumptions + +- Daytona PTY and Docker attached exec are the embedded-terminal transports for v1. +- `/api/v1/runs/{id}/ssh` stays as external access for local terminals and IDEs. +- WebSocket over plain `ws://127.0.0.1` is acceptable for local Fabro; hosted HTTPS deployments require `wss://`. +- Sources: Daytona PTY docs, Daytona SSH docs, Docker exec/Bollard APIs, and xterm.js docs. diff --git a/docs/superpowers/plans/2026-05-09-run-files-diff-scope-picker.md b/docs/superpowers/plans/2026-05-09-run-files-diff-scope-picker.md new file mode 100644 index 000000000..cc349a49d --- /dev/null +++ b/docs/superpowers/plans/2026-05-09-run-files-diff-scope-picker.md @@ -0,0 +1,111 @@ +# Run Files Diff Scope Picker Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a run-files diff scope picker for committed, uncommitted, and all sandbox changes. + +**Architecture:** Keep the existing files endpoint default behavior as committed changes. Add a `scope` query parameter for live working-tree scopes, and use patch-backed response construction for scopes that include uncommitted state. The frontend stores the selected scope in the URL and gives each scope an independent SWR cache key. + +**Tech Stack:** Rust, Axum, OpenAPI/progenitor, Bun, React, SWR, generated TypeScript Axios client. + +--- + +## Summary + +Add a picker to the run files page with three modes: + +- `Committed changes`: current behavior, comparing `base_sha..HEAD`. +- `Uncommitted changes`: staged, unstaged, and safe untracked changes in the live sandbox. +- `All changes`: committed plus uncommitted sandbox changes from `base_sha` to the current working tree. + +`Committed changes` remains the default. `Uncommitted changes` and `All changes` require a reachable live sandbox; stored `final_patch` fallback is only valid for committed changes. + +## API And Backend Changes + +- [ ] Add `scope` to `GET /api/v1/runs/{id}/files` in `docs/public/api-reference/fabro-api.yaml`. + - Accepted values: `committed`, `uncommitted`, `all`. + - Default: `committed`. + - Keep `from_sha` and `to_sha` reserved and rejected when present. +- [ ] Run `cargo build -p fabro-api` so progenitor regenerates Rust API types. +- [ ] Add a server-side scope enum in `lib/crates/fabro-server/src/run_files.rs`. + - `Committed` maps to the existing `base_sha..HEAD` materialization path. + - `Uncommitted` maps to a new live-only working-tree materialization path. + - `All` maps to the same live-only path with `base_sha` as the comparison base. +- [ ] Keep the existing committed path unchanged except for dispatching through the new scope enum. + - It may still reconnect/start the sandbox. + - It may still fall back to `RunProjection.final_patch` when live sandbox diffing cannot use the stored base. +- [ ] Implement a patch-backed working-tree path for `uncommitted` and `all`. + - For `uncommitted`, use sandbox git diff output equivalent to `git diff HEAD`. + - For `all`, use sandbox git diff output equivalent to `git diff `. + - Include staged and unstaged changes. + - Include safe untracked files by discovering untracked paths, validating repo-relative paths, applying sensitive filtering, enforcing size caps, and rendering them as added-file patch sections or placeholder entries. +- [ ] Reuse existing response semantics where possible. + - Return `PaginatedRunFileList`. + - Keep `FileDiff[]` shape. + - For patch-backed live working-tree scopes, use `unified_patch` with null file contents, matching the degraded renderer branch. + - Reuse existing sensitive, binary, symlink, submodule, truncation, file-count, and aggregate-byte behavior from the patch-section response code. +- [ ] Return `409 Conflict` for `scope=uncommitted` or `scope=all` when sandbox reconnect/start fails. + - Do not silently return committed fallback data under a live-only scope. + - Error detail: `Live sandbox access is required for this diff scope.` + +## Frontend Changes + +- [ ] Regenerate the TypeScript client. + - Run: `cd lib/packages/fabro-api-client && bun run generate`. +- [ ] Update `apps/fabro-web/app/lib/queries.ts`. + - Change `useRunFiles(id)` to `useRunFiles(id, scope)`. + - Include `scope` in the SWR key so each mode caches independently. + - Pass `scope` to `runOutputsApi.listRunFiles`. +- [ ] Update `apps/fabro-web/app/routes/run-files.tsx`. + - Parse `scope` from the page URL. + - Missing or invalid scope resolves to `committed`. + - Persist picker changes with `?scope=committed`, `?scope=uncommitted`, or `?scope=all`. + - Keep hash file deep links working with the existing `#file=...` format. +- [ ] Update `apps/fabro-web/app/routes/run-files/toolbar.tsx`. + - Add a compact segmented picker with labels: + - `All changes` + - `Uncommitted` + - `Committed` + - Keep the existing diff layout toggle and refresh button. + - Disable no mode by default; let the server decide whether live-only scopes are available. +- [ ] Show live-sandbox errors inline. + - If `scope` is `uncommitted` or `all` and the API returns `409`, show: `Live sandbox access is required for this diff.` + - Keep the selected scope visible. + - Do not route the error through the full-page initial error state when previous data exists. + +## Test Plan + +- [ ] Add server/API tests for committed default behavior. + - Request without `scope` still returns the same committed response as today. + - Request with `scope=committed` uses the same path. + - `from_sha` and `to_sha` are still rejected. +- [ ] Add server/API tests for fallback behavior. + - `scope=committed` falls back to `final_patch` when sandbox access is unavailable. + - `scope=uncommitted` returns `409` when sandbox access is unavailable. + - `scope=all` returns `409` when sandbox access is unavailable. +- [ ] Add server/API tests for working-tree scopes. + - `scope=uncommitted` includes staged changes. + - `scope=uncommitted` includes unstaged changes. + - `scope=uncommitted` includes safe untracked files. + - `scope=all` includes committed plus live working-tree changes. + - Sensitive untracked paths render as sensitive placeholders. + - Large untracked files render as truncated placeholders. +- [ ] Add frontend tests. + - Default picker state is `Committed`. + - Picker changes update the URL query string. + - `useRunFiles` calls the API with the selected scope. + - Each scope has a distinct cache key. + - `409` for live-only scopes renders the inline live-sandbox message. +- [ ] Run verification commands. + - `cargo build -p fabro-api` + - `cargo nextest run -p fabro-server run_files` + - `cd lib/packages/fabro-api-client && bun run generate` + - `cd apps/fabro-web && bun test` + - `cd apps/fabro-web && bun run typecheck` + +## Assumptions + +- Existing behavior should remain the default, so missing `scope` means `committed`. +- `Uncommitted changes` means staged, unstaged, and safe untracked files inside the run sandbox. +- Remote clone-based sandboxes cannot include submitter-side local dirty changes unless those changes were already present in the sandbox. +- Stored `final_patch` is only used for committed/final run changes.