feat(opencomputer): build managed base snapshot from Terraform#840
Conversation
OpenComputer was the only sandbox provider whose base image wasn't built by Terraform: Modal, Vercel, and Daytona each compute a source hash and run a build script via null_resource, but OpenComputer's template was a hand-set secret pointing at a manually-built snapshot. Add terraform/modules/opencomputer-infra (mirroring vercel-sandbox-infra): a data.external source hash over sandbox-runtime + build-template.ts drives a null_resource that runs 'npm run build:opencomputer-template' on change, producing a deterministic, immutable snapshot 'openinspect-runtime-<hash16>'. The name is hash-derived (not timestamped like Vercel) because OpenComputer references templates by exact name. The control-plane worker now gets OPENCOMPUTER_TEMPLATE from the managed snapshot when opencomputer_template is empty, or the pinned value when set (mirroring vercel_base_snapshot_id). opencomputer_template's required-validation is relaxed accordingly. Also wires the OPENCOMPUTER_* secrets into terraform.yml (Plan + Apply), which #818 omitted.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughOpenComputer Terraform inputs, production wiring, module execution, and control-plane bindings were updated to support managed snapshot creation and fallback template selection. ChangesOpenComputer sandbox infrastructure
Sequence Diagram(s)sequenceDiagram
participant ProductionTerraform as terraform/environments/production/opencomputer.tf
participant SourceHash as data.external.opencomputer_source_hash
participant OpenComputerInfra as module.opencomputer_infra
participant BuildScript as scripts/build-base-snapshot.sh
participant NpmBuild as npm run build:opencomputer-template
ProductionTerraform->>SourceHash: compute hash from selected source files
ProductionTerraform->>OpenComputerInfra: pass api_url, api_key, manual_snapshot_id, project_root, source_hash
OpenComputerInfra->>BuildScript: run local-exec with OpenComputer env vars
BuildScript->>NpmBuild: build base snapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
| packages/sandbox-runtime/pyproject.toml | ||
| packages/sandbox-runtime/src | ||
| packages/opencomputer-infra/src/build-template.ts | ||
| ) |
There was a problem hiding this comment.
[deep review] this source-hash boundary is too narrow for a managed immutable snapshot. build-template.ts copies every non-cache file under packages/sandbox-runtime/src/sandbox_runtime via collectRuntimeFiles, including skills markdown and any future config/data assets, but this trigger only hashes *.py, *.js, *.ts, and pyproject.toml. Editing a runtime skill prompt or adding another copied asset changes the snapshot contents without changing the Terraform trigger/name, so apply will not rebuild and the worker keeps pointing at the old openinspect-runtime-<hash>. That hidden drift is exactly what this managed snapshot path is supposed to remove. Can we make the hash reflect the builder’s actual input boundary, ideally using the same include/exclude policy as collectRuntimeFiles (plus the builder/package inputs that affect the image), instead of maintaining a narrower Terraform-only approximation?
There was a problem hiding this comment.
Confirmed and fixed in a8fb840. Verified concretely: 26 files are baked under src/sandbox_runtime but only 22 are .py/.js/.ts — the 4 missed are the skills/*/SKILL.md prompts (agent-browser, visual-verification, record-video, upload-screenshot). The hash now drops the extension filter and covers every file under packages/sandbox-runtime/src, excluding the same caches collectRuntimeFiles skips (__pycache__/.pytest_cache/.ruff_cache/*.pyc/.DS_Store), so editing a skill prompt or adding a copied asset now re-triggers the rebuild. (Hashed input set went 22 → 30 files.)
| TF_VAR_vercel_snapshot_expiration_ms: "${{ secrets.VERCEL_SNAPSHOT_EXPIRATION_MS || '0' }}" | ||
| # Sandbox provider: OpenComputer. Inert while sandbox_provider defaults to "modal"; | ||
| # when set to "opencomputer", Terraform builds the managed base snapshot during apply. | ||
| TF_VAR_opencomputer_api_url: ${{ secrets.OPENCOMPUTER_API_URL }} |
There was a problem hiding this comment.
[deep review] the workflow trigger still does not include packages/opencomputer-infra/**. After this PR, that package becomes Terraform-managed infrastructure: changes to build-template.ts should run plan/apply and, on main, build a new base snapshot. As written, OpenComputer image changes can merge without the Terraform workflow running at all unless they also touch terraform/** or packages/sandbox-runtime/**, which reintroduces the provider-specific drift this PR is trying to eliminate. Can we add packages/opencomputer-infra/** to both the push and pull_request path filters?
There was a problem hiding this comment.
Fixed in a8fb840. Added packages/opencomputer-infra/** to both the push and pull_request path filters — it was the only sandbox builder missing (modal-infra/daytona-infra/sandbox-runtime are all present), so build-template.ts changes now run plan/apply and rebuild the managed snapshot on main.
| packages/sandbox-runtime/pyproject.toml | ||
| packages/sandbox-runtime/src | ||
| packages/opencomputer-infra/src/build-template.ts | ||
| ) |
There was a problem hiding this comment.
This hash is the trigger that decides whether Terraform rebuilds the managed OpenComputer snapshot, but it only includes the runtime source/pyproject and build-template.ts. Dependency-only changes in packages/opencomputer-infra/package.json / the root lockfile can change the builder/runtime image serialization or SDK behavior without changing this hash, so Terraform will keep pointing workers at the previous deterministic snapshot name and skip the rebuild. This is especially risky because the new module is intended to make the snapshot source-managed. Please include the package dependency files that affect npm run build:opencomputer-template in the hash, similar to how the Modal hash includes dependency lockfiles.
There was a problem hiding this comment.
Fixed in a8fb840. Added packages/opencomputer-infra/package.json and the root package-lock.json to the hash, so an @opencomputer/sdk / build-toolchain bump now invalidates the snapshot — matching how the Modal hash includes its uv.lock.
There was a problem hiding this comment.
Summary
Reviewed PR #840, feat(opencomputer): build managed base snapshot from Terraform, by @ColeMurray. The change adds Terraform/CI wiring for managed OpenComputer base snapshot builds across 8 files (+162/-7); the overall approach matches the existing provider build pattern, but the snapshot rebuild trigger misses dependency inputs that can affect the generated image.
Critical Issues
- [Correctness]
terraform/environments/production/opencomputer.tf:16- The source hash only includessandbox-runtimesource/pyproject andbuild-template.ts, but notpackages/opencomputer-infra/package.jsonor the root lockfile used bynpm run build:opencomputer-template. Dependency-only changes to the OpenComputer SDK/build toolchain can change image serialization or builder behavior while Terraform keeps the same deterministic snapshot name and skips the rebuild. I left an inline comment with the requested fix.
Suggestions
- None beyond the blocking hash-input fix.
Nitpicks
- None.
Positive Feedback
- The module keeps the managed snapshot inert unless
sandbox_provider = "opencomputer", which limits impact on the default Modal path. - The deterministic snapshot naming is a good fit for exact-name snapshot references and avoids timestamp churn.
- The shell wrapper validates required build environment inputs and uses
set -euo pipefail.
Questions
- None.
Validation
bash -n terraform/modules/opencomputer-infra/scripts/build-base-snapshot.shpassed.- I could not rerun Terraform checks in this environment because the
terraformbinary is not installed.
Verdict
Request Changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
terraform/modules/opencomputer-infra/main.tf (1)
10-18: 🧹 Nitpick | 🔵 TrivialHash-based naming leaves orphaned snapshots over time.
Because
snapshot_nameis derived fromsource_hash, every source change produces a new immutable snapshot under a new name, and the prior snapshot is never destroyed by thisnull_resource. Over many builds this accumulates orphaned base snapshots on the OpenComputer side. Consider a periodic cleanup/retention process (e.g. delete snapshots matchingopeninspect-runtime-*that are no longer referenced) to bound storage. This mirrors the same limitation in the Modal/Vercel/Daytona modules, so it can be addressed cross-provider rather than here alone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@terraform/modules/opencomputer-infra/main.tf` around lines 10 - 18, The opencomputer base snapshot naming in local.snapshot_name and the opencomputer_base_snapshot null_resource is hash-derived, which creates a new immutable snapshot for every source_hash and leaves older snapshots orphaned. Update the snapshot management flow to include retention/cleanup for unused openinspect-runtime-* snapshots, ideally as part of the shared cross-provider snapshot handling used by the Modal/Vercel/Daytona modules, so stale snapshots are periodically deleted when no longer referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@terraform/environments/production/opencomputer.tf`:
- Around line 10-28: The inline `bash -c` hash script in the external data
source is swallowing failures, so harden it by enabling strict shell behavior at
the start of the script and making the `cd` to `var.project_root` fail fast if
it cannot change directories. Keep the hashing logic in place, but ensure
`find`/pipeline errors propagate instead of being masked by the final `echo`, so
`data.external` only returns a valid `hash` from the `paths` set and does not
produce an empty snapshot name in `opencomputer.tf`.
---
Nitpick comments:
In `@terraform/modules/opencomputer-infra/main.tf`:
- Around line 10-18: The opencomputer base snapshot naming in
local.snapshot_name and the opencomputer_base_snapshot null_resource is
hash-derived, which creates a new immutable snapshot for every source_hash and
leaves older snapshots orphaned. Update the snapshot management flow to include
retention/cleanup for unused openinspect-runtime-* snapshots, ideally as part of
the shared cross-provider snapshot handling used by the Modal/Vercel/Daytona
modules, so stale snapshots are periodically deleted when no longer referenced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12ea4457-a58a-4aa0-9ab0-f2c6e5b758b8
📒 Files selected for processing (8)
.github/workflows/terraform.ymlterraform/environments/production/opencomputer.tfterraform/environments/production/variables.tfterraform/environments/production/workers-control-plane.tfterraform/modules/opencomputer-infra/main.tfterraform/modules/opencomputer-infra/outputs.tfterraform/modules/opencomputer-infra/scripts/build-base-snapshot.shterraform/modules/opencomputer-infra/variables.tf
Addresses review feedback on #840: - Hash every file build-template.ts bakes in (collectRuntimeFiles copies the whole runtime tree, not just *.py/.js/.ts) so skill prompts / assets retrigger rebuilds; previously 4 skills/*/SKILL.md files were baked but unhashed. - Add packages/opencomputer-infra/package.json + root package-lock.json so an SDK/toolchain bump also invalidates the snapshot (like the Modal lockfile). - Add 'set -euo pipefail' so a failed cd/find propagates instead of yielding a degenerate empty 'openinspect-runtime-' name. - Add packages/opencomputer-infra/** to the terraform.yml push + pull_request path filters so build-template.ts changes run plan/apply and rebuild on main.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
|
Re: the hash-named snapshots orphaning over time — agreed it's a real gap, but as @coderabbitai noted it's the same for the Modal/Vercel/Daytona managed base images, so rather than special-casing OpenComputer I filed #841 to track a shared cross-provider retention/cleanup. Keeping it out of this PR's scope. |
|
Tip For best results, initiate chat on the files or code changes.
(•ᴗ•)ᕗ 🐇 ✏️ Learnings added
|
…oot (#842) ## Summary Fixes the Terraform-managed OpenComputer snapshot build (added in #840). A real `terraform apply` with `SANDBOX_PROVIDER=opencomputer` fails immediately with: ``` Missing sandbox runtime directory: ../../../packages/sandbox-runtime/src/sandbox_runtime ``` ## Root cause The script ran the builder through `npm run build:opencomputer-template`, which uses `npm -w @open-inspect/opencomputer-infra` and **moves cwd into the workspace** (`packages/opencomputer-infra`). `build-template.ts` resolves the `sandbox-runtime` source relative to that cwd, so it looked for the runtime dir under the workspace instead of the repo root. ## Fix Mirror the **sibling Vercel builder** (`terraform/modules/vercel-sandbox-infra/scripts/build-base-snapshot.sh`), which splits build from run and invokes the builder directly from the repo root: ```sh cd "${PROJECT_ROOT}" npm run build -w @open-inspect/opencomputer-infra # build the bundle (cwd irrelevant) node packages/opencomputer-infra/dist/build-template.js # run from repo root -> cwd = repo root ``` The script already `cd`s into the repo, so running `node` from there keeps cwd at the repo root and the runtime dir resolves — with **no `OPENINSPECT_REPO_ROOT` env override and no git dependency**. This keeps repo-root resolution consistent with how the other providers' Terraform builders work. ## Validation Ran the new invocation with `--print-manifest` (which builds the image and returns *before* the snapshot API call) from the repo root: - exit 0, no "Missing sandbox runtime directory" - **26** `sandbox_runtime/*` files resolved, including the 4 `skills/*/SKILL.md` prompts - cacheKey computed The original `$(pwd)` approach was also proven end-to-end against a real OpenComputer build (image built all 36 steps); this revision keeps that working while removing the env contract per review. Pure shell change — `bash -n` clean, file mode (`100755`) preserved. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed snapshot build path resolution issues caused by changing directories during the build process, improving reliability across runs. * Improved build logic to ensure the correct runtime and template resources are located consistently, even when invoked from the repository root. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 participant
What
Makes Terraform build and manage the OpenComputer base snapshot, the same way it already builds the Modal, Vercel, and Daytona base images.
Why
OpenComputer (#818) was the only sandbox provider whose base image wasn't built by Terraform. Every other provider computes a source hash and runs a build script via
null_resource:data.external.modal_source_hashmodule.modal_appdata.external.vercel_source_hashmodule.vercel_sandbox_infradata.external.daytona_source_hashmodule.daytona_infraInstead,
opencomputer_templatewas a hand-set secret pointing at a snapshot you had to build manually withnpm run build:opencomputer-template. This brings it to parity.How
New
terraform/modules/opencomputer-infra(mirrorsvercel-sandbox-infra):opencomputer.tf—data.external.opencomputer_source_hashhashespackages/sandbox-runtime/{pyproject.toml,src}+packages/opencomputer-infra/src/build-template.ts, then calls the module (both gated onlocal.use_opencomputer_backend, so it's a no-op under Modal).null_resourcewhosetriggersinclude the source hash + the build script's own hash, runningscripts/build-base-snapshot.sh→npm run build:opencomputer-templateon change.build-template.tsalready content-addresses the image (image.cacheKey()), so unchanged source is a cheap no-op.openinspect-runtime-${substr(source_hash,0,16)}. Unlike Vercel (which timestamps and resolves name→latest), OpenComputer references templates by exact name (createSandboxsendssnapshot: <name>), so the managed name must be deterministic. A source change ⇒ new name ⇒ fresh, immutable snapshot.OPENCOMPUTER_TEMPLATEnow comes frommodule.opencomputer_infra[0].snapshot_namewhen unpinned, orvar.opencomputer_templatewhen set (mirrorsvercel_base_snapshot_id). The var's required-validation is relaxed to make it an optional pin.terraform.yml— wires theOPENCOMPUTER_*secrets into the Plan + Apply env blocks (feat: Opencomputer provider #818 added the variables but never the CI plumbing, unlike the other three providers).Effect on operators
The managed build replaces the manual
build:opencomputer-template+ hand-setOPENCOMPUTER_TEMPLATEsecret. To run OpenComputer you now only setSANDBOX_PROVIDER=opencomputer+ theOPENCOMPUTER_API_URL/OPENCOMPUTER_API_KEYsecrets; Terraform builds the snapshot during apply and points the worker at it. Pinning a specific snapshot viaOPENCOMPUTER_TEMPLATEstill works as an override.Notes
terraform applyin CI, which already doesnpm ci+ Node for the existing Vercel/Modal/bot builds;var.opencomputer_api_keyis passed into the exec env.vercel.tf/modal.tf) — movingbuild-template.tsor the runtime later means updating it, or rebuilds stop triggering.sandbox_provider = "modal"(the data source + module arecount = 0).Validation
terraform fmt -recursive -checkclean;terraform validate→ Success; workflow YAML parses;bash -non the build script clean.sandbox_provider=opencomputershould confirm the snapshot builds and the worker boots from it.Builds on #818 (provider) and #838 (image deps).
Summary by CodeRabbit
New Features
Bug Fixes