Skip to content

feat(function): custom sandboxes - #6071

Open
icecrasher321 wants to merge 41 commits into
stagingfrom
staging-v11
Open

feat(function): custom sandboxes #6071
icecrasher321 wants to merge 41 commits into
stagingfrom
staging-v11

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Custom sandboxes to pre-configure deps to not have per workflow costs. Available for enterprise and Max plan simiilar to Sim Mailer. E2B templates are managed. And Daytona is on runtime generation and installations (snapshot support is too non-permissive for this feature)

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

icecrasher321 and others added 21 commits July 28, 2026 17:52
Named package sets a Function block can import from. The server
canonicalizes and hashes the list; E2B prebuilds a content-addressed
template per set, Daytona installs per execution. Create/edit is gated to
Max or Enterprise via the shared workspace entitlement check; execution is
deliberately ungated, so a downgraded workspace keeps running what it
already built.

Also on this branch:

- Extract the duplicated dropdown/combobox option-fetch lifecycle into
  use-fetched-options. Only combobox had the dependency-change reset, so
  every dropdown with dependsOn + fetchOptions cleared its list and never
  repopulated until reopened.
- Collapse the repeated Max-tier entitlement check onto one
  hasMaxTierWorkspaceAccess, shared by inbox, live sync, and sandboxes.
- Resolve a personal payer's block state through getEffectiveBillingStatus
  in getBillingEntityBlockStatus, so the client-side Max gates agree with
  the server-side ones when blockOrgMembers' fan-out is stale.
- Carve the Daytona dependency install out of the caller's execution
  budget instead of stacking on top of it.

Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	apps/sim/components/settings/navigation.ts
#	scripts/check-api-validation-contracts.ts
Staging claimed 0271 and 0272 while this branch was out, so the hand-authored
0271_workspace_sandboxes was dropped before the merge and regenerated on top
of the merged schema. Same DDL; drizzle emits plain CREATE TABLE/INDEX rather
than the hand-added IF NOT EXISTS, which matches the repo default — that
idempotent form is only needed for files with CONCURRENTLY ops below an
embedded COMMIT. Regenerating also restores the meta snapshot the
hand-authored migration never had.

Co-Authored-By: Claude <noreply@anthropic.com>
Staging independently claims idx 0273, so remove ours before merging to
avoid an add/add conflict on the drizzle migration index. Regenerated at
the next free index once the merge lands.

Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
Staging took 0273 and 0274, so the sandboxes DDL lands at the next free
index. The emitted SQL is byte-identical to the dropped 0273.

Co-Authored-By: Claude <noreply@anthropic.com>
The Max tier was spelled five ways. The odd one out — `isMax`, defined as
`isPro(plan) && credits >= 25000` — excluded both `team_25000` and
`enterprise`, and it was the sole input to the personal-workspace cap. A
delinquent Max-for-Teams org admin got 1 personal workspace while a
delinquent Max individual got 10. Only free/pro_6000/pro_25000 were tested,
so the two broken tiers were unpinned.

Separately, the server gate and the client `hasUsableMaxAccess` were
independent copies of the same rule. The settings sidebar renders Sandboxes
and Sim Mailer from the client one while the API answers 403 from the
server one, so any drift renders a feature unlocked that the API refuses.

- `MAX_TIER_CREDITS` is derived from the `CREDIT_TIERS` table; `isMaxTier`
  in plan-helpers is now the single definition, shared by the server gates,
  the client derivation, `getPlanTypeForLimits`, `plan-view`, and the cap
- `hasWorkspaceTierAccess(id, predicate, { intent, onMissingWorkspace })`
  becomes the one org-vs-personal payer fork. `intent: 'active-use'` means
  active and not billing-blocked; `'retention'` means active/past_due with
  block state ignored, so the inbox teardown guard keeps its fail-open
  semantics instead of implying them through a duplicated fork
