diff --git a/.github/workflows/terraform.yml b/.github/workflows/terraform.yml index 368be2259..b43c07a25 100644 --- a/.github/workflows/terraform.yml +++ b/.github/workflows/terraform.yml @@ -190,6 +190,8 @@ jobs: TF_VAR_modal_workspace: ${{ secrets.MODAL_WORKSPACE }} TF_VAR_modal_environment: "${{ secrets.MODAL_ENVIRONMENT || 'main' }}" TF_VAR_modal_environment_web_suffix: ${{ secrets.MODAL_ENVIRONMENT_WEB_SUFFIX }} + TF_VAR_modal_docker_sandbox_cpu: "${{ secrets.MODAL_DOCKER_SANDBOX_CPU || '4' }}" + TF_VAR_modal_docker_sandbox_memory_mb: "${{ secrets.MODAL_DOCKER_SANDBOX_MEMORY_MB || '8192' }}" TF_VAR_github_client_id: ${{ secrets.GH_OAUTH_CLIENT_ID }} TF_VAR_github_client_secret: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} TF_VAR_github_app_id: ${{ secrets.GH_APP_ID }} @@ -323,6 +325,8 @@ jobs: TF_VAR_modal_workspace: ${{ secrets.MODAL_WORKSPACE }} TF_VAR_modal_environment: "${{ secrets.MODAL_ENVIRONMENT || 'main' }}" TF_VAR_modal_environment_web_suffix: ${{ secrets.MODAL_ENVIRONMENT_WEB_SUFFIX }} + TF_VAR_modal_docker_sandbox_cpu: "${{ secrets.MODAL_DOCKER_SANDBOX_CPU || '4' }}" + TF_VAR_modal_docker_sandbox_memory_mb: "${{ secrets.MODAL_DOCKER_SANDBOX_MEMORY_MB || '8192' }}" TF_VAR_github_client_id: ${{ secrets.GH_OAUTH_CLIENT_ID }} TF_VAR_github_client_secret: ${{ secrets.GH_OAUTH_CLIENT_SECRET }} TF_VAR_github_app_id: ${{ secrets.GH_APP_ID }} diff --git a/README.md b/README.md index a556f469d..8f7bb0116 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,7 @@ Every session runs in an isolated Modal sandbox with a full development environm and UI verification - **Code-server:** Optional browser-based VS Code connected to the session workspace - **Web terminal:** ttyd-powered terminal accessible from the session UI +- **Docker:** Optional Docker Engine and Docker Compose support for Modal-backed sandboxes - **Port tunneling:** Expose up to 10 dev server ports via encrypted tunnels. URLs are available in-sandbox at `/workspace/.tunnels.env` before `.openinspect/start.sh` runs ([details](docs/HOW_IT_WORKS.md#tunnel-urls-inside-the-sandbox)) @@ -249,6 +250,8 @@ docker compose up -d postgres redis - `setup.sh` failures are non-fatal for fresh sessions, but fatal in image build mode - `start.sh` runs for every non-build session startup (fresh, repo-image, snapshot-restore) - `start.sh` failures are strict: if present and it fails, session startup fails +- When Docker is enabled, the Docker daemon starts before either hook runs; expose container ports + through sandbox tunnel ports when you need browser access - Default timeouts: - `SETUP_TIMEOUT_SECONDS` (default `300`) - `START_TIMEOUT_SECONDS` (default `120`) diff --git a/docs/GETTING_STARTED.md b/docs/GETTING_STARTED.md index f372b9bd8..f25014d6f 100644 --- a/docs/GETTING_STARTED.md +++ b/docs/GETTING_STARTED.md @@ -361,6 +361,9 @@ modal_token_secret = "your-modal-token-secret" modal_workspace = "your-modal-workspace" modal_environment = "your-modal-environment" modal_environment_web_suffix = "your-modal-web-suffix" # Lowercase letters, digits, dashes; empty for https://workspace--... endpoints +# Optional Docker-enabled sandbox resource overrides; leave unset to use Modal defaults. +# modal_docker_sandbox_cpu = 1 +# modal_docker_sandbox_memory_mb = 2048 # Daytona (only required when sandbox_provider = "daytona") # daytona_api_url = "https://app.daytona.io/api" @@ -662,46 +665,48 @@ Enable automatic deployments when you push to main by adding GitHub Secrets. Go to your fork's Settings → Secrets and variables → Actions, and add: -| Secret Name | Value | -| ------------------------------ | ------------------------------------------------------------------------------------------- | -| `CLOUDFLARE_API_TOKEN` | Your Cloudflare API token | -| `CLOUDFLARE_ACCOUNT_ID` | Your Cloudflare account ID | -| `CLOUDFLARE_WORKER_SUBDOMAIN` | Your workers.dev subdomain | -| `DEPLOYMENT_NAME` | Your deployment name | -| `R2_ACCESS_KEY_ID` | R2 access key ID | -| `R2_SECRET_ACCESS_KEY` | R2 secret access key | -| `WEB_PLATFORM` | `vercel` or `cloudflare` | -| `VERCEL_API_TOKEN` | Vercel API token _(only if `web_platform = "vercel"`)_ | -| `VERCEL_TEAM_ID` | Vercel team/account ID _(only if `web_platform = "vercel"`)_ | -| `VERCEL_PROJECT_ID` | Vercel project ID _(only if `web_platform = "vercel"`)_ | -| `NEXTAUTH_URL` | Your web app URL | -| `MODAL_TOKEN_ID` | Modal token ID | -| `MODAL_TOKEN_SECRET` | Modal token secret | -| `MODAL_WORKSPACE` | Modal workspace name | -| `MODAL_ENVIRONMENT` | Modal environment name (defaults to `main`) | -| `MODAL_ENVIRONMENT_WEB_SUFFIX` | Modal environment web suffix for endpoint URLs; lowercase letters, digits, dashes, or empty | -| `GH_OAUTH_CLIENT_ID` | GitHub App OAuth client ID | -| `GH_OAUTH_CLIENT_SECRET` | GitHub App OAuth client secret | -| `GH_APP_ID` | GitHub App ID | -| `GH_APP_PRIVATE_KEY` | GitHub App private key (PKCS#8 format) | -| `GH_APP_INSTALLATION_ID` | GitHub App installation ID | -| `ENABLE_SLACK_BOT` | `true` to deploy Slack bot, `false` to skip (default: `true`) | -| `SLACK_BOT_TOKEN` | Slack bot token (required if enabled) | -| `SLACK_SIGNING_SECRET` | Slack signing secret (required if enabled) | -| `ANTHROPIC_API_KEY` | Anthropic API key | -| `TOKEN_ENCRYPTION_KEY` | Generated encryption key (OAuth tokens) | -| `REPO_SECRETS_ENCRYPTION_KEY` | Generated encryption key (repo secrets) | -| `INTERNAL_CALLBACK_SECRET` | Generated callback secret | -| `MODAL_API_SECRET` | Generated Modal API secret | -| `NEXTAUTH_SECRET` | Generated NextAuth secret | -| `ALLOWED_USERS` | Comma-separated GitHub usernames (or empty for all users) | -| `ALLOWED_EMAIL_DOMAINS` | Comma-separated email domains (or empty for all domains) | -| `ENABLE_GITHUB_BOT` | `true` to deploy GitHub bot worker (or empty to skip) | -| `GH_WEBHOOK_SECRET` | GitHub webhook secret (required if GitHub bot enabled) | -| `GH_BOT_USERNAME` | GitHub App bot username, e.g., `my-app[bot]` (required if GitHub bot enabled) | -| `APP_NAME` | Optional display name for whitelabeling (default: `Open-Inspect`) | -| `APP_SHORT_NAME` | Optional short label for sidebar header (default: `Inspect`) | -| `APP_ICON_URL` | Optional URL to a custom logo/favicon (default: built-in icon) | +| Secret Name | Value | +| -------------------------------- | ------------------------------------------------------------------------------------------- | +| `CLOUDFLARE_API_TOKEN` | Your Cloudflare API token | +| `CLOUDFLARE_ACCOUNT_ID` | Your Cloudflare account ID | +| `CLOUDFLARE_WORKER_SUBDOMAIN` | Your workers.dev subdomain | +| `DEPLOYMENT_NAME` | Your deployment name | +| `R2_ACCESS_KEY_ID` | R2 access key ID | +| `R2_SECRET_ACCESS_KEY` | R2 secret access key | +| `WEB_PLATFORM` | `vercel` or `cloudflare` | +| `VERCEL_API_TOKEN` | Vercel API token _(only if `web_platform = "vercel"`)_ | +| `VERCEL_TEAM_ID` | Vercel team/account ID _(only if `web_platform = "vercel"`)_ | +| `VERCEL_PROJECT_ID` | Vercel project ID _(only if `web_platform = "vercel"`)_ | +| `NEXTAUTH_URL` | Your web app URL | +| `MODAL_TOKEN_ID` | Modal token ID | +| `MODAL_TOKEN_SECRET` | Modal token secret | +| `MODAL_WORKSPACE` | Modal workspace name | +| `MODAL_ENVIRONMENT` | Modal environment name (defaults to `main`) | +| `MODAL_ENVIRONMENT_WEB_SUFFIX` | Modal environment web suffix for endpoint URLs; lowercase letters, digits, dashes, or empty | +| `MODAL_DOCKER_SANDBOX_CPU` | Optional CPU cores for Docker-enabled Modal sandboxes; unset uses Modal defaults | +| `MODAL_DOCKER_SANDBOX_MEMORY_MB` | Optional memory in MB for Docker-enabled Modal sandboxes; unset uses Modal defaults | +| `GH_OAUTH_CLIENT_ID` | GitHub App OAuth client ID | +| `GH_OAUTH_CLIENT_SECRET` | GitHub App OAuth client secret | +| `GH_APP_ID` | GitHub App ID | +| `GH_APP_PRIVATE_KEY` | GitHub App private key (PKCS#8 format) | +| `GH_APP_INSTALLATION_ID` | GitHub App installation ID | +| `ENABLE_SLACK_BOT` | `true` to deploy Slack bot, `false` to skip (default: `true`) | +| `SLACK_BOT_TOKEN` | Slack bot token (required if enabled) | +| `SLACK_SIGNING_SECRET` | Slack signing secret (required if enabled) | +| `ANTHROPIC_API_KEY` | Anthropic API key | +| `TOKEN_ENCRYPTION_KEY` | Generated encryption key (OAuth tokens) | +| `REPO_SECRETS_ENCRYPTION_KEY` | Generated encryption key (repo secrets) | +| `INTERNAL_CALLBACK_SECRET` | Generated callback secret | +| `MODAL_API_SECRET` | Generated Modal API secret | +| `NEXTAUTH_SECRET` | Generated NextAuth secret | +| `ALLOWED_USERS` | Comma-separated GitHub usernames (or empty for all users) | +| `ALLOWED_EMAIL_DOMAINS` | Comma-separated email domains (or empty for all domains) | +| `ENABLE_GITHUB_BOT` | `true` to deploy GitHub bot worker (or empty to skip) | +| `GH_WEBHOOK_SECRET` | GitHub webhook secret (required if GitHub bot enabled) | +| `GH_BOT_USERNAME` | GitHub App bot username, e.g., `my-app[bot]` (required if GitHub bot enabled) | +| `APP_NAME` | Optional display name for whitelabeling (default: `Open-Inspect`) | +| `APP_SHORT_NAME` | Optional short label for sidebar header (default: `Inspect`) | +| `APP_ICON_URL` | Optional URL to a custom logo/favicon (default: built-in icon) | **Bulk upload secrets with `gh` CLI:** diff --git a/docs/HOW_IT_WORKS.md b/docs/HOW_IT_WORKS.md index e05d016e3..6b32b4eb1 100644 --- a/docs/HOW_IT_WORKS.md +++ b/docs/HOW_IT_WORKS.md @@ -143,6 +143,7 @@ development environment. - Package managers: npm, pnpm, pip, uv - agent-browser CLI + headless Chrome (for browser automation) - OpenCode (the coding agent) +- Optional Docker Engine and Docker Compose for Modal-backed sandboxes Open-Inspect supports two backend patterns: @@ -153,6 +154,10 @@ Modal is still the only backend with repo-image builds and live filesystem snaps uses persistent sandboxes instead: the control plane stops the sandbox on inactivity or stale heartbeat, then resumes that same sandbox later with the same logical sandbox ID and auth token. +Docker support is Modal-only and controlled by the sandbox `dockerEnabled` setting. Docker-enabled +sandboxes use Modal's default CPU and memory requests unless deployment operators set +`MODAL_DOCKER_SANDBOX_CPU` or `MODAL_DOCKER_SANDBOX_MEMORY_MB`. + ### Clients Clients are how users interact with sessions. The architecture is client-agnostic—any client that @@ -192,10 +197,11 @@ When you create a session for a repo without an existing snapshot: 1. **Sandbox created**: Modal spins up a new container from the base image 2. **Git sync**: Clones your repository using brokered SCM credentials from the git credential helper -3. **Setup script**: Runs `.openinspect/setup.sh` for provisioning (if present) -4. **Start script**: Runs `.openinspect/start.sh` for runtime startup (if present) -5. **Agent start**: OpenCode server starts and connects back to the control plane -6. **Ready**: Sandbox accepts prompts +3. **Docker start**: If enabled, starts `dockerd` before repository hooks run +4. **Setup script**: Runs `.openinspect/setup.sh` for provisioning (if present) +5. **Start script**: Runs `.openinspect/start.sh` for runtime startup (if present) +6. **Agent start**: OpenCode server starts and connects back to the control plane +7. **Ready**: Sandbox accepts prompts ### Restore (From Snapshot) @@ -210,8 +216,9 @@ When restoring from a previous snapshot: 1. **Restore snapshot**: Modal restores the filesystem from a saved image 2. **Quick sync**: Pulls latest changes (usually just a few commits) -3. **Start script**: Runs `.openinspect/start.sh` for runtime startup (if present) -4. **Ready**: Sandbox is ready almost instantly +3. **Docker start**: If enabled, clears stale Docker runtime state and starts `dockerd` +4. **Start script**: Runs `.openinspect/start.sh` for runtime startup (if present) +5. **Ready**: Sandbox is ready almost instantly Snapshots include installed dependencies, built artifacts, and workspace state. This is why follow-up prompts in an existing session are much faster than the first prompt. @@ -221,12 +228,17 @@ follow-up prompts in an existing session are much faster than the first prompt. When starting from a pre-built repo image: 1. **Incremental git sync**: Fast fetch + hard reset to latest branch head -2. **Setup skipped**: `.openinspect/setup.sh` already ran when the image was built -3. **Start script runs**: `.openinspect/start.sh` executes for per-session runtime startup -4. **Ready**: Agent starts once runtime hook succeeds +2. **Docker start**: If enabled, starts Docker before runtime hooks +3. **Setup skipped**: `.openinspect/setup.sh` already ran when the image was built +4. **Start script runs**: `.openinspect/start.sh` executes for per-session runtime startup +5. **Ready**: Agent starts once runtime hook succeeds If `start.sh` exists and fails, startup fails fast instead of continuing with a broken runtime. +When Docker is enabled, Open-Inspect uses `/opt/docker-data` as Docker's data root. On every boot it +removes stale daemon runtime directories and sockets while preserving images, layers, volumes, and +BuildKit cache so snapshots and repo images can still accelerate Docker-heavy projects. + ### When Snapshots Are Taken - **After successful prompt completion**: Preserves the workspace state diff --git a/docs/IMAGE_PREBUILD.md b/docs/IMAGE_PREBUILD.md index c93a45b2e..d9e81ed0d 100644 --- a/docs/IMAGE_PREBUILD.md +++ b/docs/IMAGE_PREBUILD.md @@ -73,6 +73,10 @@ The build process runs the same setup steps that a normal session would: Everything your setup script installs — dependencies, build artifacts, caches — is captured in the snapshot. +If Docker is enabled for the repository, the build sandbox starts the Docker daemon before +`.openinspect/setup.sh` runs. Docker images, volumes, layers, and BuildKit cache under +`/opt/docker-data` are included in the saved repo image. + ### What Happens When You Start a Session When you create a new session for a repository with a pre-built image: @@ -101,11 +105,33 @@ are some tips: - **Warm caches** — Running your test suite once during setup means cached files are available for subsequent runs in the session - **Pre-download large resources** — Models, datasets, or any large files the agent might need +- **Pre-pull or build Docker images** — `docker compose pull` or `docker compose build` can warm + image layers for Docker-enabled repositories Don't worry about build duration. Builds run in the background and users always get the last _successfully_ built image. A 10-minute build is worthwhile if it saves 10 minutes on every session start. +### Docker-Enabled Images + +Docker-enabled and non-Docker repo images are tracked separately. If you turn Docker on for a repo, +Open-Inspect will build or select an image that was created with Docker support instead of reusing a +non-Docker image. + +Changing Docker settings changes the required image profile for affected repositories. If a global +Docker default is enabled, any image-build-enabled repository that inherits that default will build +a Docker-profile image on the next scheduled or manual build if one is not already ready. Existing +opposite-profile images are retained rather than deleted, so they can be reused if the repository +later switches back. + +For Docker Compose projects, keep long-running services in `.openinspect/start.sh` and use +`.openinspect/setup.sh` for install, build, pull, and cache-warming work. If setup starts containers +as part of a build, stop them before the script exits, for example with `docker compose down`, so +the snapshot contains reusable image and volume data without stale running container state. + +Container ports are not automatically public. Add any browser-facing service ports to the sandbox +tunnel port settings, then start the matching Compose services from `.openinspect/start.sh`. + --- ## Troubleshooting diff --git a/docs/adr/0003-sandbox-runtime-capabilities-intent-vs-realization.md b/docs/adr/0003-sandbox-runtime-capabilities-intent-vs-realization.md new file mode 100644 index 000000000..ae5a087c9 --- /dev/null +++ b/docs/adr/0003-sandbox-runtime-capabilities-intent-vs-realization.md @@ -0,0 +1,87 @@ +# ADR 0003: Sandbox Runtime Capabilities — Intent vs. Provider Realization + +## Status + +Accepted + +## Context + +Docker support for Modal sandboxes (PR #697) surfaced a layering problem at the control-plane ↔ +sandbox-provider boundary. Docker is the first of several runtime capabilities that will span +providers: Daytona will gain Docker, Vercel is being added (also Docker-capable), and the end state +is most providers supporting Docker while it stays optional for some. Modal is the odd one out — +Docker there is an experimental opt-in that uniquely requires a different base image, a launch flag, +in-sandbox network plumbing, and dockerd supervision, so on Modal "Docker on" ⟺ "a different image +lineage." Other providers will not couple Docker to image identity. + +The initial implementation entangled three distinct concepts and shipped the wrong one across the +boundary: + +- The provider-agnostic **intent** (`dockerEnabled`) was discarded at the boundary. +- A Modal-named **image profile** was shipped to every provider as the Docker signal, conflating the + (shareable) environment **identity** with Modal's concrete image **realization**. +- Provider identity was re-derived by string comparison (`=== "modal"`, `isModalSandboxBackend`) in + routes, the durable object, and the web app, so adding Docker to a second provider meant growing + OR-lists across three packages. + +## Decision + +1. **Capabilities are the single source of truth.** `SandboxProviderCapabilities` and a + `PROVIDER_CAPABILITIES` table live in `@open-inspect/shared`. Providers set their capabilities + from the table; routes, the durable object, and the web app gate behavior on capabilities + (`supportsDocker`, `supportsPrebuiltImages`, `supportsDashboardUrl`, …) via + `getProviderCapabilities`. Code must not branch on the provider **name** outside the provider + factory. Distinct features get distinct flags — "is modal" never stands in for an unrelated + capability. + +2. **Intent crosses the boundary; providers realize it.** `RequestedSandboxRuntime` (e.g. + `{ docker }`) is declarative, provider-agnostic intent carried on `CreateSandboxConfig` / + `RestoreConfig`. Each provider translates intent into its own mechanism. Intent crosses even when + the environment id alone does not encode behavior — a provider with Docker in its base image + realizes the same environment yet still must start dockerd. + +3. **Environment identity is shared; image realization is provider-private.** The logical + environment id (`SandboxImageProfile`: `"default" | "docker"`, …) is a shared, typed concept. A + single `resolveEnvironment(intent, capabilities)` maps intent → environment id and enforces + capability gating. The lifecycle manager speaks only this shared id (snapshot/prebuilt-image + compatibility keys on it) and never sees a concrete provider image. Each provider maps the id to + a concrete image internally. + +4. **On Modal, `docker ↔ image_profile` is intentionally 1:1 and Modal-specific, and the Modal HTTP + wire is frozen.** The Modal provider realizes the shared environment id as its `image_profile` + request field; `select_runtime_image`, `DockerLaunchSettings`, the experimental launch flag, and + dockerd supervision stay inside Modal. This refactor changes only the in-process control-plane + provider boundary, not the Modal client request bodies, `web_api.py` parsing, or the + `image_profile` columns — so control-plane, Modal, and web deploy independently. + +## Consequences + +### Positive + +- Adding a provider, or flipping a capability (e.g. Daytona `supportsDocker`), is a table edit plus + the provider's own realization — no edits to routes, web gating, or the lifecycle manager. +- The next runtime capability (e.g. GPU) extends `RequestedSandboxRuntime` and each provider's + realization without reshaping the boundary. +- The environment id is shared and unifiable — the seed of a future cross-provider environment + catalog (multiple named environments per provider). + +### Negative + +- The environment-id vocabulary is intentionally duplicated across shared TypeScript, Python + (`settings.py`), and the D1 `CHECK` constraint. This is the shared contract, not an accidental + leak; the definitions must be kept in sync. +- Carrying both `requestedRuntime` (intent) and `environment` (resolved id) is mildly redundant + today (the latter is derived from the former), but it is the forward-compatible shape: intent + drives provider behavior, the environment id drives image selection and snapshot compatibility. + +## Follow-Up Rules + +- New runtime capabilities are added to `RequestedSandboxRuntime` and realized per provider; never + reshape the boundary for a single provider's mechanism. +- Gate behavior on `PROVIDER_CAPABILITIES`, never on the provider name (the provider factory is the + one allowed `name → provider` site). +- Keep the environment-id type and `resolveEnvironment` in `@open-inspect/shared`; image realization + (concrete `modal.Image` / snapshot) stays inside the provider. +- When a second provider gains a feature currently realized Modal-side (e.g. a dashboard URL), make + the realization provider-aware (or move it onto the provider) rather than widening a + Modal-specific helper behind a capability flag. diff --git a/packages/control-plane/package.json b/packages/control-plane/package.json index 4e7f6f019..027a92934 100644 --- a/packages/control-plane/package.json +++ b/packages/control-plane/package.json @@ -4,7 +4,7 @@ "private": true, "type": "module", "scripts": { - "build": "esbuild src/index.ts --bundle --format=esm --outfile=dist/index.js --platform=browser --target=es2022 --external:cloudflare:* --external:node:*", + "build": "esbuild src/index.ts --bundle --format=esm --outfile=dist/index.js --platform=browser --target=es2022 --external:cloudflare:* --external:node:* --alias:@open-inspect/shared=../shared/src/index.ts", "test": "vitest run", "test:coverage": "vitest run --coverage", "test:integration": "vitest run --config vitest.integration.config.ts", diff --git a/packages/control-plane/src/db/integration-settings.test.ts b/packages/control-plane/src/db/integration-settings.test.ts index 3b2eed8c2..8586b8580 100644 --- a/packages/control-plane/src/db/integration-settings.test.ts +++ b/packages/control-plane/src/db/integration-settings.test.ts @@ -27,6 +27,8 @@ const QUERY_PATTERNS = { DELETE_GLOBAL: /^DELETE FROM integration_settings WHERE integration_id = \?$/, SELECT_REPO: /^SELECT settings FROM integration_repo_settings WHERE integration_id = \? AND repo = \?$/, + SELECT_REPO_BATCH: + /^SELECT repo, settings FROM integration_repo_settings WHERE integration_id = \? AND repo IN \(\?(, \?)*\)$/, UPSERT_REPO: /^INSERT INTO integration_repo_settings/, DELETE_REPO: /^DELETE FROM integration_repo_settings WHERE integration_id = \? AND repo = \?$/, LIST_REPO: /^SELECT repo, settings FROM integration_repo_settings WHERE integration_id = \?$/, @@ -39,6 +41,7 @@ function normalizeQuery(query: string): string { class FakeD1Database { private globalRows = new Map(); private repoRows = new Map(); + public allQueries: Array<{ query: string; args: unknown[] }> = []; private repoKey(integrationId: string, repo: string): string { return `${integrationId}:${repo}`; @@ -68,6 +71,19 @@ class FakeD1Database { all(query: string, args: unknown[]) { const normalized = normalizeQuery(query); + this.allQueries.push({ query: normalized, args }); + + if (QUERY_PATTERNS.SELECT_REPO_BATCH.test(normalized)) { + const [integrationId, ...repos] = args as [string, ...string[]]; + const repoSet = new Set(repos); + const results: Array<{ repo: string; settings: string }> = []; + for (const row of this.repoRows.values()) { + if (row.integration_id === integrationId && repoSet.has(row.repo)) { + results.push({ repo: row.repo, settings: row.settings }); + } + } + return results; + } if (QUERY_PATTERNS.LIST_REPO.test(normalized)) { const [integrationId] = args as [string]; @@ -338,6 +354,21 @@ describe("IntegrationSettingsStore", () => { expect(repos).toEqual(["acme/gadgets", "acme/widgets"]); }); + it("batch lookup returns only requested repo overrides", async () => { + await store.setRepoSettings("github", "acme/widgets", { + model: "anthropic/claude-opus-4-6", + }); + await store.setRepoSettings("github", "acme/gadgets", { + model: "anthropic/claude-haiku-4-5", + }); + + const batch = await store.getRepoSettingsBatch("github", ["Acme/Widgets", "acme/missing"]); + + expect([...batch.keys()]).toEqual(["acme/widgets"]); + expect(batch.get("acme/widgets")?.model).toBe("anthropic/claude-opus-4-6"); + expect(db.allQueries.some((q) => q.query.includes("repo IN"))).toBe(true); + }); + it("normalizes repo name to lowercase on write and lookup", async () => { await store.setRepoSettings("github", "Acme/Widgets", { model: "anthropic/claude-opus-4-6", @@ -663,6 +694,7 @@ describe("IntegrationSettingsStore", () => { await store.setGlobal("sandbox", { defaults: { tunnelPorts: [3000, 3001], + dockerEnabled: true, maxConcurrentChildSessions: 3, maxTotalChildSessions: 8, }, @@ -672,6 +704,7 @@ describe("IntegrationSettingsStore", () => { expect(result).toEqual({ defaults: { tunnelPorts: [3000, 3001], + dockerEnabled: true, maxConcurrentChildSessions: 3, maxTotalChildSessions: 8, }, @@ -714,6 +747,35 @@ describe("IntegrationSettingsStore", () => { }); }); + it("getResolvedConfig lets repo dockerEnabled override global defaults", async () => { + await store.setGlobal("sandbox", { defaults: { dockerEnabled: true } }); + await store.setRepoSettings("sandbox", "acme/app", { dockerEnabled: false }); + + const config = await store.getResolvedConfig("sandbox", "acme/app"); + expect(config.settings.dockerEnabled).toBe(false); + }); + + it("getResolvedConfigs only reads requested repo overrides", async () => { + await store.setGlobal("sandbox", { defaults: { dockerEnabled: true } }); + await store.setRepoSettings("sandbox", "acme/app", { dockerEnabled: false }); + await store.setRepoSettings("sandbox", "acme/unrelated", { tunnelPorts: [5173] }); + + const configs = await store.getResolvedConfigs("sandbox", ["Acme/App", "acme/missing"]); + + expect(configs.get("acme/app")?.settings).toEqual({ dockerEnabled: false }); + expect(configs.get("acme/missing")?.settings).toEqual({ dockerEnabled: true }); + expect(db.allQueries.some((q) => q.query.includes("repo IN"))).toBe(true); + expect(db.allQueries.some((q) => QUERY_PATTERNS.LIST_REPO.test(q.query))).toBe(false); + }); + + it("rejects non-boolean dockerEnabled", async () => { + await expect( + store.setRepoSettings("sandbox", "acme/app", { + dockerEnabled: "true" as unknown as boolean, + }) + ).rejects.toThrow("dockerEnabled must be a boolean"); + }); + it("rejects non-array tunnelPorts", async () => { await expect( store.setGlobal("sandbox", { diff --git a/packages/control-plane/src/db/integration-settings.ts b/packages/control-plane/src/db/integration-settings.ts index 95224d948..ce644607c 100644 --- a/packages/control-plane/src/db/integration-settings.ts +++ b/packages/control-plane/src/db/integration-settings.ts @@ -17,6 +17,17 @@ import { type SettingsLevel = "global" | "repo"; const SLACK_MENTIONS_POLICIES = ["allow", "escape", "strip"] as const; +const REPO_SETTINGS_BATCH_SIZE = 99; + +function mergeDefinedSettings(defaults: object, overrides: object): Record { + const settings: Record = { ...defaults }; + for (const [key, value] of Object.entries(overrides)) { + if (value !== undefined) { + settings[key] = value; + } + } + return settings; +} export class IntegrationSettingsValidationError extends Error { constructor(message: string) { @@ -105,6 +116,36 @@ export class IntegrationSettingsStore { return JSON.parse(row.settings) as IntegrationSettingsMap[K]["repo"]; } + async getRepoSettingsBatch( + integrationId: K, + repos: string[] + ): Promise> { + const normalizedRepos = [...new Set(repos.map((repo) => repo.toLowerCase()))]; + const settingsByRepo = new Map(); + if (normalizedRepos.length === 0) return settingsByRepo; + + for (let start = 0; start < normalizedRepos.length; start += REPO_SETTINGS_BATCH_SIZE) { + const chunk = normalizedRepos.slice(start, start + REPO_SETTINGS_BATCH_SIZE); + const placeholders = chunk.map(() => "?").join(", "); + const { results } = await this.db + .prepare( + `SELECT repo, settings FROM integration_repo_settings + WHERE integration_id = ? AND repo IN (${placeholders})` + ) + .bind(integrationId, ...chunk) + .all<{ repo: string; settings: string }>(); + + for (const row of results) { + settingsByRepo.set( + row.repo.toLowerCase(), + JSON.parse(row.settings) as IntegrationSettingsMap[K]["repo"] + ); + } + } + + return settingsByRepo; + } + async setRepoSettings( integrationId: K, repo: string, @@ -163,20 +204,49 @@ export class IntegrationSettingsStore { const defaults = globalSettings?.defaults ?? {}; const overrides = repoSettings ?? {}; - - // Generic merge: repo overrides win, undefined keys don't clobber defaults - const settings: Record = { ...defaults }; - for (const [key, value] of Object.entries(overrides)) { - if (value !== undefined) { - settings[key] = value; - } - } + const settings = mergeDefinedSettings(defaults, overrides); return { enabledRepos, settings } as ResolvedIntegrationConfig< NonNullable >; } + async getResolvedConfigs( + integrationId: K, + repos: string[] + ): Promise< + Map< + string, + ResolvedIntegrationConfig> + > + > { + const normalizedRepos = [...new Set(repos.map((repo) => repo.toLowerCase()))]; + const configs = new Map< + string, + ResolvedIntegrationConfig> + >(); + if (normalizedRepos.length === 0) return configs; + + const [globalSettings, repoSettings] = await Promise.all([ + this.getGlobal(integrationId), + this.getRepoSettingsBatch(integrationId, normalizedRepos), + ]); + + const enabledRepos = + globalSettings?.enabledRepos !== undefined ? globalSettings.enabledRepos : null; + const defaults = globalSettings?.defaults ?? {}; + + for (const repo of normalizedRepos) { + const overrides = repoSettings.get(repo) ?? {}; + const settings = mergeDefinedSettings(defaults, overrides); + configs.set(repo, { enabledRepos, settings } as ResolvedIntegrationConfig< + NonNullable + >); + } + + return configs; + } + private validateAndNormalizeSettings( integrationId: K, settings: IntegrationSettingsMap[K]["repo"], @@ -315,6 +385,10 @@ export class IntegrationSettingsStore { throw new IntegrationSettingsValidationError("terminalEnabled must be a boolean"); } + if (settings.dockerEnabled !== undefined && typeof settings.dockerEnabled !== "boolean") { + throw new IntegrationSettingsValidationError("dockerEnabled must be a boolean"); + } + this.validatePositiveIntegerSetting( settings.maxConcurrentChildSessions, "maxConcurrentChildSessions" diff --git a/packages/control-plane/src/db/repo-images.test.ts b/packages/control-plane/src/db/repo-images.test.ts index 00a684e26..b93a6fc96 100644 --- a/packages/control-plane/src/db/repo-images.test.ts +++ b/packages/control-plane/src/db/repo-images.test.ts @@ -11,22 +11,24 @@ type RepoImageRow = { status: string; build_duration_seconds: number | null; error_message: string | null; + image_profile: "default" | "docker"; created_at: number; }; const QUERY_PATTERNS = { INSERT_BUILD: /^INSERT INTO repo_images/, - SELECT_BY_ID: /^SELECT repo_owner, repo_name, base_branch FROM repo_images WHERE id = \?$/, + SELECT_BY_ID: + /^SELECT repo_owner, repo_name, base_branch, image_profile FROM repo_images WHERE id = \?$/, SELECT_READY_FOR_REPO: - /^SELECT id, provider_image_id FROM repo_images WHERE repo_owner = \? AND repo_name = \? AND base_branch = \? AND status = 'ready'$/, + /^SELECT id, provider_image_id FROM repo_images WHERE repo_owner = \? AND repo_name = \? AND base_branch = \? AND image_profile = \? AND status = 'ready'$/, UPDATE_READY: /^UPDATE repo_images SET status = 'ready', provider_image_id = \?, base_sha = \?, build_duration_seconds = \? WHERE id = \?$/, DELETE_BY_ID: /^DELETE FROM repo_images WHERE id = \?$/, UPDATE_FAILED: /^UPDATE repo_images SET status = 'failed', error_message = \? WHERE id = \?$/, SELECT_LATEST_READY: - /^SELECT ri\.\* FROM repo_images ri INNER JOIN repo_metadata rm ON ri\.repo_owner = rm\.repo_owner AND ri\.repo_name = rm\.repo_name WHERE ri\.repo_owner = \? AND ri\.repo_name = \? AND ri\.status = 'ready' AND rm\.image_build_enabled = 1 ORDER BY ri\.created_at DESC LIMIT 1$/, + /^SELECT ri\.\* FROM repo_images ri INNER JOIN repo_metadata rm ON ri\.repo_owner = rm\.repo_owner AND ri\.repo_name = rm\.repo_name WHERE ri\.repo_owner = \? AND ri\.repo_name = \? AND ri\.status = 'ready' AND ri\.image_profile = \? AND rm\.image_build_enabled = 1 ORDER BY ri\.created_at DESC LIMIT 1$/, SELECT_LATEST_READY_WITH_BRANCH: - /^SELECT ri\.\* FROM repo_images ri INNER JOIN repo_metadata rm ON ri\.repo_owner = rm\.repo_owner AND ri\.repo_name = rm\.repo_name WHERE ri\.repo_owner = \? AND ri\.repo_name = \? AND ri\.base_branch = \? AND ri\.status = 'ready' AND rm\.image_build_enabled = 1 ORDER BY ri\.created_at DESC LIMIT 1$/, + /^SELECT ri\.\* FROM repo_images ri INNER JOIN repo_metadata rm ON ri\.repo_owner = rm\.repo_owner AND ri\.repo_name = rm\.repo_name WHERE ri\.repo_owner = \? AND ri\.repo_name = \? AND ri\.base_branch = \? AND ri\.status = 'ready' AND ri\.image_profile = \? AND rm\.image_build_enabled = 1 ORDER BY ri\.created_at DESC LIMIT 1$/, SELECT_STATUS: /^SELECT \* FROM repo_images WHERE repo_owner = \? AND repo_name = \? ORDER BY created_at DESC LIMIT 10$/, SELECT_ALL_STATUS: /^SELECT \* FROM repo_images ORDER BY created_at DESC LIMIT 100$/, @@ -65,17 +67,23 @@ class FakeD1Database { const [id] = args as [string]; const row = this.rows.get(id); return row - ? { repo_owner: row.repo_owner, repo_name: row.repo_name, base_branch: row.base_branch } + ? { + repo_owner: row.repo_owner, + repo_name: row.repo_name, + base_branch: row.base_branch, + image_profile: row.image_profile, + } : null; } if (QUERY_PATTERNS.SELECT_READY_FOR_REPO.test(normalized)) { - const [owner, name, branch] = args as [string, string, string]; + const [owner, name, branch, imageProfile] = args as [string, string, string, string]; for (const row of this.rows.values()) { if ( row.repo_owner === owner && row.repo_name === name && row.base_branch === branch && + row.image_profile === imageProfile && row.status === "ready" ) { return { id: row.id, provider_image_id: row.provider_image_id }; @@ -85,7 +93,7 @@ class FakeD1Database { } if (QUERY_PATTERNS.SELECT_LATEST_READY_WITH_BRANCH.test(normalized)) { - const [owner, name, branch] = args as [string, string, string]; + const [owner, name, branch, imageProfile] = args as [string, string, string, string]; if (!this.isImageBuildEnabled(owner, name)) return null; let latest: RepoImageRow | null = null; for (const row of this.rows.values()) { @@ -93,6 +101,7 @@ class FakeD1Database { row.repo_owner === owner && row.repo_name === name && row.base_branch === branch && + row.image_profile === imageProfile && row.status === "ready" ) { if (!latest || row.created_at > latest.created_at) { @@ -104,11 +113,16 @@ class FakeD1Database { } if (QUERY_PATTERNS.SELECT_LATEST_READY.test(normalized)) { - const [owner, name] = args as [string, string]; + const [owner, name, imageProfile] = args as [string, string, string]; if (!this.isImageBuildEnabled(owner, name)) return null; let latest: RepoImageRow | null = null; for (const row of this.rows.values()) { - if (row.repo_owner === owner && row.repo_name === name && row.status === "ready") { + if ( + row.repo_owner === owner && + row.repo_name === name && + row.image_profile === imageProfile && + row.status === "ready" + ) { if (!latest || row.created_at > latest.created_at) { latest = row; } @@ -149,14 +163,22 @@ class FakeD1Database { const normalized = normalizeQuery(query); if (QUERY_PATTERNS.INSERT_BUILD.test(normalized)) { - // SQL: INSERT ... VALUES (?, ?, ?, ?, '', 'building', '', ?) - // Bound args: [id, owner, name, branch, createdAt] - const [id, owner, name, branch, createdAt] = args as [string, string, string, string, number]; + // SQL: INSERT ... VALUES (?, ?, ?, ?, ?, '', 'building', '', ?) + // Bound args: [id, owner, name, branch, imageProfile, createdAt] + const [id, owner, name, branch, imageProfile, createdAt] = args as [ + string, + string, + string, + string, + "default" | "docker", + number, + ]; this.rows.set(id, { id, repo_owner: owner, repo_name: name, base_branch: branch, + image_profile: imageProfile, provider_image_id: "", status: "building", base_sha: "", @@ -296,6 +318,20 @@ describe("RepoImageStore", () => { expect(status[0].repo_name).toBe("repo"); expect(status[0].provider_image_id).toBe(""); expect(status[0].base_sha).toBe(""); + expect(status[0].image_profile).toBe("default"); + }); + + it("records Docker capability for Docker-enabled builds", async () => { + await store.registerBuild({ + id: "img-docker", + repoOwner: "Acme", + repoName: "Repo", + baseBranch: "main", + imageProfile: "docker", + }); + + const status = await store.getStatus("acme", "repo"); + expect(status[0].image_profile).toBe("docker"); }); it("normalizes owner and name to lowercase", async () => { @@ -434,6 +470,36 @@ describe("RepoImageStore", () => { expect(result!.provider_image_id).toBe("modal-img-1"); }); + it("filters latest ready images by Docker capability", async () => { + db.setImageBuildEnabled("acme", "repo", true); + await store.registerBuild({ + id: "img-plain", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + }); + await store.markReady("img-plain", "modal-img-plain", "sha1", 30); + + vi.advanceTimersByTime(1000); + + await store.registerBuild({ + id: "img-docker", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + imageProfile: "docker", + }); + await store.markReady("img-docker", "modal-img-docker", "sha2", 35); + + const plain = await store.getLatestReady("acme", "repo", "main", "default"); + expect(plain).not.toBeNull(); + expect(plain!.id).toBe("img-plain"); + + const docker = await store.getLatestReady("acme", "repo", "main", "docker"); + expect(docker).not.toBeNull(); + expect(docker!.id).toBe("img-docker"); + }); + it("returns null when image_build_enabled is false", async () => { db.setImageBuildEnabled("acme", "repo", false); await store.registerBuild({ @@ -548,6 +614,36 @@ describe("RepoImageStore", () => { expect(mainImage).not.toBeNull(); expect(mainImage!.id).toBe("img-main"); }); + + it("only replaces the previous ready image with the same Docker capability", async () => { + db.setImageBuildEnabled("acme", "repo", true); + await store.registerBuild({ + id: "img-plain", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + }); + await store.markReady("img-plain", "modal-img-plain", "sha-plain", 30); + + vi.advanceTimersByTime(1000); + + await store.registerBuild({ + id: "img-docker", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + imageProfile: "docker", + }); + const result = await store.markReady("img-docker", "modal-img-docker", "sha-docker", 25); + + expect(result.replacedImageId).toBeNull(); + const plain = await store.getLatestReady("acme", "repo", "main", "default"); + const docker = await store.getLatestReady("acme", "repo", "main", "docker"); + expect(plain).not.toBeNull(); + expect(plain!.id).toBe("img-plain"); + expect(docker).not.toBeNull(); + expect(docker!.id).toBe("img-docker"); + }); }); describe("getStatus", () => { diff --git a/packages/control-plane/src/db/repo-images.ts b/packages/control-plane/src/db/repo-images.ts index 339dac860..59a4a117c 100644 --- a/packages/control-plane/src/db/repo-images.ts +++ b/packages/control-plane/src/db/repo-images.ts @@ -1,8 +1,11 @@ +import { DEFAULT_SANDBOX_IMAGE_PROFILE, type SandboxImageProfile } from "@open-inspect/shared"; + export interface RepoImageBuild { id: string; repoOwner: string; repoName: string; baseBranch: string; + imageProfile?: SandboxImageProfile; } export interface RepoImage { @@ -15,6 +18,7 @@ export interface RepoImage { status: "building" | "ready" | "failed"; build_duration_seconds: number | null; error_message: string | null; + image_profile: SandboxImageProfile; created_at: number; } @@ -25,14 +29,15 @@ export class RepoImageStore { const now = Date.now(); await this.db .prepare( - `INSERT INTO repo_images (id, repo_owner, repo_name, base_branch, provider_image_id, status, base_sha, created_at) - VALUES (?, ?, ?, ?, '', 'building', '', ?)` + `INSERT INTO repo_images (id, repo_owner, repo_name, base_branch, image_profile, provider_image_id, status, base_sha, created_at) + VALUES (?, ?, ?, ?, ?, '', 'building', '', ?)` ) .bind( build.id, build.repoOwner.toLowerCase(), build.repoName.toLowerCase(), build.baseBranch, + build.imageProfile ?? DEFAULT_SANDBOX_IMAGE_PROFILE, now ) .run(); @@ -45,17 +50,29 @@ export class RepoImageStore { buildDurationSeconds: number ): Promise<{ replacedImageId: string | null }> { const build = await this.db - .prepare("SELECT repo_owner, repo_name, base_branch FROM repo_images WHERE id = ?") + .prepare( + "SELECT repo_owner, repo_name, base_branch, image_profile FROM repo_images WHERE id = ?" + ) .bind(buildId) - .first<{ repo_owner: string; repo_name: string; base_branch: string }>(); + .first<{ + repo_owner: string; + repo_name: string; + base_branch: string; + image_profile?: SandboxImageProfile; + }>(); if (!build) return { replacedImageId: null }; const oldReady = await this.db .prepare( - "SELECT id, provider_image_id FROM repo_images WHERE repo_owner = ? AND repo_name = ? AND base_branch = ? AND status = 'ready'" + "SELECT id, provider_image_id FROM repo_images WHERE repo_owner = ? AND repo_name = ? AND base_branch = ? AND image_profile = ? AND status = 'ready'" + ) + .bind( + build.repo_owner, + build.repo_name, + build.base_branch, + build.image_profile ?? DEFAULT_SANDBOX_IMAGE_PROFILE ) - .bind(build.repo_owner, build.repo_name, build.base_branch) .first<{ id: string; provider_image_id: string }>(); const statements: D1PreparedStatement[] = [ @@ -85,7 +102,8 @@ export class RepoImageStore { async getLatestReady( repoOwner: string, repoName: string, - baseBranch?: string + baseBranch?: string, + imageProfile: SandboxImageProfile = DEFAULT_SANDBOX_IMAGE_PROFILE ): Promise { if (baseBranch) { return this.db @@ -93,10 +111,11 @@ export class RepoImageStore { `SELECT ri.* FROM repo_images ri INNER JOIN repo_metadata rm ON ri.repo_owner = rm.repo_owner AND ri.repo_name = rm.repo_name WHERE ri.repo_owner = ? AND ri.repo_name = ? AND ri.base_branch = ? AND ri.status = 'ready' + AND ri.image_profile = ? AND rm.image_build_enabled = 1 ORDER BY ri.created_at DESC LIMIT 1` ) - .bind(repoOwner.toLowerCase(), repoName.toLowerCase(), baseBranch) + .bind(repoOwner.toLowerCase(), repoName.toLowerCase(), baseBranch, imageProfile) .first(); } return this.db @@ -104,10 +123,11 @@ export class RepoImageStore { `SELECT ri.* FROM repo_images ri INNER JOIN repo_metadata rm ON ri.repo_owner = rm.repo_owner AND ri.repo_name = rm.repo_name WHERE ri.repo_owner = ? AND ri.repo_name = ? AND ri.status = 'ready' + AND ri.image_profile = ? AND rm.image_build_enabled = 1 ORDER BY ri.created_at DESC LIMIT 1` ) - .bind(repoOwner.toLowerCase(), repoName.toLowerCase()) + .bind(repoOwner.toLowerCase(), repoName.toLowerCase(), imageProfile) .first(); } diff --git a/packages/control-plane/src/repo-images/build-profile-resolver.test.ts b/packages/control-plane/src/repo-images/build-profile-resolver.test.ts new file mode 100644 index 000000000..c135bdc2b --- /dev/null +++ b/packages/control-plane/src/repo-images/build-profile-resolver.test.ts @@ -0,0 +1,51 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { RepoImageBuildProfileResolver } from "./build-profile-resolver"; +import { + resolveSandboxSettings, + resolveSandboxSettingsForRepos, +} from "../session/integration-settings-resolution"; + +vi.mock("../session/integration-settings-resolution", () => ({ + resolveSandboxSettings: vi.fn(), + resolveSandboxSettingsForRepos: vi.fn(), +})); + +describe("RepoImageBuildProfileResolver", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("resolves a repo image profile without returning sandbox settings", async () => { + vi.mocked(resolveSandboxSettings).mockResolvedValueOnce({ dockerEnabled: true }); + + const resolver = new RepoImageBuildProfileResolver({} as D1Database); + const profile = await resolver.resolve({ repoOwner: "acme", repoName: "app" }); + + expect(profile).toEqual({ + repo: { repoOwner: "acme", repoName: "app" }, + imageProfile: "docker", + }); + expect(profile).not.toHaveProperty("sandboxSettings"); + }); + + it("resolves many repo image profiles without returning sandbox settings", async () => { + vi.mocked(resolveSandboxSettingsForRepos).mockResolvedValueOnce([ + { dockerEnabled: false }, + { dockerEnabled: true }, + ]); + + const resolver = new RepoImageBuildProfileResolver({} as D1Database); + const profiles = await resolver.resolveMany([ + { repoOwner: "acme", repoName: "app" }, + { repoOwner: "acme", repoName: "api" }, + ]); + + expect(profiles).toEqual([ + { repo: { repoOwner: "acme", repoName: "app" }, imageProfile: "default" }, + { repo: { repoOwner: "acme", repoName: "api" }, imageProfile: "docker" }, + ]); + expect(profiles[0]).not.toHaveProperty("sandboxSettings"); + expect(profiles[1]).not.toHaveProperty("sandboxSettings"); + }); +}); diff --git a/packages/control-plane/src/repo-images/build-profile-resolver.ts b/packages/control-plane/src/repo-images/build-profile-resolver.ts new file mode 100644 index 000000000..6b5fe38aa --- /dev/null +++ b/packages/control-plane/src/repo-images/build-profile-resolver.ts @@ -0,0 +1,44 @@ +import { resolveSandboxImageProfile, type SandboxImageProfile } from "@open-inspect/shared"; + +import { + resolveSandboxSettings, + resolveSandboxSettingsForRepos, +} from "../session/integration-settings-resolution"; + +export interface RepoImageBuildRepo { + repoOwner: string; + repoName: string; +} + +export interface RepoImageBuildProfile { + repo: RepoImageBuildRepo; + imageProfile: SandboxImageProfile; +} + +/** + * Resolves the repo-image build profile from repository sandbox settings. + * + * Repo image builds only need the derived compatibility key (`imageProfile`). Runtime sandbox + * settings stay in the control plane and are not forwarded to Modal's async builder. Centralizing + * this lookup keeps manual triggers and scheduler responses aligned without making route handlers + * understand Docker settings. + */ +export class RepoImageBuildProfileResolver { + constructor(private readonly db: D1Database) {} + + async resolve(repo: RepoImageBuildRepo): Promise { + const sandboxSettings = await resolveSandboxSettings(this.db, repo.repoOwner, repo.repoName); + return { + repo, + imageProfile: resolveSandboxImageProfile(sandboxSettings), + }; + } + + async resolveMany(repos: RepoImageBuildRepo[]): Promise { + const sandboxSettings = await resolveSandboxSettingsForRepos(this.db, repos); + return repos.map((repo, index) => ({ + repo, + imageProfile: resolveSandboxImageProfile(sandboxSettings[index]), + })); + } +} diff --git a/packages/control-plane/src/routes/repo-images.ts b/packages/control-plane/src/routes/repo-images.ts index 98af799dc..c00d0631c 100644 --- a/packages/control-plane/src/routes/repo-images.ts +++ b/packages/control-plane/src/routes/repo-images.ts @@ -13,8 +13,9 @@ import { RepoMetadataStore } from "../db/repo-metadata"; import { GlobalSecretsStore } from "../db/global-secrets"; import { RepoSecretsStore } from "../db/repo-secrets"; import { mergeSecrets } from "../db/secrets-validation"; +import { RepoImageBuildProfileResolver } from "../repo-images/build-profile-resolver"; import { createModalClient } from "../sandbox/client"; -import { isModalSandboxBackend } from "../sandbox/provider-name"; +import { getProviderCapabilities } from "@open-inspect/shared"; import { createLogger } from "../logger"; import type { Env } from "../types"; import { @@ -31,12 +32,12 @@ import { const logger = createLogger("router:repo-images"); -function requireModalRepoImages(env: Env): Response | null { - if (isModalSandboxBackend(env.SANDBOX_PROVIDER)) { +function requirePrebuiltImageSupport(env: Env): Response | null { + if (getProviderCapabilities(env.SANDBOX_PROVIDER).supportsPrebuiltImages) { return null; } - return error("Repo images are only available when SANDBOX_PROVIDER=modal", 501); + return error("Repo images are not supported by the configured sandbox provider", 501); } /** @@ -49,7 +50,7 @@ async function handleBuildComplete( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -135,7 +136,7 @@ async function handleBuildFailed( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -184,7 +185,7 @@ async function handleTriggerBuild( match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -202,16 +203,20 @@ async function handleTriggerBuild( const { owner, name } = params; const store = new RepoImageStore(env.DB); + const profileResolver = new RepoImageBuildProfileResolver(env.DB); const now = Date.now(); const buildId = `img-${owner}-${name}-${now}`; try { + const buildProfile = await profileResolver.resolve({ repoOwner: owner, repoName: name }); + // Register the build in D1 await store.registerBuild({ id: buildId, repoOwner: owner, repoName: name, baseBranch: "main", + imageProfile: buildProfile.imageProfile, }); // Construct callback URL @@ -278,6 +283,7 @@ async function handleTriggerBuild( buildId, callbackUrl, userEnvVars, + imageProfile: buildProfile.imageProfile, }, { trace_id: ctx.trace_id, request_id: ctx.request_id } ); @@ -286,6 +292,7 @@ async function handleTriggerBuild( build_id: buildId, repo_owner: owner, repo_name: name, + image_profile: buildProfile.imageProfile, request_id: ctx.request_id, trace_id: ctx.trace_id, }); @@ -313,7 +320,7 @@ async function handleGetStatus( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -355,7 +362,7 @@ async function handleMarkStale( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -405,7 +412,7 @@ async function handleCleanup( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -455,7 +462,7 @@ async function handleToggleImageBuild( match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -509,7 +516,7 @@ async function handleGetEnabledRepos( _match: RegExpMatchArray, ctx: RequestContext ): Promise { - const providerError = requireModalRepoImages(env); + const providerError = requirePrebuiltImageSupport(env); if (providerError) return providerError; if (!env.DB) { @@ -517,10 +524,16 @@ async function handleGetEnabledRepos( } const metadataStore = new RepoMetadataStore(env.DB); + const profileResolver = new RepoImageBuildProfileResolver(env.DB); try { const repos = await metadataStore.getImageBuildEnabledRepos(); - return json({ repos }); + const buildProfiles = await profileResolver.resolveMany(repos); + const reposWithProfiles = buildProfiles.map(({ repo, imageProfile }) => ({ + ...repo, + imageProfile, + })); + return json({ repos: reposWithProfiles }); } catch (e) { logger.error("repo_image.enabled_repos_error", { error: e instanceof Error ? e.message : String(e), diff --git a/packages/control-plane/src/sandbox/client.test.ts b/packages/control-plane/src/sandbox/client.test.ts index 32e4f509f..e97c546f6 100644 --- a/packages/control-plane/src/sandbox/client.test.ts +++ b/packages/control-plane/src/sandbox/client.test.ts @@ -88,4 +88,29 @@ describe("ModalClient", () => { "https://acme-prod-web--open-inspect-api-health.modal.run" ); }); + + it("includes the image profile in repo-image build requests", async () => { + const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response( + JSON.stringify({ success: true, data: { build_id: "img-1", status: "building" } }), + { + status: 200, + headers: { "Content-Type": "application/json" }, + } + ) + ); + + const client = createModalClient("secret", "acme"); + await client.buildRepoImage({ + repoOwner: "acme", + repoName: "repo", + buildId: "img-1", + callbackUrl: "https://cp.example/repo-images/build-complete", + imageProfile: "docker", + }); + + const body = JSON.parse((fetchMock.mock.calls[0][1] as RequestInit).body as string); + expect(body).not.toHaveProperty("sandbox_settings"); + expect(body.image_profile).toBe("docker"); + }); }); diff --git a/packages/control-plane/src/sandbox/client.ts b/packages/control-plane/src/sandbox/client.ts index af7e95e04..0ee14e3fa 100644 --- a/packages/control-plane/src/sandbox/client.ts +++ b/packages/control-plane/src/sandbox/client.ts @@ -5,7 +5,11 @@ * All requests are authenticated using HMAC-signed tokens. */ -import { generateInternalToken, type SandboxSettings } from "@open-inspect/shared"; +import { + generateInternalToken, + type SandboxImageProfile, + type SandboxRuntimeSettings, +} from "@open-inspect/shared"; import type { McpServerConfig } from "@open-inspect/shared"; import { createLogger } from "../logger"; import type { CorrelationContext } from "../logger"; @@ -54,7 +58,6 @@ export interface CreateSandboxRequest { repoName: string; controlPlaneUrl: string; sandboxAuthToken: string; - snapshotId?: string; opencodeSessionId?: string; provider?: string; model?: string; @@ -66,7 +69,8 @@ export interface CreateSandboxRequest { codeServerEnabled?: boolean; agentSlackNotifyEnabled?: boolean; mcpServers?: McpServerConfig[]; - sandboxSettings?: SandboxSettings; + sandboxSettings?: SandboxRuntimeSettings; + imageProfile: SandboxImageProfile; } export interface CreateSandboxResponse { @@ -96,7 +100,8 @@ export interface RestoreSandboxRequest { codeServerEnabled?: boolean; agentSlackNotifyEnabled?: boolean; mcpServers?: McpServerConfig[]; - sandboxSettings?: SandboxSettings; + sandboxSettings?: SandboxRuntimeSettings; + imageProfile: SandboxImageProfile; } export interface RestoreSandboxResponse { @@ -140,6 +145,7 @@ export interface BuildRepoImageRequest { buildId: string; callbackUrl: string; userEnvVars?: Record; + imageProfile: SandboxImageProfile; } export interface BuildRepoImageResponse { @@ -249,7 +255,6 @@ export class ModalClient { repo_name: request.repoName, control_plane_url: request.controlPlaneUrl, sandbox_auth_token: request.sandboxAuthToken, - snapshot_id: request.snapshotId || null, opencode_session_id: request.opencodeSessionId || null, provider: request.provider || "anthropic", model: request.model || "claude-sonnet-4-6", @@ -262,6 +267,7 @@ export class ModalClient { agent_slack_notify_enabled: request.agentSlackNotifyEnabled ?? false, mcp_servers: request.mcpServers || null, sandbox_settings: request.sandboxSettings ?? null, + image_profile: request.imageProfile, }), }); @@ -349,6 +355,7 @@ export class ModalClient { code_server_enabled: request.codeServerEnabled ?? false, agent_slack_notify_enabled: request.agentSlackNotifyEnabled ?? false, sandbox_settings: request.sandboxSettings ?? null, + image_profile: request.imageProfile, }), }); @@ -561,6 +568,7 @@ export class ModalClient { build_id: request.buildId, callback_url: request.callbackUrl, user_env_vars: request.userEnvVars, + image_profile: request.imageProfile, }), }); diff --git a/packages/control-plane/src/sandbox/index.ts b/packages/control-plane/src/sandbox/index.ts index 04db35bf7..b36f087b8 100644 --- a/packages/control-plane/src/sandbox/index.ts +++ b/packages/control-plane/src/sandbox/index.ts @@ -43,11 +43,7 @@ export { type DaytonaSandboxResponse, type DaytonaCreateSandboxParams, } from "./daytona-rest-client"; -export { - resolveSandboxBackendName, - isModalSandboxBackend, - type SandboxBackendName, -} from "./provider-name"; +export { parseSandboxBackendName, type SandboxBackendName } from "@open-inspect/shared"; // Lifecycle decisions export { diff --git a/packages/control-plane/src/sandbox/lifecycle/manager.test.ts b/packages/control-plane/src/sandbox/lifecycle/manager.test.ts index a6dbf6c71..87cbdda04 100644 --- a/packages/control-plane/src/sandbox/lifecycle/manager.test.ts +++ b/packages/control-plane/src/sandbox/lifecycle/manager.test.ts @@ -71,6 +71,7 @@ function createMockSandbox( modal_object_id: "modal-obj-123", snapshot_id: null, snapshot_image_id: null, + snapshot_image_profile: null, auth_token: "auth-token-123", auth_token_hash: "auth-token-hash-123", status: "ready", @@ -137,9 +138,12 @@ function createMockStorage( calls.push(`updateSandboxModalObjectId:${id}`); if (sandbox) sandbox.modal_object_id = id; }), - updateSandboxSnapshotImageId: vi.fn((sandboxId: string, imageId: string) => { - calls.push(`updateSandboxSnapshotImageId:${imageId}`); - if (sandbox) sandbox.snapshot_image_id = imageId; + updateSandboxSnapshotImageId: vi.fn((sandboxId: string, imageId: string, imageProfile) => { + calls.push(`updateSandboxSnapshotImageId:${imageId}:${imageProfile}`); + if (sandbox) { + sandbox.snapshot_image_id = imageId; + sandbox.snapshot_image_profile = imageProfile; + } }), updateSandboxLastActivity: vi.fn((timestamp: number) => { calls.push("updateSandboxLastActivity"); @@ -268,6 +272,7 @@ function createMockProvider( supportsSnapshots: true, supportsRestore: true, supportsWarm: true, + supportsDocker: true, ...overrides.capabilities, }, createSandbox: @@ -529,6 +534,40 @@ describe("SandboxLifecycleManager", () => { expect(provider.createSandbox).not.toHaveBeenCalled(); }); + it("spawns fresh when the saved snapshot profile does not match current settings", async () => { + const session = createMockSession({ + sandbox_settings: '{"dockerEnabled":true}', + }); + const sandbox = createMockSandbox({ + status: "stopped", + snapshot_image_id: "img-default", + snapshot_image_profile: "default", + }); + const storage = createMockStorage(session, sandbox); + const provider = createMockProvider(); + + const manager = new SandboxLifecycleManager( + provider, + storage, + createMockBroadcaster(), + createMockWebSocketManager(false), + createMockAlarmScheduler(), + createMockIdGenerator(), + createTestConfig() + ); + + await manager.spawnSandbox(); + + expect(provider.restoreFromSnapshot).not.toHaveBeenCalled(); + expect(provider.createSandbox).toHaveBeenCalledWith( + expect.objectContaining({ + sandboxSettings: {}, + requestedRuntime: { docker: true }, + environment: "docker", + }) + ); + }); + it("schedules connecting timeout alarm after restore", async () => { const sandbox = createMockSandbox({ status: "stopped", @@ -945,7 +984,7 @@ describe("SandboxLifecycleManager", () => { await manager.triggerSnapshot("test_reason"); expect(provider.takeSnapshot).toHaveBeenCalled(); - expect(storage.calls).toContain("updateSandboxSnapshotImageId:snapshot-img-123"); + expect(storage.calls).toContain("updateSandboxSnapshotImageId:snapshot-img-123:default"); expect( broadcaster.messages.some((m) => (m as { type: string }).type === "snapshot_saved") ).toBe(true); @@ -1001,7 +1040,30 @@ describe("SandboxLifecycleManager", () => { await manager.triggerSnapshot("execution_complete"); - expect(storage.calls).toContain("updateSandboxSnapshotImageId:custom-snapshot-id"); + expect(storage.calls).toContain("updateSandboxSnapshotImageId:custom-snapshot-id:default"); + }); + + it("stores the current image profile with the returned snapshot imageId", async () => { + const session = createMockSession({ + sandbox_settings: '{"dockerEnabled":true}', + }); + const sandbox = createMockSandbox({ status: "ready" }); + const storage = createMockStorage(session, sandbox); + + const manager = new SandboxLifecycleManager( + createMockProvider(), + storage, + createMockBroadcaster(), + createMockWebSocketManager(), + createMockAlarmScheduler(), + createMockIdGenerator(), + createTestConfig() + ); + + await manager.triggerSnapshot("execution_complete"); + + expect(storage.calls).toContain("updateSandboxSnapshotImageId:snapshot-img-123:docker"); + expect(sandbox.snapshot_image_profile).toBe("docker"); }); it("handles snapshot errors gracefully", async () => { @@ -1623,7 +1685,42 @@ describe("SandboxLifecycleManager", () => { expect(repoImageLookup.getLatestReady).toHaveBeenCalledWith( "testowner", "testrepo", - "feature/xyz" + "feature/xyz", + "default" + ); + }); + + it("passes image profile to repo image lookup", async () => { + const session = createMockSession({ + sandbox_settings: '{"dockerEnabled":true}', + }); + const sandbox = createMockSandbox({ status: "pending", created_at: Date.now() - 60000 }); + const storage = createMockStorage(session, sandbox); + const provider = createMockProvider(); + + const repoImageLookup: RepoImageLookup = { + getLatestReady: vi.fn(async () => null), + }; + + const manager = new SandboxLifecycleManager( + provider, + storage, + createMockBroadcaster(), + createMockWebSocketManager(false), + createMockAlarmScheduler(), + createMockIdGenerator(), + createTestConfig(), + {}, + repoImageLookup + ); + + await manager.spawnSandbox(); + + expect(repoImageLookup.getLatestReady).toHaveBeenCalledWith( + "testowner", + "testrepo", + "main", + "docker" ); }); @@ -1658,7 +1755,8 @@ describe("SandboxLifecycleManager", () => { describe("sandbox settings", () => { it("doSpawn() passes sandboxSettings from session to provider config", async () => { const session = createMockSession({ - sandbox_settings: '{"tunnelPorts":[3000]}', + sandbox_settings: + '{"tunnelPorts":[3000],"terminalEnabled":true,"dockerEnabled":true,"maxConcurrentChildSessions":2}', }); const sandbox = createMockSandbox({ status: "pending", created_at: Date.now() - 60000 }); const storage = createMockStorage(session, sandbox); @@ -1678,11 +1776,40 @@ describe("SandboxLifecycleManager", () => { expect(provider.createSandbox).toHaveBeenCalledWith( expect.objectContaining({ - sandboxSettings: { tunnelPorts: [3000] }, + sandboxSettings: { tunnelPorts: [3000], terminalEnabled: true }, + requestedRuntime: { docker: true }, + environment: "docker", }) ); }); + it("doSpawn() resolves environment to default when provider does not support Docker", async () => { + const session = createMockSession({ + sandbox_settings: '{"dockerEnabled":true}', + }); + const sandbox = createMockSandbox({ status: "pending", created_at: Date.now() - 60000 }); + const storage = createMockStorage(session, sandbox); + const provider = createMockProvider({ capabilities: { supportsDocker: false } }); + + const manager = new SandboxLifecycleManager( + provider, + storage, + createMockBroadcaster(), + createMockWebSocketManager(false), + createMockAlarmScheduler(), + createMockIdGenerator(), + createTestConfig() + ); + + await manager.spawnSandbox(); + + const createConfig = vi.mocked(provider.createSandbox).mock.calls[0][0]; + // Intent still crosses the boundary; the environment is capability-gated. + expect(createConfig.requestedRuntime).toEqual({ docker: true }); + expect(createConfig.environment).toBe("default"); + expect(createConfig.sandboxSettings).toEqual({}); + }); + it("doSpawn() passes empty settings when sandbox_settings is null", async () => { const session = createMockSession({ sandbox_settings: null }); const sandbox = createMockSandbox({ status: "pending", created_at: Date.now() - 60000 }); @@ -1704,6 +1831,7 @@ describe("SandboxLifecycleManager", () => { expect(provider.createSandbox).toHaveBeenCalledWith( expect.objectContaining({ sandboxSettings: {}, + environment: "default", }) ); }); @@ -1731,6 +1859,7 @@ describe("SandboxLifecycleManager", () => { expect(provider.createSandbox).toHaveBeenCalledWith( expect.objectContaining({ sandboxSettings: { tunnelPorts: [3000] }, + environment: "default", }) ); }); @@ -1828,6 +1957,7 @@ describe("SandboxLifecycleManager", () => { expect(provider.restoreFromSnapshot).toHaveBeenCalledWith( expect.objectContaining({ sandboxSettings: {}, + environment: "default", }) ); }); diff --git a/packages/control-plane/src/sandbox/lifecycle/manager.ts b/packages/control-plane/src/sandbox/lifecycle/manager.ts index 60e8a8b96..dc05a6c78 100644 --- a/packages/control-plane/src/sandbox/lifecycle/manager.ts +++ b/packages/control-plane/src/sandbox/lifecycle/manager.ts @@ -10,7 +10,14 @@ * spawn attempts within the same request. */ -import { MAX_TUNNEL_PORTS, type SandboxSettings } from "@open-inspect/shared"; +import { + DEFAULT_SANDBOX_IMAGE_PROFILE, + normalizeSandboxRuntimeSettings, + resolveEnvironment, + type RequestedSandboxRuntime, + type SandboxImageProfile, + type SandboxSettings, +} from "@open-inspect/shared"; import type { SandboxStatus } from "../../types"; import type { SandboxRow, SessionRow } from "../../session/types"; import type { McpServerConfig } from "@open-inspect/shared"; @@ -53,6 +60,7 @@ export interface SandboxCircuitBreakerInfo { created_at: number; modal_object_id: string | null; snapshot_image_id: string | null; + snapshot_image_profile: SandboxImageProfile | null; spawn_failure_count: number | null; last_spawn_failure: number | null; } @@ -82,8 +90,12 @@ export interface SandboxStorage { updateSandboxForResume?(data: { status: SandboxStatus; createdAt: number }): void; /** Update sandbox Modal object ID (for snapshot API) */ updateSandboxModalObjectId(modalObjectId: string): void; - /** Update sandbox snapshot image ID */ - updateSandboxSnapshotImageId(sandboxId: string, imageId: string): void; + /** Update sandbox snapshot image ID and the image profile it was created from */ + updateSandboxSnapshotImageId( + sandboxId: string, + imageId: string, + imageProfile: SandboxImageProfile + ): void; /** Update last activity timestamp */ updateSandboxLastActivity(timestamp: number): void; /** Increment circuit breaker failure count */ @@ -204,7 +216,8 @@ export interface RepoImageLookup { getLatestReady( repoOwner: string, repoName: string, - baseBranch?: string + baseBranch?: string, + imageProfile?: SandboxImageProfile ): Promise<{ provider_image_id: string; base_sha: string } | null>; } @@ -301,12 +314,17 @@ export class SandboxLifecycleManager { return; } + const session = this.storage.getSession(); + const requestedEnvironment = session + ? this.resolveSessionEnvironment(session) + : DEFAULT_SANDBOX_IMAGE_PROFILE; + // Evaluate spawn decision const spawnState = { status: (sandboxState?.status || "pending") as SandboxStatus, createdAt: sandboxState?.created_at || 0, providerObjectId: sandboxState?.modal_object_id || null, - snapshotImageId: sandboxState?.snapshot_image_id || null, + snapshotImageId: this.getCompatibleSnapshotImageId(sandboxState, requestedEnvironment), hasActiveWebSocket: this.wsManager.getSandboxWebSocket() !== null, }; @@ -392,6 +410,10 @@ export class SandboxLifecycleManager { const userEnvVars = await this.storage.getUserEnvVars(); const { provider, model: modelId } = this.resolveProviderAndModel(session); + const storedSandboxSettings = this.parseSandboxSettings(session); + const sandboxSettings = normalizeSandboxRuntimeSettings(storedSandboxSettings); + const requestedRuntime = this.resolveRequestedRuntime(storedSandboxSettings); + const environment = resolveEnvironment(requestedRuntime, this.provider.capabilities); // Look up pre-built repo image (graceful fallback on failure) let repoImageId: string | null = null; @@ -401,7 +423,8 @@ export class SandboxLifecycleManager { const repoImage = await this.repoImageLookup.getLatestReady( session.repo_owner, session.repo_name, - session.base_branch + session.base_branch, + environment ); if (repoImage) { repoImageId = repoImage.provider_image_id; @@ -426,7 +449,6 @@ export class SandboxLifecycleManager { const codeServerEnabled = session.code_server_enabled === 1; const agentSlackNotifyEnabled = await this.resolveAgentSlackNotifyEnabled(session); - const sandboxSettings = this.parseSandboxSettings(session); const createConfig: CreateSandboxConfig = { sessionId, sandboxId: expectedSandboxId, @@ -445,6 +467,8 @@ export class SandboxLifecycleManager { agentSlackNotifyEnabled, mcpServers, sandboxSettings, + requestedRuntime, + environment, }; const result = await this.provider.createSandbox(createConfig); @@ -608,7 +632,10 @@ export class SandboxLifecycleManager { const codeServerEnabled = session.code_server_enabled === 1; const agentSlackNotifyEnabled = await this.resolveAgentSlackNotifyEnabled(session); const mcpServers = await this.loadMcpServers(session); - const sandboxSettings = this.parseSandboxSettings(session); + const storedSandboxSettings = this.parseSandboxSettings(session); + const sandboxSettings = normalizeSandboxRuntimeSettings(storedSandboxSettings); + const requestedRuntime = this.resolveRequestedRuntime(storedSandboxSettings); + const environment = resolveEnvironment(requestedRuntime, this.provider.capabilities); const result = await this.provider.restoreFromSnapshot({ snapshotImageId, sessionId: session.session_name || session.id, @@ -626,6 +653,8 @@ export class SandboxLifecycleManager { agentSlackNotifyEnabled, mcpServers, sandboxSettings, + requestedRuntime, + environment, }); if (result.success) { @@ -734,7 +763,7 @@ export class SandboxLifecycleManager { sandboxId: sandbox.modal_sandbox_id, timeoutSeconds, codeServerEnabled: session.code_server_enabled === 1, - sandboxSettings: this.parseSandboxSettings(session), + sandboxSettings: normalizeSandboxRuntimeSettings(this.parseSandboxSettings(session)), }); if (!result.success) { @@ -826,10 +855,12 @@ export class SandboxLifecycleManager { }); if (result.success && result.imageId) { - this.storage.updateSandboxSnapshotImageId(sandbox.id, result.imageId); + const environment = this.resolveSessionEnvironment(session); + this.storage.updateSandboxSnapshotImageId(sandbox.id, result.imageId, environment); this.log.info("Snapshot saved", { event: "sandbox.snapshot_saved", image_id: result.imageId, + image_profile: environment, reason, }); this.broadcaster.broadcast({ @@ -1189,28 +1220,49 @@ export class SandboxLifecycleManager { try { const parsed: unknown = JSON.parse(session.sandbox_settings); if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + return parsed as SandboxSettings; + } catch { + this.log.warn("Failed to parse sandbox_settings, using defaults"); + return {}; + } + } - const settings = parsed as Record; - const result: SandboxSettings = {}; + /** Map persisted sandbox settings to provider-agnostic runtime intent. */ + private resolveRequestedRuntime(settings: SandboxSettings): RequestedSandboxRuntime { + return { docker: settings.dockerEnabled === true }; + } - // Validate tunnelPorts at the boundary — data may come from untrusted callers - if (settings.tunnelPorts !== undefined) { - if (!Array.isArray(settings.tunnelPorts)) return {}; - const valid = settings.tunnelPorts.filter( - (p: unknown) => typeof p === "number" && Number.isInteger(p) && p >= 1 && p <= 65535 - ); - result.tunnelPorts = valid.slice(0, MAX_TUNNEL_PORTS); - } + /** + * Resolve the logical environment id for a session, gated by the active + * provider's capabilities. The manager speaks this shared id — never a + * provider image. + */ + private resolveSessionEnvironment(session: SessionRow): SandboxImageProfile { + return resolveEnvironment( + this.resolveRequestedRuntime(this.parseSandboxSettings(session)), + this.provider.capabilities + ); + } - if (typeof settings.terminalEnabled === "boolean") { - result.terminalEnabled = settings.terminalEnabled; - } + private getCompatibleSnapshotImageId( + sandboxState: SandboxCircuitBreakerInfo | null, + requestedEnvironment: SandboxImageProfile + ): string | null { + if (!sandboxState?.snapshot_image_id) return null; - return result; - } catch { - this.log.warn("Failed to parse sandbox_settings, using defaults"); - return {}; + const snapshotEnvironment = + sandboxState.snapshot_image_profile ?? DEFAULT_SANDBOX_IMAGE_PROFILE; + if (snapshotEnvironment === requestedEnvironment) { + return sandboxState.snapshot_image_id; } + + this.log.info("Ignoring snapshot image with mismatched environment", { + event: "sandbox.snapshot_profile_mismatch", + snapshot_image_id: sandboxState.snapshot_image_id, + snapshot_image_profile: snapshotEnvironment, + requested_image_profile: requestedEnvironment, + }); + return null; } private async storeAndBroadcastTunnelUrls( diff --git a/packages/control-plane/src/sandbox/provider-name.test.ts b/packages/control-plane/src/sandbox/provider-name.test.ts deleted file mode 100644 index 73afea528..000000000 --- a/packages/control-plane/src/sandbox/provider-name.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { resolveSandboxBackendName, isModalSandboxBackend } from "./provider-name"; - -describe("resolveSandboxBackendName", () => { - it("defaults to modal when undefined", () => { - expect(resolveSandboxBackendName(undefined)).toBe("modal"); - }); - - it("defaults to modal when empty string", () => { - expect(resolveSandboxBackendName("")).toBe("modal"); - }); - - it("defaults to modal when whitespace-only", () => { - expect(resolveSandboxBackendName(" ")).toBe("modal"); - }); - - it('returns "modal" for "modal"', () => { - expect(resolveSandboxBackendName("modal")).toBe("modal"); - }); - - it('returns "daytona" for "daytona"', () => { - expect(resolveSandboxBackendName("daytona")).toBe("daytona"); - }); - - it("is case-insensitive", () => { - expect(resolveSandboxBackendName("MODAL")).toBe("modal"); - expect(resolveSandboxBackendName("Daytona")).toBe("daytona"); - expect(resolveSandboxBackendName("DAYTONA")).toBe("daytona"); - }); - - it("trims whitespace", () => { - expect(resolveSandboxBackendName(" modal ")).toBe("modal"); - expect(resolveSandboxBackendName(" daytona ")).toBe("daytona"); - }); - - it("throws for unsupported provider", () => { - expect(() => resolveSandboxBackendName("k8s")).toThrow("Unsupported SANDBOX_PROVIDER: k8s"); - expect(() => resolveSandboxBackendName("fly")).toThrow("Unsupported SANDBOX_PROVIDER: fly"); - }); -}); - -describe("isModalSandboxBackend", () => { - it("returns true for modal", () => { - expect(isModalSandboxBackend("modal")).toBe(true); - }); - - it("returns true for undefined (default)", () => { - expect(isModalSandboxBackend(undefined)).toBe(true); - }); - - it("returns false for daytona", () => { - expect(isModalSandboxBackend("daytona")).toBe(false); - }); -}); diff --git a/packages/control-plane/src/sandbox/provider-name.ts b/packages/control-plane/src/sandbox/provider-name.ts deleted file mode 100644 index 32234a137..000000000 --- a/packages/control-plane/src/sandbox/provider-name.ts +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Sandbox backend selection utilities. - */ - -export type SandboxBackendName = "modal" | "daytona"; - -/** - * Resolve the configured sandbox backend. - * - * Defaults to Modal to preserve existing deployments. - */ -export function resolveSandboxBackendName(value: string | undefined): SandboxBackendName { - const normalized = value?.trim().toLowerCase(); - - if (!normalized || normalized === "modal") { - return "modal"; - } - - if (normalized === "daytona") { - return "daytona"; - } - - throw new Error(`Unsupported SANDBOX_PROVIDER: ${value}`); -} - -export function isModalSandboxBackend(value: string | undefined): boolean { - return resolveSandboxBackendName(value) === "modal"; -} diff --git a/packages/control-plane/src/sandbox/provider.ts b/packages/control-plane/src/sandbox/provider.ts index 48c715fec..8f5977ca8 100644 --- a/packages/control-plane/src/sandbox/provider.ts +++ b/packages/control-plane/src/sandbox/provider.ts @@ -5,30 +5,24 @@ * enabling unit testing and future provider support. */ -import type { SandboxSettings } from "@open-inspect/shared"; +import type { + RequestedSandboxRuntime, + SandboxImageProfile, + SandboxProviderCapabilities, + SandboxRuntimeSettings, +} from "@open-inspect/shared"; import type { CorrelationContext } from "../logger"; import type { McpServerConfig } from "@open-inspect/shared"; +// The provider capability descriptor lives in @open-inspect/shared so the +// control plane and the web app read the same source of truth. Re-exported +// here for the many call sites that import it alongside the provider config +// types. +export type { SandboxProviderCapabilities }; + /** Default sandbox lifetime in seconds (2 hours). */ export const DEFAULT_SANDBOX_TIMEOUT_SECONDS = 7200; -/** - * Capabilities supported by a sandbox provider. - * Providers can support different feature sets. - */ -export interface SandboxProviderCapabilities { - /** Whether the provider supports filesystem snapshots */ - supportsSnapshots: boolean; - /** Whether the provider supports restoring from snapshots */ - supportsRestore: boolean; - /** Whether the provider supports pre-warming sandboxes */ - supportsWarm: boolean; - /** Whether the provider can resume a previously stopped sandbox in place */ - supportsPersistentResume?: boolean; - /** Whether the provider can stop a sandbox explicitly via API */ - supportsExplicitStop?: boolean; -} - /** * Configuration for creating a new sandbox. */ @@ -73,8 +67,21 @@ export interface CreateSandboxConfig { agentSlackNotifyEnabled?: boolean; /** MCP servers to inject into the agent session */ mcpServers?: McpServerConfig[]; - /** Sandbox settings (tunnel ports, etc.) resolved from integration settings */ - sandboxSettings?: SandboxSettings; + /** Runtime sandbox settings resolved from integration settings */ + sandboxSettings?: SandboxRuntimeSettings; + /** + * Provider-agnostic declarative runtime intent (e.g. Docker). The provider + * realizes it with its own mechanism; the manager does not pre-translate it. + */ + requestedRuntime?: RequestedSandboxRuntime; + /** + * Resolved logical environment id (shared, provider-agnostic) for this spawn. + * Drives image selection and snapshot/prebuilt-image compatibility. Each + * provider maps it to a concrete image internally; the control plane never + * sees a provider image. Computed once by the lifecycle manager via + * resolveEnvironment(requestedRuntime, capabilities). + */ + environment: SandboxImageProfile; } /** @@ -135,8 +142,12 @@ export interface RestoreConfig { codeServerEnabled?: boolean; /** Resolved fresh on each restore — see CreateSandboxConfig. */ agentSlackNotifyEnabled?: boolean; - /** Sandbox settings (tunnel ports, etc.) resolved from integration settings */ - sandboxSettings?: SandboxSettings; + /** Runtime sandbox settings resolved from integration settings */ + sandboxSettings?: SandboxRuntimeSettings; + /** Provider-agnostic declarative runtime intent (e.g. Docker). See CreateSandboxConfig. */ + requestedRuntime?: RequestedSandboxRuntime; + /** Resolved logical environment id for this restore. See CreateSandboxConfig. */ + environment: SandboxImageProfile; } /** @@ -201,8 +212,8 @@ export interface ResumeConfig { timeoutSeconds?: number; /** Whether code-server should be exposed */ codeServerEnabled?: boolean; - /** Sandbox settings (tunnel ports, etc.) resolved from integration settings */ - sandboxSettings?: SandboxSettings; + /** Runtime sandbox settings resolved from integration settings */ + sandboxSettings?: SandboxRuntimeSettings; /** Correlation context for downstream tracing */ correlation?: CorrelationContext; } diff --git a/packages/control-plane/src/sandbox/providers/daytona-provider.test.ts b/packages/control-plane/src/sandbox/providers/daytona-provider.test.ts index 6dc27a5ba..6d56a7bbd 100644 --- a/packages/control-plane/src/sandbox/providers/daytona-provider.test.ts +++ b/packages/control-plane/src/sandbox/providers/daytona-provider.test.ts @@ -85,6 +85,7 @@ const baseCreateConfig: CreateSandboxConfig = { sandboxAuthToken: "auth-token-abc", provider: "anthropic", model: "anthropic/claude-sonnet-4-5", + environment: "default", }; const baseResumeConfig: ResumeConfig = { @@ -116,6 +117,9 @@ describe("DaytonaSandboxProvider", () => { supportsWarm: false, supportsPersistentResume: true, supportsExplicitStop: true, + supportsDocker: false, + supportsPrebuiltImages: false, + supportsDashboardUrl: false, }); }); }); diff --git a/packages/control-plane/src/sandbox/providers/daytona-provider.ts b/packages/control-plane/src/sandbox/providers/daytona-provider.ts index 698cd2785..5333e9576 100644 --- a/packages/control-plane/src/sandbox/providers/daytona-provider.ts +++ b/packages/control-plane/src/sandbox/providers/daytona-provider.ts @@ -5,7 +5,12 @@ * code-server password derivation that previously lived in the Python shim. */ -import { computeHmacHex, MAX_TUNNEL_PORTS, type SandboxSettings } from "@open-inspect/shared"; +import { + computeHmacHex, + MAX_TUNNEL_PORTS, + PROVIDER_CAPABILITIES, + type SandboxRuntimeSettings, +} from "@open-inspect/shared"; import { createLogger } from "../../logger"; import type { SourceControlProviderName } from "../../source-control"; import type { DaytonaRestClient, DaytonaCreateSandboxParams } from "../daytona-rest-client"; @@ -49,13 +54,7 @@ export interface DaytonaProviderConfig { export class DaytonaSandboxProvider implements SandboxProvider { readonly name = "daytona"; - readonly capabilities: SandboxProviderCapabilities = { - supportsSnapshots: false, - supportsRestore: false, - supportsWarm: false, - supportsPersistentResume: true, - supportsExplicitStop: true, - }; + readonly capabilities: SandboxProviderCapabilities = PROVIDER_CAPABILITIES.daytona; constructor( private readonly client: DaytonaRestClient, @@ -71,6 +70,11 @@ export class DaytonaSandboxProvider implements SandboxProvider { const envVars = await this.buildEnvVars(config); const labels = this.buildLabels(config); + // Daytona uses a single base snapshot, so it ignores config.environment. + // TODO: realize config.requestedRuntime?.docker here when Daytona Docker + // support lands (its own mechanism — a create flag or snapshot variant — + // not the Modal image_profile), and flip + // PROVIDER_CAPABILITIES.daytona.supportsDocker. const params: DaytonaCreateSandboxParams = { name: config.sandboxId, snapshot: this.client.config.baseSnapshot, @@ -262,7 +266,7 @@ export class DaytonaSandboxProvider implements SandboxProvider { logicalSandboxId: string, timeoutSeconds: number | undefined, codeServerEnabled: boolean | undefined, - sandboxSettings: SandboxSettings | undefined + sandboxSettings: SandboxRuntimeSettings | undefined ): Promise<{ codeServerUrl?: string; codeServerPassword?: string; diff --git a/packages/control-plane/src/sandbox/providers/modal-provider.test.ts b/packages/control-plane/src/sandbox/providers/modal-provider.test.ts index dedc9425c..59c2a2bbe 100644 --- a/packages/control-plane/src/sandbox/providers/modal-provider.test.ts +++ b/packages/control-plane/src/sandbox/providers/modal-provider.test.ts @@ -62,6 +62,7 @@ const testConfig = { sandboxAuthToken: "auth-token", provider: "anthropic", model: "anthropic/claude-sonnet-4-5", + environment: "default" as const, }; // ==================== Tests ==================== @@ -76,6 +77,44 @@ describe("ModalSandboxProvider", () => { expect(provider.capabilities.supportsSnapshots).toBe(true); expect(provider.capabilities.supportsRestore).toBe(true); expect(provider.capabilities.supportsWarm).toBe(true); + expect(provider.capabilities.supportsDocker).toBe(true); + }); + }); + + describe("image realization (environment -> Modal wire)", () => { + it("maps the resolved environment to the Modal image_profile request field", async () => { + const client = createMockModalClient(); + const provider = new ModalSandboxProvider(client); + + await provider.createSandbox({ ...testConfig, environment: "docker" }); + + expect(client.createSandbox).toHaveBeenCalledWith( + expect.objectContaining({ imageProfile: "docker" }), + undefined + ); + }); + + it("passes the environment through to image_profile on restore", async () => { + const client = createMockModalClient(); + const provider = new ModalSandboxProvider(client); + + await provider.restoreFromSnapshot({ + snapshotImageId: "img-123", + sessionId: "session-123", + sandboxId: "sandbox-123", + sandboxAuthToken: "token", + controlPlaneUrl: "https://test.com", + repoOwner: "owner", + repoName: "repo", + provider: "anthropic", + model: "anthropic/claude-sonnet-4-5", + environment: "default", + }); + + expect(client.restoreSandbox).toHaveBeenCalledWith( + expect.objectContaining({ imageProfile: "default" }), + undefined + ); }); }); @@ -486,6 +525,7 @@ describe("ModalSandboxProvider", () => { repoName: "repo", provider: "anthropic", model: "anthropic/claude-sonnet-4-5", + environment: "default", }); expect.fail("Should have thrown"); } catch (e) { @@ -513,6 +553,7 @@ describe("ModalSandboxProvider", () => { repoName: "repo", provider: "anthropic", model: "anthropic/claude-sonnet-4-5", + environment: "default", }); expect.fail("Should have thrown"); } catch (e) { @@ -562,6 +603,7 @@ describe("ModalSandboxProvider", () => { repoName: "repo", provider: "anthropic", model: "anthropic/claude-sonnet-4-5", + environment: "default", }); expect(result.success).toBe(true); diff --git a/packages/control-plane/src/sandbox/providers/modal-provider.ts b/packages/control-plane/src/sandbox/providers/modal-provider.ts index d2f52fffe..ca799d851 100644 --- a/packages/control-plane/src/sandbox/providers/modal-provider.ts +++ b/packages/control-plane/src/sandbox/providers/modal-provider.ts @@ -7,6 +7,7 @@ import { ModalApiError } from "../client"; import type { ModalClient } from "../client"; +import { PROVIDER_CAPABILITIES } from "@open-inspect/shared"; import { DEFAULT_SANDBOX_TIMEOUT_SECONDS, SandboxProviderError, @@ -43,13 +44,7 @@ import { export class ModalSandboxProvider implements SandboxProvider { readonly name = "modal"; - readonly capabilities: SandboxProviderCapabilities = { - supportsSnapshots: true, - supportsRestore: true, - supportsWarm: true, - supportsPersistentResume: false, - supportsExplicitStop: false, - }; + readonly capabilities: SandboxProviderCapabilities = PROVIDER_CAPABILITIES.modal; constructor(private readonly client: ModalClient) {} @@ -78,6 +73,9 @@ export class ModalSandboxProvider implements SandboxProvider { agentSlackNotifyEnabled: config.agentSlackNotifyEnabled, mcpServers: config.mcpServers, sandboxSettings: config.sandboxSettings, + // Modal realizes the shared environment id as its image_profile. The + // Modal HTTP contract (image_profile) is intentionally unchanged. + imageProfile: config.environment, }, config.correlation ); @@ -120,6 +118,7 @@ export class ModalSandboxProvider implements SandboxProvider { agentSlackNotifyEnabled: config.agentSlackNotifyEnabled, mcpServers: config.mcpServers, sandboxSettings: config.sandboxSettings, + imageProfile: config.environment, }, config.correlation ); diff --git a/packages/control-plane/src/session/durable-object.ts b/packages/control-plane/src/session/durable-object.ts index db66629fe..d497edcd2 100644 --- a/packages/control-plane/src/session/durable-object.ts +++ b/packages/control-plane/src/session/durable-object.ts @@ -10,13 +10,17 @@ import { DurableObject } from "cloudflare:workers"; import { initSchema } from "./schema"; import { buildSessionInternalUrl, SessionInternalPaths } from "./contracts"; -import { resolveAppName, timingSafeEqual } from "@open-inspect/shared"; +import { + getProviderCapabilities, + parseSandboxBackendName, + resolveAppName, + timingSafeEqual, +} from "@open-inspect/shared"; import { generateId, hashToken, encryptToken, decryptToken } from "../auth/crypto"; import { buildModalSandboxDashboardUrl, createModalClient } from "../sandbox/client"; import { createDaytonaRestClient } from "../sandbox/daytona-rest-client"; import { createModalProvider } from "../sandbox/providers/modal-provider"; import { createDaytonaProvider } from "../sandbox/providers/daytona-provider"; -import { resolveSandboxBackendName } from "../sandbox/provider-name"; import { createLogger, parseLogLevel } from "../logger"; import type { Logger } from "../logger"; import { @@ -548,7 +552,7 @@ export class SessionDO extends DurableObject { * Create the lifecycle manager with all required adapters. */ private createLifecycleManager(): SandboxLifecycleManager { - const sandboxBackend = resolveSandboxBackendName(this.env.SANDBOX_PROVIDER); + const sandboxBackend = parseSandboxBackendName(this.env.SANDBOX_PROVIDER); const provider = sandboxBackend === "daytona" @@ -613,8 +617,8 @@ export class SessionDO extends DurableObject { updateSandboxForSpawn: (data) => this.repository.updateSandboxForSpawn(data), updateSandboxForResume: (data) => this.repository.updateSandboxForResume(data), updateSandboxModalObjectId: (id) => this.repository.updateSandboxModalObjectId(id), - updateSandboxSnapshotImageId: (sandboxId, imageId) => - this.repository.updateSandboxSnapshotImageId(sandboxId, imageId), + updateSandboxSnapshotImageId: (sandboxId, imageId, imageProfile) => + this.repository.updateSandboxSnapshotImageId(sandboxId, imageId, imageProfile), updateSandboxLastActivity: (timestamp) => this.repository.updateSandboxLastActivity(timestamp), incrementCircuitBreakerFailure: (timestamp) => @@ -736,8 +740,8 @@ export class SessionDO extends DurableObject { if (this.env.DB && sandboxBackend === "modal") { const repoImageStore = new RepoImageStore(this.env.DB); repoImageLookup = { - getLatestReady: (repoOwner, repoName, baseBranch) => - repoImageStore.getLatestReady(repoOwner, repoName, baseBranch), + getLatestReady: (repoOwner, repoName, baseBranch, imageProfile) => + repoImageStore.getLatestReady(repoOwner, repoName, baseBranch, imageProfile), }; } @@ -1725,7 +1729,11 @@ export class SessionDO extends DurableObject { } private getSandboxDashboardUrl(providerObjectId: string | null | undefined): string | null { - if (resolveSandboxBackendName(this.env.SANDBOX_PROVIDER) !== "modal") return null; + // Gated on a capability, not a provider-name comparison. The URL builder is + // still Modal-specific because Modal is the only backend exposing a console + // URL today; when a second provider gains one, make the builder + // provider-aware (or move it onto the provider) alongside flipping the flag. + if (!getProviderCapabilities(this.env.SANDBOX_PROVIDER).supportsDashboardUrl) return null; return buildModalSandboxDashboardUrl({ workspace: this.env.MODAL_WORKSPACE, environment: this.env.MODAL_ENVIRONMENT, diff --git a/packages/control-plane/src/session/http/handlers/child-sessions.handler.test.ts b/packages/control-plane/src/session/http/handlers/child-sessions.handler.test.ts index f1e5401f4..4eddbd9d0 100644 --- a/packages/control-plane/src/session/http/handlers/child-sessions.handler.test.ts +++ b/packages/control-plane/src/session/http/handlers/child-sessions.handler.test.ts @@ -68,6 +68,7 @@ function createSandbox(overrides: Partial = {}): SandboxRow { modal_object_id: null, snapshot_id: null, snapshot_image_id: null, + snapshot_image_profile: null, auth_token: null, auth_token_hash: null, status: "running", diff --git a/packages/control-plane/src/session/http/handlers/session-lifecycle.handler.test.ts b/packages/control-plane/src/session/http/handlers/session-lifecycle.handler.test.ts index db15a197e..d53cb6d04 100644 --- a/packages/control-plane/src/session/http/handlers/session-lifecycle.handler.test.ts +++ b/packages/control-plane/src/session/http/handlers/session-lifecycle.handler.test.ts @@ -38,6 +38,7 @@ function createSandbox(overrides: Partial = {}): SandboxRow { modal_object_id: null, snapshot_id: null, snapshot_image_id: null, + snapshot_image_profile: null, auth_token: null, auth_token_hash: null, status: "running", diff --git a/packages/control-plane/src/session/integration-settings-resolution.ts b/packages/control-plane/src/session/integration-settings-resolution.ts index 3af2954a7..4a4ec1a48 100644 --- a/packages/control-plane/src/session/integration-settings-resolution.ts +++ b/packages/control-plane/src/session/integration-settings-resolution.ts @@ -4,6 +4,15 @@ import { createLogger } from "../logger"; const logger = createLogger("session-integration-settings"); +type RepoRef = { + repoOwner: string; + repoName: string; +}; + +function repoSettingsKey(repoOwner: string, repoName: string): string { + return `${repoOwner}/${repoName}`.toLowerCase(); +} + /** * Resolve whether code-server should be enabled for a given repo, * checking both the `enabled` setting and the `enabledRepos` allowlist. @@ -40,12 +49,12 @@ export async function resolveSandboxSettings( repoName: string ): Promise { if (!db) return {}; - const repo = `${repoOwner}/${repoName}`; + const repo = repoSettingsKey(repoOwner, repoName); try { const store = new IntegrationSettingsStore(db); const { enabledRepos, settings } = await store.getResolvedConfig("sandbox", repo); // enabledRepos: null -> all repos, [] -> none, [...] -> allowlist - if (enabledRepos !== null && !enabledRepos.includes(repo.toLowerCase())) return {}; + if (enabledRepos !== null && !enabledRepos.includes(repo)) return {}; return settings as SandboxSettings; } catch (e) { logger.warn("Failed to resolve sandbox settings, using defaults", { @@ -54,3 +63,32 @@ export async function resolveSandboxSettings( return {}; } } + +/** + * Resolve sandbox settings for multiple repos with one global-settings read and one repo-settings + * read. Returned settings are aligned with the input repos. + */ +export async function resolveSandboxSettingsForRepos( + db: D1Database | undefined, + repos: RepoRef[] +): Promise { + if (!db || repos.length === 0) return repos.map(() => ({})); + + try { + const repoKeys = repos.map((repo) => repoSettingsKey(repo.repoOwner, repo.repoName)); + const store = new IntegrationSettingsStore(db); + const configs = await store.getResolvedConfigs("sandbox", repoKeys); + + return repoKeys.map((repo) => { + const config = configs.get(repo); + if (!config) return {}; + if (config.enabledRepos !== null && !config.enabledRepos.includes(repo)) return {}; + return config.settings as SandboxSettings; + }); + } catch (e) { + logger.warn("Failed to resolve sandbox settings in batch, using defaults", { + error: e instanceof Error ? e.message : String(e), + }); + return repos.map(() => ({})); + } +} diff --git a/packages/control-plane/src/session/repository.test.ts b/packages/control-plane/src/session/repository.test.ts index 66c093008..c97f43f4b 100644 --- a/packages/control-plane/src/session/repository.test.ts +++ b/packages/control-plane/src/session/repository.test.ts @@ -275,12 +275,13 @@ describe("SessionRepository", () => { }); describe("updateSandboxSnapshotImageId", () => { - it("updates snapshot image ID for specific sandbox", () => { - repo.updateSandboxSnapshotImageId("sb-1", "img-123"); + it("updates snapshot image ID and profile for specific sandbox", () => { + repo.updateSandboxSnapshotImageId("sb-1", "img-123", "docker"); expect(mock.calls.length).toBe(1); expect(mock.calls[0].query).toContain("UPDATE sandbox SET snapshot_image_id"); - expect(mock.calls[0].params).toEqual(["img-123", "sb-1"]); + expect(mock.calls[0].query).toContain("snapshot_image_profile"); + expect(mock.calls[0].params).toEqual(["img-123", "docker", "sb-1"]); }); }); diff --git a/packages/control-plane/src/session/repository.ts b/packages/control-plane/src/session/repository.ts index 1d484aec6..f34bb5beb 100644 --- a/packages/control-plane/src/session/repository.ts +++ b/packages/control-plane/src/session/repository.ts @@ -29,6 +29,7 @@ import { type EventListCursor, type EventTimelineCursor, } from "./event-cursor"; +import { isSandboxImageProfile, type SandboxImageProfile } from "@open-inspect/shared"; type TokenEvent = Extract; type ExecutionCompleteEvent = Extract; @@ -54,6 +55,7 @@ export interface SandboxCircuitBreakerState { created_at: number; modal_object_id: string | null; snapshot_image_id: string | null; + snapshot_image_profile: SandboxImageProfile | null; spawn_failure_count: number | null; last_spawn_failure: number | null; } @@ -351,7 +353,7 @@ export class SessionRepository { getSandboxWithCircuitBreaker(): SandboxCircuitBreakerState | null { const result = this.sql.exec( - `SELECT status, created_at, modal_object_id, snapshot_image_id, spawn_failure_count, last_spawn_failure FROM sandbox LIMIT 1` + `SELECT status, created_at, modal_object_id, snapshot_image_id, snapshot_image_profile, spawn_failure_count, last_spawn_failure FROM sandbox LIMIT 1` ); const rows = this.rows(result); return rows[0] ?? null; @@ -411,8 +413,23 @@ export class SessionRepository { ); } - updateSandboxSnapshotImageId(sandboxId: string, imageId: string): void { - this.sql.exec(`UPDATE sandbox SET snapshot_image_id = ? WHERE id = ?`, imageId, sandboxId); + updateSandboxSnapshotImageId( + sandboxId: string, + imageId: string, + imageProfile: SandboxImageProfile + ): void { + // The DO `snapshot_image_profile` column is bare TEXT (no CHECK, unlike the + // D1 repo_images.image_profile column). Validate against the shared allow-set + // so an unexpected value can never corrupt snapshot-compatibility matching. + if (!isSandboxImageProfile(imageProfile)) { + throw new Error(`Invalid sandbox image profile for snapshot: ${String(imageProfile)}`); + } + this.sql.exec( + `UPDATE sandbox SET snapshot_image_id = ?, snapshot_image_profile = ? WHERE id = ?`, + imageId, + imageProfile, + sandboxId + ); } updateSandboxHeartbeat(timestamp: number): void { diff --git a/packages/control-plane/src/session/schema.ts b/packages/control-plane/src/session/schema.ts index 1338d1668..43ca5ade5 100644 --- a/packages/control-plane/src/session/schema.ts +++ b/packages/control-plane/src/session/schema.ts @@ -94,6 +94,7 @@ CREATE TABLE IF NOT EXISTS sandbox ( modal_object_id TEXT, -- Legacy provider object ID (Modal object ID or Daytona handle) snapshot_id TEXT, snapshot_image_id TEXT, -- Modal Image ID for filesystem snapshot restoration + snapshot_image_profile TEXT, -- Sandbox image profile used to create snapshot_image_id auth_token TEXT, -- Token for sandbox to authenticate back to control plane auth_token_hash TEXT, -- SHA-256 hash of sandbox auth token (preferred) status TEXT DEFAULT 'pending', -- 'pending', 'spawning', 'connecting', 'warming', 'syncing', 'ready', 'running', 'stale', 'snapshotting', 'stopped', 'failed' @@ -383,6 +384,11 @@ export const MIGRATIONS: readonly SchemaMigration[] = [ description: "Add total_cost to session", run: `ALTER TABLE session ADD COLUMN total_cost REAL NOT NULL DEFAULT 0`, }, + { + id: 31, + description: "Add snapshot_image_profile to sandbox", + run: `ALTER TABLE sandbox ADD COLUMN snapshot_image_profile TEXT`, + }, ]; /** diff --git a/packages/control-plane/src/session/types.ts b/packages/control-plane/src/session/types.ts index 25a4eb5f0..93e40f5ac 100644 --- a/packages/control-plane/src/session/types.ts +++ b/packages/control-plane/src/session/types.ts @@ -15,6 +15,7 @@ import type { EventType, } from "../types"; import type { GitPushSpec } from "../source-control"; +import type { SandboxImageProfile } from "@open-inspect/shared"; // Database row types (match SQLite schema) @@ -97,6 +98,7 @@ export interface SandboxRow { modal_object_id: string | null; // Legacy column: provider object ID (Modal object ID or Daytona handle) snapshot_id: string | null; snapshot_image_id: string | null; // Modal Image ID for filesystem snapshot restoration + snapshot_image_profile: SandboxImageProfile | null; // Image profile used when snapshot_image_id was created auth_token: string | null; auth_token_hash: string | null; // SHA-256 hash of sandbox auth token status: SandboxStatus; diff --git a/packages/control-plane/src/session/websocket-manager.test.ts b/packages/control-plane/src/session/websocket-manager.test.ts index 70f5fb0b0..c4adbb806 100644 --- a/packages/control-plane/src/session/websocket-manager.test.ts +++ b/packages/control-plane/src/session/websocket-manager.test.ts @@ -156,6 +156,7 @@ function createSandboxRow(modalSandboxId: string): SandboxRow { modal_object_id: null, snapshot_id: null, snapshot_image_id: null, + snapshot_image_profile: null, auth_token: null, auth_token_hash: null, status: "ready", diff --git a/packages/control-plane/test/integration/integration-settings.test.ts b/packages/control-plane/test/integration/integration-settings.test.ts index 2a6e173cc..dbd81d0e0 100644 --- a/packages/control-plane/test/integration/integration-settings.test.ts +++ b/packages/control-plane/test/integration/integration-settings.test.ts @@ -542,7 +542,7 @@ describe("Integration settings API", () => { headers, body: JSON.stringify({ settings: { - defaults: { tunnelPorts: [3000] }, + defaults: { tunnelPorts: [3000], dockerEnabled: true }, }, }), }); @@ -554,10 +554,11 @@ describe("Integration settings API", () => { expect(getRes.status).toBe(200); const body = await getRes.json<{ settings: { - defaults: { tunnelPorts: number[] }; + defaults: { tunnelPorts: number[]; dockerEnabled: boolean }; }; }>(); expect(body.settings.defaults.tunnelPorts).toEqual([3000]); + expect(body.settings.defaults.dockerEnabled).toBe(true); }); it("PUT /integration-settings/sandbox with invalid tunnelPorts returns 400", async () => { @@ -575,6 +576,21 @@ describe("Integration settings API", () => { expect(body.error).toContain("tunnelPorts must be an array"); }); + it("PUT /integration-settings/sandbox with invalid dockerEnabled returns 400", async () => { + const headers = await authHeaders(); + const response = await SELF.fetch( + "https://test.local/integration-settings/sandbox/repos/acme/widgets", + { + method: "PUT", + headers, + body: JSON.stringify({ settings: { dockerEnabled: "true" } }), + } + ); + expect(response.status).toBe(400); + const body = await response.json<{ error: string }>(); + expect(body.error).toContain("dockerEnabled must be a boolean"); + }); + it("GET /integration-settings/sandbox/resolved returns default empty tunnelPorts when unconfigured", async () => { const headers = await authHeaders(); const res = await SELF.fetch( diff --git a/packages/control-plane/test/integration/repo-images.test.ts b/packages/control-plane/test/integration/repo-images.test.ts index 2841cd13a..c99894b6e 100644 --- a/packages/control-plane/test/integration/repo-images.test.ts +++ b/packages/control-plane/test/integration/repo-images.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach } from "vitest"; import { SELF, env } from "cloudflare:test"; import { generateInternalToken } from "../../src/auth/internal"; +import { IntegrationSettingsStore } from "../../src/db/integration-settings"; import { RepoImageStore } from "../../src/db/repo-images"; import { RepoMetadataStore } from "../../src/db/repo-metadata"; import { cleanD1Tables } from "./cleanup"; @@ -33,6 +34,7 @@ describe("D1 RepoImageStore", () => { base_branch: "main", provider_image_id: "", base_sha: "", + image_profile: "default", }); expect(status[0].created_at).toBeGreaterThan(0); }); @@ -176,6 +178,34 @@ describe("D1 RepoImageStore", () => { expect(result!.id).toBe("img-1"); }); + it("getLatestReady filters by Docker capability", async () => { + await metadataStore.setImageBuildEnabled("acme", "repo", true); + await store.registerBuild({ + id: "img-plain", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + }); + await store.markReady("img-plain", "modal-img-plain", "sha1", 30); + + await store.registerBuild({ + id: "img-docker", + repoOwner: "acme", + repoName: "repo", + baseBranch: "main", + imageProfile: "docker", + }); + await store.markReady("img-docker", "modal-img-docker", "sha2", 35); + + const plain = await store.getLatestReady("acme", "repo", "main", "default"); + expect(plain).not.toBeNull(); + expect(plain!.id).toBe("img-plain"); + + const docker = await store.getLatestReady("acme", "repo", "main", "docker"); + expect(docker).not.toBeNull(); + expect(docker!.id).toBe("img-docker"); + }); + it("getStatus returns builds ordered by created_at DESC", async () => { await store.registerBuild({ id: "img-1", @@ -420,11 +450,33 @@ describe("Repo image HTTP routes", () => { expect(response.status).toBe(200); const body = await response.json<{ - repos: Array<{ repoOwner: string; repoName: string }>; + repos: Array<{ repoOwner: string; repoName: string; imageProfile: string }>; }>(); expect(body.repos).toHaveLength(2); const names = body.repos.map((r) => r.repoName).sort(); expect(names).toEqual(["repo-a", "repo-c"]); + expect(body.repos.every((r) => r.imageProfile === "default")).toBe(true); + }); + + it("GET /repo-images/enabled-repos includes resolved Docker setting", async () => { + const metadataStore = new RepoMetadataStore(env.DB); + await metadataStore.setImageBuildEnabled("acme", "repo-a", true); + await new IntegrationSettingsStore(env.DB).setRepoSettings("sandbox", "acme/repo-a", { + dockerEnabled: true, + }); + + const headers = await authHeaders(); + delete (headers as Record)["Content-Type"]; + + const response = await SELF.fetch("https://test.local/repo-images/enabled-repos", { + headers, + }); + + expect(response.status).toBe(200); + const body = await response.json<{ + repos: Array<{ repoOwner: string; repoName: string; imageProfile: string }>; + }>(); + expect(body.repos).toEqual([{ repoOwner: "acme", repoName: "repo-a", imageProfile: "docker" }]); }); it("PUT /repo-images/toggle/:owner/:name enables image build", async () => { diff --git a/packages/control-plane/tsconfig.json b/packages/control-plane/tsconfig.json index 9c5af8d57..bc364b6ab 100644 --- a/packages/control-plane/tsconfig.json +++ b/packages/control-plane/tsconfig.json @@ -16,8 +16,10 @@ "noUnusedLocals": false, "noUnusedParameters": false, "noFallthroughCasesInSwitch": true, + "baseUrl": ".", "paths": { - "@/*": ["./src/*"] + "@/*": ["./src/*"], + "@open-inspect/shared": ["../shared/src/index.ts"] } }, "include": ["src/**/*.ts"], diff --git a/packages/control-plane/vitest.config.ts b/packages/control-plane/vitest.config.ts index 4bed8b2a2..8c15eebae 100644 --- a/packages/control-plane/vitest.config.ts +++ b/packages/control-plane/vitest.config.ts @@ -1,6 +1,15 @@ import { defineConfig } from "vitest/config"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); export default defineConfig({ + resolve: { + alias: { + "@open-inspect/shared": path.resolve(__dirname, "../shared/src/index.ts"), + }, + }, test: { environment: "node", include: ["src/**/*.test.ts"], diff --git a/packages/control-plane/vitest.integration.config.ts b/packages/control-plane/vitest.integration.config.ts index 8a54440c4..cab9a9cbe 100644 --- a/packages/control-plane/vitest.integration.config.ts +++ b/packages/control-plane/vitest.integration.config.ts @@ -47,5 +47,10 @@ export default defineWorkersConfig(async () => { }, }, }, + resolve: { + alias: { + "@open-inspect/shared": path.resolve(__dirname, "../shared/src/index.ts"), + }, + }, }; }); diff --git a/packages/modal-infra/src/app.py b/packages/modal-infra/src/app.py index ae264d2f0..98b25df4e 100644 --- a/packages/modal-infra/src/app.py +++ b/packages/modal-infra/src/app.py @@ -61,6 +61,8 @@ # ALLOWED_CONTROL_PLANE_HOSTS: comma-separated list of permitted callback hosts # SCM_PROVIDER: "github" (default) or "gitlab" — selects the clone credential type # GITLAB_ACCESS_TOKEN: GitLab PAT used as clone credential when SCM_PROVIDER=gitlab +# MODAL_DOCKER_SANDBOX_CPU: optional CPU override for Docker-enabled sandboxes +# MODAL_DOCKER_SANDBOX_MEMORY_MB: optional memory override for Docker-enabled sandboxes internal_api_secret = modal.Secret.from_name( "internal-api", required_keys=["MODAL_API_SECRET", "INTERNAL_CALLBACK_SECRET"], diff --git a/packages/modal-infra/src/images/__init__.py b/packages/modal-infra/src/images/__init__.py index 79ab9f4a6..1e05cd39c 100644 --- a/packages/modal-infra/src/images/__init__.py +++ b/packages/modal-infra/src/images/__init__.py @@ -1,5 +1,5 @@ """Image definitions for Open-Inspect sandboxes.""" -from .base import OPENCODE_VERSION, base_image +from .base import OPENCODE_VERSION, base_image, docker_image -__all__ = ["OPENCODE_VERSION", "base_image"] +__all__ = ["OPENCODE_VERSION", "base_image", "docker_image"] diff --git a/packages/modal-infra/src/images/base.py b/packages/modal-infra/src/images/base.py index 340e80b2b..b843aed66 100644 --- a/packages/modal-infra/src/images/base.py +++ b/packages/modal-infra/src/images/base.py @@ -11,12 +11,19 @@ - Sandbox entrypoint and bridge code """ +import os from pathlib import Path import modal import sandbox_runtime +# Modal reads image builder version from process/workspace config while resolving +# images, not from an individual image option. Docker-in-sandbox launch support +# requires the 2025.06 builder, so set a default for every image defined here +# while preserving an explicit deployment override. +os.environ.setdefault("MODAL_IMAGE_BUILDER_VERSION", "2025.06") + # Get the path to the sandbox runtime code (provider-agnostic) SANDBOX_RUNTIME_DIR = Path(sandbox_runtime.__file__).parent @@ -44,6 +51,13 @@ # ttyd version to install (pinned for reproducible images) TTYD_VERSION = "1.7.7" TTYD_SHA256 = "8a217c968aba172e0dbf3f34447218dc015bc4d5e59bf51db2f2cd12b7be4f55" +DOCKER_CE_VERSION = "5:29.5.2-1~debian.12~bookworm" +DOCKER_CE_CLI_VERSION = "5:29.5.2-1~debian.12~bookworm" +CONTAINERD_VERSION = "2.2.4-1~debian.12~bookworm" +DOCKER_BUILDX_PLUGIN_VERSION = "0.34.1-1~debian.12~bookworm" +DOCKER_COMPOSE_PLUGIN_VERSION = "5.1.4-1~debian.12~bookworm" +RUNC_VERSION = "1.3.0" +RUNC_SHA256 = "028986516ab5646370edce981df2d8e8a8d12188deaf837142a02097000ae2f2" # Cache buster - change this to force Modal image rebuild # v51: SCM credential helper backed by control plane; remove embedded VCS tokens @@ -217,6 +231,40 @@ "npm cache clean --force", ) +# Image variant with Docker Engine, Buildx, and Docker Compose for Modal's +# Docker-in-Sandboxes alpha. The runtime starts dockerd on demand. +docker_image = ( + base_image.apt_install("iproute2", "iptables", "wget") + .run_commands( + "install -m 0755 -d /etc/apt/keyrings", + "curl -fsSL https://download.docker.com/linux/debian/gpg -o /etc/apt/keyrings/docker.asc", + "chmod a+r /etc/apt/keyrings/docker.asc", + 'echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] ' + 'https://download.docker.com/linux/debian $(. /etc/os-release && echo "$VERSION_CODENAME") stable" ' + "> /etc/apt/sources.list.d/docker.list", + "apt-get update && apt-get install -y " + f"docker-ce={DOCKER_CE_VERSION} " + f"docker-ce-cli={DOCKER_CE_CLI_VERSION} " + f"containerd.io={CONTAINERD_VERSION} " + f"docker-buildx-plugin={DOCKER_BUILDX_PLUGIN_VERSION} " + f"docker-compose-plugin={DOCKER_COMPOSE_PLUGIN_VERSION} " + "&& rm -rf /var/lib/apt/lists/*", + ) + .run_commands( + "rm -f $(command -v runc)", + f"wget -q https://github.com/opencontainers/runc/releases/download/v{RUNC_VERSION}/runc.amd64", + f'echo "{RUNC_SHA256} runc.amd64" | sha256sum -c -', + "chmod +x runc.amd64", + "mv runc.amd64 /usr/local/bin/runc", + "update-alternatives --set iptables /usr/sbin/iptables-legacy", + "update-alternatives --set ip6tables /usr/sbin/ip6tables-legacy", + "mkdir -p /opt/docker-data", + "docker --version", + "docker compose version", + "runc --version", + ) +) + # Image variant optimized for Python projects python_image = base_image.run_commands( # Pre-create virtual environment diff --git a/packages/modal-infra/src/sandbox/launch_options.py b/packages/modal-infra/src/sandbox/launch_options.py new file mode 100644 index 000000000..3b312a94a --- /dev/null +++ b/packages/modal-infra/src/sandbox/launch_options.py @@ -0,0 +1,105 @@ +"""Modal launch option planning for sandbox runtime settings.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +import modal + +from sandbox_runtime.constants import EXPECTED_TUNNEL_PORTS_ENV_VAR + +from ..app import app +from ..images.base import base_image, docker_image +from .settings import ( + DOCKER_IMAGE_PROFILE, + DockerDeployPolicy, + DockerLaunchSettings, + RuntimePortSettings, + SandboxImageProfile, + SandboxRuntimeSettings, +) + + +@dataclass(frozen=True, slots=True) +class RuntimeLaunchOptions: + """Modal launch options derived from parsed sandbox runtime settings.""" + + image_profile: SandboxImageProfile + docker: DockerLaunchSettings + terminal_enabled: bool = False + exposed_ports: tuple[int, ...] = () + tunnel_ports: tuple[int, ...] = () + + @classmethod + def for_session( + cls, + settings: SandboxRuntimeSettings, + code_server_enabled: bool, + image_profile: SandboxImageProfile, + docker_policy: DockerDeployPolicy | None = None, + ) -> RuntimeLaunchOptions: + ports = RuntimePortSettings.from_settings(settings, code_server_enabled) + return cls( + image_profile=image_profile, + docker=DockerLaunchSettings.from_profile(image_profile, docker_policy), + terminal_enabled=settings.terminal_enabled, + exposed_ports=ports.exposed_ports, + tunnel_ports=ports.tunnel_ports, + ) + + @classmethod + def for_image_build( + cls, + image_profile: SandboxImageProfile, + docker_policy: DockerDeployPolicy | None = None, + ) -> RuntimeLaunchOptions: + return cls( + image_profile=image_profile, + docker=DockerLaunchSettings.from_profile(image_profile, docker_policy), + ) + + +def select_base_image(image_profile: SandboxImageProfile) -> modal.Image: + return docker_image if image_profile == DOCKER_IMAGE_PROFILE else base_image + + +def select_runtime_image( + image_profile: SandboxImageProfile, + *, + repo_image_id: str | None = None, +) -> tuple[modal.Image, str]: + if repo_image_id: + return modal.Image.from_id(repo_image_id), "repo" + return select_base_image(image_profile), "base" + + +def build_modal_create_kwargs( + launch_options: RuntimeLaunchOptions, + *, + image: modal.Image, + secrets: list[Any], + timeout_seconds: int, + env_vars: dict[str, str], +) -> dict[str, Any]: + launch_env_vars = dict(env_vars) + if launch_options.terminal_enabled: + launch_env_vars["TERMINAL_ENABLED"] = "true" + if launch_options.tunnel_ports: + launch_env_vars[EXPECTED_TUNNEL_PORTS_ENV_VAR] = ",".join( + str(p) for p in launch_options.tunnel_ports + ) + launch_env_vars.update(launch_options.docker.env_vars()) + + create_kwargs: dict[str, Any] = { + "image": image, + "app": app, + "secrets": secrets, + "timeout": timeout_seconds, + "workdir": "/workspace", + "env": launch_env_vars, + } + if launch_options.exposed_ports: + create_kwargs["encrypted_ports"] = list(launch_options.exposed_ports) + create_kwargs.update(launch_options.docker.modal_create_kwargs()) + return create_kwargs diff --git a/packages/modal-infra/src/sandbox/manager.py b/packages/modal-infra/src/sandbox/manager.py index bab361f18..f48cacfd7 100644 --- a/packages/modal-infra/src/sandbox/manager.py +++ b/packages/modal-infra/src/sandbox/manager.py @@ -15,28 +15,31 @@ import os import secrets import time -from dataclasses import dataclass -from typing import Any +from dataclasses import dataclass, field import modal from sandbox_runtime.constants import ( CODE_SERVER_PORT, - EXPECTED_TUNNEL_PORTS_ENV_VAR, TTYD_PROXY_PORT, TUNNEL_ENV_FILE_PATH, ) from sandbox_runtime.log_config import get_logger from sandbox_runtime.types import SandboxStatus, SessionConfig -from ..app import app, llm_secrets -from ..images.base import base_image +from ..app import llm_secrets +from .launch_options import ( + RuntimeLaunchOptions, + build_modal_create_kwargs, + select_base_image, + select_runtime_image, +) +from .settings import DockerDeployPolicy, SandboxImageProfile, SandboxRuntimeSettings log = get_logger("manager") DEFAULT_SANDBOX_TIMEOUT_SECONDS = 7200 # 2 hours SNAPSHOT_FILESYSTEM_TIMEOUT_SECONDS = 300 -MAX_TUNNEL_PORTS = 10 @dataclass @@ -46,7 +49,6 @@ class SandboxConfig: repo_owner: str repo_name: str sandbox_id: str | None = None # Expected sandbox ID from control plane - snapshot_id: str | None = None session_config: SessionConfig | None = None control_plane_url: str = "" sandbox_auth_token: str = "" @@ -59,9 +61,8 @@ class SandboxConfig: agent_slack_notify_enabled: bool = ( False # Whether to install the agent-initiated slack-notify tool ) - settings: dict[str, Any] | None = ( - None # Sandbox settings (tunnelPorts, etc.) from control plane - ) + settings: SandboxRuntimeSettings = field(default_factory=SandboxRuntimeSettings.default) + image_profile: SandboxImageProfile = "default" @dataclass @@ -149,40 +150,6 @@ async def _resolve_tunnels( await asyncio.sleep(backoff * (attempt + 1)) return resolved - @staticmethod - def _validate_ports(raw: list) -> list[int]: - """Validate and sanitize tunnel ports: must be int, 1-65535, max MAX_TUNNEL_PORTS.""" - ports: list[int] = [] - for p in raw: - if isinstance(p, int) and 1 <= p <= 65535: - ports.append(p) - if len(ports) >= MAX_TUNNEL_PORTS: - break - return ports - - @staticmethod - def _collect_exposed_ports( - code_server_enabled: bool, - terminal_enabled: bool, - settings: dict[str, Any] | None, - ) -> tuple[list[int], list[int]]: - """Return (all_exposed_ports, extra_tunnel_ports) from settings and feature flags.""" - reserved: set[int] = set() - exposed: list[int] = [] - if code_server_enabled: - exposed.append(CODE_SERVER_PORT) - reserved.add(CODE_SERVER_PORT) - if terminal_enabled: - exposed.append(TTYD_PROXY_PORT) - reserved.add(TTYD_PROXY_PORT) - - raw_ports = (settings or {}).get("tunnelPorts", []) - tunnel_ports = SandboxManager._validate_ports(raw_ports) if raw_ports else [] - # Remove reserved ports from tunnel_ports to avoid duplicates - tunnel_ports = [p for p in tunnel_ports if p not in reserved] - exposed.extend(tunnel_ports) - return exposed, tunnel_ports - @staticmethod async def _resolve_and_setup_tunnels( sandbox: modal.Sandbox, @@ -302,8 +269,7 @@ async def create_sandbox( """ Create a new sandbox for a session. - If a snapshot_id is provided, restores from that snapshot. - Otherwise, creates from the latest image for the repo. + Creates from the latest compatible repo image when one is provided. Args: config: Sandbox configuration including repo info and session config @@ -336,7 +302,7 @@ async def create_sandbox( } ) - # A boot from a pre-built image (session snapshot or repo image) may + # A boot from a pre-built repo image may # run an entrypoint built before the credential-helper migration: no # helper, and the old entrypoint expects VCS_CLONE_TOKEN in env to # rewrite origin. Pass the fresh token through for those (with the @@ -345,7 +311,7 @@ async def create_sandbox( # helper and need no token in env. Repo images are selected by SHA and # aren't rebuilt by a CACHE_BUSTER bump, so we can't assume they're # current. - boots_from_prebuilt_image = bool(config.snapshot_id or config.repo_image_id) + boots_from_prebuilt_image = bool(config.repo_image_id) self._inject_vcs_env_vars( env_vars, clone_token=config.clone_token if boots_from_prebuilt_image else None, @@ -357,9 +323,13 @@ async def create_sandbox( code_server_password = self._generate_code_server_password() env_vars["CODE_SERVER_PASSWORD"] = code_server_password - terminal_enabled = bool((config.settings or {}).get("terminalEnabled", False)) - if terminal_enabled: - env_vars["TERMINAL_ENABLED"] = "true" + runtime_settings = config.settings + launch_options = RuntimeLaunchOptions.for_session( + runtime_settings, + config.code_server_enabled, + config.image_profile, + docker_policy=DockerDeployPolicy.from_env(), + ) if config.agent_slack_notify_enabled: env_vars["AGENT_SLACK_NOTIFY_ENABLED"] = "true" @@ -367,32 +337,21 @@ async def create_sandbox( if config.session_config: env_vars["SESSION_CONFIG"] = config.session_config.model_dump_json() - # Determine image to use (priority: session snapshot > repo image > base image) - if config.snapshot_id: - image = modal.Image.from_registry(f"open-inspect-snapshot:{config.snapshot_id}") - elif config.repo_image_id: - image = modal.Image.from_id(config.repo_image_id) + image, image_source = select_runtime_image( + launch_options.image_profile, + repo_image_id=config.repo_image_id, + ) + if config.repo_image_id: env_vars["FROM_REPO_IMAGE"] = "true" env_vars["REPO_IMAGE_SHA"] = config.repo_image_sha or "" - else: - image = base_image - exposed_ports, tunnel_ports = self._collect_exposed_ports( - config.code_server_enabled, terminal_enabled, config.settings + create_kwargs = build_modal_create_kwargs( + launch_options, + image=image, + secrets=[llm_secrets], + timeout_seconds=config.timeout_seconds, + env_vars=env_vars, ) - if tunnel_ports: - env_vars[EXPECTED_TUNNEL_PORTS_ENV_VAR] = ",".join(str(p) for p in tunnel_ports) - - create_kwargs: dict = { - "image": image, - "app": app, - "secrets": [llm_secrets], - "timeout": config.timeout_seconds, - "workdir": "/workspace", - "env": env_vars, - } - if exposed_ports: - create_kwargs["encrypted_ports"] = exposed_ports sandbox = await modal.Sandbox.create.aio( "python", @@ -403,7 +362,11 @@ async def create_sandbox( modal_object_id = sandbox.object_id code_server_url, ttyd_url, extra_tunnel_urls = await self._resolve_and_setup_tunnels( - sandbox, sandbox_id, config.code_server_enabled, terminal_enabled, tunnel_ports + sandbox, + sandbox_id, + config.code_server_enabled, + launch_options.terminal_enabled, + list(launch_options.tunnel_ports), ) duration_ms = int((time.time() - start_time) * 1000) @@ -415,6 +378,9 @@ async def create_sandbox( repo_name=config.repo_name, duration_ms=duration_ms, outcome="success", + docker_enabled=launch_options.docker.enabled, + image_profile=launch_options.image_profile, + image_source=image_source, ) return SandboxHandle( @@ -422,7 +388,6 @@ async def create_sandbox( modal_sandbox=sandbox, status=SandboxStatus.WARMING, created_at=time.time(), - snapshot_id=config.snapshot_id, modal_object_id=modal_object_id, code_server_url=code_server_url, code_server_password=code_server_password, @@ -437,6 +402,7 @@ async def create_build_sandbox( default_branch: str = "main", clone_token: str = "", user_env_vars: dict[str, str] | None = None, + image_profile: SandboxImageProfile = "default", ) -> SandboxHandle: """ Create a sandbox specifically for image building. @@ -445,7 +411,7 @@ async def create_build_sandbox( - Sets IMAGE_BUILD_MODE=true (exits after setup, no OpenCode/bridge) - No SANDBOX_AUTH_TOKEN, CONTROL_PLANE_URL, or LLM secrets - Shorter timeout (30 min vs 2 hours) - - Always uses base_image (builds start from the universal base) + - Uses the Docker-capable base when the docker image profile is requested Note: MCP servers are not available during image builds (no session config). MCP packages are installed at first use via npx instead. @@ -473,17 +439,23 @@ async def create_build_sandbox( ) self._inject_vcs_env_vars(env_vars, clone_token or None) + launch_options = RuntimeLaunchOptions.for_image_build( + image_profile, + docker_policy=DockerDeployPolicy.from_env(), + ) + create_kwargs = build_modal_create_kwargs( + launch_options, + image=select_base_image(launch_options.image_profile), + secrets=[], + timeout_seconds=BUILD_TIMEOUT_SECONDS, + env_vars=env_vars, + ) sandbox = await modal.Sandbox.create.aio( "python", "-m", "sandbox_runtime.entrypoint", - image=base_image, - app=app, - secrets=[], - timeout=BUILD_TIMEOUT_SECONDS, - workdir="/workspace", - env=env_vars, + **create_kwargs, ) modal_object_id = sandbox.object_id @@ -496,6 +468,9 @@ async def create_build_sandbox( repo_name=repo_name, duration_ms=duration_ms, outcome="success", + docker_enabled=launch_options.docker.enabled, + image_profile=launch_options.image_profile, + image_source="base", ) return SandboxHandle( @@ -626,7 +601,8 @@ async def restore_from_snapshot( timeout_seconds: int = DEFAULT_SANDBOX_TIMEOUT_SECONDS, code_server_enabled: bool = False, agent_slack_notify_enabled: bool = False, - settings: dict[str, Any] | None = None, + settings: SandboxRuntimeSettings | None = None, + image_profile: SandboxImageProfile = "default", ) -> SandboxHandle: """ Create a new sandbox from a filesystem snapshot Image. @@ -699,29 +675,24 @@ async def restore_from_snapshot( code_server_password = self._generate_code_server_password() env_vars["CODE_SERVER_PASSWORD"] = code_server_password - terminal_enabled = bool((settings or {}).get("terminalEnabled", False)) - if terminal_enabled: - env_vars["TERMINAL_ENABLED"] = "true" + runtime_settings = settings or SandboxRuntimeSettings.default() + launch_options = RuntimeLaunchOptions.for_session( + runtime_settings, + code_server_enabled, + image_profile, + docker_policy=DockerDeployPolicy.from_env(), + ) if agent_slack_notify_enabled: env_vars["AGENT_SLACK_NOTIFY_ENABLED"] = "true" - exposed_ports, tunnel_ports = self._collect_exposed_ports( - code_server_enabled, terminal_enabled, settings + create_kwargs = build_modal_create_kwargs( + launch_options, + image=image, + secrets=[llm_secrets], + timeout_seconds=timeout_seconds, + env_vars=env_vars, ) - if tunnel_ports: - env_vars[EXPECTED_TUNNEL_PORTS_ENV_VAR] = ",".join(str(p) for p in tunnel_ports) - - create_kwargs: dict = { - "image": image, - "app": app, - "secrets": [llm_secrets], - "timeout": timeout_seconds, - "workdir": "/workspace", - "env": env_vars, - } - if exposed_ports: - create_kwargs["encrypted_ports"] = exposed_ports sandbox = await modal.Sandbox.create.aio( "python", @@ -732,7 +703,11 @@ async def restore_from_snapshot( modal_object_id = sandbox.object_id code_server_url, ttyd_url, extra_tunnel_urls = await self._resolve_and_setup_tunnels( - sandbox, sandbox_id, code_server_enabled, terminal_enabled, tunnel_ports + sandbox, + sandbox_id, + code_server_enabled, + launch_options.terminal_enabled, + list(launch_options.tunnel_ports), ) duration_ms = int((time.time() - start_time) * 1000) @@ -745,6 +720,9 @@ async def restore_from_snapshot( repo_name=repo_name, duration_ms=duration_ms, outcome="success", + docker_enabled=launch_options.docker.enabled, + image_profile=launch_options.image_profile, + image_source="snapshot", ) return SandboxHandle( diff --git a/packages/modal-infra/src/sandbox/settings.py b/packages/modal-infra/src/sandbox/settings.py new file mode 100644 index 000000000..0382dc621 --- /dev/null +++ b/packages/modal-infra/src/sandbox/settings.py @@ -0,0 +1,177 @@ +"""Typed sandbox runtime settings parsed from control-plane payloads.""" + +from __future__ import annotations + +import os +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any, Literal + +from sandbox_runtime.constants import CODE_SERVER_PORT, TTYD_PROXY_PORT + +MAX_TUNNEL_PORTS = 10 +DOCKER_DATA_ROOT = "/opt/docker-data" +DEFAULT_IMAGE_PROFILE = "default" +DOCKER_IMAGE_PROFILE = "docker" +SandboxImageProfile = Literal["default", "docker"] +IMAGE_PROFILES = frozenset({DEFAULT_IMAGE_PROFILE, DOCKER_IMAGE_PROFILE}) + + +def _optional_env_float(name: str) -> float | None: + raw = os.environ.get(name) + if not raw: + return None + try: + value = float(raw) + return value if value > 0 else None + except ValueError: + return None + + +def _optional_env_int(name: str) -> int | None: + raw = os.environ.get(name) + if not raw: + return None + try: + value = int(raw) + return value if value > 0 else None + except ValueError: + return None + + +def validate_tunnel_ports(raw: object) -> tuple[int, ...]: + """Validate and sanitize tunnel ports: int, 1-65535, capped at MAX_TUNNEL_PORTS.""" + if not isinstance(raw, list | tuple): + return () + + ports: list[int] = [] + for port in raw: + if type(port) is int and 1 <= port <= 65535: + ports.append(port) + if len(ports) >= MAX_TUNNEL_PORTS: + break + return tuple(ports) + + +def parse_bool_setting(value: object) -> bool: + """Only real JSON booleans enable runtime features.""" + return value is True + + +def parse_sandbox_image_profile(value: object) -> SandboxImageProfile: + if value is None: + return DEFAULT_IMAGE_PROFILE + if value in IMAGE_PROFILES: + return value + raise ValueError(f"Invalid sandbox image profile: {value!r}") + + +@dataclass(frozen=True, slots=True) +class RuntimePortSettings: + """Modal port exposure derived from user sandbox settings.""" + + exposed_ports: tuple[int, ...] = () + tunnel_ports: tuple[int, ...] = () + + @classmethod + def from_settings( + cls, settings: SandboxRuntimeSettings, code_server_enabled: bool + ) -> RuntimePortSettings: + reserved: set[int] = set() + exposed: list[int] = [] + if code_server_enabled: + exposed.append(CODE_SERVER_PORT) + reserved.add(CODE_SERVER_PORT) + if settings.terminal_enabled: + exposed.append(TTYD_PROXY_PORT) + reserved.add(TTYD_PROXY_PORT) + + tunnel_ports = tuple(p for p in settings.tunnel_ports if p not in reserved) + exposed.extend(tunnel_ports) + return cls(exposed_ports=tuple(exposed), tunnel_ports=tunnel_ports) + + +@dataclass(frozen=True, slots=True) +class DockerDeployPolicy: + """Deploy-level Docker resource overrides. + + Read once from the environment at the request/config layer (see + ``from_env``) and injected into ``DockerLaunchSettings.from_profile`` so the + launch-settings value object stays pure and testable without monkeypatching + ``os.environ``. + """ + + cpu: float | None = None + memory_mb: int | None = None + + @classmethod + def from_env(cls) -> DockerDeployPolicy: + return cls( + cpu=_optional_env_float("MODAL_DOCKER_SANDBOX_CPU"), + memory_mb=_optional_env_int("MODAL_DOCKER_SANDBOX_MEMORY_MB"), + ) + + +@dataclass(frozen=True, slots=True) +class DockerLaunchSettings: + """Docker-specific Modal launch settings after deploy policy is applied.""" + + enabled: bool = False + data_root: str = DOCKER_DATA_ROOT + cpu: float | None = None + memory_mb: int | None = None + + @classmethod + def from_profile( + cls, + image_profile: SandboxImageProfile, + policy: DockerDeployPolicy | None = None, + ) -> DockerLaunchSettings: + if image_profile != DOCKER_IMAGE_PROFILE: + return cls() + policy = policy or DockerDeployPolicy() + return cls( + enabled=True, + cpu=policy.cpu, + memory_mb=policy.memory_mb, + ) + + def env_vars(self) -> dict[str, str]: + if not self.enabled: + return {} + return { + "OPENINSPECT_DOCKER_ENABLED": "true", + "DOCKER_DATA_ROOT": self.data_root, + } + + def modal_create_kwargs(self) -> dict[str, Any]: + if not self.enabled: + return {} + kwargs: dict[str, Any] = { + "experimental_options": {"enable_docker": True}, + } + if self.cpu is not None: + kwargs["cpu"] = self.cpu + if self.memory_mb is not None: + kwargs["memory"] = self.memory_mb + return kwargs + + +@dataclass(frozen=True, slots=True) +class SandboxRuntimeSettings: + """Sandbox settings parsed from a control-plane request.""" + + tunnel_ports: tuple[int, ...] = () + terminal_enabled: bool = False + + @classmethod + def default(cls) -> SandboxRuntimeSettings: + return cls() + + @classmethod + def from_raw(cls, raw: Mapping[str, Any] | None) -> SandboxRuntimeSettings: + payload: Mapping[str, Any] = raw if isinstance(raw, Mapping) else {} + return cls( + tunnel_ports=validate_tunnel_ports(payload.get("tunnelPorts", [])), + terminal_enabled=parse_bool_setting(payload.get("terminalEnabled")), + ) diff --git a/packages/modal-infra/src/scheduler/image_builder.py b/packages/modal-infra/src/scheduler/image_builder.py index 054467199..7703ffa00 100644 --- a/packages/modal-infra/src/scheduler/image_builder.py +++ b/packages/modal-infra/src/scheduler/image_builder.py @@ -39,6 +39,7 @@ ) from ..auth import generate_internal_token from ..log_config import get_logger +from ..sandbox.settings import DEFAULT_IMAGE_PROFILE, parse_sandbox_image_profile log = get_logger("image_builder") @@ -48,7 +49,6 @@ # Build log errors are surfaced through callbacks; keep them concise. BUILD_FAILURE_MESSAGE_MAX_CHARS = 500 - _SETUP_FAILURE_EVENTS = {"setup.failed", "setup.timeout", "setup.error"} _SUPERVISOR_FAILURE_EVENTS = {"supervisor.error", "supervisor.fatal"} @@ -247,6 +247,7 @@ async def build_repo_image( callback_url: str = "", build_id: str = "", user_env_vars: dict[str, str] | None = None, + image_profile: str = DEFAULT_IMAGE_PROFILE, ) -> None: """ Async worker: create build sandbox, await exit, snapshot, callback. @@ -261,6 +262,7 @@ async def build_repo_image( callback_url: URL to POST success result to build_id: Build identifier from the control plane user_env_vars: User-defined environment variables (repo secrets) injected into the build sandbox + image_profile: Resolved sandbox image profile for this build """ from ..sandbox.manager import SNAPSHOT_FILESYSTEM_TIMEOUT_SECONDS, SandboxManager @@ -269,6 +271,12 @@ async def build_repo_image( log.error("build.invalid_callback_url", url=callback_url, build_id=build_id) return + try: + resolved_image_profile = parse_sandbox_image_profile(image_profile) + except ValueError as e: + log.error("build.invalid_image_profile", build_id=build_id, error=str(e)) + return + start_time = time.time() manager = SandboxManager() handle = None @@ -292,6 +300,7 @@ async def build_repo_image( default_branch=default_branch, clone_token=clone_token, user_env_vars=user_env_vars, + image_profile=resolved_image_profile, ) # 3. Stream stdout until build completes (sandbox stays alive for snapshotting) @@ -471,6 +480,7 @@ def _should_rebuild( repo_name: str, remote_sha: str, all_images: list[dict], + image_profile: str = DEFAULT_IMAGE_PROFILE, ) -> bool: """ Determine if a repo needs a rebuild based on current image status. @@ -486,6 +496,7 @@ def _should_rebuild( for img in all_images if img.get("repo_owner", "").lower() == owner_lower and img.get("repo_name", "").lower() == name_lower + and (img.get("image_profile") or DEFAULT_IMAGE_PROFILE) == image_profile ] # Check if there's already a build in progress @@ -570,6 +581,18 @@ async def rebuild_repo_images(): for repo in enabled_repos: repo_owner = repo.get("repoOwner", "") repo_name = repo.get("repoName", "") + try: + image_profile = parse_sandbox_image_profile( + repo.get("imageProfile") or DEFAULT_IMAGE_PROFILE + ) + except ValueError as e: + log.error( + "scheduler.invalid_image_profile", + repo_owner=repo_owner, + repo_name=repo_name, + error=str(e), + ) + continue if not repo_owner or not repo_name: continue @@ -578,16 +601,17 @@ async def rebuild_repo_images(): if not remote_sha: continue - if _should_rebuild(repo_owner, repo_name, remote_sha, all_images): + if _should_rebuild(repo_owner, repo_name, remote_sha, all_images, image_profile): try: await _api_post( - f"{control_plane_url}/repo-images/trigger/{repo_owner}/{repo_name}", + f"{control_plane_url}/repo-images/trigger/{repo_owner}/{repo_name}" ) builds_triggered += 1 log.info( "scheduler.build_triggered", repo_owner=repo_owner, repo_name=repo_name, + image_profile=image_profile, ) except Exception as e: log.error( diff --git a/packages/modal-infra/src/web_api.py b/packages/modal-infra/src/web_api.py index d547ae950..14da25c32 100644 --- a/packages/modal-infra/src/web_api.py +++ b/packages/modal-infra/src/web_api.py @@ -107,6 +107,15 @@ def require_valid_control_plane_url(url: str | None) -> None: ) +def parse_request_image_profile(request: dict): + from .sandbox.settings import parse_sandbox_image_profile + + try: + return parse_sandbox_image_profile(request.get("image_profile")) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + + @app.function( image=function_image, secrets=[github_app_secrets, internal_api_secret], @@ -133,7 +142,6 @@ async def api_create_sandbox( "repo_name": "...", "control_plane_url": "...", "sandbox_auth_token": "...", - "snapshot_id": null, "provider": "anthropic", "model": "claude-sonnet-4-6" } @@ -151,12 +159,13 @@ async def api_create_sandbox( # Import types and manager directly from .sandbox import SessionConfig from .sandbox.manager import SandboxConfig, SandboxManager + from .sandbox.settings import SandboxRuntimeSettings manager = SandboxManager() - snapshot_id = request.get("snapshot_id") repo_image_id = request.get("repo_image_id") or None - clone_token = _resolve_clone_token() if snapshot_id or repo_image_id else None + clone_token = _resolve_clone_token() if repo_image_id else None + image_profile = parse_request_image_profile(request) session_config = SessionConfig( session_id=request.get("session_id"), @@ -173,7 +182,6 @@ async def api_create_sandbox( repo_owner=request.get("repo_owner"), repo_name=request.get("repo_name"), sandbox_id=request.get("sandbox_id"), # Use control-plane-provided ID for auth - snapshot_id=snapshot_id, session_config=session_config, control_plane_url=control_plane_url, sandbox_auth_token=request.get("sandbox_auth_token"), @@ -183,7 +191,8 @@ async def api_create_sandbox( repo_image_sha=request.get("repo_image_sha") or None, code_server_enabled=bool(request.get("code_server_enabled", False)), agent_slack_notify_enabled=bool(request.get("agent_slack_notify_enabled", False)), - settings=request.get("sandbox_settings") or None, + settings=SandboxRuntimeSettings.from_raw(request.get("sandbox_settings")), + image_profile=image_profile, ) handle = await manager.create_sandbox(config) @@ -201,6 +210,10 @@ async def api_create_sandbox( "tunnel_urls": handle.tunnel_urls, }, } + except HTTPException as e: + outcome = "error" + http_status = e.status_code + raise except Exception as e: outcome = "error" http_status = 500 @@ -459,11 +472,14 @@ async def api_restore_sandbox( try: from .sandbox.manager import DEFAULT_SANDBOX_TIMEOUT_SECONDS, SandboxManager + from .sandbox.settings import SandboxRuntimeSettings session_config = request.get("session_config", {}) sandbox_id = request.get("sandbox_id") sandbox_auth_token = request.get("sandbox_auth_token", "") user_env_vars = request.get("user_env_vars") or None + sandbox_settings = SandboxRuntimeSettings.from_raw(request.get("sandbox_settings")) + image_profile = parse_request_image_profile(request) timeout_seconds = int(request.get("timeout_seconds", DEFAULT_SANDBOX_TIMEOUT_SECONDS)) manager = SandboxManager() @@ -471,7 +487,6 @@ async def api_restore_sandbox( code_server_enabled = bool(request.get("code_server_enabled", False)) agent_slack_notify_enabled = bool(request.get("agent_slack_notify_enabled", False)) - sandbox_settings = request.get("sandbox_settings") or None # Restore sandbox from snapshot handle = await manager.restore_from_snapshot( @@ -486,6 +501,7 @@ async def api_restore_sandbox( code_server_enabled=code_server_enabled, agent_slack_notify_enabled=agent_slack_notify_enabled, settings=sandbox_settings, + image_profile=image_profile, ) return { @@ -570,6 +586,7 @@ async def api_build_repo_image( build_id = request.get("build_id", "") callback_url = request.get("callback_url", "") user_env_vars = request.get("user_env_vars") or None + image_profile = parse_request_image_profile(request) if not repo_owner or not repo_name: raise HTTPException(status_code=400, detail="repo_owner and repo_name are required") @@ -585,6 +602,7 @@ async def api_build_repo_image( callback_url=callback_url, build_id=build_id, user_env_vars=user_env_vars, + image_profile=image_profile, ) return { diff --git a/packages/modal-infra/tests/test_build_sandbox.py b/packages/modal-infra/tests/test_build_sandbox.py index 2aa14cf17..ef5b9ad1e 100644 --- a/packages/modal-infra/tests/test_build_sandbox.py +++ b/packages/modal-infra/tests/test_build_sandbox.py @@ -255,3 +255,25 @@ async def test_system_vars_override_user_env_vars(monkeypatch): env = captured["env"] assert env["IMAGE_BUILD_MODE"] == "true" assert env["SANDBOX_ID"].startswith("build-acme-my-repo-") + + +@pytest.mark.asyncio +async def test_docker_build_sandbox_sets_docker_env_and_experimental_option(monkeypatch): + """Docker-enabled repo image builds should use Modal's Docker sandbox option.""" + captured = {} + monkeypatch.setattr("src.sandbox.manager.modal.Sandbox.create", _fake_sandbox_create(captured)) + + manager = SandboxManager() + await manager.create_build_sandbox( + repo_owner="acme", + repo_name="my-repo", + image_profile="docker", + ) + + env = captured["env"] + assert env["OPENINSPECT_DOCKER_ENABLED"] == "true" + assert env["DOCKER_DATA_ROOT"] == "/opt/docker-data" + assert "OPENINSPECT_SANDBOX_IMAGE_PROFILE" not in env + assert captured["kwargs"]["experimental_options"] == {"enable_docker": True} + assert "cpu" not in captured["kwargs"] + assert "memory" not in captured["kwargs"] diff --git a/packages/modal-infra/tests/test_image_builder_v2.py b/packages/modal-infra/tests/test_image_builder_v2.py index 0803e83a1..14e1d028c 100644 --- a/packages/modal-infra/tests/test_image_builder_v2.py +++ b/packages/modal-infra/tests/test_image_builder_v2.py @@ -406,6 +406,32 @@ async def test_uses_snapshot_timeout_and_terminates_on_success(self): assert callback_payload["provider_image_id"] == "im-test" assert callback_payload["base_sha"] == "abc123" + @pytest.mark.asyncio + async def test_passes_image_profile_to_build_sandbox(self): + handle, _snapshot_aio, _terminate_aio = self._build_handle() + manager = SimpleNamespace(create_build_sandbox=AsyncMock(return_value=handle)) + + with ( + patch("src.scheduler.image_builder.validate_control_plane_url", return_value=True), + patch("src.scheduler.image_builder._generate_clone_token", return_value="gh-token"), + patch("src.sandbox.manager.SandboxManager", return_value=manager), + patch( + "src.scheduler.image_builder._callback_with_retry", + new_callable=AsyncMock, + return_value=True, + ), + ): + await build_repo_image.local( + repo_owner="acme", + repo_name="repo", + callback_url="https://cp.test/repo-images/build-complete", + build_id="img-1", + image_profile="docker", + ) + + manager.create_build_sandbox.assert_awaited_once() + assert manager.create_build_sandbox.await_args.kwargs["image_profile"] == "docker" + @pytest.mark.asyncio async def test_terminates_and_reports_failure_when_snapshot_times_out(self): handle, snapshot_aio, terminate_aio = self._build_handle( diff --git a/packages/modal-infra/tests/test_launch_options.py b/packages/modal-infra/tests/test_launch_options.py new file mode 100644 index 000000000..b5481d6d8 --- /dev/null +++ b/packages/modal-infra/tests/test_launch_options.py @@ -0,0 +1,73 @@ +"""Unit tests for Docker launch settings and deploy-policy injection. + +These exercise the value objects directly — no os.environ monkeypatching — +proving DockerLaunchSettings.from_profile is pure once the deploy policy is +injected. +""" + +from src.sandbox.launch_options import RuntimeLaunchOptions +from src.sandbox.settings import ( + DockerDeployPolicy, + DockerLaunchSettings, + SandboxRuntimeSettings, +) + + +def test_from_profile_default_profile_is_disabled(): + settings = DockerLaunchSettings.from_profile("default") + assert settings.enabled is False + assert settings.cpu is None + assert settings.memory_mb is None + + +def test_from_profile_docker_without_policy_has_no_resource_overrides(): + settings = DockerLaunchSettings.from_profile("docker") + assert settings.enabled is True + assert settings.cpu is None + assert settings.memory_mb is None + + +def test_from_profile_docker_applies_injected_policy(): + policy = DockerDeployPolicy(cpu=2.5, memory_mb=6144) + settings = DockerLaunchSettings.from_profile("docker", policy) + assert settings.enabled is True + assert settings.cpu == 2.5 + assert settings.memory_mb == 6144 + + +def test_from_profile_default_ignores_policy(): + policy = DockerDeployPolicy(cpu=2.5, memory_mb=6144) + settings = DockerLaunchSettings.from_profile("default", policy) + assert settings.enabled is False + assert settings.cpu is None + assert settings.memory_mb is None + + +def test_deploy_policy_from_env_reads_overrides(monkeypatch): + monkeypatch.setenv("MODAL_DOCKER_SANDBOX_CPU", "1.5") + monkeypatch.setenv("MODAL_DOCKER_SANDBOX_MEMORY_MB", "4096") + policy = DockerDeployPolicy.from_env() + assert policy.cpu == 1.5 + assert policy.memory_mb == 4096 + + +def test_deploy_policy_from_env_defaults_to_none(monkeypatch): + monkeypatch.delenv("MODAL_DOCKER_SANDBOX_CPU", raising=False) + monkeypatch.delenv("MODAL_DOCKER_SANDBOX_MEMORY_MB", raising=False) + policy = DockerDeployPolicy.from_env() + assert policy.cpu is None + assert policy.memory_mb is None + + +def test_runtime_launch_options_threads_policy_into_docker_kwargs(): + policy = DockerDeployPolicy(cpu=3.0, memory_mb=8192) + options = RuntimeLaunchOptions.for_session( + SandboxRuntimeSettings.default(), + code_server_enabled=False, + image_profile="docker", + docker_policy=policy, + ) + kwargs = options.docker.modal_create_kwargs() + assert kwargs["cpu"] == 3.0 + assert kwargs["memory"] == 8192 + assert kwargs["experimental_options"] == {"enable_docker": True} diff --git a/packages/modal-infra/tests/test_sandbox_env_vars.py b/packages/modal-infra/tests/test_sandbox_env_vars.py index 154a921bf..c9a1f1f04 100644 --- a/packages/modal-infra/tests/test_sandbox_env_vars.py +++ b/packages/modal-infra/tests/test_sandbox_env_vars.py @@ -1,9 +1,45 @@ import json +import re +from pathlib import Path import pytest from sandbox_runtime.types import SessionConfig from src.sandbox.manager import DEFAULT_SANDBOX_TIMEOUT_SECONDS, SandboxConfig, SandboxManager +from src.sandbox.settings import IMAGE_PROFILES, SandboxRuntimeSettings, parse_sandbox_image_profile + + +def test_parse_sandbox_image_profile_accepts_known_profiles(): + assert parse_sandbox_image_profile("default") == "default" + assert parse_sandbox_image_profile("docker") == "docker" + assert parse_sandbox_image_profile(None) == "default" + + +def test_parse_sandbox_image_profile_rejects_unknown_profiles(): + with pytest.raises(ValueError): + parse_sandbox_image_profile("bogus") + + +def test_sandbox_image_profiles_match_shared_typescript_contract(): + shared_types = ( + Path(__file__).resolve().parents[2] / "shared" / "src" / "types" / "integrations.ts" + ).read_text() + match = re.search(r"export type SandboxImageProfile = ([^;]+);", shared_types) + assert match is not None + + ts_profiles = set(re.findall(r'"([^"]+)"', match.group(1))) + assert ts_profiles == IMAGE_PROFILES + + +def test_runtime_settings_ignore_non_boolean_feature_flags(): + for raw in ( + {"dockerEnabled": "false", "terminalEnabled": "true"}, + {"dockerEnabled": 1, "terminalEnabled": 1}, + {"dockerEnabled": {}, "terminalEnabled": []}, + ): + settings = SandboxRuntimeSettings.from_raw(raw) + + assert settings.terminal_enabled is False @pytest.mark.asyncio @@ -41,6 +77,114 @@ class FakeSandbox: assert env_vars["CUSTOM_SECRET"] == "value" +@pytest.mark.asyncio +async def test_create_sandbox_enables_docker_when_profile_is_docker(monkeypatch): + captured = {} + + async def fake_create_aio(*args, **kwargs): + captured["env"] = kwargs.get("env") + captured["experimental_options"] = kwargs.get("experimental_options") + captured["cpu"] = kwargs.get("cpu") + captured["memory"] = kwargs.get("memory") + + class FakeSandbox: + object_id = "obj-docker" + stdout = None + + return FakeSandbox() + + fake_create_aio.aio = fake_create_aio + monkeypatch.setattr("src.sandbox.manager.modal.Sandbox.create", fake_create_aio) + + manager = SandboxManager() + await manager.create_sandbox( + SandboxConfig( + repo_owner="acme", + repo_name="repo", + control_plane_url="https://control-plane.example", + sandbox_auth_token="token-123", + image_profile="docker", + ) + ) + + assert captured["env"]["OPENINSPECT_DOCKER_ENABLED"] == "true" + assert captured["env"]["DOCKER_DATA_ROOT"] == "/opt/docker-data" + assert "OPENINSPECT_SANDBOX_IMAGE_PROFILE" not in captured["env"] + assert captured["experimental_options"] == {"enable_docker": True} + assert captured["cpu"] is None + assert captured["memory"] is None + + +@pytest.mark.asyncio +async def test_create_sandbox_does_not_enable_docker_when_setting_is_off(monkeypatch): + captured = {} + + async def fake_create_aio(*args, **kwargs): + captured["env"] = kwargs.get("env") + captured["experimental_options"] = kwargs.get("experimental_options") + captured["cpu"] = kwargs.get("cpu") + captured["memory"] = kwargs.get("memory") + + class FakeSandbox: + object_id = "obj-plain" + stdout = None + + return FakeSandbox() + + fake_create_aio.aio = fake_create_aio + monkeypatch.setattr("src.sandbox.manager.modal.Sandbox.create", fake_create_aio) + + manager = SandboxManager() + await manager.create_sandbox( + SandboxConfig( + repo_owner="acme", + repo_name="repo", + control_plane_url="https://control-plane.example", + sandbox_auth_token="token-123", + ) + ) + + assert "OPENINSPECT_DOCKER_ENABLED" not in captured["env"] + assert "OPENINSPECT_SANDBOX_IMAGE_PROFILE" not in captured["env"] + assert captured["experimental_options"] is None + assert captured["cpu"] is None + assert captured["memory"] is None + + +@pytest.mark.asyncio +async def test_docker_resources_are_configurable(monkeypatch): + captured = {} + + async def fake_create_aio(*args, **kwargs): + captured["cpu"] = kwargs.get("cpu") + captured["memory"] = kwargs.get("memory") + + class FakeSandbox: + object_id = "obj-docker-resources" + stdout = None + + return FakeSandbox() + + fake_create_aio.aio = fake_create_aio + monkeypatch.setattr("src.sandbox.manager.modal.Sandbox.create", fake_create_aio) + monkeypatch.setenv("MODAL_DOCKER_SANDBOX_CPU", "2.5") + monkeypatch.setenv("MODAL_DOCKER_SANDBOX_MEMORY_MB", "6144") + + manager = SandboxManager() + await manager.create_sandbox( + SandboxConfig( + repo_owner="acme", + repo_name="repo", + control_plane_url="https://control-plane.example", + sandbox_auth_token="token-123", + image_profile="docker", + ) + ) + + assert captured["cpu"] == 2.5 + assert captured["memory"] == 6144 + + @pytest.mark.asyncio async def test_restore_user_env_vars_override_order(monkeypatch): captured = {} @@ -465,29 +609,6 @@ class FakeImage: assert "OI_GITHUB_TOKEN_IS_FALLBACK" not in env -@pytest.mark.asyncio -async def test_session_snapshot_boot_preserves_clone_token(monkeypatch): - """A session-snapshot boot has the same legacy-compat need as repo images.""" - captured = {} - - monkeypatch.setattr("src.sandbox.manager.modal.Image.from_registry", lambda *a, **kw: object()) - monkeypatch.setattr("src.sandbox.manager.modal.Sandbox.create", _fake_sandbox_create(captured)) - monkeypatch.delenv("SCM_PROVIDER", raising=False) - - manager = SandboxManager() - config = SandboxConfig( - repo_owner="acme", - repo_name="repo", - clone_token="ghs_snapshot_token", - snapshot_id="snap-1", - ) - await manager.create_sandbox(config) - - env = captured["env"] - assert env["VCS_CLONE_TOKEN"] == "ghs_snapshot_token" - assert env["OI_GITHUB_TOKEN_IS_FALLBACK"] == "1" - - @pytest.mark.asyncio async def test_restore_preserves_vcs_clone_token_for_legacy_snapshots(monkeypatch): """Snapshot restore still injects VCS_CLONE_TOKEN. diff --git a/packages/modal-infra/tests/test_scheduler.py b/packages/modal-infra/tests/test_scheduler.py index 0c6735d1f..728f07d0f 100644 --- a/packages/modal-infra/tests/test_scheduler.py +++ b/packages/modal-infra/tests/test_scheduler.py @@ -159,6 +159,20 @@ def test_ignores_other_repos(self): result = _should_rebuild("acme", "repo", "abc123", images) assert result is True + def test_rebuild_when_only_default_image_exists_for_docker_profile(self): + """Docker profile rebuilds require a Docker-profile ready image.""" + images = [ + { + "repo_owner": "acme", + "repo_name": "repo", + "status": "ready", + "base_sha": "abc123", + "image_profile": "default", + } + ] + result = _should_rebuild("acme", "repo", "abc123", images, image_profile="docker") + assert result is True + class TestRebuildRepoImages: """Test the rebuild_repo_images cron function (integration-level with mocks).""" @@ -206,7 +220,9 @@ async def test_triggers_build_on_sha_mismatch(self): "MODAL_API_SECRET": "test-secret", } - mock_enabled = {"repos": [{"repoOwner": "acme", "repoName": "repo"}]} + mock_enabled = { + "repos": [{"repoOwner": "acme", "repoName": "repo", "imageProfile": "docker"}] + } mock_status = { "images": [ { @@ -265,6 +281,7 @@ async def mock_post_side_effect(url, payload=None, **kwargs): trigger_calls = [c for c in mock_post.call_args_list if "trigger" in str(c)] assert len(trigger_calls) == 1 assert "acme/repo" in str(trigger_calls[0]) + assert len(trigger_calls[0].args) == 1 @pytest.mark.asyncio async def test_skips_build_when_sha_matches(self): @@ -380,3 +397,46 @@ async def mock_post_side_effect(url, payload=None, **kwargs): cleanup_calls = [c for c in mock_post.call_args_list if "cleanup" in str(c)] assert len(cleanup_calls) == 1 + + @pytest.mark.asyncio + async def test_skips_repo_with_invalid_image_profile(self): + env = { + "CONTROL_PLANE_URL": "https://cp.test", + "MODAL_API_SECRET": "test-secret", + } + + async def mock_get_side_effect(url, **kwargs): + if "enabled-repos" in url: + return { + "repos": [{"repoOwner": "acme", "repoName": "repo", "imageProfile": "bogus"}] + } + if "status" in url: + return {"images": []} + return {} + + async def mock_post_side_effect(url, payload=None, **kwargs): + return {"ok": True, "markedFailed": 0, "deleted": 0} + + with ( + patch.dict("os.environ", env, clear=False), + patch( + "src.scheduler.image_builder._api_get", + new_callable=AsyncMock, + side_effect=mock_get_side_effect, + ), + patch( + "src.scheduler.image_builder._api_post", + new_callable=AsyncMock, + side_effect=mock_post_side_effect, + ) as mock_post, + patch( + "sandbox_runtime.auth.github_app.generate_installation_token", + return_value="gh-token", + ), + ): + from src.scheduler.image_builder import rebuild_repo_images + + await rebuild_repo_images.local() + + trigger_calls = [c for c in mock_post.call_args_list if "trigger" in str(c)] + assert trigger_calls == [] diff --git a/packages/modal-infra/tests/test_ttyd.py b/packages/modal-infra/tests/test_ttyd.py index fae751c52..13f06207a 100644 --- a/packages/modal-infra/tests/test_ttyd.py +++ b/packages/modal-infra/tests/test_ttyd.py @@ -4,50 +4,49 @@ import pytest -from sandbox_runtime.constants import TTYD_PORT +from sandbox_runtime.constants import CODE_SERVER_PORT, TTYD_PORT, TTYD_PROXY_PORT from src.sandbox.manager import ( - CODE_SERVER_PORT, - TTYD_PROXY_PORT, SandboxConfig, SandboxManager, ) +from src.sandbox.settings import RuntimePortSettings, SandboxRuntimeSettings + + +def _settings(raw: dict | None = None) -> SandboxRuntimeSettings: + return SandboxRuntimeSettings.from_raw(raw) class TestCollectExposedPortsTerminal: - """_collect_exposed_ports with terminal_enabled flag.""" + """RuntimePortSettings with terminal_enabled flag.""" def test_terminal_enabled_includes_proxy_port(self): - exposed, _extra = SandboxManager._collect_exposed_ports( - code_server_enabled=False, terminal_enabled=True, settings=None + ports = RuntimePortSettings.from_settings( + _settings({"terminalEnabled": True}), code_server_enabled=False ) - assert TTYD_PROXY_PORT in exposed + assert TTYD_PROXY_PORT in ports.exposed_ports # ttyd raw port should NOT be exposed (only the proxy port) - assert TTYD_PORT not in exposed + assert TTYD_PORT not in ports.exposed_ports def test_terminal_disabled_excludes_proxy_port(self): - exposed, _extra = SandboxManager._collect_exposed_ports( - code_server_enabled=False, terminal_enabled=False, settings=None - ) - assert TTYD_PROXY_PORT not in exposed + ports = RuntimePortSettings.from_settings(_settings(), code_server_enabled=False) + assert TTYD_PROXY_PORT not in ports.exposed_ports def test_terminal_and_code_server_both_enabled(self): - exposed, _extra = SandboxManager._collect_exposed_ports( - code_server_enabled=True, terminal_enabled=True, settings=None + ports = RuntimePortSettings.from_settings( + _settings({"terminalEnabled": True}), code_server_enabled=True ) - assert CODE_SERVER_PORT in exposed - assert TTYD_PROXY_PORT in exposed + assert CODE_SERVER_PORT in ports.exposed_ports + assert TTYD_PROXY_PORT in ports.exposed_ports def test_terminal_port_deduped_from_tunnel_ports(self): """If user explicitly lists TTYD_PROXY_PORT in tunnelPorts, it should not duplicate.""" - settings = {"tunnelPorts": [TTYD_PROXY_PORT, 3000]} - exposed, extra = SandboxManager._collect_exposed_ports( - code_server_enabled=False, terminal_enabled=True, settings=settings - ) - assert exposed.count(TTYD_PROXY_PORT) == 1 - assert 3000 in exposed + settings = _settings({"terminalEnabled": True, "tunnelPorts": [TTYD_PROXY_PORT, 3000]}) + ports = RuntimePortSettings.from_settings(settings, code_server_enabled=False) + assert ports.exposed_ports.count(TTYD_PROXY_PORT) == 1 + assert 3000 in ports.exposed_ports # TTYD_PROXY_PORT should not be in extra (reserved) - assert TTYD_PROXY_PORT not in extra - assert 3000 in extra + assert TTYD_PROXY_PORT not in ports.tunnel_ports + assert 3000 in ports.tunnel_ports class TestResolveTunnelsTerminal: @@ -137,7 +136,7 @@ class FakeSandbox: control_plane_url="https://cp.example.com", sandbox_auth_token="token-123", code_server_enabled=False, - settings={"terminalEnabled": True}, + settings=_settings({"terminalEnabled": True}), ) handle = await manager.create_sandbox(config) @@ -229,7 +228,7 @@ class FakeSandbox: control_plane_url="https://cp.example.com", sandbox_auth_token="token-456", code_server_enabled=False, - settings={"terminalEnabled": True}, + settings=_settings({"terminalEnabled": True}), ) assert handle.ttyd_url == "https://ttyd-restored.example.com" diff --git a/packages/modal-infra/tests/test_tunnel_ports.py b/packages/modal-infra/tests/test_tunnel_ports.py index 0bd5bb27e..db1b402df 100644 --- a/packages/modal-infra/tests/test_tunnel_ports.py +++ b/packages/modal-infra/tests/test_tunnel_ports.py @@ -5,11 +5,17 @@ import pytest from sandbox_runtime.constants import ( + CODE_SERVER_PORT, EXPECTED_TUNNEL_PORTS_ENV_VAR, TTYD_PROXY_PORT, TUNNEL_ENV_FILE_PATH, ) -from src.sandbox.manager import CODE_SERVER_PORT, SandboxConfig, SandboxManager +from src.sandbox.manager import SandboxConfig, SandboxManager +from src.sandbox.settings import RuntimePortSettings, SandboxRuntimeSettings, validate_tunnel_ports + + +def _settings(raw: dict | None = None) -> SandboxRuntimeSettings: + return SandboxRuntimeSettings.from_raw(raw) def _mock_sandbox_with_open() -> tuple[MagicMock, AsyncMock]: @@ -303,7 +309,7 @@ class FakeSandbox: SandboxConfig( repo_owner="acme", repo_name="repo", - settings={"tunnelPorts": [3000, 5173]}, + settings=_settings({"tunnelPorts": [3000, 5173]}), ) ) @@ -368,74 +374,73 @@ class FakeSandbox: await manager.restore_from_snapshot( snapshot_image_id="img-abc", session_config={"repo_owner": "acme", "repo_name": "repo"}, - settings={"tunnelPorts": [3000]}, + settings=_settings({"tunnelPorts": [3000]}), ) assert captured["env"][EXPECTED_TUNNEL_PORTS_ENV_VAR] == "3000" class TestCollectExposedPorts: - """SandboxManager._collect_exposed_ports tests.""" + """RuntimePortSettings tests.""" def test_no_ports_when_no_settings(self): - exposed, tunnel = SandboxManager._collect_exposed_ports(False, False, None) - assert exposed == [] - assert tunnel == [] + ports = RuntimePortSettings.from_settings(_settings(), False) + assert ports.exposed_ports == () + assert ports.tunnel_ports == () def test_code_server_only(self): - exposed, tunnel = SandboxManager._collect_exposed_ports(True, False, None) - assert exposed == [CODE_SERVER_PORT] - assert tunnel == [] + ports = RuntimePortSettings.from_settings(_settings(), True) + assert ports.exposed_ports == (CODE_SERVER_PORT,) + assert ports.tunnel_ports == () def test_tunnel_ports_only(self): - exposed, tunnel = SandboxManager._collect_exposed_ports( - False, False, {"tunnelPorts": [3000, 5173]} - ) - assert exposed == [3000, 5173] - assert tunnel == [3000, 5173] + ports = RuntimePortSettings.from_settings(_settings({"tunnelPorts": [3000, 5173]}), False) + assert ports.exposed_ports == (3000, 5173) + assert ports.tunnel_ports == (3000, 5173) def test_combined_code_server_and_tunnels(self): - exposed, tunnel = SandboxManager._collect_exposed_ports( - True, False, {"tunnelPorts": [3000]} - ) - assert exposed == [CODE_SERVER_PORT, 3000] - assert tunnel == [3000] + ports = RuntimePortSettings.from_settings(_settings({"tunnelPorts": [3000]}), True) + assert ports.exposed_ports == (CODE_SERVER_PORT, 3000) + assert ports.tunnel_ports == (3000,) def test_terminal_only(self): - exposed, tunnel = SandboxManager._collect_exposed_ports(False, True, None) - assert exposed == [TTYD_PROXY_PORT] - assert tunnel == [] + ports = RuntimePortSettings.from_settings(_settings({"terminalEnabled": True}), False) + assert ports.exposed_ports == (TTYD_PROXY_PORT,) + assert ports.tunnel_ports == () def test_deduplicates_ttyd_port_from_tunnels(self): - exposed, tunnel = SandboxManager._collect_exposed_ports( - False, True, {"tunnelPorts": [TTYD_PROXY_PORT, 3000]} + ports = RuntimePortSettings.from_settings( + _settings({"terminalEnabled": True, "tunnelPorts": [TTYD_PROXY_PORT, 3000]}), False ) - assert exposed == [TTYD_PROXY_PORT, 3000] - assert tunnel == [3000] + assert ports.exposed_ports == (TTYD_PROXY_PORT, 3000) + assert ports.tunnel_ports == (3000,) def test_deduplicates_code_server_port_from_tunnels(self): - exposed, tunnel = SandboxManager._collect_exposed_ports( - True, False, {"tunnelPorts": [CODE_SERVER_PORT, 3000]} + ports = RuntimePortSettings.from_settings( + _settings({"tunnelPorts": [CODE_SERVER_PORT, 3000]}), True ) - assert exposed == [CODE_SERVER_PORT, 3000] - assert tunnel == [3000] + assert ports.exposed_ports == (CODE_SERVER_PORT, 3000) + assert ports.tunnel_ports == (3000,) class TestValidatePorts: - """SandboxManager._validate_ports tests.""" + """validate_tunnel_ports tests.""" def test_accepts_valid_ports(self): - assert SandboxManager._validate_ports([80, 3000, 65535]) == [80, 3000, 65535] + assert validate_tunnel_ports([80, 3000, 65535]) == (80, 3000, 65535) def test_rejects_out_of_range(self): - assert SandboxManager._validate_ports([0, -1, 65536, 3000]) == [3000] + assert validate_tunnel_ports([0, -1, 65536, 3000]) == (3000,) def test_rejects_non_integers(self): - assert SandboxManager._validate_ports(["3000", 3.5, None, 8080]) == [8080] + assert validate_tunnel_ports(["3000", 3.5, None, 8080]) == (8080,) + + def test_rejects_boolean_ports(self): + assert validate_tunnel_ports([True, False, 8080]) == (8080,) def test_caps_at_ten(self): ports = list(range(1, 20)) - assert len(SandboxManager._validate_ports(ports)) == 10 + assert len(validate_tunnel_ports(ports)) == 10 def test_empty_list(self): - assert SandboxManager._validate_ports([]) == [] + assert validate_tunnel_ports([]) == () diff --git a/packages/modal-infra/tests/test_web_api_create_sandbox.py b/packages/modal-infra/tests/test_web_api_create_sandbox.py index b1c653d25..f85c7e527 100644 --- a/packages/modal-infra/tests/test_web_api_create_sandbox.py +++ b/packages/modal-infra/tests/test_web_api_create_sandbox.py @@ -7,6 +7,7 @@ from sandbox_runtime.types import SandboxStatus from src import web_api from src.sandbox import manager as manager_module +from src.scheduler import image_builder as image_builder_module def _patch_auth(monkeypatch: pytest.MonkeyPatch) -> None: @@ -43,6 +44,15 @@ async def _call_create_sandbox(request: dict) -> dict: ) +async def _call_build_repo_image(request: dict) -> dict: + return await web_api.api_build_repo_image.get_raw_f()( + request, + authorization="Bearer test", + x_trace_id=None, + x_request_id=None, + ) + + @pytest.mark.asyncio async def test_create_sandbox_does_not_resolve_clone_token_for_fresh_boot(monkeypatch): """Fresh base-image boots authenticate via the credential helper only.""" @@ -97,3 +107,34 @@ def resolve_clone_token() -> str: assert result["success"] is True assert calls == [True] assert captured["config"].clone_token == "ghs_prebuilt" + + +@pytest.mark.asyncio +async def test_build_repo_image_forwards_image_profile(monkeypatch): + captured = {} + + _patch_auth(monkeypatch) + + async def fake_spawn_aio(**kwargs): + captured.update(kwargs) + + monkeypatch.setattr( + image_builder_module, + "build_repo_image", + SimpleNamespace(spawn=SimpleNamespace(aio=fake_spawn_aio)), + ) + + result = await _call_build_repo_image( + { + "repo_owner": "acme", + "repo_name": "repo", + "default_branch": "main", + "build_id": "build-1", + "callback_url": "https://control-plane.example/repo-images/build-complete", + "image_profile": "docker", + } + ) + + assert result["success"] is True + assert "sandbox_settings" not in captured + assert captured["image_profile"] == "docker" diff --git a/packages/sandbox-runtime/src/sandbox_runtime/credentials/git_credential_helper.py b/packages/sandbox-runtime/src/sandbox_runtime/credentials/git_credential_helper.py index 4b144a10b..a79aa2c47 100644 --- a/packages/sandbox-runtime/src/sandbox_runtime/credentials/git_credential_helper.py +++ b/packages/sandbox-runtime/src/sandbox_runtime/credentials/git_credential_helper.py @@ -160,7 +160,7 @@ def _read_cached() -> dict[str, object] | None: cached = cast("dict[str, object]", raw_cached) expires_at_ms = cached.get("expires_at_epoch_ms") - if not isinstance(expires_at_ms, (int, float)): + if not isinstance(expires_at_ms, int | float): return None seconds_remaining = expires_at_ms / 1000 - time.time() @@ -201,7 +201,7 @@ def _fetch_from_control_plane(endpoint: tuple[str, str, str]) -> dict[str, objec if not isinstance(data, dict) or not data.get("username") or not data.get("password"): raise RuntimeError("control plane response missing username/password") expires_at = data.get("expires_at_epoch_ms") - if not isinstance(expires_at, (int, float)) or expires_at <= 0: + if not isinstance(expires_at, int | float) or expires_at <= 0: # Fail loud rather than cache a credential that _read_cached would # immediately reject, which would silently refetch on every git op. raise RuntimeError("control plane response has invalid expires_at_epoch_ms") diff --git a/packages/sandbox-runtime/src/sandbox_runtime/docker_service.py b/packages/sandbox-runtime/src/sandbox_runtime/docker_service.py new file mode 100644 index 000000000..36287bf50 --- /dev/null +++ b/packages/sandbox-runtime/src/sandbox_runtime/docker_service.py @@ -0,0 +1,199 @@ +"""Docker daemon supervision for sandbox runtime processes.""" + +import asyncio +import shutil +import time +from contextlib import suppress +from pathlib import Path + +DEFAULT_DOCKER_DATA_ROOT = Path("/opt/docker-data") +DEFAULT_IP_FORWARD_PATH = Path("/proc/sys/net/ipv4/ip_forward") +DOCKER_COMMAND_TIMEOUT_SECONDS = 5.0 +DEFAULT_RUN_PATHS = ( + Path("/var/run/docker.sock"), + Path("/var/run/docker.pid"), + Path("/var/run/docker"), + Path("/run/containerd"), +) + + +class DockerService: + """Starts and stops a configured in-sandbox Docker daemon.""" + + STALE_DATA_DIRS = ("network", "containers", "containerd", "runtimes", "tmp") + READY_TIMEOUT_SECONDS = 30.0 + + def __init__( + self, + log, + data_root: Path = DEFAULT_DOCKER_DATA_ROOT, + run_paths: tuple[Path, ...] = DEFAULT_RUN_PATHS, + ip_forward_path: Path = DEFAULT_IP_FORWARD_PATH, + ): + self.log = log + self.data_root = data_root + self.run_paths = run_paths + self.ip_forward_path = ip_forward_path + self.process: asyncio.subprocess.Process | None = None + self._log_task: asyncio.Task[None] | None = None + + @property + def exit_code(self) -> int | None: + if not self.process: + return None + return self.process.returncode + + def has_crashed(self) -> bool: + return self.exit_code is not None + + def prepare_runtime_state(self) -> None: + """Remove stale daemon/container metadata while preserving images, cache, and volumes.""" + self.data_root.mkdir(parents=True, exist_ok=True) + for dirname in self.STALE_DATA_DIRS: + path = self.data_root / dirname + if path.is_dir(): + shutil.rmtree(path, ignore_errors=True) + elif path.exists(): + path.unlink(missing_ok=True) + + for path in self.run_paths: + if path.is_dir(): + shutil.rmtree(path, ignore_errors=True) + else: + path.unlink(missing_ok=True) + + async def _run_command( + self, + *args: str, + check: bool = True, + timeout_seconds: float = DOCKER_COMMAND_TIMEOUT_SECONDS, + ) -> tuple[int, str]: + proc = await asyncio.create_subprocess_exec( + *args, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.STDOUT, + ) + try: + stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout_seconds) + except TimeoutError: + with suppress(ProcessLookupError): + proc.kill() + await proc.wait() + raise RuntimeError(f"{' '.join(args)} timed out after {timeout_seconds:.1f}s") from None + output = (stdout or b"").decode(errors="replace") + if check and proc.returncode != 0: + raise RuntimeError(f"{' '.join(args)} failed: {output.strip()}") + return proc.returncode or 0, output + + async def configure_network(self) -> None: + """Configure SNAT for Docker containers in Modal's gVisor environment.""" + _, route_output = await self._run_command("ip", "route", "show", "default") + default_line = next( + (line for line in route_output.splitlines() if line.startswith("default ")), + "", + ) + pieces = default_line.split() + if "dev" not in pieces: + raise RuntimeError("Could not determine default network device for Docker") + dev = pieces[pieces.index("dev") + 1] + + _, addr_output = await self._run_command("ip", "-4", "addr", "show", "dev", dev) + address = "" + for line in addr_output.splitlines(): + stripped = line.strip() + if stripped.startswith("inet "): + address = stripped.split()[1].split("/")[0] + break + if not address: + raise RuntimeError(f"Could not determine IPv4 address for Docker device {dev}") + + try: + self.ip_forward_path.write_text("1\n") + except OSError as e: + self.log.warn("docker.ip_forward_failed", error=str(e)) + + for proto in ("tcp", "udp"): + rule = ( + "POSTROUTING", + "-o", + dev, + "-p", + proto, + "-j", + "SNAT", + "--to-source", + address, + ) + exists, _ = await self._run_command( + "iptables-legacy", "-t", "nat", "-C", *rule, check=False + ) + if exists != 0: + await self._run_command("iptables-legacy", "-t", "nat", "-A", *rule) + + async def start(self) -> None: + self.prepare_runtime_state() + await self.configure_network() + + self.process = await asyncio.create_subprocess_exec( + "dockerd", + f"--data-root={self.data_root}", + "--host=unix:///var/run/docker.sock", + "--iptables=false", + "--ip6tables=false", + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.STDOUT, + ) + self._log_task = asyncio.create_task(self._forward_logs()) + try: + await self.wait_until_ready() + except Exception: + await self.stop() + raise + self.log.info("docker.ready", data_root=str(self.data_root)) + + async def wait_until_ready(self) -> None: + start = time.time() + while time.time() - start < self.READY_TIMEOUT_SECONDS: + if self.process and self.process.returncode is not None: + raise RuntimeError(f"dockerd exited early with code {self.process.returncode}") + returncode, _ = await self._run_command("docker", "info", check=False) + if returncode == 0: + return + await asyncio.sleep(0.5) + raise RuntimeError("dockerd did not become ready before timeout") + + async def _forward_logs(self) -> None: + if not self.process or not self.process.stdout: + return + try: + async for line in self.process.stdout: + self.log.info("docker.stdout", line=line.decode(errors="replace").rstrip()) + except Exception as e: + self.log.warn("docker.log_forward_error", exc=e) + + async def _cancel_log_task(self) -> None: + if not self._log_task: + return + if not self._log_task.done(): + self._log_task.cancel() + with suppress(asyncio.CancelledError): + await self._log_task + self._log_task = None + + async def stop(self) -> None: + process = self.process + if not process: + await self._cancel_log_task() + return + + if process.returncode is None: + self.log.info("docker.terminating") + process.terminate() + try: + await asyncio.wait_for(process.wait(), timeout=5.0) + except TimeoutError: + process.kill() + await process.wait() + + await self._cancel_log_task() + self.process = None diff --git a/packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py b/packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py index 904fd082b..e591083d2 100644 --- a/packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py +++ b/packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py @@ -29,15 +29,41 @@ TTYD_PROXY_PORT, TUNNEL_ENV_FILE_PATH, ) +from .docker_service import DEFAULT_DOCKER_DATA_ROOT, DockerService from .log_config import configure_logging, get_logger +from .runtime_services import RuntimeServices configure_logging() +DOCKER_DATA_ROOT_ENV_VAR = "DOCKER_DATA_ROOT" +DOCKER_ENABLED_ENV_VAR = "OPENINSPECT_DOCKER_ENABLED" + AGENT_TOOLS_GATED_ON_ENV: dict[str, str] = { "slack-notify.js": "AGENT_SLACK_NOTIFY_ENABLED", } + +def build_runtime_services( + log, + *, + docker_enabled: bool, + docker_data_root: Path, +) -> RuntimeServices: + if not docker_enabled: + log.info("runtime_services.docker_disabled") + return RuntimeServices(log) + + log.info( + "runtime_services.docker_enabled", + docker_data_root=str(docker_data_root), + ) + return RuntimeServices( + log, + docker=DockerService(log, data_root=docker_data_root), + ) + + # Wrapper installed at /usr/local/bin/gh (ahead of the real /usr/bin/gh in # PATH). The git credential helper can't authenticate the GitHub CLI — gh # reads GH_TOKEN/GITHUB_TOKEN from the environment, not git's protocol — so @@ -129,6 +155,10 @@ def __init__(self): self.repo_owner = os.environ.get("REPO_OWNER", "") self.repo_name = os.environ.get("REPO_NAME", "") self.vcs_host = os.environ.get("VCS_HOST", "github.com") + self.docker_enabled = os.environ.get(DOCKER_ENABLED_ENV_VAR, "").lower() == "true" + self.docker_data_root = Path( + os.environ.get(DOCKER_DATA_ROOT_ENV_VAR, str(DEFAULT_DOCKER_DATA_ROOT)) + ) # Note: VCS credentials are no longer captured at sandbox start. Git # operations authenticate per-call via the system-wide credential # helper (`/usr/local/bin/oi-git-credentials`), which fetches fresh @@ -151,6 +181,11 @@ def __init__(self): sandbox_id=self.sandbox_id, session_id=session_id, ) + self.runtime_services = build_runtime_services( + self.log, + docker_enabled=self.docker_enabled, + docker_data_root=self.docker_data_root, + ) @property def base_branch(self) -> str: @@ -998,6 +1033,11 @@ async def monitor_processes(self) -> None: ttyd_proxy_restart_count = 0 while not self.shutdown_event.is_set(): + if not await self.runtime_services.ensure_healthy(self._report_fatal_error): + self.log.error("runtime_services.unhealthy") + self.shutdown_event.set() + break + # Check OpenCode process if self.opencode_process and self.opencode_process.returncode is not None: exit_code = self.opencode_process.returncode @@ -1445,14 +1485,17 @@ async def run(self) -> None: self.log.info("git.sync_complete", head_sha=head_sha) self.git_sync_complete.set() - # Phase 2: Run setup script only for fresh or build boots. + # Phase 2: Start Docker before repository hooks when requested. + await self.runtime_services.start_before_hooks() + + # Phase 3: Run setup script only for fresh or build boots. setup_success: bool | None = None if self.boot_mode in ("fresh", "build"): setup_success = await self.run_setup_script() if image_build_mode and not setup_success: raise RuntimeError("setup hook failed in build mode") - # Phase 3: Run runtime start hook for all non-build boots. Wait for + # Phase 4: Run runtime start hook for all non-build boots. Wait for # tunnel URLs first so dev servers booted by start.sh see fresh data. start_success: bool | None = None if self.boot_mode != "build": @@ -1472,7 +1515,7 @@ async def run(self) -> None: await self.shutdown_event.wait() return - # Phase 3.5: Start optional sidecars (best-effort, non-fatal) + # Phase 4.5: Start optional sidecars (best-effort, non-fatal) for sidecar_name, starter in ( ("code_server", self.start_code_server), ("ttyd", self.start_ttyd), @@ -1492,11 +1535,11 @@ async def run(self) -> None: except Exception as e: self.log.warn("ttyd_proxy.start_failed", exc=e) - # Phase 4: Start OpenCode server (in repo directory) + # Phase 5: Start OpenCode server (in repo directory) await self.start_opencode() opencode_ready = True - # Phase 5: Start bridge (after OpenCode is ready) + # Phase 6: Start bridge (after OpenCode is ready) await self.start_bridge() # Emit sandbox.startup wide event @@ -1516,7 +1559,7 @@ async def run(self) -> None: outcome="success", ) - # Phase 6: Monitor processes + # Phase 7: Monitor processes await self.monitor_processes() except Exception as e: @@ -1535,6 +1578,19 @@ async def shutdown(self) -> None: """Graceful shutdown of all processes.""" self.log.info("supervisor.shutdown_start") + try: + await asyncio.wait_for( + self.runtime_services.stop(), + timeout=self.SIDECAR_TIMEOUT_SECONDS, + ) + except TimeoutError: + self.log.warn( + "runtime_services.stop_timeout", + timeout_seconds=self.SIDECAR_TIMEOUT_SECONDS, + ) + except Exception as e: + self.log.warn("runtime_services.stop_error", exc=e) + # Terminate bridge first if self.bridge_process and self.bridge_process.returncode is None: self.bridge_process.terminate() diff --git a/packages/sandbox-runtime/src/sandbox_runtime/runtime_services.py b/packages/sandbox-runtime/src/sandbox_runtime/runtime_services.py new file mode 100644 index 000000000..da21ed481 --- /dev/null +++ b/packages/sandbox-runtime/src/sandbox_runtime/runtime_services.py @@ -0,0 +1,40 @@ +"""Optional runtime service lifecycle for sandbox supervisor.""" + +from collections.abc import Awaitable, Callable + +from .docker_service import DockerService + +ReportFatalError = Callable[[str], Awaitable[None]] + + +class RuntimeServices: + """Owns optional sidecar services started by the sandbox supervisor.""" + + def __init__(self, log, docker: DockerService | None = None): + self.log = log + self.docker = docker + + async def start_before_hooks(self) -> None: + if not self.docker: + return + await self.docker.start() + + async def ensure_healthy(self, report_fatal_error: ReportFatalError) -> bool: + if not self.docker: + return True + + if not self.docker.has_crashed(): + return True + + # Docker-backed hooks and Compose services depend on dockerd staying up. + # Restarting only the daemon would not rerun start.sh, so fail the + # session instead of leaving it alive with missing runtime services. + exit_code = self.docker.exit_code + self.log.error("docker.crash", exit_code=exit_code) + await report_fatal_error(f"dockerd exited unexpectedly with code {exit_code}") + return False + + async def stop(self) -> None: + if not self.docker: + return + await self.docker.stop() diff --git a/packages/sandbox-runtime/tests/test_docker_service.py b/packages/sandbox-runtime/tests/test_docker_service.py new file mode 100644 index 000000000..303e83e0b --- /dev/null +++ b/packages/sandbox-runtime/tests/test_docker_service.py @@ -0,0 +1,273 @@ +"""Tests for Docker daemon supervision in the sandbox entrypoint.""" + +import asyncio +import os +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from sandbox_runtime.docker_service import DockerService +from sandbox_runtime.entrypoint import SandboxSupervisor, build_runtime_services +from sandbox_runtime.runtime_services import RuntimeServices + + +def test_build_runtime_services_omits_docker_when_disabled(tmp_path): + data_root = tmp_path / "docker-data" + log = MagicMock() + + services = build_runtime_services( + log, + docker_enabled=False, + docker_data_root=data_root, + ) + + assert services.docker is None + log.info.assert_called_with("runtime_services.docker_disabled") + + +def test_build_runtime_services_includes_docker_when_enabled(tmp_path): + data_root = tmp_path / "docker-data" + log = MagicMock() + + services = build_runtime_services( + log, + docker_enabled=True, + docker_data_root=data_root, + ) + + assert isinstance(services.docker, DockerService) + assert services.docker.data_root == data_root + log.info.assert_called_with( + "runtime_services.docker_enabled", + docker_data_root=str(data_root), + ) + + +@pytest.mark.asyncio +async def test_runtime_services_treats_dockerd_crash_as_fatal(): + docker = MagicMock() + docker.has_crashed.return_value = True + docker.exit_code = 2 + log = MagicMock() + report_fatal_error = AsyncMock() + + services = RuntimeServices(log, docker=docker) + + assert await services.ensure_healthy(report_fatal_error) is False + log.error.assert_called_once_with("docker.crash", exit_code=2) + report_fatal_error.assert_awaited_once_with("dockerd exited unexpectedly with code 2") + + +def test_prepare_runtime_state_preserves_snapshot_content(tmp_path): + data_root = tmp_path / "docker-data" + stale_dirs = ["network", "containers", "containerd", "runtimes", "tmp"] + preserved_dirs = ["image", "vfs", "overlay2", "volumes", "buildkit"] + + for name in stale_dirs + preserved_dirs: + path = data_root / name + path.mkdir(parents=True) + (path / "marker").write_text(name) + + socket_path = tmp_path / "docker.sock" + socket_path.write_text("stale") + run_dir = tmp_path / "containerd" + run_dir.mkdir() + (run_dir / "marker").write_text("stale") + + service = DockerService(MagicMock(), data_root=data_root, run_paths=(socket_path, run_dir)) + service.prepare_runtime_state() + + for name in stale_dirs: + assert not (data_root / name).exists() + for name in preserved_dirs: + assert (data_root / name / "marker").read_text() == name + assert not socket_path.exists() + assert not run_dir.exists() + + +@pytest.mark.asyncio +async def test_supervisor_starts_docker_before_setup_in_build_mode(): + env = { + "SANDBOX_ID": "test-sandbox", + "REPO_OWNER": "acme", + "REPO_NAME": "repo", + "SESSION_CONFIG": "{}", + "IMAGE_BUILD_MODE": "true", + "OPENINSPECT_DOCKER_ENABLED": "true", + } + + order: list[str] = [] + + with patch.dict(os.environ, env, clear=False): + supervisor = SandboxSupervisor() + + supervisor.perform_git_sync = AsyncMock(side_effect=lambda: order.append("git") or True) + supervisor.runtime_services.start_before_hooks = AsyncMock( + side_effect=lambda: order.append("docker") + ) + supervisor.run_setup_script = AsyncMock(side_effect=lambda: order.append("setup") or True) + supervisor.shutdown = AsyncMock() + supervisor.shutdown_event.set() + + with patch.dict(os.environ, env, clear=False): + await supervisor.run() + + assert order == ["git", "docker", "setup"] + + +@pytest.mark.asyncio +async def test_supervisor_shutdown_times_out_runtime_services(): + env = { + "SANDBOX_ID": "test-sandbox", + "REPO_OWNER": "acme", + "REPO_NAME": "repo", + "SESSION_CONFIG": "{}", + } + + async def slow_stop(): + await asyncio.sleep(1) + + with patch.dict(os.environ, env, clear=False): + supervisor = SandboxSupervisor() + + supervisor.SIDECAR_TIMEOUT_SECONDS = 0.01 + supervisor.runtime_services.stop = slow_stop + supervisor.log = MagicMock() + + await supervisor.shutdown() + + supervisor.log.warn.assert_called_with( + "runtime_services.stop_timeout", + timeout_seconds=0.01, + ) + + +@pytest.mark.asyncio +async def test_configure_network_adds_snat_rules(tmp_path): + calls: list[tuple[str, ...]] = [] + + async def fake_run_command(*args: str, check: bool = True): + calls.append(args) + if args == ("ip", "route", "show", "default"): + return 0, "default via 10.0.0.1 dev eth0\n" + if args == ("ip", "-4", "addr", "show", "dev", "eth0"): + return 0, "2: eth0: \n inet 10.0.0.2/24 scope global eth0\n" + if args[:4] == ("iptables-legacy", "-t", "nat", "-C"): + return 1, "" + return 0, "" + + ip_forward = tmp_path / "ip_forward" + + service = DockerService( + MagicMock(), + data_root=tmp_path / "docker-data", + ip_forward_path=ip_forward, + ) + service._run_command = fake_run_command + + await service.configure_network() + + assert ip_forward.read_text() == "1\n" + assert ( + "iptables-legacy", + "-t", + "nat", + "-A", + "POSTROUTING", + "-o", + "eth0", + "-p", + "tcp", + "-j", + "SNAT", + "--to-source", + "10.0.0.2", + ) in calls + assert ( + "iptables-legacy", + "-t", + "nat", + "-A", + "POSTROUTING", + "-o", + "eth0", + "-p", + "udp", + "-j", + "SNAT", + "--to-source", + "10.0.0.2", + ) in calls + + +@pytest.mark.asyncio +async def test_run_command_times_out_and_kills_process(): + service = DockerService(MagicMock()) + + with pytest.raises(RuntimeError, match="timed out after"): + await service._run_command( + sys.executable, + "-c", + "import time; time.sleep(10)", + timeout_seconds=0.01, + ) + + +@pytest.mark.asyncio +async def test_start_cleans_up_dockerd_when_readiness_fails(monkeypatch, tmp_path): + class FakeProcess: + stdout = None + returncode = None + terminated = False + killed = False + + def terminate(self): + self.terminated = True + + def kill(self): + self.killed = True + + async def wait(self): + self.returncode = 0 + + process = FakeProcess() + service = DockerService( + MagicMock(), + data_root=tmp_path / "docker-data", + run_paths=(tmp_path / "docker.sock", tmp_path / "docker.pid"), + ) + service.configure_network = AsyncMock() + service.wait_until_ready = AsyncMock(side_effect=RuntimeError("not ready")) + monkeypatch.setattr( + "sandbox_runtime.docker_service.asyncio.create_subprocess_exec", + AsyncMock(return_value=process), + ) + + with pytest.raises(RuntimeError, match="not ready"): + await service.start() + + assert process.terminated is True + assert service.process is None + assert service._log_task is None + + +@pytest.mark.asyncio +async def test_monitor_logs_when_runtime_services_are_unhealthy(): + env = { + "SANDBOX_ID": "test-sandbox", + "REPO_OWNER": "acme", + "REPO_NAME": "repo", + "SESSION_CONFIG": "{}", + } + + with patch.dict(os.environ, env, clear=False): + supervisor = SandboxSupervisor() + + supervisor.runtime_services.ensure_healthy = AsyncMock(return_value=False) + supervisor.log = MagicMock() + + await supervisor.monitor_processes() + + supervisor.log.error.assert_any_call("runtime_services.unhealthy") + assert supervisor.shutdown_event.is_set() diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 917bad5e3..1ed670488 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -799,3 +799,4 @@ export interface ListAutomationRunsResponse { } export * from "./integrations"; +export * from "./sandbox-provider"; diff --git a/packages/shared/src/types/integrations.test.ts b/packages/shared/src/types/integrations.test.ts new file mode 100644 index 000000000..d029b8963 --- /dev/null +++ b/packages/shared/src/types/integrations.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from "vitest"; +import { normalizeSandboxRuntimeSettings, resolveSandboxImageProfile } from "./integrations"; + +describe("sandbox settings helpers", () => { + it("normalizes runtime sandbox settings at untrusted boundaries", () => { + expect( + normalizeSandboxRuntimeSettings({ + tunnelPorts: ["3000", 5173, -1, 70000, 3001], + terminalEnabled: "true", + dockerEnabled: true, + maxConcurrentChildSessions: 2, + maxTotalChildSessions: 0, + }) + ).toEqual({ + tunnelPorts: [5173, 3001], + }); + }); + + it("resolves Docker image profile from normalized settings", () => { + expect(resolveSandboxImageProfile({ dockerEnabled: true })).toBe("docker"); + expect(resolveSandboxImageProfile({ dockerEnabled: "true" as unknown as boolean })).toBe( + "default" + ); + }); +}); diff --git a/packages/shared/src/types/integrations.ts b/packages/shared/src/types/integrations.ts index 8f8a82238..f3352b504 100644 --- a/packages/shared/src/types/integrations.ts +++ b/packages/shared/src/types/integrations.ts @@ -48,18 +48,67 @@ export const DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS = 5; /** Default maximum agent-spawned child sessions per parent session. */ export const DEFAULT_MAX_TOTAL_CHILD_SESSIONS = 15; +/** Repo image/runtime profile that changes the base sandbox image requirements. */ +export type SandboxImageProfile = "default" | "docker"; + +export const DEFAULT_SANDBOX_IMAGE_PROFILE: SandboxImageProfile = "default"; + +export function isSandboxImageProfile(value: unknown): value is SandboxImageProfile { + return value === DEFAULT_SANDBOX_IMAGE_PROFILE || value === "docker"; +} + /** Sandbox environment settings. Provider-agnostic: describes what the user wants, not how it's done. */ export interface SandboxSettings { /** Extra ports to expose via tunnels (e.g., dev server ports 3000, 5173). */ tunnelPorts?: number[]; /** Enable a browser-based terminal (ttyd) in sandbox sessions. */ terminalEnabled?: boolean; + /** Enable Docker Engine and Docker Compose inside Modal-backed sandboxes. */ + dockerEnabled?: boolean; /** Maximum active agent-spawned child sessions per parent session. */ maxConcurrentChildSessions?: number; /** Maximum total agent-spawned child sessions per parent session. */ maxTotalChildSessions?: number; } +/** Runtime settings passed to sandbox providers. Excludes control-plane-only settings. */ +export type SandboxRuntimeSettings = Pick; + +function isPositiveInteger(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value >= 1; +} + +function normalizeTunnelPorts(value: unknown): number[] | undefined { + if (value === undefined) return undefined; + if (!Array.isArray(value)) return []; + return value + .filter((port): port is number => isPositiveInteger(port) && port <= 65535) + .slice(0, MAX_TUNNEL_PORTS); +} + +export function normalizeSandboxRuntimeSettings(value: unknown): SandboxRuntimeSettings { + if (!value || typeof value !== "object" || Array.isArray(value)) return {}; + + const settings = value as Record; + const normalized: SandboxRuntimeSettings = {}; + const tunnelPorts = normalizeTunnelPorts(settings.tunnelPorts); + + if (tunnelPorts !== undefined) { + normalized.tunnelPorts = tunnelPorts; + } + if (typeof settings.terminalEnabled === "boolean") { + normalized.terminalEnabled = settings.terminalEnabled; + } + + return normalized; +} + +export function resolveSandboxImageProfile( + settings: Pick | null | undefined +): SandboxImageProfile { + return settings?.dockerEnabled === true ? "docker" : DEFAULT_SANDBOX_IMAGE_PROFILE; +} + export type SlackMentionsPolicy = "allow" | "escape" | "strip"; /** Per-repo Slack overrides. Mentions policy is workspace-wide and cannot be overridden per repo. */ diff --git a/packages/shared/src/types/sandbox-provider.test.ts b/packages/shared/src/types/sandbox-provider.test.ts new file mode 100644 index 000000000..01716b11f --- /dev/null +++ b/packages/shared/src/types/sandbox-provider.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { + getProviderCapabilities, + parseSandboxBackendName, + PROVIDER_CAPABILITIES, +} from "./sandbox-provider"; + +describe("parseSandboxBackendName", () => { + it("defaults to modal when undefined / empty / whitespace", () => { + expect(parseSandboxBackendName(undefined)).toBe("modal"); + expect(parseSandboxBackendName("")).toBe("modal"); + expect(parseSandboxBackendName(" ")).toBe("modal"); + }); + + it("returns the named backend", () => { + expect(parseSandboxBackendName("modal")).toBe("modal"); + expect(parseSandboxBackendName("daytona")).toBe("daytona"); + }); + + it("is case-insensitive and trims whitespace", () => { + expect(parseSandboxBackendName("MODAL")).toBe("modal"); + expect(parseSandboxBackendName(" Daytona ")).toBe("daytona"); + }); + + it("throws for an unsupported provider", () => { + expect(() => parseSandboxBackendName("k8s")).toThrow("Unsupported SANDBOX_PROVIDER: k8s"); + expect(() => parseSandboxBackendName("fly")).toThrow("Unsupported SANDBOX_PROVIDER: fly"); + }); +}); + +describe("getProviderCapabilities", () => { + it("defaults to the modal capability row", () => { + expect(getProviderCapabilities(undefined)).toBe(PROVIDER_CAPABILITIES.modal); + }); + + it("resolves capabilities by backend name", () => { + expect(getProviderCapabilities("daytona")).toBe(PROVIDER_CAPABILITIES.daytona); + }); + + it("models docker and prebuilt images as distinct capabilities", () => { + // Modal is the only backend with prebuilt images / a dashboard URL today. + expect(PROVIDER_CAPABILITIES.modal.supportsDocker).toBe(true); + expect(PROVIDER_CAPABILITIES.modal.supportsPrebuiltImages).toBe(true); + expect(PROVIDER_CAPABILITIES.modal.supportsDashboardUrl).toBe(true); + + expect(PROVIDER_CAPABILITIES.daytona.supportsDocker).toBe(false); + expect(PROVIDER_CAPABILITIES.daytona.supportsPrebuiltImages).toBe(false); + expect(PROVIDER_CAPABILITIES.daytona.supportsDashboardUrl).toBe(false); + }); +}); diff --git a/packages/shared/src/types/sandbox-provider.ts b/packages/shared/src/types/sandbox-provider.ts new file mode 100644 index 000000000..9edd420fe --- /dev/null +++ b/packages/shared/src/types/sandbox-provider.ts @@ -0,0 +1,145 @@ +// Sandbox provider capabilities and backend selection. +// +// This module is the single, provider-agnostic source of truth for: +// - which sandbox backends exist and how the SANDBOX_PROVIDER value is parsed +// - what each backend can do (the capability table) +// +// The control plane (lifecycle manager, routes, durable object) and the web app +// both read capabilities from here instead of re-deriving behavior by comparing +// the provider name (e.g. `=== "modal"`). Adding or changing a provider is a +// single edit to PROVIDER_CAPABILITIES. + +import { DEFAULT_SANDBOX_IMAGE_PROFILE, type SandboxImageProfile } from "./integrations"; + +/** + * Canonical sandbox backends. + * + * Vercel is on the roadmap but not yet implemented as a provider; it is added + * to this union (and to PROVIDER_CAPABILITIES, the factory, and the web parser) + * together with its provider class. + */ +export type SandboxBackendName = "modal" | "daytona"; + +/** + * Parse the configured sandbox backend name. + * + * Defaults to Modal to preserve existing deployments. Throws on an unknown + * value so misconfiguration fails loudly rather than silently falling back. + */ +export function parseSandboxBackendName(value: string | undefined): SandboxBackendName { + const normalized = value?.trim().toLowerCase(); + + if (!normalized || normalized === "modal") { + return "modal"; + } + if (normalized === "daytona") { + return "daytona"; + } + + throw new Error(`Unsupported SANDBOX_PROVIDER: ${value}`); +} + +/** + * Capabilities supported by a sandbox provider. + * + * Each flag describes provider-agnostic intent ("supports Docker", "supports + * prebuilt images") so callers can gate behavior on the capability rather than + * on the provider name. Distinct features get distinct flags — never let "is + * modal" stand in for an unrelated capability. + */ +export interface SandboxProviderCapabilities { + /** Whether the provider supports filesystem snapshots */ + supportsSnapshots: boolean; + /** Whether the provider supports restoring from snapshots */ + supportsRestore: boolean; + /** Whether the provider supports pre-warming sandboxes */ + supportsWarm: boolean; + /** Whether the provider can resume a previously stopped sandbox in place */ + supportsPersistentResume?: boolean; + /** Whether the provider can stop a sandbox explicitly via API */ + supportsExplicitStop?: boolean; + /** Whether the provider can run Docker Engine inside sandboxes */ + supportsDocker?: boolean; + /** Whether the provider supports pre-built per-repo images (repo image builds) */ + supportsPrebuiltImages?: boolean; + /** Whether the provider exposes a management dashboard URL for sandbox objects */ + supportsDashboardUrl?: boolean; +} + +/** + * Authoritative capability table, keyed by backend. + * + * This is build-time data shipped to both the control plane and the web bundle. + * Provider classes set their `capabilities` from this table; routes and the web + * client read it via {@link getProviderCapabilities}. + */ +export const PROVIDER_CAPABILITIES: Record = { + modal: { + supportsSnapshots: true, + supportsRestore: true, + supportsWarm: true, + supportsPersistentResume: false, + supportsExplicitStop: false, + supportsDocker: true, + supportsPrebuiltImages: true, + supportsDashboardUrl: true, + }, + daytona: { + supportsSnapshots: false, + supportsRestore: false, + supportsWarm: false, + supportsPersistentResume: true, + supportsExplicitStop: true, + supportsDocker: false, + supportsPrebuiltImages: false, + supportsDashboardUrl: false, + }, +}; + +/** + * Look up provider capabilities from a raw SANDBOX_PROVIDER value. + * + * Defaults to Modal (via {@link parseSandboxBackendName}); throws on an unknown + * provider name. + */ +export function getProviderCapabilities(value: string | undefined): SandboxProviderCapabilities { + return PROVIDER_CAPABILITIES[parseSandboxBackendName(value)]; +} + +/** + * Provider-agnostic, declarative runtime intent for a sandbox. + * + * Describes what the user wants ("Docker available"), not how a provider + * realizes it. Crosses the provider boundary so a provider can act on the + * intent directly (e.g. start dockerd) even when the environment id alone does + * not encode that behavior. Extend this object (e.g. `gpu?`) to add the next + * runtime capability without reshaping the boundary. + */ +export interface RequestedSandboxRuntime { + /** User wants Docker Engine + Docker Compose available inside the sandbox. */ + docker?: boolean; +} + +/** + * Resolve runtime intent to a logical environment id, gated by provider + * capabilities. This is the single place intent → environment lives. + * + * The environment id is shared and provider-agnostic; it drives image + * selection and snapshot/prebuilt-image compatibility. Each provider maps the + * id to a concrete image internally. Today this is a 1:1 map (docker intent on + * a docker-capable provider → "docker"); it is the seed of a future shared + * environment catalog where the boundary may instead carry an explicit + * environment selection. + * + * Capability gating happens here: a Docker request on a provider that does not + * support Docker resolves to the default environment. + */ +export function resolveEnvironment( + requested: RequestedSandboxRuntime | undefined, + capabilities: SandboxProviderCapabilities +): SandboxImageProfile { + if (requested?.docker === true && capabilities.supportsDocker === true) { + return "docker"; + } + return DEFAULT_SANDBOX_IMAGE_PROFILE; +} diff --git a/packages/web/src/components/settings/sandbox-settings-model.ts b/packages/web/src/components/settings/sandbox-settings-model.ts new file mode 100644 index 000000000..0637a14cf --- /dev/null +++ b/packages/web/src/components/settings/sandbox-settings-model.ts @@ -0,0 +1,101 @@ +import type { SandboxSettings } from "@open-inspect/shared"; + +export type DockerMode = "inherit" | "enabled" | "disabled"; + +export function isValidPort(value: string): boolean { + return /^\d+$/.test(value) && Number(value) >= 1 && Number(value) <= 65535; +} + +export function isPositiveInteger(value: string): boolean { + return /^\d+$/.test(value) && Number(value) >= 1; +} + +export function normalizePorts(input: string[]): { ports: number[]; invalid: string[] } { + const nonEmpty = input.filter((row) => row.trim() !== ""); + const invalid = nonEmpty.filter((row) => !isValidPort(row.trim())); + const ports = [ + ...new Set(nonEmpty.filter((row) => isValidPort(row.trim())).map((row) => Number(row.trim()))), + ]; + return { ports, invalid }; +} + +export function resolveDockerMode({ + isGlobal, + globalDefaults, + repoSettings, +}: { + isGlobal: boolean; + globalDefaults?: SandboxSettings; + repoSettings?: SandboxSettings | null; +}): DockerMode { + if (isGlobal) { + return globalDefaults?.dockerEnabled ? "enabled" : "disabled"; + } + if (repoSettings?.dockerEnabled === undefined) { + return "inherit"; + } + return repoSettings.dockerEnabled ? "enabled" : "disabled"; +} + +export function resolveDockerEnabled( + dockerMode: DockerMode, + globalDefaults?: SandboxSettings +): boolean { + return dockerMode === "enabled" || (dockerMode === "inherit" && !!globalDefaults?.dockerEnabled); +} + +export function buildSettingsPayload({ + baseSettings, + isGlobal, + ports, + terminalEnabled, + dockerMode, + dockerSupported, + maxConcurrentChildSessions, + maxTotalChildSessions, + maxConcurrentChildSessionsEdited, + maxTotalChildSessionsEdited, + repoSettings, +}: { + baseSettings?: SandboxSettings | null; + isGlobal: boolean; + ports: number[]; + terminalEnabled: boolean; + dockerMode: DockerMode; + dockerSupported: boolean; + maxConcurrentChildSessions: string; + maxTotalChildSessions: string; + maxConcurrentChildSessionsEdited: boolean; + maxTotalChildSessionsEdited: boolean; + repoSettings?: SandboxSettings | null; +}): SandboxSettings { + const settingsPayload: SandboxSettings = { + ...baseSettings, + tunnelPorts: ports, + terminalEnabled, + }; + + if (dockerSupported) { + if (!isGlobal && dockerMode === "inherit") { + delete settingsPayload.dockerEnabled; + } else { + settingsPayload.dockerEnabled = dockerMode === "enabled"; + } + } + if ( + isGlobal || + maxConcurrentChildSessionsEdited || + repoSettings?.maxConcurrentChildSessions !== undefined + ) { + settingsPayload.maxConcurrentChildSessions = Number(maxConcurrentChildSessions); + } + if ( + isGlobal || + maxTotalChildSessionsEdited || + repoSettings?.maxTotalChildSessions !== undefined + ) { + settingsPayload.maxTotalChildSessions = Number(maxTotalChildSessions); + } + + return settingsPayload; +} diff --git a/packages/web/src/components/settings/sandbox-settings.test.tsx b/packages/web/src/components/settings/sandbox-settings.test.tsx index 1ef584847..a22523f48 100644 --- a/packages/web/src/components/settings/sandbox-settings.test.tsx +++ b/packages/web/src/components/settings/sandbox-settings.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom /// -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import * as matchers from "@testing-library/jest-dom/matchers"; @@ -34,6 +34,10 @@ vi.mock("@/hooks/use-repos", () => ({ const SETTINGS_KEY = "/api/integration-settings/sandbox"; +beforeEach(() => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "modal"); +}); + function globalSettings( tunnelPorts: number[], enabledRepos?: string[], @@ -71,6 +75,7 @@ function renderWithSWR(fallbackData: unknown) { afterEach(() => { cleanup(); vi.restoreAllMocks(); + vi.unstubAllEnvs(); reposMock.repos = []; reposMock.loading = false; }); @@ -194,6 +199,7 @@ describe("SandboxSettingsPage — tunnel ports editor", () => { defaults: { tunnelPorts: [8080], terminalEnabled: false, + dockerEnabled: false, maxConcurrentChildSessions: DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, maxTotalChildSessions: DEFAULT_MAX_TOTAL_CHILD_SESSIONS, }, @@ -257,6 +263,7 @@ describe("SandboxSettingsPage — tunnel ports editor", () => { defaults: { tunnelPorts: [], terminalEnabled: false, + dockerEnabled: false, maxConcurrentChildSessions: 2, maxTotalChildSessions: 7, }, @@ -393,6 +400,7 @@ describe("SandboxSettingsPage — tunnel ports editor", () => { defaults: { tunnelPorts: [3000], terminalEnabled: false, + dockerEnabled: false, maxConcurrentChildSessions: DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, maxTotalChildSessions: DEFAULT_MAX_TOTAL_CHILD_SESSIONS, }, @@ -417,4 +425,370 @@ describe("SandboxSettingsPage — tunnel ports editor", () => { expect(screen.getByText("Save Settings").closest("button")).toBeDisabled(); }); + + it("hides Docker control for non-Modal sandbox providers", () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "daytona"); + renderWithSWR(globalSettings([])); + expect(screen.queryByText("Docker")).not.toBeInTheDocument(); + }); + + it("preserves Docker settings when saving other global settings for non-Modal providers", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "daytona"); + + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { + [SETTINGS_KEY]: { + integrationId: "sandbox", + settings: { defaults: { tunnelPorts: [], dockerEnabled: true } }, + }, + }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByText("Add port")); + await user.type(screen.getByPlaceholderText("e.g. 3000"), "3000"); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + SETTINGS_KEY, + expect.objectContaining({ method: "PUT" }) + ); + }); + + const putCall = fetchMock.mock.calls.find(([, init]) => init?.method === "PUT"); + expect(putCall).toBeDefined(); + const [, init] = putCall!; + expect(JSON.parse(String(init?.body))).toEqual({ + settings: { + defaults: { + tunnelPorts: [3000], + terminalEnabled: false, + dockerEnabled: true, + maxConcurrentChildSessions: DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, + maxTotalChildSessions: DEFAULT_MAX_TOTAL_CHILD_SESSIONS, + }, + }, + }); + }); + + it("does not add Docker settings when saving non-Modal sandbox settings", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "daytona"); + + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { + [SETTINGS_KEY]: { + integrationId: "sandbox", + settings: { defaults: { tunnelPorts: [] } }, + }, + }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByText("Add port")); + await user.type(screen.getByPlaceholderText("e.g. 3000"), "3000"); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + SETTINGS_KEY, + expect.objectContaining({ method: "PUT" }) + ); + }); + + const putCall = fetchMock.mock.calls.find(([, init]) => init?.method === "PUT"); + expect(putCall).toBeDefined(); + const [, init] = putCall!; + expect(JSON.parse(String(init?.body))).toEqual({ + settings: { + defaults: { + tunnelPorts: [3000], + terminalEnabled: false, + maxConcurrentChildSessions: DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, + maxTotalChildSessions: DEFAULT_MAX_TOTAL_CHILD_SESSIONS, + }, + }, + }); + }); + + it("sends dockerEnabled in the global payload when enabled", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "modal"); + + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { [SETTINGS_KEY]: globalSettings([]) }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByRole("switch", { name: "Docker" })); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + SETTINGS_KEY, + expect.objectContaining({ + method: "PUT", + body: JSON.stringify({ + settings: { + defaults: { + tunnelPorts: [], + terminalEnabled: false, + dockerEnabled: true, + maxConcurrentChildSessions: DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS, + maxTotalChildSessions: DEFAULT_MAX_TOTAL_CHILD_SESSIONS, + }, + }, + }), + }) + ); + }); + }); + + it("lets repo Docker settings inherit without saving a dockerEnabled override", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "modal"); + Element.prototype.scrollIntoView = vi.fn(); + reposMock.repos = [ + { + id: 1, + fullName: "acme/app", + owner: "acme", + name: "app", + description: null, + private: false, + defaultBranch: "main", + }, + ]; + const repoSettingsKey = "/api/integration-settings/sandbox/repos/acme/app"; + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { + [SETTINGS_KEY]: { + integrationId: "sandbox", + settings: { defaults: { tunnelPorts: [], dockerEnabled: true } }, + }, + [repoSettingsKey]: { integrationId: "sandbox", repo: "acme/app", settings: null }, + }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByText("All Repositories (Global)")); + await user.click(screen.getByRole("option", { name: /app/ })); + expect(screen.getByLabelText("Docker")).toHaveValue("inherit"); + + await user.click(screen.getByText("Add port")); + await user.type(screen.getByPlaceholderText("e.g. 3000"), "3000"); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + repoSettingsKey, + expect.objectContaining({ + method: "PUT", + body: JSON.stringify({ + settings: { tunnelPorts: [3000], terminalEnabled: false }, + }), + }) + ); + }); + }); + + it("clears an existing repo Docker override when switching back to inherit", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "modal"); + Element.prototype.scrollIntoView = vi.fn(); + reposMock.repos = [ + { + id: 1, + fullName: "acme/app", + owner: "acme", + name: "app", + description: null, + private: false, + defaultBranch: "main", + }, + ]; + const repoSettingsKey = "/api/integration-settings/sandbox/repos/acme/app"; + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { + [SETTINGS_KEY]: { + integrationId: "sandbox", + settings: { defaults: { tunnelPorts: [], dockerEnabled: true } }, + }, + [repoSettingsKey]: { + integrationId: "sandbox", + repo: "acme/app", + settings: { tunnelPorts: [], terminalEnabled: false, dockerEnabled: false }, + }, + }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByText("All Repositories (Global)")); + await user.click(screen.getByRole("option", { name: /app/ })); + expect(screen.getByLabelText("Docker")).toHaveValue("disabled"); + + await user.selectOptions(screen.getByLabelText("Docker"), "inherit"); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + repoSettingsKey, + expect.objectContaining({ + method: "PUT", + body: JSON.stringify({ + settings: { tunnelPorts: [], terminalEnabled: false }, + }), + }) + ); + }); + }); + + it("saves explicit disabled Docker override for a repo", async () => { + vi.stubEnv("NEXT_PUBLIC_SANDBOX_PROVIDER", "modal"); + Element.prototype.scrollIntoView = vi.fn(); + reposMock.repos = [ + { + id: 1, + fullName: "acme/app", + owner: "acme", + name: "app", + description: null, + private: false, + defaultBranch: "main", + }, + ]; + const repoSettingsKey = "/api/integration-settings/sandbox/repos/acme/app"; + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method === "PUT") { + return new Response(JSON.stringify({}), { status: 200 }); + } + throw new Error("unexpected fetch"); + }); + vi.stubGlobal("fetch", fetchMock); + + render( + new Map(), + fallback: { + [SETTINGS_KEY]: { + integrationId: "sandbox", + settings: { defaults: { tunnelPorts: [], dockerEnabled: true } }, + }, + [repoSettingsKey]: { integrationId: "sandbox", repo: "acme/app", settings: null }, + }, + dedupingInterval: Infinity, + revalidateOnFocus: false, + revalidateIfStale: false, + revalidateOnReconnect: false, + }} + > + + + ); + + await user.click(screen.getByText("All Repositories (Global)")); + await user.click(screen.getByRole("option", { name: /app/ })); + await user.selectOptions(screen.getByLabelText("Docker"), "disabled"); + await user.click(screen.getByText("Save Settings")); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + repoSettingsKey, + expect.objectContaining({ + method: "PUT", + body: JSON.stringify({ + settings: { tunnelPorts: [], terminalEnabled: false, dockerEnabled: false }, + }), + }) + ); + }); + }); }); diff --git a/packages/web/src/components/settings/sandbox-settings.tsx b/packages/web/src/components/settings/sandbox-settings.tsx index c802b5156..038ae003d 100644 --- a/packages/web/src/components/settings/sandbox-settings.tsx +++ b/packages/web/src/components/settings/sandbox-settings.tsx @@ -13,6 +13,15 @@ import { DEFAULT_MAX_TOTAL_CHILD_SESSIONS, MAX_TUNNEL_PORTS, } from "@open-inspect/shared"; +import { + buildSettingsPayload, + isPositiveInteger, + normalizePorts, + resolveDockerEnabled, + resolveDockerMode, + type DockerMode, +} from "./sandbox-settings-model"; +import { supportsSandboxDocker } from "@/lib/sandbox-provider"; const GLOBAL_SCOPE = "__global__"; @@ -29,14 +38,6 @@ interface RepoSettingsResponse { const fetcher = (url: string) => fetch(url).then((r) => r.json()); -function isValidPort(value: string): boolean { - return /^\d+$/.test(value) && Number(value) >= 1 && Number(value) <= 65535; -} - -function isPositiveInteger(value: string): boolean { - return /^\d+$/.test(value) && Number(value) >= 1; -} - function SandboxSettingsEditor({ scope, owner, @@ -73,6 +74,8 @@ function SandboxSettingsEditor({ const currentTerminalEnabled: boolean = isGlobal ? ((data as GlobalSettingsResponse)?.settings?.defaults?.terminalEnabled ?? false) : ((data as RepoSettingsResponse)?.settings?.terminalEnabled ?? false); + const currentDockerMode = resolveDockerMode({ isGlobal, globalDefaults, repoSettings }); + const dockerSupported = supportsSandboxDocker(); const currentMaxConcurrentChildSessions: number = isGlobal ? (globalDefaults?.maxConcurrentChildSessions ?? DEFAULT_MAX_CONCURRENT_CHILD_SESSIONS) @@ -88,6 +91,7 @@ function SandboxSettingsEditor({ const [portRows, setPortRows] = useState(null); const [terminalEnabled, setTerminalEnabled] = useState(null); + const [dockerMode, setDockerMode] = useState(null); const [maxConcurrentChildSessions, setMaxConcurrentChildSessions] = useState(null); const [maxTotalChildSessions, setMaxTotalChildSessions] = useState(null); const [saving, setSaving] = useState(false); @@ -96,6 +100,8 @@ function SandboxSettingsEditor({ // Resolve terminal toggle: local edit or server state const resolvedTerminalEnabled = terminalEnabled ?? currentTerminalEnabled; + const resolvedDockerMode = dockerMode ?? currentDockerMode; + const resolvedDockerEnabled = resolveDockerEnabled(resolvedDockerMode, globalDefaults); // Use server state unless user is editing const rows = portRows ?? currentPorts.map(String); @@ -120,16 +126,6 @@ function SandboxSettingsEditor({ setPortRows(updated); }; - /** Trim, filter empty, validate, parse to number, dedupe. */ - const normalizePorts = (input: string[]): { ports: number[]; invalid: string[] } => { - const nonEmpty = input.filter((r) => r.trim() !== ""); - const invalid = nonEmpty.filter((r) => !isValidPort(r.trim())); - const ports = [ - ...new Set(nonEmpty.filter((r) => isValidPort(r.trim())).map((r) => Number(r.trim()))), - ]; - return { ports, invalid }; - }; - const handleSave = useCallback(async () => { setError(null); setSuccess(false); @@ -153,24 +149,19 @@ function SandboxSettingsEditor({ const existingEnabledRepos = isGlobal ? (data as GlobalSettingsResponse)?.settings?.enabledRepos : undefined; - const settingsPayload: SandboxSettings = { - tunnelPorts: ports, + const settingsPayload = buildSettingsPayload({ + baseSettings: isGlobal ? globalDefaults : repoSettings, + isGlobal, + ports, terminalEnabled: resolvedTerminalEnabled, - }; - if ( - isGlobal || - maxConcurrentChildSessions !== null || - repoSettings?.maxConcurrentChildSessions !== undefined - ) { - settingsPayload.maxConcurrentChildSessions = Number(resolvedMaxConcurrentChildSessions); - } - if ( - isGlobal || - maxTotalChildSessions !== null || - repoSettings?.maxTotalChildSessions !== undefined - ) { - settingsPayload.maxTotalChildSessions = Number(resolvedMaxTotalChildSessions); - } + dockerMode: resolvedDockerMode, + dockerSupported, + maxConcurrentChildSessions: resolvedMaxConcurrentChildSessions, + maxTotalChildSessions: resolvedMaxTotalChildSessions, + maxConcurrentChildSessionsEdited: maxConcurrentChildSessions !== null, + maxTotalChildSessionsEdited: maxTotalChildSessions !== null, + repoSettings, + }); const body = isGlobal ? { settings: { defaults: settingsPayload, enabledRepos: existingEnabledRepos } } : { settings: settingsPayload }; @@ -189,6 +180,7 @@ function SandboxSettingsEditor({ await mutate(); setPortRows(null); setTerminalEnabled(null); + setDockerMode(null); setMaxConcurrentChildSessions(null); setMaxTotalChildSessions(null); setSuccess(true); @@ -204,19 +196,23 @@ function SandboxSettingsEditor({ apiUrl, mutate, data, + globalDefaults, resolvedTerminalEnabled, + resolvedDockerMode, + dockerSupported, resolvedMaxConcurrentChildSessions, resolvedMaxTotalChildSessions, maxConcurrentChildSessions, maxTotalChildSessions, - repoSettings?.maxConcurrentChildSessions, - repoSettings?.maxTotalChildSessions, + repoSettings, ]); const hasPortChanges = portRows !== null && JSON.stringify(normalizePorts(portRows).ports) !== JSON.stringify(currentPorts); const hasTerminalChange = terminalEnabled !== null && terminalEnabled !== currentTerminalEnabled; + const hasDockerChange = + dockerSupported && dockerMode !== null && dockerMode !== currentDockerMode; const hasConcurrentLimitChange = maxConcurrentChildSessions !== null && maxConcurrentChildSessions !== String(currentMaxConcurrentChildSessions); @@ -224,7 +220,11 @@ function SandboxSettingsEditor({ maxTotalChildSessions !== null && maxTotalChildSessions !== String(currentMaxTotalChildSessions); const hasChanges = - hasPortChanges || hasTerminalChange || hasConcurrentLimitChange || hasTotalLimitChange; + hasPortChanges || + hasTerminalChange || + hasDockerChange || + hasConcurrentLimitChange || + hasTotalLimitChange; if (isLoading || isLoadingGlobal) { return

Loading...

; @@ -259,6 +259,48 @@ function SandboxSettingsEditor({ + {dockerSupported ? ( +
+
+
+ +

+ Enable Docker Engine and Docker Compose in Modal sandboxes. +

+
+ {isGlobal ? ( + + ) : ( + + )} +
+
+ ) : null} +
diff --git a/packages/web/src/lib/sandbox-provider.ts b/packages/web/src/lib/sandbox-provider.ts index b7a3bcb1d..d879ed4fa 100644 --- a/packages/web/src/lib/sandbox-provider.ts +++ b/packages/web/src/lib/sandbox-provider.ts @@ -1,23 +1,30 @@ /** * Public sandbox backend helpers for the web app. + * + * Backend parsing and capability lookups are delegated to @open-inspect/shared + * so the web app and the control plane agree on a single source of truth. */ -export type PublicSandboxProvider = "modal" | "daytona"; +import { + getProviderCapabilities, + parseSandboxBackendName, + type SandboxBackendName, +} from "@open-inspect/shared"; -export function getPublicSandboxProvider(): PublicSandboxProvider { - const rawValue = process.env.NEXT_PUBLIC_SANDBOX_PROVIDER ?? process.env.SANDBOX_PROVIDER; - if (!rawValue || rawValue.trim() === "") { - return "modal"; - } +export type PublicSandboxProvider = SandboxBackendName; - const value = rawValue.trim().toLowerCase(); - if (value === "modal" || value === "daytona") { - return value; - } +function rawSandboxProvider(): string | undefined { + return process.env.NEXT_PUBLIC_SANDBOX_PROVIDER ?? process.env.SANDBOX_PROVIDER; +} - throw new Error(`Invalid sandbox provider: ${rawValue}`); +export function getPublicSandboxProvider(): PublicSandboxProvider { + return parseSandboxBackendName(rawSandboxProvider()); } export function supportsRepoImages(): boolean { - return getPublicSandboxProvider() === "modal"; + return getProviderCapabilities(rawSandboxProvider()).supportsPrebuiltImages === true; +} + +export function supportsSandboxDocker(): boolean { + return getProviderCapabilities(rawSandboxProvider()).supportsDocker === true; } diff --git a/terraform/README.md b/terraform/README.md index d418d08de..2a3bd799b 100644 --- a/terraform/README.md +++ b/terraform/README.md @@ -180,6 +180,8 @@ MODAL_TOKEN_SECRET MODAL_WORKSPACE MODAL_ENVIRONMENT # Optional; defaults to main MODAL_ENVIRONMENT_WEB_SUFFIX # Optional; lowercase letters, digits, dashes; empty for workspace--... endpoints +MODAL_DOCKER_SANDBOX_CPU # Optional; overrides Modal's default CPU request for Docker-enabled sandboxes +MODAL_DOCKER_SANDBOX_MEMORY_MB # Optional; overrides Modal's default memory request for Docker-enabled sandboxes # GitHub OAuth App GH_OAUTH_CLIENT_ID diff --git a/terraform/d1/migrations/0022_add_repo_image_profile.sql b/terraform/d1/migrations/0022_add_repo_image_profile.sql new file mode 100644 index 000000000..21ee86867 --- /dev/null +++ b/terraform/d1/migrations/0022_add_repo_image_profile.sql @@ -0,0 +1,7 @@ +-- Track which sandbox image profile a repo image was built for. +ALTER TABLE repo_images + ADD COLUMN image_profile TEXT NOT NULL DEFAULT 'default' + CHECK (image_profile IN ('default', 'docker')); + +CREATE INDEX IF NOT EXISTS idx_repo_images_repo_profile_status + ON repo_images(repo_owner, repo_name, image_profile, status, created_at DESC); diff --git a/terraform/environments/production/modal.tf b/terraform/environments/production/modal.tf index fb3b487a3..9c249e0b2 100644 --- a/terraform/environments/production/modal.tf +++ b/terraform/environments/production/modal.tf @@ -52,12 +52,20 @@ module "modal_app" { }, { name = "internal-api" - values = { - MODAL_API_SECRET = var.modal_api_secret - INTERNAL_CALLBACK_SECRET = var.internal_callback_secret - ALLOWED_CONTROL_PLANE_HOSTS = local.control_plane_host - CONTROL_PLANE_URL = local.control_plane_url - } + values = merge( + { + MODAL_API_SECRET = var.modal_api_secret + INTERNAL_CALLBACK_SECRET = var.internal_callback_secret + ALLOWED_CONTROL_PLANE_HOSTS = local.control_plane_host + CONTROL_PLANE_URL = local.control_plane_url + }, + var.modal_docker_sandbox_cpu == null ? {} : { + MODAL_DOCKER_SANDBOX_CPU = tostring(var.modal_docker_sandbox_cpu) + }, + var.modal_docker_sandbox_memory_mb == null ? {} : { + MODAL_DOCKER_SANDBOX_MEMORY_MB = tostring(var.modal_docker_sandbox_memory_mb) + } + ) } ] } diff --git a/terraform/environments/production/terraform.tfvars.example b/terraform/environments/production/terraform.tfvars.example index 01f2b45c1..6668e47c2 100644 --- a/terraform/environments/production/terraform.tfvars.example +++ b/terraform/environments/production/terraform.tfvars.example @@ -62,6 +62,11 @@ modal_workspace = "" modal_environment = "main" modal_environment_web_suffix = "" +# Optional resource overrides for Docker-enabled Modal sandboxes. +# Leave unset to use Modal's default CPU and memory requests. +# modal_docker_sandbox_cpu = 1 +# modal_docker_sandbox_memory_mb = 2048 + # Daytona REST API # Only required when sandbox_provider = "daytona" # Create an API key at https://app.daytona.io with permissions: diff --git a/terraform/environments/production/variables.tf b/terraform/environments/production/variables.tf index 3bc0c31a0..9f0493541 100644 --- a/terraform/environments/production/variables.tf +++ b/terraform/environments/production/variables.tf @@ -94,6 +94,28 @@ variable "modal_environment_web_suffix" { } } +variable "modal_docker_sandbox_cpu" { + description = "Optional CPU cores requested for Docker-enabled Modal sandboxes. Leave null to use Modal's default." + type = number + default = null + + validation { + condition = var.modal_docker_sandbox_cpu == null || var.modal_docker_sandbox_cpu > 0 + error_message = "modal_docker_sandbox_cpu must be null or greater than 0." + } +} + +variable "modal_docker_sandbox_memory_mb" { + description = "Optional memory in MB requested for Docker-enabled Modal sandboxes. Leave null to use Modal's default." + type = number + default = null + + validation { + condition = var.modal_docker_sandbox_memory_mb == null || var.modal_docker_sandbox_memory_mb > 0 + error_message = "modal_docker_sandbox_memory_mb must be null or greater than 0." + } +} + # ============================================================================= # GitHub OAuth App Credentials # =============================================================================