chore: plans

This commit is contained in:
Bryan Helmkamp 2026-05-10 10:24:07 -04:00
parent ee3ef3a3b2
commit e9461c0ce1
No known key found for this signature in database
3 changed files with 468 additions and 0 deletions

View file

@ -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<DateTime<Utc>>`
- `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.

View file

@ -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<String>`
- `id: Option<String>`
- `state: SandboxState`
- `native_state: Option<String>`
- `region: Option<String>`
- `image: Option<String>`
- `resources: SandboxResources`
- `labels: BTreeMap<String, String>`
- `timestamps: SandboxTimestamps`
`SandboxResources`:
- `cpu_cores: Option<f64>`
- `memory_bytes: Option<u64>`
- `disk_bytes: Option<u64>`
`SandboxTimestamps`:
- `created_at: Option<DateTime<Utc>>`
- `last_activity_at: Option<DateTime<Utc>>`
`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<String>, daytona_organization_id: Option<String>, run_id: Option<RunId>) -> Result<SandboxDetails>`
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.

View file

@ -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<RunSpec>`.
- `status: RunStatus`, not `Option<RunStatus>`.
- `status_updated_at: DateTime<Utc>` and `last_event_at: DateTime<Utc>`, 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<String>`.
- 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<String>, summary: Option<DiffSummary> }`.
- Change `RunProjection.checkpoints`:
- From `Vec<(u32, Checkpoint)>` to `Vec<CheckpointRecord>`.
- 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<Utc>`.
- 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<RunProjection>` 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.