- `isWorkspaceOnEnterprisePlan`'s personal branch now applies the status and
  block checks its own org branch always had, and its TSDoc names its real
  consumer (copilot BYOK, not Access Control)
- the client live-sync gate gained the server's `isHosted` branch, so a
  self-hosted deploy with billing on no longer locks an interval the API
  accepts. It reads both flags directly rather than taking one as a
  parameter the callers sourced from the same module
- `sqlIsPro`/`sqlIsTeam` escape the `_` LIKE wildcard, matching the already
  correct hand-rolled filter in seat-drift
- deletes the `TERMINAL_SUBSCRIPTION_STATUSES` and `ENTITLED_STATUSES`
  shadow constants, and corrects three test mocks that asserted `trialing`
  was entitled or usable

`max-tier-parity.test.ts` asserts the client and server answers match for
every plan name. Both new guards were checked against the old code: the
parity test fails 3 assertions with the previous predicate, and the
self-hosted test fails without the `isHosted` branch.

Co-Authored-By: Claude <noreply@anthropic.com>
Staging has claimed 0275 (table_views) and 0276 (drop_legacy_folder_tables)
since the last merge, so our 0275_workspace_sandboxes collides on the index.

Dropping ours first — the .sql, meta/0275_snapshot.json, and the journal
entry — leaves packages/db/migrations byte-identical to the merge-base, so
the merge sees no add/add conflict at all. Regenerated on the far side.

Ours is the droppable side: plain additive DDL with no hand edits, which
drizzle reproduces exactly. Staging's migrations are hand-written and must
survive.

Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	apps/sim/lib/core/config/feature-flags.ts
#	scripts/check-api-validation-contracts.ts
Staging claimed 0275 (table_views) and 0276 (drop_legacy_folder_tables), so
the sandboxes migration dropped before the merge comes back on top as 0277.

The emitted SQL is byte-identical to what was dropped — the original had no
hand edits, so there is nothing to reapply. It is purely additive: two enums,
sandbox_image and workspace_sandbox, their two FKs and six indexes. That it
regenerated unchanged also confirms the schema.ts auto-merge was correct —
had it lost staging's legacy-folder-table drops, drizzle would have emitted
CREATE TABLE for them here.

Snapshot chain is continuous (0273 -> 0277, each prevId matching the previous
id) and the table counts track the DDL: 100 -> 101 (table_views) -> 99
(legacy folder tables dropped) -> 101 (the two sandbox tables).

Co-Authored-By: Claude <noreply@anthropic.com>
…ut switch

Sandboxes shipped behind `custom-sandboxes`, an AppConfig rollout flag falling
back to a `CUSTOM_SANDBOXES` secret. That made it the only Max-gated surface
with no self-hosted path: `INBOX_ENABLED` can force Sim Mailer on for an
operator running their own billing, and `ENTERPRISE_ENABLED` turns on the
other nine features at once, but neither reached sandboxes. A self-hoster had
to find a separately-named variable that was not part of that family, and one
running with billing enabled could not enable it at all.

Sandboxes now joins the enterprise feature set and the rollout flag is gone:

- `sandboxes` is an `EnterpriseFeature` with `SANDBOXES_ENABLED` and its
  `NEXT_PUBLIC_` twin, so the master switch and the per-feature override both
  reach it like every sibling
- `hasWorkspaceSandboxAccess` takes the inbox's shape exactly — the override
  wins, then a deployment without billing is unrestricted, then the workspace
  payer needs usable Max or Enterprise
- the settings nav gains `selfHostedOverride`, so the section resolves through
  the same path as Sim Mailer instead of a second entitlement AND-ed in
- `custom-sandboxes`, the `CUSTOM_SANDBOXES` secret, the now-unreachable
  `SANDBOXES_UNAVAILABLE` 403 copy, and the route's kill-switch branch are
  deleted

