[EN-1048] fix(api): heal sandbox missing from eviction index by jakubno · Pull Request #3199 · e2b-dev/infra

jakubno · 2026-07-03T08:45:58Z

Adds executionID to global eviction index to prevent TOCTOU
Adds heal routine, which adds sandbox to global eviction index, if out of sync

cursor · 2026-07-03T08:46:04Z

PR Summary

High Risk
Changes sit on the sandbox eviction and TTL path in Redis; mistakes could skip eviction, evict the wrong execution, or leave sandboxes running indefinitely, though behavior is heavily covered by new race and heal tests.

Overview
Fixes cases where sandboxes could outlive their TTL or lose eviction coverage when the global Redis expiration index drifted from stored sandbox records or when the same sandbox ID was reused across executions.

Expiration index members are now keyed by execution ID (with a rolling-deploy path for the old teamID:sandboxID shape). Add, Update, and Remove keep the ZSET aligned with the execution actually stored or deleted; Remove uses the Lua script’s returned JSON so index cleanup cannot drop a newer execution’s member. ExpiredItems sweeps orphans and stale execution members, upgrades legacy entries, and re-scores drifted scores, with counters for healed, swept, and rescored events.

A background healer on each API pod periodically scans team sandboxes and ZADD NX’s any missing index members (batched, grace period for in-flight adds), addressing the production “immortal sandbox” gap when index writes were lost.

^{Reviewed by Cursor Bugbot for commit 48134ad. Bugbot is set up for automated code reviews on this repo. Configure here.}

linear-code · 2026-07-03T08:46:25Z

EN-1048

codecov · 2026-07-03T08:47:29Z

Codecov Report

❌ Patch coverage is 73.60000% with 66 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ackages/api/internal/sandbox/storage/redis/heal.go	63.88%	29 Missing and 10 partials ⚠️
...ackages/api/internal/sandbox/storage/redis/main.go	61.29%	8 Missing and 4 partials ⚠️
...ckages/api/internal/sandbox/storage/redis/items.go	90.14%	2 Missing and 5 partials ⚠️
...s/api/internal/sandbox/storage/redis/operations.go	70.58%	2 Missing and 3 partials ⚠️
...ckages/api/internal/sandbox/storage/redis/utils.go	86.95%	2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

gemini-code-assist · 2026-07-03T08:57:06Z

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

Prevents accidentally report duration than actual sandbox duration. Prerequisite for #3199

claude

This PR touches core Redis-backed sandbox eviction paths and introduces a background healer, legacy-member migration, and multiple new race-handling code paths — worth a human review despite the good test coverage.

Extended reasoning...

Overview

The PR changes the global expiration ZSET member format from teamID:sandboxID to teamID:sandboxID:executionID to make ZREM structurally safe against TOCTOU where a lockless Add for a fresh execution of the same sandbox ID races a Remove or the evictor's stale sweep. It also adds a background healer that scans stored sandboxes and ZADD NX-re-adds missing index members, and expands ExpiredItems to sweep orphans and dead-execution members, lazily upgrade legacy members, and re-score drifted entries. removeSandboxScript now returns the deleted JSON so Remove can scope the ZREM to the execution it actually deleted. New OTEL counters (healed, swept, rescored) provide observability.

Security risks

None material. Changes are internal Redis bookkeeping; no auth/crypto/permission surface is touched. The new metrics attributes are static strings, not user input.

Level of scrutiny

High. Sandbox eviction is production-critical: getting index membership wrong can either leave sandboxes immortal (the incident this fixes) or falsely evict live sandboxes. Several code paths interact non-trivially (Add / Remove / Update / ExpiredItems / healer, plus a rolling-deploy migration window with legacy members coexisting), and the correctness argument relies on subtle ordering (ZADD NX before ZREM of legacy, XX-only rescoring, execution-scoped ZREM). Human eyes should confirm the invariants hold under Redis Cluster and rolling deploys.

Other factors

