diff --git a/docs/plans/2026-05-10-sandbox-capabilities-follow-on-plan.md b/docs/plans/2026-05-10-sandbox-capabilities-follow-on-plan.md new file mode 100644 index 000000000..14a345214 --- /dev/null +++ b/docs/plans/2026-05-10-sandbox-capabilities-follow-on-plan.md @@ -0,0 +1,202 @@ +# Sandbox Capabilities Follow-On Plan + +## Summary + +After the `SandboxDetails` tab lands, add the interactive sandbox capabilities that are useful without OTLP: terminal, file browser, and browser-controlled VNC. + +This is a follow-on PR plan. It deliberately excludes logs, traces, and metrics. It also excludes Daytona Dashboard embedding; Fabro should own the UI and call provider APIs through the Fabro server. + +The current `Sandbox` tab already combines sandbox details and terminal access. It uses a two-column layout: the left column shows sandbox information, and the right column currently always shows the terminal. This follow-on changes the right column into a mode surface with a toggle for `Terminal`, `Filesystem`, and `VNC`. + +## Scope + +- Keep `Terminal` available inside the existing `Sandbox` tab, building on the existing `/runs/{id}/terminal` WebSocket route and xterm UI. +- Add a provider-neutral sandbox file browser as a right-column `Sandbox` tab mode that uses existing run-scoped sandbox file APIs. +- Add Daytona-backed VNC as a right-column `Sandbox` tab mode using a signed preview URL for the sandbox noVNC service. +- Keep provider credentials server-side only. +- Do not add OTLP plumbing, logs, traces, metrics, or provider dashboard iframes. + +## Sandbox Tab Layout + +The top-level run detail tabs should remain focused. Do not add new top-level `Terminal`, `Filesystem`, or `VNC` tabs for this work. The run page should keep a single sandbox workspace entry: + +- `Overview` +- `Stages` +- `Files Changed` +- `Sandbox` +- `Billing` + +The `Sandbox` tab requires a run sandbox. Inside the `Sandbox` tab: + +- Left column: persistent `SandboxDetails` panel. +- Right column: interactive workspace mode. +- Right-column modes: + - `Terminal` + - `Filesystem` + - `VNC` + +The right-column mode control should be a segmented control or tablist local to the `Sandbox` page, not top-level app navigation. `Terminal` should remain the default mode. + +Routing options: + +- Preferred simple route: store the active mode in query state, e.g. `/runs/:id/sandbox?mode=filesystem`. +- Acceptable alternative: nested sandbox routes, e.g. `/runs/:id/sandbox/filesystem`, if React Router integration is cleaner. + +Keep `/runs/:id/files` reserved for the existing diff-oriented `Files Changed` tab. + +## Terminal + +The embedded terminal already exists in the codebase: + +- Frontend: `apps/fabro-web/app/routes/run-terminal.tsx` +- Server: `GET /api/v1/runs/{id}/terminal` +- Sandbox adapter: `lib/crates/fabro-sandbox/src/terminal.rs` + +Follow-on work should move or adapt the existing terminal UI into the right column of `apps/fabro-web/app/routes/run-sandbox.tsx` unless implementation shows a cleaner shared-component extraction. + +- Preserve the existing terminal transport and WebSocket protocol. +- Keep the terminal gated by run sandbox presence through the containing `Sandbox` route. +- Align error/empty states with the new sandbox capability pages. +- Do not move terminal transport into OpenAPI; the WebSocket protocol remains hand-authored. +- Remove any now-redundant top-level `Terminal` tab/route only if the current implementation still exposes one after `Sandbox` absorbed it. + +## Filesystem + +Build a true sandbox filesystem browser in the right column of the `Sandbox` tab, separate from `Files Changed`. + +Existing server/API pieces: + +- `GET /api/v1/runs/{id}/sandbox/files` +- `GET /api/v1/runs/{id}/sandbox/file` +- `PUT /api/v1/runs/{id}/sandbox/file` +- Types: `SandboxFileEntry`, `SandboxFileListResponse` +- Server handler: `lib/crates/fabro-server/src/server/handler/sandbox.rs` + +Frontend work: + +- Prefer a right-column component such as `apps/fabro-web/app/routes/run-sandbox/filesystem-panel.tsx` or a nearby component under the existing sandbox route. +- Add generated-client query/mutation hooks in `apps/fabro-web/app/lib/queries.ts` or a focused route-local data layer if the interactions are too stateful for generic hooks. +- The filesystem mode itself can use a two-pane layout within the right column: + - Left side of the mode: directory tree/list with refresh and path navigation. + - Right side of the mode: file preview or empty state. +- First version capabilities: + - Browse directories. + - Preview text files with a size cap. + - Download files through the existing file endpoint. + - Upload/replace a file through the existing `PUT` endpoint. +- Defer destructive and complex editing actions unless needed immediately: + - Delete + - Rename/move + - chmod/chown + - search/replace + +Server/API follow-up if needed: + +- Add file metadata fields if the current `SandboxFileEntry` is too thin for the UI. +- Add explicit directory creation/delete/move endpoints only when the UI implements those actions. +- Keep path validation and provider access server-side. + +## VNC + +Add browser-controlled desktop access for Daytona sandboxes with VNC support. + +Documented Daytona path: + +- Start/manage VNC through Daytona Computer Use / VNC support. +- Expose browser noVNC through a signed Daytona preview URL. +- Use signed preview URLs for iframes because standard preview URLs require headers the browser iframe cannot set. + +Backend endpoint: + +- `POST /api/v1/runs/{id}/sandbox/vnc` + +Response: + +- `url: String` +- `expires_at: Option>` +- `provider: String` +- `port: u16` + +Behavior: + +- Load the run sandbox record. +- Only Daytona is supported in the first VNC PR. +- Reconnect to the Daytona sandbox. +- Start or verify Computer Use / VNC processes. +- Generate a signed preview URL for the configured noVNC port. +- Return `409` if the sandbox is not ready or VNC startup fails. +- Return `501` for Docker/local until there is a concrete provider-backed VNC story. + +Configuration: + +- Add a server/run setting for the Daytona noVNC port only if the port is not already stable in our images. +- Default to the image contract used by Fabro's VNC-capable snapshots. +- Keep the value provider-specific; do not pretend all providers have the same VNC port. + +Frontend route: + +- Add a right-column VNC component near the existing sandbox route, e.g. `apps/fabro-web/app/routes/run-sandbox/vnc-panel.tsx`. +- When the VNC mode is selected, call the VNC endpoint and render an iframe: + - `src` is the signed preview URL. + - `allow` includes clipboard and fullscreen permissions. + - Provide refresh/reconnect action. + - Show unsupported-provider, startup-failed, and expired-link states. +- Do not expose Daytona API keys, preview tokens, or provider connection metadata outside the signed URL. + +Security notes: + +- Treat the VNC iframe as remote desktop control of the sandbox. +- Keep it same run-auth gated as terminal and filesystem. +- Use signed preview URLs with short TTLs. +- Avoid embedding arbitrary provider dashboard URLs. + +## API And Type Placement + +- Add OpenAPI schemas only for HTTP endpoints: + - VNC response/request types. + - Any new filesystem metadata or mutation types. +- Do not put WebSocket terminal protocol into OpenAPI. +- Put reusable product/API DTOs in `fabro-types` only when they become shared vocabulary. Route-local response types can remain generated API types if there is no internal semantic owner. +- Regenerate both Rust and TypeScript clients for any OpenAPI changes: + - `cargo build -p fabro-api` + - `cd lib/packages/fabro-api-client && bun run generate` + +## Test Plan + +- Terminal: + - Existing `run-terminal` tests continue to pass. + - Sandbox route tests confirm `Terminal` is the default right-column mode. + - If a top-level `Terminal` route/tab is removed, update or delete tests that asserted it as separate navigation. +- Filesystem server tests: + - Existing list/download/upload tests remain green. + - Add route tests for any new metadata or mutation endpoints. + - Missing sandbox and unsupported provider states render as API errors, not panics. +- Filesystem frontend tests: + - Selecting `Filesystem` switches only the right column and leaves sandbox details visible in the left column. + - Browse root directory and nested directory. + - Selecting a file requests file contents and renders a preview. + - Large/binary file states do not try to render unsafe content inline. + - Upload success refreshes the current directory. +- VNC server tests: + - Missing run/sandbox returns `404`. + - Docker/local VNC returns `501`. + - Daytona path starts or verifies VNC and requests a signed preview URL for the configured port. + - Daytona startup or preview failure returns `409`. +- VNC frontend tests: + - Selecting `VNC` switches only the right column and leaves sandbox details visible in the left column. + - Loading state while the signed URL is requested. + - Successful response renders an iframe with the returned URL. + - Unsupported and failure responses render actionable empty states. + - Refresh action requests a fresh signed URL. +- Manual acceptance: + - Daytona run with VNC-capable image: select the `VNC` mode inside the `Sandbox` tab, interact with desktop, type into a terminal/browser inside the desktop, refresh Fabro page, reconnect. + - Daytona run filesystem: browse `/workspace`, preview a text file, upload a file, verify it appears from the terminal. + - Confirm no Daytona secret appears in browser devtools except the intended short-lived signed preview URL. + +## Assumptions + +- Fabro-owned Daytona images already include VNC/noVNC support. +- The noVNC port is either stable by image contract or configurable before the VNC PR lands. +- The existing terminal implementation is the baseline; this follow-on PR should adapt its UI placement rather than replace its transport. +- Filesystem UI can ship incrementally with browse/preview/download/upload before destructive operations. +- Logs, traces, and metrics remain out of scope because we are avoiding OTLP plumbing. diff --git a/docs/plans/2026-05-10-sandbox-details-tab-plan.md b/docs/plans/2026-05-10-sandbox-details-tab-plan.md new file mode 100644 index 000000000..1bed5fefa --- /dev/null +++ b/docs/plans/2026-05-10-sandbox-details-tab-plan.md @@ -0,0 +1,161 @@ +# Sandbox Details Tab Plan + +## Summary + +Add a provider-neutral `SandboxDetails` API model and a new `Sandbox` tab on the run detail page before `Terminal`. + +The tab shows live sandbox identity, state, placement, image/snapshot, resources, labels, and timestamps for the run-owned sandbox. It does not add lifecycle controls, terminal/VNC/file-browser capabilities, metrics, traces, or logs. Those are separate follow-on work. + +## Scope + +- Add `GET /api/v1/runs/{id}/sandbox` returning `SandboxDetails`. +- Source live details from the sandbox provider where possible: + - Daytona: use the existing Daytona reconnect path and SDK sandbox data. + - Docker: inspect the managed container through Docker/Bollard. + - Local: return a minimal unsupported/local detail only if the run has a local sandbox record; otherwise keep missing sandbox as `404`. +- Add a `Sandbox` tab at `/runs/:id/sandbox`, gated by the same run sandbox presence check as `Terminal`. +- Show the data in a compact panel layout consistent with the run detail UI. +- Ignore lifecycle settings by design: no auto-stop, auto-archive, auto-delete rows. + +## API Model + +Create the canonical Rust model in `lib/crates/fabro-types/src/sandbox_details.rs` and re-export it from `lib/crates/fabro-types/src/lib.rs`. + +`SandboxDetails`: + +- `provider: String` +- `name: Option` +- `id: Option` +- `state: SandboxState` +- `native_state: Option` +- `region: Option` +- `image: Option` +- `resources: SandboxResources` +- `labels: BTreeMap` +- `timestamps: SandboxTimestamps` + +`SandboxResources`: + +- `cpu_cores: Option` +- `memory_bytes: Option` +- `disk_bytes: Option` + +`SandboxTimestamps`: + +- `created_at: Option>` +- `last_activity_at: Option>` + +`SandboxState` should be the UI/control-plane normalized state: + +- `unknown` +- `provisioning` +- `starting` +- `running` +- `stopping` +- `stopped` +- `paused` +- `deleting` +- `deleted` +- `archived` +- `restoring` +- `resizing` +- `error` + +Keep `native_state` so provider-specific truth is visible and debugging does not require expanding the normalized enum forever. + +## Provider Mapping + +Add a sandbox-details inspection function in `fabro-sandbox`, not on the existing `Sandbox` trait unless implementation proves the trait boundary is the simplest fit. A focused free function keeps this as control-plane inspection instead of execution behavior. + +Suggested location: + +- `lib/crates/fabro-sandbox/src/details.rs` +- exported from `lib/crates/fabro-sandbox/src/lib.rs` + +Suggested interface: + +- `sandbox_details(record: &SandboxRecord, daytona_api_key: Option, daytona_organization_id: Option, run_id: Option) -> Result` + +Provider behavior: + +- Daytona: + - Reuse `DaytonaSandbox::reconnect(...)` or a lower-level SDK client helper. + - Map Daytona `name` to `name`, UUID/id to `id` if available, `snapshot` to `image`, target/region to `region`, SDK resources to `resources`, SDK labels to `labels`, and SDK timestamps to `timestamps`. + - Normalize Daytona states into `SandboxState` and preserve the original state string in `native_state`. +- Docker: + - Reuse `DockerSandbox::reconnect(...)` and inspect the container with Bollard. + - Map Docker container name to `name`, container id to `id`, image to `image`, labels to `labels`, `created` to `timestamps.created_at`, and state status to `native_state`. + - Compute CPU cores from `HostConfig.cpu_quota / cpu_period` when present; otherwise leave null. + - Use memory limit when present and non-zero; leave disk null unless Docker inspect provides a reliable configured limit. + - Set `region` to null; the UI renders Docker as local. +- Local: + - Return `provider = "local"`, `state = "running"` or `unknown`, `name = null`, `id = null`, `region = null`, `image = null`, empty labels, null resources/timestamps. + +## Server Changes + +- Update `docs/public/api-reference/fabro-api.yaml`: + - Add `GET /api/v1/runs/{id}/sandbox`. + - Add schemas for `SandboxDetails`, `SandboxState`, `SandboxResources`, and `SandboxTimestamps`. +- Update `lib/crates/fabro-api/build.rs` with `with_replacement(...)` entries for the new `fabro-types` types. +- Add a type identity / JSON parity test under `lib/crates/fabro-api/tests/`, following `run_summary_round_trip.rs`. +- Add the handler to `lib/crates/fabro-server/src/server/handler/sandbox.rs`. +- Reuse `load_run_sandbox_record(...)` and current auth behavior from the existing sandbox routes. +- Return: + - `404` when the run or sandbox record is missing. + - `409` when the provider exists but inspection fails because the sandbox/container is gone or inaccessible. + - `501` only for a provider that has a sandbox record but no details implementation. + +## Frontend Changes + +- Regenerate `lib/packages/fabro-api-client`. +- Add the generated API surface to `apps/fabro-web/app/lib/api-client.ts` if it lands on a new generated API class, or use the existing class if the operation groups with `HumanInTheLoopApi`. +- Add a SWR hook in `apps/fabro-web/app/lib/queries.ts`, with a stable key in `apps/fabro-web/app/lib/query-keys.ts`. +- Add route: + - `apps/fabro-web/app/routes/run-sandbox.tsx` + - router entry in `apps/fabro-web/app/router.tsx`: `route("sandbox", RunSandbox)` +- Update `apps/fabro-web/app/routes/run-detail.tsx`: + - Add `{ name: "Sandbox", path: "/sandbox", requiresSandbox: true }`. + - Place it before `Terminal`. +- UI layout: + - Status strip: provider, normalized state, native state when different. + - Overview panel: name, id, region/local, image. + - Resources panel: CPU, memory, disk, with unavailable values rendered as muted em dashes. + - Labels panel: key/value rows; empty state when none. + - Timestamps panel: created at and last activity, nullable. +- Use existing run detail spacing, panel borders, text colors, and tab styles. Do not introduce a new design system primitive for this. + +## Test Plan + +- Rust unit tests: + - Daytona state normalization covers representative active, provisioning, stopped/archived, and error states. + - Docker state normalization covers `created`, `running`, `paused`, `restarting`, `removing`, `exited`, and `dead`. + - Docker resource conversion handles quota/period, missing quota, zero memory, and configured memory. + - `SandboxDetails` serializes with snake_case fields and nullable optional fields. +- Server tests: + - Missing run returns `404`. + - Run without sandbox record returns `404`. + - Docker sandbox record calls Docker details adapter and returns normalized fields. + - Daytona sandbox record calls Daytona details adapter and returns normalized fields. + - Provider inspection failure returns `409` with a useful message. +- API tests: + - Add `fabro-api` replacement parity test proving generated `SandboxDetails` is the shared `fabro_types::SandboxDetails`. + - Run `cargo build -p fabro-api`. +- Frontend tests: + - `run-detail.test.ts` verifies the `Sandbox` tab appears for sandbox-backed runs, is before `Terminal`, and is hidden when no sandbox is present. + - `run-sandbox` route renders overview/resources/labels/timestamps and handles null fields without layout breakage. + - Query-key test covers the new sandbox details key. +- Verification: + - `cargo nextest run -p fabro-server` + - `cargo nextest run -p fabro-api` + - relevant `fabro-sandbox` tests + - `cd lib/packages/fabro-api-client && bun run generate` + - `cd apps/fabro-web && bun test && bun run typecheck` + +## Assumptions + +- `SandboxDetails` belongs in `fabro-types` because it is shared product vocabulary and should be reused by `fabro-api`. +- The existing persisted `SandboxRecord` remains the minimal reconnect record; do not overload it with live provider details. +- Docker disk size is nullable until there is a reliable configured/container-specific limit. +- `native_state` is for display/debugging only; UI behavior keys off normalized `state`. +- Lifecycle settings and actions are intentionally out of scope for this PR. + diff --git a/docs/plans/run-projection-simplification.md b/docs/plans/run-projection-simplification.md new file mode 100644 index 000000000..3fb958338 --- /dev/null +++ b/docs/plans/run-projection-simplification.md @@ -0,0 +1,105 @@ +# Canonical RunProjection Simplification + +## Summary + +Refactor `RunProjection` into a stricter canonical run-state type with no compatibility shims. Because there are no production deployments, make the breaking JSON/API changes directly and remove old field names, tuple encodings, default-empty projection states, and generated/client traces. + +The end state: a `RunProjection` always represents a real run initialized from `run.created`; it has required `spec`, `status`, `status_updated_at`, and `last_event_at`; checkpoint history is named records; graph source lives in `RunSpec`; terminal diff lives in `Conclusion`; checkpoint diff lives on checkpoint records. + +## Key Type And API Changes + +- Change `RunProjection`: + - `spec: RunSpec`, not `Option`. + - `status: RunStatus`, not `Option`. + - `status_updated_at: DateTime` and `last_event_at: DateTime`, not optional. + - Remove top-level `graph_source`, `checkpoint`, `final_patch`, and `diff_summary`. + - Keep `start`, `sandbox`, `pending_control`, `conclusion`, `pull_request`, and `superseded_by` optional because those are genuinely conditional lifecycle facts. +- Change `RunSpec`: + - Add `graph_source: Option`. + - Keep existing optional git/provenance/blob/fork fields as optional. +- Change `StartRecord`: + - Remove redundant `run_id`; keep `start_time`, `run_branch`, `base_sha`. +- Add canonical types: + - `CheckpointRecord { seq: u32, checkpoint: Checkpoint, diff: RunDiff }`. + - `RunDiff { patch: Option, summary: Option }`. +- Change `RunProjection.checkpoints`: + - From `Vec<(u32, Checkpoint)>` to `Vec`. + - Replace `current_checkpoint()` with `checkpoints.last().map(|record| &record.checkpoint)`. +- Change `Conclusion`: + - Add `diff: RunDiff`. + - Store terminal `final_patch` as `conclusion.diff.patch`. + - Store terminal `diff_summary` as `conclusion.diff.summary`. +- Change `PendingInterviewRecord.started_at` to required `DateTime`. +- Change `StageProjection.state` to required `StageState`; initialize new stage projections as `Running` unless immediately set to a terminal/skipped/retrying state. + +## Implementation Changes + +- Rework projection construction: + - Remove `Default` from `RunProjection`. + - Replace `RunProjection::default() + apply_event` with initialization from the first `run.created` event. + - `apply_events([])` should error at the reducer level; store/cache code may still return `None` when a run has no events. + - The first valid projection state is `Submitted`, with both timestamps set to the `run.created` timestamp. + - `run.submitted` only attaches `definition_blob`; it should not be needed to make the projection valid. +- Rework incremental projection caches: + - Store projection cache state as `Option` until `run.created` arrives. + - Applying any non-`run.created` event before initialization is an invalid event error. +- Update reducer mappings: + - `run.created.workflow_source` -> `projection.spec.graph_source`. + - `run.started` -> `StartRecord { start_time, run_branch, base_sha }`. + - `checkpoint.completed` -> push `CheckpointRecord { seq, checkpoint, diff }`. + - `run.completed` / `run.failed` -> set `conclusion.diff`. + - Checkpoint diff summaries should be sourced from `checkpoints.last().diff.summary`; terminal summaries from `conclusion.diff.summary`. +- Keep `RunSummary.diff_summary` as a summary/list convenience field, derived from terminal conclusion diff when present, otherwise latest checkpoint diff. Do not reintroduce a top-level projection diff field. +- Update OpenAPI and generated clients: + - Update `RunProjection`, `RunSpec`, `Conclusion`, `PendingInterviewRecord`, `StageProjection`. + - Add `CheckpointRecord` and `RunDiff`. + - Remove tuple checkpoint schema and generated `run-projection-checkpoints-inner-inner.ts`. + - Regenerate Rust API and TypeScript client after schema changes. + +## Cleanup: Leave No Trace + +- Remove all code references to: + - `RunProjection::default()`. + - `projection.graph_source`. + - `projection.checkpoint`. + - `projection.final_patch`. + - `projection.diff_summary`. + - `state.status.unwrap_or(...)`. + - tuple checkpoint destructuring like `(seq, checkpoint)`. +- Remove obsolete tests and snapshots that assert old raw projection JSON with `graph_source`, `checkpoint`, `final_patch`, nullable `spec`, nullable `status`, or tuple checkpoints. +- Remove compatibility aliases, legacy deserializers, serde aliases, and migration logic for the old projection shape. +- Remove stale OpenAPI schemas and generated TypeScript models produced solely by the old tuple/nullable shape. +- Update comments and docs that refer to `RunProjection.final_patch`; use `conclusion.diff.patch`. +- Update dump/export code so `run.json` uses the new canonical projection, graph source is read from `spec.graph_source`, and checkpoint dump entries iterate `CheckpointRecord`. + +## Test Plan + +- Rust type/API parity: + - Update `fabro-api` round-trip tests for `RunProjection`, `StageProjection`, `PendingInterviewRecord`, and add tests for `CheckpointRecord` and `RunDiff`. + - Ensure `fabro-api` generated types still reuse canonical `fabro_types` replacements. +- Reducer behavior: + - Projection initializes only from `run.created`. + - Empty/missing-created event sequences fail clearly. + - Required timestamps/status/spec are always present after initialization. + - Checkpoint history serializes as named records and `current_checkpoint()` derives from the last record. + - Terminal events populate `conclusion.diff`. + - Checkpoint events populate `CheckpointRecord.diff`. + - `RunSummary.diff_summary` derives from terminal diff first, latest checkpoint diff otherwise. +- Server/API behavior: + - `/api/v1/runs/{id}/state` returns the new non-null canonical shape. + - `/api/v1/runs/{id}/checkpoint` still returns latest checkpoint or null. + - start/resume checks use derived current checkpoint. + - file fallback uses `conclusion.diff.patch`. +- Frontend/CLI behavior: + - Update consumers of generated `RunProjection`. + - CLI inspect/dump/rewind/fork tests use derived current checkpoint and new diff location. + - Web run detail still shows diff summary via `RunSummary.diff_summary`. + +## Verification + +- Run `cargo build -p fabro-api` after OpenAPI changes. +- Regenerate TypeScript API client. +- Run focused tests for `fabro-types`, `fabro-store`, `fabro-api`, `fabro-server`, `fabro-cli`, and `apps/fabro-web`. +- Run `cargo nextest run --workspace`. +- Run `cd apps/fabro-web && bun test && bun run typecheck`. +- Finish with searches proving no old traces remain: `graph_source` only under `RunSpec`, no `RunProjection::default`, no `projection.checkpoint`, no `projection.final_patch`, no tuple checkpoint generated model.