Its legacy default is `true`, matching `inbox`: the gate already returns true
whenever billing is off, so `false` would leave the nav override disagreeing
with the gate that answers the request. Self-hosted builds run on the
operator's own E2B/Daytona credentials, so there is no Sim-side cost to
withhold — the docs now say so, since enabling the feature without a provider
configured is the obvious trap.

The new gate tests run with billing enabled on purpose; the `!isBillingEnabled`
bail would otherwise answer every case and hide whether the override is wired.
Verified by deleting the override line — exactly the one assertion fails.

Co-Authored-By: Claude <noreply@anthropic.com>
`matchTriggerWidth={false}` exists for the opposite case — a narrow trigger
whose option labels would truncate, letting the menu grow past it. The language
field is a full-width form control with two short labels, so the override
shrank the menu to "JavaScript" and pinned it to the right edge instead.

The default (`true`) is correct here. Every other consumer passing `false` is a
genuinely narrow trigger — a role picker in a member row, a table filter chip.

Co-Authored-By: Claude <noreply@anthropic.com>
…able

`ensureSandboxImage` only ran when a sandbox was saved, so resolution treated
an unusable image as terminal and told the user to go fix a definition that was
never wrong. Three states stuck permanently until someone re-saved in Settings:

- a build that failed
- a build whose worker died mid-flight, stranding the row in `building`
- every sandbox created while the deployment ran a `runtime` provider, after a
  switch to a `prebuilt` one — `runtime` writes no image rows at all, so the
  whole fleet resolved to "no completed build" with nothing to repair it

Resolution now re-queues through the registry's existing idempotent entry point
before failing, and says a build is on its way instead of pointing at Settings.
The conflict guard already claims only a `failed` row or a stale `pending`/
`building` one, so executions arriving during a healthy build enqueue nothing —
no thundering herd from a hot workflow.

The registry is imported dynamically for the same reason `sandboxDb` is: it
pulls `@sim/db` into the static graph, which this module keeps out of the
executor bundle. That also avoids a cycle, since the registry imports
`invalidateSandboxResolution` from here. A repair that itself fails is logged
and swallowed — it must never replace the build error naming the sandbox.

Verified by deleting the repair call: exactly the three new assertions fail.

Co-Authored-By: Claude <noreply@anthropic.com>
The label read "Test · Python · 1 package". The block's own list is already
scoped to the language its sibling `language` subblock selects, so the language
repeated on every row said nothing, and the package count is decoration next to
the name that identifies the sandbox.

The language stays for the one caller that cannot filter — agent tool-input
renders this field under a synthetic id where the sibling `language` value is
unreachable, so its list spans both languages and the name alone is ambiguous.
That is the same missing value which disables filtering, so `showLanguage` is
derived from it directly rather than passed independently and left to drift.

A failed build is still marked: that suffix is the difference between a
selection that runs and one that does not.

Passing the flag also means dropping `.map(toSandboxOption)` for an explicit
arrow — `Array.map` hands the index to the second parameter.

Co-Authored-By: Claude <noreply@anthropic.com>
The card printed "443f4934-26ab-44ab-8...". `resolveDropdownLabel` only reads a
subblock's static `options` array, and the sandbox picker is a `combobox` whose
options load asynchronously, so its array is empty and the raw stored id fell
through to the label.

Resolved the same way skills and tools already are: a `resolveSandboxLabel` in
the display layer, fed from the shared sandbox list query — the same cache entry
the picker reads, so this adds no request.

Two deliberate scopings:

- the query is subscribed only for the sandbox row. `SubBlockRow` is memoized
  per subblock, and the list query polls while a build is in flight, so an
  unconditional hook would re-render every row on the canvas on each poll tick
- the resolver matches the field id, not just the type. There is no dedicated
  subblock type for it, and matching `combobox` alone would relabel unrelated
  pickers

An id with no matching sandbox resolves to null rather than a guess, so a
deleted sandbox falls through to the caller's placeholder. The template preview
surface is left alone: it is explicitly hook-free and passes empty lists for
tools and skills too.

