From de294c0b9c0c0d6301244ef2888373ba635aa60b Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 27 Aug 2026 13:39:34 -0500 Subject: [PATCH 1/3] feat: track latest OpenShell SDK and wire new v0.0.113 API surface Bump github.com/NVIDIA/OpenShell/sdk/go to latest (v0.0.115 upstream) and adopt the additive API surface that shipped since the pinned Aug-14 build. - Sandbox stop/start (StopSandbox/StartSandbox) with STOPPING/STOPPED/ STARTING phases: BFF handlers + routes, frontend actions on the sandbox list and detail pages, PhaseLabel handling. - SandboxStatus.exitCode surfaced in the detail view (aids ERROR triage). - Draft-policy review_token: resolved server-side on approve (now required by ApproveDraftChunk) and exposed on chunks alongside applicationError and current/candidate effective policies for a future before/after diff. Also refresh the stale "no stop/start" docs (openshell-api rule, CLAUDE.md, CONTRIBUTING, README, create-sandbox modal copy). All green: go build/vet/test, tsc, eslint, jest (178 passing). Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Gage Krumbach --- .claude/rules/openshell-api.md | 4 +- CLAUDE.md | 2 +- CONTRIBUTING.md | 2 +- README.md | 2 +- backend/go.mod | 4 +- backend/go.sum | 4 +- backend/internal/api/app.go | 2 + backend/internal/api/drafts_handler.go | 33 +++++++++++++- backend/internal/api/mock_sdk_test.go | 6 +-- backend/internal/api/sandboxes_handler.go | 23 ++++++++++ backend/internal/models/models.go | 4 ++ backend/internal/models/observability.go | 15 +++++++ backend/internal/models/sdk_converters.go | 12 +++++ frontend/src/api/client.ts | 7 ++- frontend/src/api/sandboxes.ts | 34 ++++++++++++++ .../src/components/CreateSandboxModal.tsx | 2 +- frontend/src/components/PhaseLabel.tsx | 13 +++++- .../components/sandbox/SandboxTableRow.tsx | 10 +++++ frontend/src/pages/SandboxDetailPage.tsx | 44 ++++++++++++++++++- frontend/src/pages/SandboxListPage.tsx | 17 ++++++- .../pages/__tests__/SandboxListPage.spec.tsx | 2 + frontend/src/types/index.ts | 29 ++++++++++-- 22 files changed, 248 insertions(+), 23 deletions(-) diff --git a/.claude/rules/openshell-api.md b/.claude/rules/openshell-api.md index fc828d92..9089cdf0 100644 --- a/.claude/rules/openshell-api.md +++ b/.claude/rules/openshell-api.md @@ -12,14 +12,14 @@ alwaysApply: false | Service | Proto | What we use | |---------|-------|-------------| -| `openshell.v1.OpenShell` (64 RPCs) | openshell.proto | Sandbox, workspace, member, provider, profile, policy, draft policy, logs, SSH, services | +| `openshell.v1.OpenShell` (68 RPCs) | openshell.proto | Sandbox (incl. stop/start), workspace, member, provider, profile, policy, draft policy, logs, SSH, services | | `openshell.inference.v1.Inference` (4 RPCs) | inference.proto | Inference route CRUD | Skip `GatewayInterceptor`, `SupervisorMiddleware`, `ComputeDriver` — internal/operator only. ## Hard facts — do not violate these -1. **No sandbox stop/start/suspend/restart.** `SandboxPhase` = UNSPECIFIED, PROVISIONING, READY, ERROR, DELETING, UNKNOWN. Lifecycle: Create → Ready/Error → Delete. Never generate stop/start endpoints, buttons, or "Suspended" states. +1. **Sandbox stop/start exists (as of v0.0.113); no suspend/restart.** `StopSandbox` retains persistent state; `StartSandbox` resumes. `SandboxPhase` = UNSPECIFIED, PROVISIONING, READY, ERROR, DELETING, STOPPING, STOPPED, STARTING, UNKNOWN. Lifecycle: Create → Ready/Error → (Stop ⇄ Start) → Delete. There is still no suspend/restart RPC and no "Suspended" state — do not invent those. 2. **No workspace-scoped named-policy resource.** Policy exists as: (a) `SandboxSpec.policy` — **required** on `CreateSandbox`, then versioned revisions per sandbox; (b) gateway-global via `UpdateConfig(global=true)`. No CreatePolicy/DeletePolicy/ListPolicies-by-workspace RPCs exist. 3. **Sandbox-scoped `UpdateConfig` may only change `network_policies` and inference fields.** filesystem/landlock/process are immutable after create — render read-only. 4. **No OCSF events API.** Observability = `GetSandboxLogs` (structured `fields` map on log lines) and `WatchSandbox` platform events. Never build an events query endpoint. diff --git a/CLAUDE.md b/CLAUDE.md index e96b04d8..6c564167 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ Each rule has a corresponding Architecture Decision Record in [`docs/adrs/`](doc ## OpenShell API reference -The gateway exposes 68 gRPC RPCs across 2 services (64 in `OpenShell`, 4 in `Inference`). Proto files are in `backend/proto/`. +The gateway exposes 72 gRPC RPCs across 2 services (68 in `OpenShell`, 4 in `Inference`). Proto files are in `backend/proto/`. ## Personas diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ef09125..05565203 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -77,7 +77,7 @@ This is a legal requirement for Apache 2.0 licensed projects. CI will reject uns **The proto files are the source of truth** (see `.claude/rules/openshell-api.md`). Do not invent RPCs, fields, or lifecycle states. If a UI concept has no backing RPC, open an issue — do not fabricate an endpoint. Common mistakes to avoid: -- No sandbox stop/start/suspend (lifecycle is create → ready/error → delete) +- Sandbox stop/start exists (v0.0.113+); still no suspend/restart (lifecycle is create → ready/error → stop ⇄ start → delete) - No workspace-level policy CRUD (policy is per-sandbox or gateway-global) - No events API (use GetSandboxLogs + polling) - Provider credentials are write-only — never return secrets to the frontend diff --git a/README.md b/README.md index df8c5199..176a7e21 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ Browser ── REST ──► Go BFF ── gRPC (bearer) ──► OpenShell ga - **Proto is source of truth.** `backend/proto/` is copied from `NVIDIA/OpenShell/proto/`; `make proto` regenerates `backend/gen/`. Wrappers in `backend/internal/gateway/` cover the Phase 1 user-facing RPCs only. - **Polling for status**: sandbox state uses polling (5s via React Query `refetchInterval`). WebSockets are used only for the interactive terminal. - **Secrets never reach the browser**: provider credentials are write-only; the BFF serializes only credential key names. -- **No sandbox stop/start**: the OpenShell lifecycle is create → ready/error → delete. The UI reflects the API as-is. +- **Sandbox stop/start** (OpenShell v0.0.113+): the lifecycle is create → ready/error → (stop ⇄ start) → delete. Stopping retains persistent state; there is still no suspend/restart. The UI reflects the API as-is. - Sandbox **policy is required at create**: the form ships client-side starter templates (the gateway has no server-side policy library). See `CLAUDE.md` and `.claude/rules/` for contributor conventions. diff --git a/backend/go.mod b/backend/go.mod index 4d1579ce..bf6a9c13 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -3,10 +3,11 @@ module github.com/Gkrumbach07/openshell-dashboard/backend go 1.25.1 require ( - github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260814001211-c4b500a7de64 + github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260827175501-197b41371d28 github.com/go-chi/chi/v5 v5.3.1 github.com/gorilla/websocket v1.5.3 google.golang.org/grpc v1.82.1 + google.golang.org/protobuf v1.36.11 ) require ( @@ -16,5 +17,4 @@ require ( golang.org/x/sys v0.43.0 // indirect golang.org/x/text v0.36.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect - google.golang.org/protobuf v1.36.11 // indirect ) diff --git a/backend/go.sum b/backend/go.sum index 8df43756..54a29093 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -1,5 +1,5 @@ -github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260814001211-c4b500a7de64 h1:1bxTs8ykTjWia0soV7vw6m/0ZhWDjcOD38kGOcH/7r4= -github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260814001211-c4b500a7de64/go.mod h1:PPbbhH5tmSfoWzVcb5CXurnMkuJOYpesZmUB1itrlFY= +github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260827175501-197b41371d28 h1:Grmzq50KhPbrLFm3xt64gwn3jDMHNMMiqZX2PLbcXXs= +github.com/NVIDIA/OpenShell/sdk/go v0.0.0-20260827175501-197b41371d28/go.mod h1:fWdNw+NcUcKcvMLTa0/q66wVUJHGwWWV+CZOh/BdMZw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/backend/internal/api/app.go b/backend/internal/api/app.go index a8fec14b..2236ae91 100644 --- a/backend/internal/api/app.go +++ b/backend/internal/api/app.go @@ -98,6 +98,8 @@ func (app *App) Routes() http.Handler { r.Post("/sandboxes", app.CreateSandbox) r.Get("/sandboxes/{name}", app.GetSandbox) r.Delete("/sandboxes/{name}", app.DeleteSandbox) + r.Post("/sandboxes/{name}/stop", app.StopSandbox) + r.Post("/sandboxes/{name}/start", app.StartSandbox) r.Get("/sandboxes/{name}/logs", app.GetSandboxLogs) r.Get("/sandboxes/{name}/terminal", app.Terminal) r.Get("/sandboxes/{name}/providers", app.ListSandboxProviders) diff --git a/backend/internal/api/drafts_handler.go b/backend/internal/api/drafts_handler.go index b04082f6..3f012bb8 100644 --- a/backend/internal/api/drafts_handler.go +++ b/backend/internal/api/drafts_handler.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "log/slog" "net/http" @@ -46,7 +47,21 @@ func (app *App) GetDraftPolicy(w http.ResponseWriter, r *http.Request) { // ApproveDraftChunk merges one proposed rule into the active policy. func (app *App) ApproveDraftChunk(w http.ResponseWriter, r *http.Request) { - result, err := app.sdk.Policy().ApproveDraftChunk(r.Context(), chi.URLParam(r, "workspace"), chi.URLParam(r, "name"), chi.URLParam(r, "chunk")) + workspace := chi.URLParam(r, "workspace") + name := chi.URLParam(r, "name") + chunkID := chi.URLParam(r, "chunk") + + // The gateway binds each approval to a review_token that pins the exact + // evaluated candidate (optimistic concurrency). Resolve the current token + // for this chunk from the draft before approving; older gateways return an + // empty token, which the RPC accepts. + reviewToken, err := app.resolveDraftReviewToken(r.Context(), workspace, name, chunkID) + if err != nil { + writeSDKError(w, err) + return + } + + result, err := app.sdk.Policy().ApproveDraftChunk(r.Context(), workspace, name, chunkID, reviewToken) if err != nil { writeSDKError(w, err) return @@ -57,6 +72,22 @@ func (app *App) ApproveDraftChunk(w http.ResponseWriter, r *http.Request) { }) } +// resolveDraftReviewToken returns the review token bound to the given draft +// chunk, or an empty string if the chunk carries none. The token pins an +// approval to the exact candidate the gateway last evaluated. +func (app *App) resolveDraftReviewToken(ctx context.Context, workspace, name, chunkID string) (string, error) { + draft, err := app.sdk.Policy().GetDraft(ctx, workspace, name) + if err != nil { + return "", err + } + for i := range draft.Chunks { + if draft.Chunks[i].ID == chunkID { + return draft.Chunks[i].ReviewToken, nil + } + } + return "", nil +} + // RejectDraftChunkRequest carries the optional reviewer reason, surfaced back // to the in-sandbox agent. type RejectDraftChunkRequest struct { diff --git a/backend/internal/api/mock_sdk_test.go b/backend/internal/api/mock_sdk_test.go index 2b7d5b48..a7cd9ad6 100644 --- a/backend/internal/api/mock_sdk_test.go +++ b/backend/internal/api/mock_sdk_test.go @@ -331,7 +331,7 @@ func (m *mockSDKWorkspaces) ListMembers(ctx context.Context, workspace string, o type mockSDKPolicy struct { getDraftFn func(ctx context.Context, workspace, sandboxName string, opts ...openshell.GetDraftOption) (*openshell.DraftPolicy, error) - approveFn func(ctx context.Context, workspace, sandboxName, chunkID string) (*openshell.ApproveResult, error) + approveFn func(ctx context.Context, workspace, sandboxName, chunkID, reviewToken string) (*openshell.ApproveResult, error) rejectFn func(ctx context.Context, workspace, sandboxName, chunkID, reason string) error approveAllFn func(ctx context.Context, workspace, sandboxName string, opts ...openshell.ApproveAllOption) (*openshell.ApproveAllResult, error) clearFn func(ctx context.Context, workspace, sandboxName string) (*openshell.ClearResult, error) @@ -349,9 +349,9 @@ func (m *mockSDKPolicy) GetDraft(ctx context.Context, workspace, sandboxName str return &openshell.DraftPolicy{}, nil } -func (m *mockSDKPolicy) ApproveDraftChunk(ctx context.Context, workspace, sandboxName, chunkID string) (*openshell.ApproveResult, error) { +func (m *mockSDKPolicy) ApproveDraftChunk(ctx context.Context, workspace, sandboxName, chunkID, reviewToken string) (*openshell.ApproveResult, error) { if m.approveFn != nil { - return m.approveFn(ctx, workspace, sandboxName, chunkID) + return m.approveFn(ctx, workspace, sandboxName, chunkID, reviewToken) } return &openshell.ApproveResult{}, nil } diff --git a/backend/internal/api/sandboxes_handler.go b/backend/internal/api/sandboxes_handler.go index de988331..99d18cf2 100644 --- a/backend/internal/api/sandboxes_handler.go +++ b/backend/internal/api/sandboxes_handler.go @@ -74,6 +74,29 @@ func (app *App) GetSandbox(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, models.FromSDKSandbox(sandbox)) } +// StopSandbox stops a running sandbox while retaining its persistent state. +// The sandbox transitions through STOPPING to STOPPED and can be resumed with +// StartSandbox. +func (app *App) StopSandbox(w http.ResponseWriter, r *http.Request) { + sandbox, err := app.sdk.Sandboxes().Stop(r.Context(), chi.URLParam(r, "workspace"), chi.URLParam(r, "name")) + if err != nil { + writeSDKError(w, err) + return + } + writeJSON(w, http.StatusOK, models.FromSDKSandbox(sandbox)) +} + +// StartSandbox resumes a previously stopped sandbox. The sandbox transitions +// through STARTING back to READY. +func (app *App) StartSandbox(w http.ResponseWriter, r *http.Request) { + sandbox, err := app.sdk.Sandboxes().Start(r.Context(), chi.URLParam(r, "workspace"), chi.URLParam(r, "name")) + if err != nil { + writeSDKError(w, err) + return + } + writeJSON(w, http.StatusOK, models.FromSDKSandbox(sandbox)) +} + func (app *App) DeleteSandbox(w http.ResponseWriter, r *http.Request) { if err := app.sdk.Sandboxes().Delete(r.Context(), chi.URLParam(r, "workspace"), chi.URLParam(r, "name")); err != nil { writeSDKError(w, err) diff --git a/backend/internal/models/models.go b/backend/internal/models/models.go index eca4ad3a..b61af19e 100644 --- a/backend/internal/models/models.go +++ b/backend/internal/models/models.go @@ -48,6 +48,10 @@ type SandboxStatus struct { Phase string `json:"phase"` Conditions []SandboxCondition `json:"conditions,omitempty"` CurrentPolicyVersion uint32 `json:"currentPolicyVersion"` + // ExitCode is the main process exit code once the sandbox has exited (nil + // while running). Signal exits are reported as 128+signal. Surfaced to + // explain ERROR-phase sandboxes. + ExitCode *int32 `json:"exitCode,omitempty"` } // SandboxSpec is the dashboard view of openshell.v1.SandboxSpec. Policy is diff --git a/backend/internal/models/observability.go b/backend/internal/models/observability.go index 30308748..53802ac5 100644 --- a/backend/internal/models/observability.go +++ b/backend/internal/models/observability.go @@ -65,6 +65,21 @@ type PolicyChunk struct { DecidedAtMs int64 `json:"decidedAtMs,omitempty"` Confidence float32 `json:"confidence"` HitCount int32 `json:"hitCount"` + // ReviewToken pins an approval to the exact evaluated candidate (optimistic + // concurrency). The BFF resolves it server-side on approve; exposed so a + // client can echo it directly. + ReviewToken string `json:"reviewToken,omitempty"` + // ApplicationError is set when a prover-clean chunk still fails to apply to + // the complete candidate policy (explains "clean but not applicable"). + ApplicationError string `json:"applicationError,omitempty"` + // CurrentEffectivePolicyHash and CandidateEffectivePolicyHash identify the + // before/after effective policies for a diff view. + CurrentEffectivePolicyHash string `json:"currentEffectivePolicyHash,omitempty"` + CandidateEffectivePolicyHash string `json:"candidateEffectivePolicyHash,omitempty"` + // CurrentEffectivePolicy and CandidateEffectivePolicy carry the full + // before/after policies (protojson) so the draft inbox can render a diff. + CurrentEffectivePolicy json.RawMessage `json:"currentEffectivePolicy,omitempty"` + CandidateEffectivePolicy json.RawMessage `json:"candidateEffectivePolicy,omitempty"` } // DraftPolicy mirrors GetDraftPolicyResponse. diff --git a/backend/internal/models/sdk_converters.go b/backend/internal/models/sdk_converters.go index e97bcc25..b74b4dd2 100644 --- a/backend/internal/models/sdk_converters.go +++ b/backend/internal/models/sdk_converters.go @@ -54,6 +54,7 @@ func FromSDKSandbox(sandbox *openshell.Sandbox) Sandbox { AgentPod: sandbox.Status.AgentPod, Phase: strings.ToUpper(string(sandbox.Status.Phase)), CurrentPolicyVersion: sandbox.Status.CurrentPolicyVersion, + ExitCode: sandbox.Status.ExitCode, } for _, cond := range sandbox.Status.Conditions { out.Status.Conditions = append(out.Status.Conditions, SandboxCondition{ @@ -472,10 +473,21 @@ func FromSDKDraftPolicy(draft *openshell.DraftPolicy) DraftPolicy { Binary: chunk.Binary, ValidationResult: chunk.ValidationResult, RejectionReason: chunk.RejectionReason, + ReviewToken: chunk.ReviewToken, + ApplicationError: chunk.ApplicationError, + + CurrentEffectivePolicyHash: chunk.CurrentEffectivePolicyHash, + CandidateEffectivePolicyHash: chunk.CandidateEffectivePolicyHash, } if chunk.ProposedRule != nil { item.ProposedRule = MarshalSDKNetworkPolicyRule(chunk.ProposedRule) } + if chunk.CurrentEffectivePolicy != nil { + item.CurrentEffectivePolicy = marshalSDKPolicy(chunk.CurrentEffectivePolicy) + } + if chunk.CandidateEffectivePolicy != nil { + item.CandidateEffectivePolicy = marshalSDKPolicy(chunk.CandidateEffectivePolicy) + } out.Chunks = append(out.Chunks, item) } return out diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 97d2da2c..9219d8b9 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -69,8 +69,11 @@ export const apiFetch = async ( export const get = (path: string): Promise => apiFetch(path); -export const post = (path: string, body: unknown): Promise => - apiFetch(path, { method: 'POST', body: JSON.stringify(body) }); +export const post = (path: string, body?: unknown): Promise => + apiFetch(path, { + method: 'POST', + body: body === undefined ? undefined : JSON.stringify(body), + }); export const put = (path: string, body: unknown): Promise => apiFetch(path, { method: 'PUT', body: JSON.stringify(body) }); diff --git a/frontend/src/api/sandboxes.ts b/frontend/src/api/sandboxes.ts index c3ab6fb3..cf93f913 100644 --- a/frontend/src/api/sandboxes.ts +++ b/frontend/src/api/sandboxes.ts @@ -45,6 +45,22 @@ export const deleteSandbox = ( `/api/v1/workspaces/${encodeURIComponent(workspace)}/sandboxes/${encodeURIComponent(name)}`, ); +export const stopSandbox = ( + workspace: string, + name: string, +): Promise => + post( + `/api/v1/workspaces/${encodeURIComponent(workspace)}/sandboxes/${encodeURIComponent(name)}/stop`, + ); + +export const startSandbox = ( + workspace: string, + name: string, +): Promise => + post( + `/api/v1/workspaces/${encodeURIComponent(workspace)}/sandboxes/${encodeURIComponent(name)}/start`, + ); + export const useSandboxes = (workspace: string, labelSelector?: string) => useQuery({ queryKey: sandboxKeys.list(workspace, labelSelector), @@ -184,6 +200,24 @@ export const useDeleteSandbox = (workspace: string) => { }); }; +export const useStopSandbox = (workspace: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (name: string) => stopSandbox(workspace, name), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: sandboxKeys.scope(workspace) }), + }); +}; + +export const useStartSandbox = (workspace: string) => { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (name: string) => startSandbox(workspace, name), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: sandboxKeys.scope(workspace) }), + }); +}; + // --- Service endpoints --- export const listServices = ( diff --git a/frontend/src/components/CreateSandboxModal.tsx b/frontend/src/components/CreateSandboxModal.tsx index a885b6c4..75fc0c62 100644 --- a/frontend/src/components/CreateSandboxModal.tsx +++ b/frontend/src/components/CreateSandboxModal.tsx @@ -68,7 +68,7 @@ const CreateSandboxModal: React.FC = ({ >
= ({ phase }) => { switch (phase) { case 'READY': @@ -38,11 +39,19 @@ const PhaseLabel: React.FC = ({ phase }) => { ); case 'PROVISIONING': + case 'STOPPING': + case 'STARTING': return ( ); + case 'STOPPED': + return ( + + ); case 'DELETING': case 'TERMINATING': return ( diff --git a/frontend/src/components/sandbox/SandboxTableRow.tsx b/frontend/src/components/sandbox/SandboxTableRow.tsx index 5334892d..1370224b 100644 --- a/frontend/src/components/sandbox/SandboxTableRow.tsx +++ b/frontend/src/components/sandbox/SandboxTableRow.tsx @@ -27,6 +27,8 @@ type SandboxTableRowProps = { onSelect: (isSelecting: boolean) => void; onNameClick?: () => void; onDelete: () => void; + onStop?: () => void; + onStart?: () => void; onViewLogs: () => void; onOpenTerminal?: () => void; policyView?: SandboxPolicyView; @@ -47,6 +49,8 @@ const SandboxTableRow: React.FC = ({ onSelect, onNameClick, onDelete, + onStop, + onStart, onViewLogs, onOpenTerminal, policyView, @@ -63,6 +67,12 @@ const SandboxTableRow: React.FC = ({ const actionItems = [ ...(onOpenTerminal ? [{ title: 'Terminal', onClick: onOpenTerminal }] : []), { title: 'Logs', onClick: onViewLogs }, + ...(onStop && sandbox.status.phase === 'READY' + ? [{ title: 'Stop', onClick: onStop }] + : []), + ...(onStart && sandbox.status.phase === 'STOPPED' + ? [{ title: 'Start', onClick: onStart }] + : []), { title: 'Delete', onClick: onDelete }, ]; diff --git a/frontend/src/pages/SandboxDetailPage.tsx b/frontend/src/pages/SandboxDetailPage.tsx index 500a753a..9cf2b55f 100644 --- a/frontend/src/pages/SandboxDetailPage.tsx +++ b/frontend/src/pages/SandboxDetailPage.tsx @@ -28,7 +28,11 @@ import { useSearchParams } from 'react-router-dom'; import { useFeatureFlags } from '../api/auth'; import { useDraftPolicy, useSandboxPolicy } from '../api/policy'; import { useProviderExpiry } from '../api/providers'; -import { useSandbox } from '../api/sandboxes'; +import { + useSandbox, + useStartSandbox, + useStopSandbox, +} from '../api/sandboxes'; import ConnectCard from '../components/ConnectCard'; import LabelsList from '../components/LabelsList'; import PhaseLabel from '../components/PhaseLabel'; @@ -52,6 +56,8 @@ type SandboxDetailPageProps = { const SandboxDetailPage: React.FC = (props) => { const { workspace, sandboxName } = props; const sandbox = useSandbox(workspace, sandboxName); + const stopSandbox = useStopSandbox(workspace); + const startSandbox = useStartSandbox(workspace); const features = useFeatureFlags(); const policyQuery = useSandboxPolicy(workspace, sandboxName); const draftsQuery = useDraftPolicy(workspace, sandboxName); @@ -126,6 +132,31 @@ const SandboxDetailPage: React.FC = (props) => { + + {data.status.phase === 'STOPPED' ? ( + + ) : ( + + )} + = (props) => { {data.status.currentPolicyVersion || '-'} + {data.status.exitCode !== undefined && ( + + Exit code + + {data.status.exitCode} + + + )} Attached providers diff --git a/frontend/src/pages/SandboxListPage.tsx b/frontend/src/pages/SandboxListPage.tsx index 2336948f..86322319 100644 --- a/frontend/src/pages/SandboxListPage.tsx +++ b/frontend/src/pages/SandboxListPage.tsx @@ -32,7 +32,12 @@ import { useNavigate } from 'react-router-dom'; import { useFeatureFlags } from '../api/auth'; import { useDraftNotifications, useSandboxPolicies } from '../api/policy'; import { useProviderExpiry } from '../api/providers'; -import { deleteSandbox, useSandboxes } from '../api/sandboxes'; +import { + deleteSandbox, + useSandboxes, + useStartSandbox, + useStopSandbox, +} from '../api/sandboxes'; import { useAlerts } from '../app/AlertContext'; import ConfirmDeleteModal from '../components/ConfirmDeleteModal'; import CreateSandboxModal from '../components/CreateSandboxModal'; @@ -101,6 +106,8 @@ const SandboxListPage: React.FC = ({ const providerExpiry = useProviderExpiry(workspace); const drafts = useDraftNotifications(features.draftPolicy); const { addSuccess } = useAlerts(); + const stopSandbox = useStopSandbox(workspace); + const startSandbox = useStartSandbox(workspace); const bulkDelete = useBulkDelete( (name) => deleteSandbox(workspace, name), ['sandboxes', workspace], @@ -277,6 +284,14 @@ const SandboxListPage: React.FC = ({ } onNameClick={() => onSelect?.(sandbox.metadata.name)} onDelete={() => setDeleteTargets([sandbox.metadata.name])} + onStop={() => { + stopSandbox.mutate(sandbox.metadata.name); + addSuccess(`Stopping sandbox ${sandbox.metadata.name}`); + }} + onStart={() => { + startSandbox.mutate(sandbox.metadata.name); + addSuccess(`Starting sandbox ${sandbox.metadata.name}`); + }} onViewLogs={() => viewSandbox(sandbox.metadata.name, 'logs')} onOpenTerminal={ sandbox.status.phase === 'READY' && features.terminal diff --git a/frontend/src/pages/__tests__/SandboxListPage.spec.tsx b/frontend/src/pages/__tests__/SandboxListPage.spec.tsx index 2026e723..5bfa502f 100644 --- a/frontend/src/pages/__tests__/SandboxListPage.spec.tsx +++ b/frontend/src/pages/__tests__/SandboxListPage.spec.tsx @@ -27,6 +27,8 @@ const mockSandbox: Sandbox = { jest.mock('../../api/sandboxes', () => ({ useSandboxes: jest.fn(), deleteSandbox: jest.fn(), + useStopSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), + useStartSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), useCreateSandbox: jest.fn(() => ({ mutate: jest.fn(), reset: jest.fn(), diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index 5c26415c..ec7b5826 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -31,10 +31,20 @@ export type WorkspaceMember = { role: WorkspaceRole | 'UNSPECIFIED'; }; -// openshell.v1.SandboxPhase — the full lifecycle. There is no stopped or -// suspended state; a sandbox runs until deleted. +// openshell.v1.SandboxPhase — the full lifecycle. Sandboxes can be stopped +// (retaining persistent state) and started again: +// PROVISIONING → READY → STOPPING → STOPPED → STARTING → READY, plus ERROR and +// DELETING terminal/transitional states. export type SandboxPhase = - 'PROVISIONING' | 'READY' | 'ERROR' | 'DELETING' | 'UNKNOWN' | 'UNSPECIFIED'; + | 'PROVISIONING' + | 'READY' + | 'ERROR' + | 'DELETING' + | 'STOPPING' + | 'STOPPED' + | 'STARTING' + | 'UNKNOWN' + | 'UNSPECIFIED'; export type SandboxCondition = { type: string; @@ -50,6 +60,9 @@ export type SandboxStatus = { conditions?: SandboxCondition[]; phase: SandboxPhase; currentPolicyVersion: number; + // Main process exit code once the sandbox has exited (undefined while + // running). Signal exits are reported as 128+signal. + exitCode?: number; }; // --- openshell.sandbox.v1.SandboxPolicy (protojson camelCase) --- @@ -409,6 +422,16 @@ export type PolicyChunk = { // Gateway prover verdict — there is no separate verify RPC. validationResult?: string; rejectionReason?: string; + // Pins an approval to the exact evaluated candidate (optimistic concurrency). + reviewToken?: string; + // Set when a prover-clean chunk still fails to apply to the complete + // candidate policy. + applicationError?: string; + // Before/after effective policy identity and full policies for a diff view. + currentEffectivePolicyHash?: string; + candidateEffectivePolicyHash?: string; + currentEffectivePolicy?: SandboxPolicy; + candidateEffectivePolicy?: SandboxPolicy; }; export type DraftPolicy = { From 96056ddadb59c59009c277caf6e334834e44e4a9 Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 27 Aug 2026 13:54:44 -0500 Subject: [PATCH 2/3] fix: satisfy golangci-lint fieldalignment and prettier CI checks - Reorder SandboxStatus and PolicyChunk fields to minimize pointer-scan bytes (govet fieldalignment), preserving doc comments. - Run prettier on SandboxDetailPage.tsx. Both check-backend and check-frontend CI jobs were failing on these. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Gage Krumbach --- backend/internal/models/models.go | 8 ++--- backend/internal/models/observability.go | 38 ++++++++++++------------ frontend/src/pages/SandboxDetailPage.tsx | 6 +--- 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/backend/internal/models/models.go b/backend/internal/models/models.go index b61af19e..ecc819f1 100644 --- a/backend/internal/models/models.go +++ b/backend/internal/models/models.go @@ -43,15 +43,15 @@ type SandboxCondition struct { // SandboxStatus mirrors openshell.v1.SandboxStatus. type SandboxStatus struct { + // ExitCode is the main process exit code once the sandbox has exited (nil + // while running). Signal exits are reported as 128+signal. Surfaced to + // explain ERROR-phase sandboxes. + ExitCode *int32 `json:"exitCode,omitempty"` SandboxName string `json:"sandboxName,omitempty"` AgentPod string `json:"agentPod,omitempty"` Phase string `json:"phase"` Conditions []SandboxCondition `json:"conditions,omitempty"` CurrentPolicyVersion uint32 `json:"currentPolicyVersion"` - // ExitCode is the main process exit code once the sandbox has exited (nil - // while running). Signal exits are reported as 128+signal. Surfaced to - // explain ERROR-phase sandboxes. - ExitCode *int32 `json:"exitCode,omitempty"` } // SandboxSpec is the dashboard view of openshell.v1.SandboxSpec. Policy is diff --git a/backend/internal/models/observability.go b/backend/internal/models/observability.go index 53802ac5..7cb2fd38 100644 --- a/backend/internal/models/observability.go +++ b/backend/internal/models/observability.go @@ -52,34 +52,34 @@ type PolicyUpdateResult struct { // ProposedRule is protojson of a NetworkPolicyRule. ValidationResult carries // the gateway prover verdict (there is no separate verify RPC). type PolicyChunk struct { - ValidationResult string `json:"validationResult,omitempty"` - Status string `json:"status"` - RuleName string `json:"ruleName,omitempty"` - Rationale string `json:"rationale,omitempty"` - SecurityNotes string `json:"securityNotes,omitempty"` - ID string `json:"id"` - RejectionReason string `json:"rejectionReason,omitempty"` - Binary string `json:"binary,omitempty"` - ProposedRule json.RawMessage `json:"proposedRule,omitempty"` - CreatedAtMs int64 `json:"createdAtMs"` - DecidedAtMs int64 `json:"decidedAtMs,omitempty"` - Confidence float32 `json:"confidence"` - HitCount int32 `json:"hitCount"` // ReviewToken pins an approval to the exact evaluated candidate (optimistic // concurrency). The BFF resolves it server-side on approve; exposed so a // client can echo it directly. - ReviewToken string `json:"reviewToken,omitempty"` - // ApplicationError is set when a prover-clean chunk still fails to apply to - // the complete candidate policy (explains "clean but not applicable"). - ApplicationError string `json:"applicationError,omitempty"` - // CurrentEffectivePolicyHash and CandidateEffectivePolicyHash identify the + ReviewToken string `json:"reviewToken,omitempty"` + Status string `json:"status"` + RuleName string `json:"ruleName,omitempty"` + Rationale string `json:"rationale,omitempty"` + SecurityNotes string `json:"securityNotes,omitempty"` + ID string `json:"id"` + RejectionReason string `json:"rejectionReason,omitempty"` + Binary string `json:"binary,omitempty"` + ValidationResult string `json:"validationResult,omitempty"` + // CandidateEffectivePolicyHash and CurrentEffectivePolicyHash identify the // before/after effective policies for a diff view. - CurrentEffectivePolicyHash string `json:"currentEffectivePolicyHash,omitempty"` CandidateEffectivePolicyHash string `json:"candidateEffectivePolicyHash,omitempty"` + CurrentEffectivePolicyHash string `json:"currentEffectivePolicyHash,omitempty"` + // ApplicationError is set when a prover-clean chunk still fails to apply to + // the complete candidate policy (explains "clean but not applicable"). + ApplicationError string `json:"applicationError,omitempty"` + ProposedRule json.RawMessage `json:"proposedRule,omitempty"` // CurrentEffectivePolicy and CandidateEffectivePolicy carry the full // before/after policies (protojson) so the draft inbox can render a diff. CurrentEffectivePolicy json.RawMessage `json:"currentEffectivePolicy,omitempty"` CandidateEffectivePolicy json.RawMessage `json:"candidateEffectivePolicy,omitempty"` + DecidedAtMs int64 `json:"decidedAtMs,omitempty"` + CreatedAtMs int64 `json:"createdAtMs"` + HitCount int32 `json:"hitCount"` + Confidence float32 `json:"confidence"` } // DraftPolicy mirrors GetDraftPolicyResponse. diff --git a/frontend/src/pages/SandboxDetailPage.tsx b/frontend/src/pages/SandboxDetailPage.tsx index 9cf2b55f..e751e198 100644 --- a/frontend/src/pages/SandboxDetailPage.tsx +++ b/frontend/src/pages/SandboxDetailPage.tsx @@ -28,11 +28,7 @@ import { useSearchParams } from 'react-router-dom'; import { useFeatureFlags } from '../api/auth'; import { useDraftPolicy, useSandboxPolicy } from '../api/policy'; import { useProviderExpiry } from '../api/providers'; -import { - useSandbox, - useStartSandbox, - useStopSandbox, -} from '../api/sandboxes'; +import { useSandbox, useStartSandbox, useStopSandbox } from '../api/sandboxes'; import ConnectCard from '../components/ConnectCard'; import LabelsList from '../components/LabelsList'; import PhaseLabel from '../components/PhaseLabel'; From 5a32c945fed2c54ad3e63c475483903ac3178eb0 Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 27 Aug 2026 14:19:55 -0500 Subject: [PATCH 3/3] fix: address self-review findings on sandbox stop/start + draft approve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix false-positive success toasts: list-page Stop/Start now report success/failure from the mutation's actual outcome, not immediately on click. Detail-page Stop/Start gained the error feedback they were missing entirely. - Extract canStopSandbox/canStartSandbox so the list row and detail page page can't disagree on which phases allow which action. - Add STOPPED/STOPPING/STARTING colors to StatusDot (PhaseLabel already had them). - Drop the delete-confirmation copy claiming deletion is "the only way to stop a running sandbox" — no longer true now that Stop exists. - ApproveDraftChunk: accept an optional client-supplied reviewToken (the frontend already has it from GetDraftPolicy) and only fall back to the extra GetDraft round-trip when the client didn't send one. - Add handler tests for StopSandbox/StartSandbox (previously untested; the SDK mock's Stop/Start literally panicked) and for both ApproveDraftChunk token paths. - Correct .claude/rules/openshell-api.md and CLAUDE.md: backend/proto/ and backend/internal/gateway/ were removed by the SDK migration (#43) and are stale/unused — the vendored Go SDK is the source of truth now, not a local proto copy. Also documented ApproveDraftChunk's new review_token under the optimistic-concurrency hard fact. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Gage Krumbach --- .claude/rules/openshell-api.md | 16 ++-- CLAUDE.md | 2 +- backend/internal/api/drafts_handler.go | 30 +++++-- backend/internal/api/mock_sdk_test.go | 16 +++- backend/internal/api/policies_handler_test.go | 66 ++++++++++++-- .../internal/api/sandboxes_handler_test.go | 87 +++++++++++++++++++ frontend/src/api/policy.ts | 10 ++- .../SandboxDraftsTab/DraftChunkRow.tsx | 7 +- frontend/src/components/StatusDot.tsx | 3 + .../components/sandbox/SandboxTableRow.tsx | 5 +- frontend/src/pages/SandboxDetailPage.tsx | 25 +++++- frontend/src/pages/SandboxListPage.tsx | 34 +++++--- frontend/src/utils/sandboxLifecycle.ts | 10 +++ 13 files changed, 261 insertions(+), 50 deletions(-) create mode 100644 frontend/src/utils/sandboxLifecycle.ts diff --git a/.claude/rules/openshell-api.md b/.claude/rules/openshell-api.md index 9089cdf0..3591e766 100644 --- a/.claude/rules/openshell-api.md +++ b/.claude/rules/openshell-api.md @@ -1,19 +1,19 @@ --- -description: OpenShell gRPC API reference for dashboard development — proto is source of truth -globs: "backend/internal/gateway/**,backend/proto/**,frontend/src/api/**,frontend/src/types/**" +description: OpenShell gRPC API reference for dashboard development — the vendored Go SDK is source of truth +globs: "backend/internal/sdkclient/**,backend/internal/api/**,frontend/src/api/**,frontend/src/types/**" alwaysApply: false --- # OpenShell API Reference -**THE PROTO FILES IN `backend/proto/` ARE THE SOURCE OF TRUTH.** Before writing any gateway wrapper method, REST handler, or TypeScript type, read the actual RPC and message definitions in the proto. Never invent RPCs, fields, or lifecycle states. If a UI idea has no backing RPC, flag it — do not fabricate an endpoint. +**THE VENDORED GO SDK IS THE SOURCE OF TRUTH.** Since the SDK migration (PR #43), the BFF calls `github.com/NVIDIA/OpenShell/sdk/go` directly — there is no local `backend/proto/` copy or `backend/gen/` anymore (both were removed; `backend/proto/*.proto` predates the migration and is stale/unused — do not edit or regenerate from it). Before writing any handler or TypeScript type, read the actual RPC/message definitions in `$(go env GOMODCACHE)/github.com/!n!v!i!d!i!a/!open!shell/sdk/go@/openshell/v1/` (hand-written client wrappers with doc comments; `types/*.go` for message shapes) or via `go doc`. Never invent RPCs, fields, or lifecycle states. If a UI idea has no backing RPC, flag it — do not fabricate an endpoint. Track upstream via `go get github.com/NVIDIA/OpenShell/sdk/go@latest`. ## Services -| Service | Proto | What we use | -|---------|-------|-------------| -| `openshell.v1.OpenShell` (68 RPCs) | openshell.proto | Sandbox (incl. stop/start), workspace, member, provider, profile, policy, draft policy, logs, SSH, services | -| `openshell.inference.v1.Inference` (4 RPCs) | inference.proto | Inference route CRUD | +| Service | SDK package | What we use | +|---------|-------------|-------------| +| `openshell.v1.OpenShell` | `openshell/v1` (`Client.Sandboxes()`, `.Workspaces()`, `.Providers()`, `.Policy()`, ...) | Sandbox (incl. stop/start), workspace, member, provider, profile, policy, draft policy, logs, SSH, services | +| `openshell.inference.v1.Inference` | `openshell/v1` (`Client.Inference()`) | Inference route CRUD | Skip `GatewayInterceptor`, `SupervisorMiddleware`, `ComputeDriver` — internal/operator only. @@ -29,7 +29,7 @@ Skip `GatewayInterceptor`, `SupervisorMiddleware`, `ComputeDriver` — internal/ 8. **Provider model** (`datamodel.v1.Provider`): metadata, type (profile slug like "claude"/"gitlab"), credentials (map, `secret` option — strip before returning to browser), config (map), credential_expires_at_ms, profile_workspace. No endpoint-URL/status/model fields. Valid types come from `ListProviderProfiles`. 9. **No list-images API.** Sandbox images are free-text OCI refs; community images by convention `ghcr.io/nvidia/openshell-community/sandboxes/`. 10. **`GetGatewayInfo` returns only** status, gateway_version, compute_drivers[]. No uptime/db/TLS/auth-mode fields. -11. **Optimistic concurrency:** `AttachSandboxProvider`, `DetachSandboxProvider`, `UpdateConfig`, `UpdateProviderProfiles` accept `expected_resource_version` — pass the ObjectMeta.resource_version from the last read. +11. **Optimistic concurrency:** `AttachSandboxProvider`, `DetachSandboxProvider`, `UpdateConfig`, `UpdateProviderProfiles` accept `expected_resource_version` — pass the ObjectMeta.resource_version from the last read. `ApproveDraftChunk` similarly takes a `review_token` (from the chunk's last `GetDraft` read) pinning the exact evaluated candidate; the BFF falls back to resolving it server-side if the client doesn't send one. 12. **Workspace scoping:** most requests carry a `workspace` field (empty = "default"); list RPCs offer `all_workspaces`. Workspaces themselves are top-level. 13. **Secret fields** are annotated `[(openshell.options.v1.secret) = true]` in proto — grep for `secret` when adding a wrapper and never serialize those fields to the frontend. diff --git a/CLAUDE.md b/CLAUDE.md index 6c564167..6ddb504e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,7 @@ Each rule has a corresponding Architecture Decision Record in [`docs/adrs/`](doc ## OpenShell API reference -The gateway exposes 72 gRPC RPCs across 2 services (68 in `OpenShell`, 4 in `Inference`). Proto files are in `backend/proto/`. +The gateway exposes RPCs across 2 services (`OpenShell`, `Inference`), consumed via the vendored `github.com/NVIDIA/OpenShell/sdk/go` (see `.claude/rules/openshell-api.md`) — not a local proto copy. ## Personas diff --git a/backend/internal/api/drafts_handler.go b/backend/internal/api/drafts_handler.go index 3f012bb8..ef58f6c4 100644 --- a/backend/internal/api/drafts_handler.go +++ b/backend/internal/api/drafts_handler.go @@ -45,22 +45,38 @@ func (app *App) GetDraftPolicy(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, models.FromSDKDraftPolicy(draft)) } +// ApproveDraftChunkRequest optionally carries the chunk's review token, which +// the frontend already has from its last GetDraftPolicy fetch. Absent for +// older clients or the approve-from-notification path. +type ApproveDraftChunkRequest struct { + ReviewToken string `json:"reviewToken,omitempty"` +} + // ApproveDraftChunk merges one proposed rule into the active policy. func (app *App) ApproveDraftChunk(w http.ResponseWriter, r *http.Request) { workspace := chi.URLParam(r, "workspace") name := chi.URLParam(r, "name") chunkID := chi.URLParam(r, "chunk") - // The gateway binds each approval to a review_token that pins the exact - // evaluated candidate (optimistic concurrency). Resolve the current token - // for this chunk from the draft before approving; older gateways return an - // empty token, which the RPC accepts. - reviewToken, err := app.resolveDraftReviewToken(r.Context(), workspace, name, chunkID) - if err != nil { - writeSDKError(w, err) + var body ApproveDraftChunkRequest + if r.ContentLength > 0 && !decodeBody(w, r, &body) { return } + // The gateway binds each approval to a review_token that pins the exact + // evaluated candidate (optimistic concurrency). Use the token the client + // already has; only fall back to a GetDraft round-trip when it didn't send + // one. Older gateways return an empty token, which the RPC accepts. + reviewToken := body.ReviewToken + if reviewToken == "" { + var err error + reviewToken, err = app.resolveDraftReviewToken(r.Context(), workspace, name, chunkID) + if err != nil { + writeSDKError(w, err) + return + } + } + result, err := app.sdk.Policy().ApproveDraftChunk(r.Context(), workspace, name, chunkID, reviewToken) if err != nil { writeSDKError(w, err) diff --git a/backend/internal/api/mock_sdk_test.go b/backend/internal/api/mock_sdk_test.go index a7cd9ad6..66dad27a 100644 --- a/backend/internal/api/mock_sdk_test.go +++ b/backend/internal/api/mock_sdk_test.go @@ -46,6 +46,8 @@ type mockSDKSandboxes struct { detachFn func(ctx context.Context, workspace, sandboxName, providerName string, expectedResourceVersion uint64) (*openshell.DetachProviderResult, error) listProvidersFn func(ctx context.Context, workspace, sandboxName string) ([]*openshell.Provider, error) getLogsFn func(ctx context.Context, workspace, name string, opts ...openshell.LogOption) (*openshell.LogResult, error) + stopFn func(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) + startFn func(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) } func (m *mockSDKSandboxes) List(ctx context.Context, workspace string, opts ...openshell.ListOptions) ([]*openshell.Sandbox, error) { @@ -112,12 +114,18 @@ func (m *mockSDKSandboxes) GetLogs(ctx context.Context, workspace, name string, return &openshell.LogResult{}, nil } -func (m *mockSDKSandboxes) Start(_ context.Context, _, _ string) (*openshell.Sandbox, error) { - panic("not implemented") +func (m *mockSDKSandboxes) Start(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) { + if m.startFn != nil { + return m.startFn(ctx, workspace, name) + } + return &openshell.Sandbox{Name: name}, nil } -func (m *mockSDKSandboxes) Stop(_ context.Context, _, _ string) (*openshell.Sandbox, error) { - panic("not implemented") +func (m *mockSDKSandboxes) Stop(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) { + if m.stopFn != nil { + return m.stopFn(ctx, workspace, name) + } + return &openshell.Sandbox{Name: name}, nil } func (m *mockSDKSandboxes) WaitStopped(_ context.Context, _, _ string, _ ...openshell.WaitOptions) (*openshell.Sandbox, error) { diff --git a/backend/internal/api/policies_handler_test.go b/backend/internal/api/policies_handler_test.go index 60f226a6..f49f4d15 100644 --- a/backend/internal/api/policies_handler_test.go +++ b/backend/internal/api/policies_handler_test.go @@ -210,15 +210,63 @@ func TestGetDraftPolicy(t *testing.T) { } func TestApproveDraftChunk(t *testing.T) { - app := newTestAppWithSDK(&mockSDK{}) - r := chi.NewRouter() - r.Post("/workspaces/{workspace}/sandboxes/{name}/drafts/{chunk}/approve", app.ApproveDraftChunk) - req := httptest.NewRequest(http.MethodPost, "/workspaces/default/sandboxes/my-sandbox/drafts/c1/approve", nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } + t.Run("uses client-supplied review token without resolving the draft", func(t *testing.T) { + sdk := &mockSDK{} + sdk.policy.getDraftFn = func(_ context.Context, _, _ string, _ ...openshell.GetDraftOption) (*openshell.DraftPolicy, error) { + t.Fatal("GetDraft should not be called when the client supplies a reviewToken") + return nil, nil + } + var gotToken string + sdk.policy.approveFn = func(_ context.Context, _, _, _, reviewToken string) (*openshell.ApproveResult, error) { + gotToken = reviewToken + return &openshell.ApproveResult{PolicyVersion: 2}, nil + } + app := newTestAppWithSDK(sdk) + r := chi.NewRouter() + r.Post("/workspaces/{workspace}/sandboxes/{name}/drafts/{chunk}/approve", app.ApproveDraftChunk) + req := httptest.NewRequest( + http.MethodPost, + "/workspaces/default/sandboxes/my-sandbox/drafts/c1/approve", + strings.NewReader(`{"reviewToken":"client-token"}`), + ) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d; body: %s", w.Code, w.Body.String()) + } + if gotToken != "client-token" { + t.Errorf("reviewToken passed to ApproveDraftChunk = %q, want %q", gotToken, "client-token") + } + }) + + t.Run("resolves the review token from the draft when the client sends none", func(t *testing.T) { + sdk := &mockSDK{} + sdk.policy.getDraftFn = func(_ context.Context, _, _ string, _ ...openshell.GetDraftOption) (*openshell.DraftPolicy, error) { + return &openshell.DraftPolicy{ + Chunks: []openshell.PolicyChunk{ + {ID: "c1", ReviewToken: "resolved-token"}, + }, + }, nil + } + var gotToken string + sdk.policy.approveFn = func(_ context.Context, _, _, _, reviewToken string) (*openshell.ApproveResult, error) { + gotToken = reviewToken + return &openshell.ApproveResult{PolicyVersion: 2}, nil + } + app := newTestAppWithSDK(sdk) + r := chi.NewRouter() + r.Post("/workspaces/{workspace}/sandboxes/{name}/drafts/{chunk}/approve", app.ApproveDraftChunk) + req := httptest.NewRequest(http.MethodPost, "/workspaces/default/sandboxes/my-sandbox/drafts/c1/approve", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d; body: %s", w.Code, w.Body.String()) + } + if gotToken != "resolved-token" { + t.Errorf("reviewToken passed to ApproveDraftChunk = %q, want %q", gotToken, "resolved-token") + } + }) } func TestRejectDraftChunk(t *testing.T) { diff --git a/backend/internal/api/sandboxes_handler_test.go b/backend/internal/api/sandboxes_handler_test.go index f88614a0..cf7a2f06 100644 --- a/backend/internal/api/sandboxes_handler_test.go +++ b/backend/internal/api/sandboxes_handler_test.go @@ -235,6 +235,93 @@ func TestGetSandbox(t *testing.T) { } } +func TestStopSandbox(t *testing.T) { + tests := []struct { + stopFn func(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) + name string + wantStatus int + }{ + { + name: "success", + stopFn: func(_ context.Context, _, name string) (*openshell.Sandbox, error) { + return &openshell.Sandbox{ID: "id-1", Name: name, Status: openshell.SandboxStatus{Phase: openshell.SandboxStopping}}, nil + }, + wantStatus: http.StatusOK, + }, + { + name: "not found", + stopFn: func(_ context.Context, _, _ string) (*openshell.Sandbox, error) { + return nil, &openshell.StatusError{Code: openshell.ErrorNotFound, Message: "sandbox not found"} + }, + wantStatus: http.StatusNotFound, + }, + { + name: "already stopped conflicts", + stopFn: func(_ context.Context, _, _ string) (*openshell.Sandbox, error) { + return nil, &openshell.StatusError{Code: openshell.ErrorConflict, Message: "sandbox is not running"} + }, + wantStatus: http.StatusConflict, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sdk := &mockSDK{} + sdk.sandboxes.stopFn = tc.stopFn + app := newTestAppWithSDK(sdk) + r := chi.NewRouter() + r.Post("/workspaces/{workspace}/sandboxes/{name}/stop", app.StopSandbox) + + req := httptest.NewRequest(http.MethodPost, "/workspaces/default/sandboxes/my-sandbox/stop", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != tc.wantStatus { + t.Errorf("status = %d, want %d; body: %s", w.Code, tc.wantStatus, w.Body.String()) + } + }) + } +} + +func TestStartSandbox(t *testing.T) { + tests := []struct { + startFn func(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) + name string + wantStatus int + }{ + { + name: "success", + startFn: func(_ context.Context, _, name string) (*openshell.Sandbox, error) { + return &openshell.Sandbox{ID: "id-1", Name: name, Status: openshell.SandboxStatus{Phase: openshell.SandboxStarting}}, nil + }, + wantStatus: http.StatusOK, + }, + { + name: "not found", + startFn: func(_ context.Context, _, _ string) (*openshell.Sandbox, error) { + return nil, &openshell.StatusError{Code: openshell.ErrorNotFound, Message: "sandbox not found"} + }, + wantStatus: http.StatusNotFound, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sdk := &mockSDK{} + sdk.sandboxes.startFn = tc.startFn + app := newTestAppWithSDK(sdk) + r := chi.NewRouter() + r.Post("/workspaces/{workspace}/sandboxes/{name}/start", app.StartSandbox) + + req := httptest.NewRequest(http.MethodPost, "/workspaces/default/sandboxes/my-sandbox/start", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != tc.wantStatus { + t.Errorf("status = %d, want %d; body: %s", w.Code, tc.wantStatus, w.Body.String()) + } + }) + } +} + func TestDeleteSandbox(t *testing.T) { tests := []struct { deleteFn func(ctx context.Context, workspace, name string) error diff --git a/frontend/src/api/policy.ts b/frontend/src/api/policy.ts index 0f9bd2db..7254f514 100644 --- a/frontend/src/api/policy.ts +++ b/frontend/src/api/policy.ts @@ -63,10 +63,11 @@ export const approveDraftChunk = ( workspace: string, name: string, chunkId: string, + reviewToken?: string, ): Promise => post( `${sandboxBase(workspace, name)}/drafts/${encodeURIComponent(chunkId)}/approve`, - {}, + reviewToken ? { reviewToken } : {}, ); export const rejectDraftChunk = ( @@ -190,8 +191,11 @@ const useDraftMutation = ( }; export const useApproveDraftChunk = (workspace: string, name: string) => - useDraftMutation(workspace, name, (chunkId: string) => - approveDraftChunk(workspace, name, chunkId), + useDraftMutation( + workspace, + name, + (args: { chunkId: string; reviewToken?: string }) => + approveDraftChunk(workspace, name, args.chunkId, args.reviewToken), ); export const useRejectDraftChunk = (workspace: string, name: string) => diff --git a/frontend/src/components/SandboxDraftsTab/DraftChunkRow.tsx b/frontend/src/components/SandboxDraftsTab/DraftChunkRow.tsx index 55fe1905..322bd922 100644 --- a/frontend/src/components/SandboxDraftsTab/DraftChunkRow.tsx +++ b/frontend/src/components/SandboxDraftsTab/DraftChunkRow.tsx @@ -114,7 +114,12 @@ const DraftChunkRow: React.FC = ({ @@ -145,9 +155,16 @@ const SandboxDetailPage: React.FC = (props) => { data-testid="sandbox-stop-button" isLoading={stopSandbox.isPending} isDisabled={ - stopSandbox.isPending || data.status.phase !== 'READY' + stopSandbox.isPending || !canStopSandbox(data.status.phase) + } + onClick={() => + stopSandbox.mutate(sandboxName, { + onError: (err) => + addDanger( + `Failed to stop sandbox: ${(err as Error).message}`, + ), + }) } - onClick={() => stopSandbox.mutate(sandboxName)} > Stop diff --git a/frontend/src/pages/SandboxListPage.tsx b/frontend/src/pages/SandboxListPage.tsx index 86322319..a0a7402f 100644 --- a/frontend/src/pages/SandboxListPage.tsx +++ b/frontend/src/pages/SandboxListPage.tsx @@ -105,7 +105,7 @@ const SandboxListPage: React.FC = ({ const policyViews = useSandboxPolicies(workspace, visibleNames); const providerExpiry = useProviderExpiry(workspace); const drafts = useDraftNotifications(features.draftPolicy); - const { addSuccess } = useAlerts(); + const { addSuccess, addDanger } = useAlerts(); const stopSandbox = useStopSandbox(workspace); const startSandbox = useStartSandbox(workspace); const bulkDelete = useBulkDelete( @@ -284,14 +284,26 @@ const SandboxListPage: React.FC = ({ } onNameClick={() => onSelect?.(sandbox.metadata.name)} onDelete={() => setDeleteTargets([sandbox.metadata.name])} - onStop={() => { - stopSandbox.mutate(sandbox.metadata.name); - addSuccess(`Stopping sandbox ${sandbox.metadata.name}`); - }} - onStart={() => { - startSandbox.mutate(sandbox.metadata.name); - addSuccess(`Starting sandbox ${sandbox.metadata.name}`); - }} + onStop={() => + stopSandbox.mutate(sandbox.metadata.name, { + onSuccess: () => + addSuccess(`Stopping sandbox ${sandbox.metadata.name}`), + onError: (err) => + addDanger( + `Failed to stop sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, + ), + }) + } + onStart={() => + startSandbox.mutate(sandbox.metadata.name, { + onSuccess: () => + addSuccess(`Starting sandbox ${sandbox.metadata.name}`), + onError: (err) => + addDanger( + `Failed to start sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, + ), + }) + } onViewLogs={() => viewSandbox(sandbox.metadata.name, 'logs')} onOpenTerminal={ sandbox.status.phase === 'READY' && features.terminal @@ -343,8 +355,8 @@ const SandboxListPage: React.FC = ({ } body={ deleteTargets && deleteTargets.length > 1 - ? `${deleteTargets.length} sandboxes will be permanently deleted. This is the only way to stop running sandboxes.` - : `Sandbox "${deleteTargets?.[0] ?? ''}" will be permanently deleted. This is the only way to stop a running sandbox.` + ? `${deleteTargets.length} sandboxes will be permanently deleted. This cannot be undone.` + : `Sandbox "${deleteTargets?.[0] ?? ''}" will be permanently deleted. This cannot be undone.` } confirmName={deleteTargets?.length === 1 ? deleteTargets[0] : undefined} isOpen={deleteTargets !== null} diff --git a/frontend/src/utils/sandboxLifecycle.ts b/frontend/src/utils/sandboxLifecycle.ts new file mode 100644 index 00000000..3a9be108 --- /dev/null +++ b/frontend/src/utils/sandboxLifecycle.ts @@ -0,0 +1,10 @@ +import type { SandboxPhase } from '../types'; + +// A sandbox can only be stopped from READY, and only started from STOPPED — +// shared by the list row actions and the detail page header so the two +// surfaces never disagree on which action is available. +export const canStopSandbox = (phase: SandboxPhase): boolean => + phase === 'READY'; + +export const canStartSandbox = (phase: SandboxPhase): boolean => + phase === 'STOPPED';