No bugs surfaced by the automated bug hunter.
Extensive race-focused unit tests exercise the key scenarios (fresh-execution not unindexed by Remove, stale-sweep execution-scoped, legacy upgrade, drift rescore, healer restoration, healer grace period, no-clobber).
Patch coverage is ~72% per Codecov, with the healer's error paths mostly uncovered — acceptable for warn-and-continue paths, but a reviewer should sanity-check.
No prior human review on this PR yet.

arkamar

Not necessarily request for changes, it's rather a request for discussion.

SMEMBERS + single MGET/ZMSCORE over a whole team index produces huge commands and replies for teams with many sandboxes, stalling Redis. Walk the index with SSCAN and heal in 256-id batches; ZADD NX keeps duplicate SSCAN returns idempotent.

Split-and-switch reads clearer than Cut + LastIndexByte and no longer returns sandbox IDs containing ':' — malformed members (non-UUID execution tag, extra segments) are rejected instead of folded into a legacy sandbox ID. Co-authored-by: Petr Vaněk <6185600+arkamar@users.noreply.github.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1572212514

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

arkamar

I think the cancel() for startHealer() makes sense. Otherwise LGTM.

Co-authored-by: Petr Vaněk <petr.vanek@e2b.dev>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48134ad9ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-07T18:11:50Z

+	// a newer execution wrote a different member, so it can never be
+	// unindexed here. If the key was already gone, any leftover execution
+	// member is swept by ExpiredItems once its score passes.
+	members := []any{legacyExpirationMember(teamID.String(), sandboxID)}


Avoid deleting fresh legacy members during rollout

When a new pod removes an old execution while an old pod is still in the rolling deploy path, that old pod can write the fresh execution's legacy team:sandbox member between the DEL/SREM script and this unconditional legacy ZREM; this line then deletes the fresh execution's only expiration-index entry, leaving the newly started sandbox invisible to ExpiredItems until the periodic healer eventually recreates it. The file already treats legacy members as still written during rolling deploys, so this race remains possible for the exact compatibility window the migration supports.

Useful? React with 👍 / 👎.

jakubno added 2 commits July 3, 2026 08:24

fix(api): heal sandbox missing from eviction index

727622d

chore: add metrics

3256394

cla-bot Bot added the cla-signed label Jul 3, 2026

jakubno changed the title ~~Fix/heal sandbox missing from eviction index~~ [EN-1048] fix(api): heal sandbox missing from eviction index Jul 3, 2026

jakubno mentioned this pull request Jul 3, 2026

chore(api): clamp sandbox duration #3200

Merged

jakubno added a commit that referenced this pull request Jul 3, 2026

chore(api): clamp sandbox duration (#3200)

9791ec2

Prevents accidentally report duration than actual sandbox duration. Prerequisite for #3199

jakubno marked this pull request as ready for review July 3, 2026 15:22

jakubno requested review from ValentaTomas and dobrac as code owners July 3, 2026 15:22

Merge branch 'main' into fix/heal-sandbox-missing-from-eviction-index

609845f

claude Bot reviewed Jul 3, 2026

View reviewed changes

arkamar requested changes Jul 7, 2026

View reviewed changes

Comment thread packages/api/internal/sandbox/storage/redis/utils.go

Comment thread packages/api/internal/sandbox/storage/redis/heal.go

Comment thread packages/api/internal/sandbox/storage/redis/items.go

jakubno and others added 3 commits July 7, 2026 15:51

chore: add todo to clean up migration artifacts

fb1bf45

jakubno requested a review from arkamar July 7, 2026 15:55

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread packages/api/internal/sandbox/storage/redis/main.go

arkamar approved these changes Jul 7, 2026

View reviewed changes

Comment thread packages/api/internal/sandbox/storage/redis/main.go

Update main.go

48134ad

Co-authored-by: Petr Vaněk <petr.vanek@e2b.dev>

chatgpt-codex-connector Bot reviewed Jul 7, 2026

View reviewed changes

Uh oh!

Conversation

jakubno commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

linear-code Bot commented Jul 3, 2026

Uh oh!

codecov Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot commented Jul 3, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

arkamar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

arkamar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jakubno commented Jul 3, 2026 •

edited

Loading

cursor Bot commented Jul 3, 2026 •

edited

Loading

codecov Bot commented Jul 3, 2026 •

edited

Loading