Co-Authored-By: Claude <noreply@anthropic.com>
Entitlement decides whether a workspace may author sandboxes; nothing decided
whether anything could run one. A self-hosted deployment with SANDBOXES_ENABLED
but no E2B or Daytona credentials got a fully functional tab whose output no
Function block could select — the picker is gated on the provider vars, the tab
was not.

Both navigation planes now drop the section when neither
NEXT_PUBLIC_SANDBOX_ENABLED nor the pre-Daytona NEXT_PUBLIC_E2B_ENABLED is set —
the same pair the picker's `showWhenEnvSet` reads, so the two cannot disagree.
Dropped rather than locked: an upgrade does not conjure a provider.

The unified plane drops it in `buildUnifiedSettingsNavigation` rather than in the
sidebar's filter, because the sidebar's `selfHostedOverride` short-circuit runs
before its `requiresMax` check and would have revealed the tab anyway. It reads
the browser twins, not the server's `isRemoteSandboxEnabled`, since this module
renders on both sides.

The predicate is a function, not a module constant, because the constant form was
untestable and ambient: the env mock falls through to `process.env`, and
`apps/sim/.env` (gitignored, so absent on CI) sets NEXT_PUBLIC_E2B_ENABLED=true.
The nav tests passed locally and failed 6 assertions with the flag cleared. They
now pin both flags, so the suite is identical with and without a local env file —
verified by running it both ways.

Co-Authored-By: Claude <noreply@anthropic.com>
The Sandboxes section described behavior two commits on this branch changed, and
led with an internal detail no reader needs.

- entitlement is no longer Max/Enterprise only: self-hosted deployments unlock
  sandboxes with SANDBOXES_ENABLED, and the section is hidden outright when a
  deployment has no sandbox provider, which is the state a self-hoster is most
  likely to hit and least likely to diagnose
- a build that is not Ready is no longer terminal. It is queued again on the next
  run, so the advice is to wait and re-run, not to go edit a package list that
  was never wrong
- deleting a sandbox frees its build once nothing else references it. Builds are
  shared by content, so this is the one place a reader could reasonably assume
  deletion is immediate

Dropped the `ModuleNotFoundError` aside: what the old code did instead is not
something a reader needs to know to use the feature.

The page is hand-written — `function` has category 'blocks' and is absent from
`NATIVE_RESOURCE_BLOCK_TYPES`, so generate-docs skips it and these edits will not
be overwritten.

Co-Authored-By: Claude <noreply@anthropic.com>
Deleting a sandbox only removed its row, leaving the built template in E2B until
the 30-day retention sweep — up to a month of paying to store an image nothing
could select. Editing a package list had the same effect on the old content
address, which is the more common case since every edit re-points the sandbox.

`releaseSandboxImage(specHash)` now deletes the provider image and its row from
both paths. It reuses the sweep's provider call and its ordering: image first,
row second, so a refused delete leaves the row for the sweep to retry rather than
orphaning a remote template nothing points at.

Two guards make eager deletion safe:

- builds are keyed by content, not by workspace, so two workspaces declaring the
  same package list share one image. The release no-ops while any sandbox still
  references the hash — otherwise one workspace's delete would break the other's
- an in-flight build is left alone rather than raced; the sweep collects it once
  it settles

Called detached from both routes. The row is already committed by then, so the
user's action has succeeded whatever the provider says, and awaiting would hold a
UI delete open on a remote call the sweep would retry anyway. Every failure inside
is logged and swallowed for the same reason.

E2B's delete verified against their API reference: DELETE /templates/{templateID}
with X-API-Key, 204 on success. The existing implementation already matched, so
this commit only adds the call sites and the guards.

Co-Authored-By: Claude <noreply@anthropic.com>
…atus dot

Two follow-ups to the resolution repair.

The repair had no rate limit. `ensureSandboxImage` re-claims a `failed` row on
sight, and a bad package name fails in seconds, so the in-flight guard never
closed the window: a workflow on a one-minute schedule would enqueue a build a
minute against a package list that will never resolve, each one real provider
build compute. Before the repair existed resolution simply threw, so this was
introduced with it.

The two callers want different things, so the cooldown is opt-in. A save is a
person explicitly asking for another attempt and still retries immediately;
resolution passes `FAILED_BUILD_RETRY_COOLDOWN_MS` and gets at most one attempt
per window no matter how often the workflow runs. Ten minutes: long enough that
per-minute runs cannot drive per-minute builds, short enough that a transient
registry outage clears within the hour.

The status line loses its colour dot. `size-[6px] rounded-full` appeared in
exactly one file in the repo, so it was a new primitive rather than a pattern,
and it duplicated state the text colour already carries — the label now turns
`--text-error` on a failed build, which is what every other status row in
settings does. `ChipTag` was the wrong home for this: its variants are
`mono`/`invite`, with no semantic tone, so a status version would have meant
overriding its chrome from the consumer.

Also corrects the docs line this changes: a failed build is retried periodically,
and saving is the way to retry now, so "wait a moment and run again" no longer
describes it.

Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	apps/sim/blocks/types.ts
#	apps/sim/lib/core/config/env.ts
#	apps/sim/lib/execution/remote-sandbox/conformance.test.ts
#	apps/sim/lib/execution/remote-sandbox/index.ts
#	apps/sim/lib/execution/remote-sandbox/types.ts
@gitguardian

gitguardian Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
35187658 Triggered Username Password 41bc220 apps/desktop/src/main/browser-credentials/vault.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@cursor

cursor Bot commented Jul 30, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes remote code execution (custom images, dependency install), secret injection for agent tools, and billing/entitlement gates across server and client—mistakes could break existing Function blocks or widen secret exposure.

Overview
Adds workspace sandboxes—named Python/npm dependency sets that Function blocks can select so imports run against a prebuilt E2B image (or install at runtime on Daytona)—with Settings → Sandboxes (Max/Enterprise or SANDBOXES_ENABLED), CRUD APIs, image build/retention jobs, and docs.

Function execution now accepts sandboxId and passes it into remote sandbox resolution; the block config adds an advanced Sandbox combobox (language-scoped, empty = default image) and Secret access tool-only fields so agent tools can limit which workspace secrets are injected—scoped before {{VAR}} resolution so narrowed lists cannot leak via template tags.

Billing and gates align Max-tier features: isMaxTier / MAX_TIER_CREDITS as the single threshold, hasWorkspaceSandboxAccess (override like inbox), shared hasWorkspaceTierAccess for inbox/live sync/sandboxes, and personal enterprise payer checks match org mode; getBillingEntityBlockStatus for users follows effective org-owner block state.

Editor polish: shared useFetchedOptions for combobox/dropdown, context: 'tool-input' / emptyIsValid subblock flags, wand sandbox package enrichment, and connector live-sync entitlement mirrors server isHosted behavior.

Reviewed by Cursor Bugbot for commit 4e21e64. Configure here.

@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Jul 30, 2026 7:01pm

Request Review

Comment thread apps/sim/lib/execution/remote-sandbox/image-registry.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds workspace custom sandboxes with content-addressed prebuilt images (E2B) and runtime installs (Daytona), plus registry lifecycle, billing/entitlements, settings UI, and Function-block selection.

  • Image registry: ensure/build/release/cleanup with atomic claim-before-delete and mid-delete re-adopt rebuild
  • Create-time repairMissingSandboxImage when the provider reports a missing template
  • Workspace sandbox APIs, DB migration, plan gates (enterprise/Max), docs and Helm bits

Confidence Score: 5/5

Safe to merge; prior shared-image race findings are addressed and no blocking failure remains on the follow-up scope.

Claim-before-delete, sweep parity, post-delete rebuild, refused-delete restore isolation, dead-row drop, and create-time missing-image repair are present in HEAD; no remaining incomplete fix on those threads.

Important Files Changed

Filename Overview
apps/sim/lib/execution/remote-sandbox/image-registry.ts Shared claim-and-delete, re-adopt rebuild with imageKnownGone, and drop-on-failed-rebuild close the prior shared-image race class.
apps/sim/lib/execution/remote-sandbox/resolve.ts Resolution fail-closed plus create-time missing-image repair with imageKnownGone and no cooldown.
apps/sim/lib/execution/remote-sandbox/e2b.ts Prebuilt templates, narrow isMissingImage classifier, and REST delete for retention/release.
apps/sim/lib/execution/remote-sandbox/index.ts createSelectedSandbox routes missing-template create failures into rebuild messaging.

Sequence Diagram

sequenceDiagram
  participant Exec as Execution
  participant Resolve as resolveWorkspaceSandbox
  participant Reg as sandbox_image registry
  participant Prov as E2B provider

  Exec->>Resolve: sandboxId + workspaceId
  Resolve->>Reg: read image by specHash
  alt ready + imageRef
    Resolve-->>Exec: ResolvedSandbox
    Exec->>Prov: create(imageRef)
    alt template missing
      Prov-->>Exec: NotFoundError
      Exec->>Reg: ensureSandboxImage(imageKnownGone)
      Exec-->>Exec: fail closed with rebuild message
    else ok
      Prov-->>Exec: sandbox handle
    end
  else missing/failed/in-flight
    Resolve->>Reg: scheduleImageRepair
    Resolve-->>Exec: throw unusable image
  end
Loading

Reviews (12): Last reviewed commit: "fix(sandboxes): key the build trigger by..." | Re-trigger Greptile

icecrasher321 and others added 2 commits July 29, 2026 17:29
…atement

Greptile P1. Reading references in one statement and deleting in another left a
window — a wide one, since a provider delete is a network call — where a second
workspace could declare the same package list, inherit the `ready` row, and have
its next run fail against a template already on its way out. Content addressing
is what makes that reachable: the image is shared, so one workspace's delete can
strand another's sandbox.

The reference check now lives in the conditional DELETE itself, so winning the
delete is the proof that nothing referenced the hash. A workspace that adopts the
hash first makes the delete match nothing and the release becomes a no-op.

Claiming the row before the provider call would otherwise strand a template
nothing points at if the provider then refused, so that path puts the row back
and the retention sweep inherits the retry — the same property the previous
ordering had.

The sweep is deliberately left as it is: its equivalent window needs a hash
unreferenced AND unused for 30 days, and its provider-first ordering encodes the
documented retry-on-refusal behaviour this path now reproduces explicitly.

No transaction is opened. The provider call sits between discrete statements
rather than inside one, so no pooled connection is held across it — which is why
this uses a conditional delete instead of the repo's `pg_advisory_xact_lock`
pattern, whose lock only releases at commit.

Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/execution/remote-sandbox/image-registry.ts
icecrasher321 and others added 2 commits July 29, 2026 19:33
Cursor and Greptile, independently, on the same code. `deleteImage` and
`rebuildIfReadopted` shared one try/catch, so a rebuild failure after a *successful*
provider delete was handled as if the provider had refused: the catch put the
claimed row back, `ready` status and all, pointing at a template that no longer
exists.

That is the one state resolution cannot repair — it fixes a row that is missing or
failed, never one claiming to be ready — so it reintroduced the permanent breakage
an earlier commit had just closed, through the error path rather than the happy one.

Restoring now belongs strictly to a refused delete. Once the template is gone the
row stays gone, and the rebuild runs past that catch. The rebuild also swallows its
own failures: it follows a delete that already succeeded, so it must not be reported
as a failed release, and inside the sweep it must not reject the rest of its chunk.
The adopter's next run still reaches the normal repair path.

The regression test drives a rebuild failure and asserts no row is restored. It
fails against the original shape — rebuild inside the shared try, no inner catch —
which is what the two reviewers were describing.

Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/execution/remote-sandbox/image-registry.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 29b7442. Configure here.

…cheduled

Greptile, one layer under the previous fix. Making the post-delete rebuild swallow
its own failures kept it from being reported as a failed release, but left the
adopter's row claiming a `ready` image whose template is already deleted — the one
state resolution cannot repair, since it rebuilds a row that is missing or failed
and never one that says ready.

So the row is now dropped when the rebuild does not take. That turns the adopter
into the missing-row case, which the next execution repairs on its own, instead of
a sandbox that stays broken until someone re-saves it by hand. A failure to drop it
is logged at error, because at that point two writes in a row have failed and there
is nothing further this path can do.

Also gives the release tests a default "nothing re-adopted" select. Without it the
rebuild threw on an unstubbed mock and the cleanup delete overwrote the predicate
the claim assertions read, so two of them were passing on the wrong statement.

Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f09dc99. Configure here.

Comment thread apps/sim/lib/execution/remote-sandbox/image-registry.ts
… observable

Six review rounds narrowed the window between deleting a shared template and
another workspace adopting its content hash, and each fix exposed the next facet.
They all share a cause: the registry row and the provider template are two systems
with no shared transaction, so any scheme that keeps them in step is guessing.

Create is the one step that does not have to guess. It either gets a sandbox or it
does not, so a `ready` row pointing at a deleted template now corrects itself the
first time it is used, rather than needing someone to re-save the sandbox.

- `SandboxImageBuilder.isMissingImage` asks the provider to classify its own
  failure. Prebuilt-only, because a runtime provider has no image to miss
- E2B answers it off `NotFoundError`, which the SDK maps from a 404. The only
  resource a create names is the template, and the two subclasses that describe
  other calls — a missing file, an exited sandbox — are excluded. The classifier
  stays deliberately narrow: treating auth or rate-limit failures as a missing
  image would turn a provider outage into a build storm
- `repairMissingSandboxImage` invalidates the cache, rebuilds with
  `imageKnownGone` (no cooldown, since this observed the image is gone rather than
  inferring it), and returns copy telling the author to run again
- `ResolvedSandbox` carries `specHash` so the failing execution can name what to
  rebuild

This subsumes the open facets rather than adding another guard beside them: the
stale per-replica cache, an adopter left `ready` against a deleted ref, and a
rebuild that never took all end at the same place — the next run repairs itself.

Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/execution/remote-sandbox/image-registry.ts
Cursor Bugbot. The Trigger.dev idempotency key was the content address alone, so a
second attempt at the same spec was deduped against the first: the SDK returns the
finished run instead of starting one, and the row that `ensureSandboxImage` just
flipped to `pending` sits there with no worker. Nothing can re-claim a `pending`
row until it goes stale, so a retry inside the 5-minute TTL did nothing for the
next half hour.

That silently disabled every repair path — save-to-retry, which the docs name
explicitly, and both the resolution and create-time rebuilds.

The key's own comment already said it exists "to collapse concurrent saves of the
same spec into one build, not to suppress a retry after one failed". The conditional
update above it is what actually collapses concurrent saves: only one caller gets a
row back, so only one ever reaches the trigger. Keying by the claim's `updatedAt`
keeps that property and makes each genuine attempt distinct, while a duplicate
delivery of one attempt still collapses.

Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4e21e64. Configure here.

Resolves the route-count baseline in scripts/check-api-validation-contracts.ts,
which git merged silently but wrongly. Both sides independently bumped
BASELINE.totalRoutes/zodRoutes 994 -> 997, three routes each; the literal is
identical on both sides, so git takes it as a convergent change with no
conflict while the merged tree actually holds 1000 routes. Set to 1000/1000/0
-- merge-base plus both deltas -- and proved by the audit's own scan rather
than the arithmetic: check:api-validation:strict reports total routes: 1000
against baseline: total=1000, and fails when the scan exceeds the baseline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant