Skip to content

Commit c98e3c7

Browse files
authored
fix(sandbox): judge session lease coverage from the request's own clock reading (#8665)
ensureSessionSandbox subtracted elapsed time from the lease using one Date.now() read, and the handle's outlives() added it back using a second read. When the millisecond ticked between the two and the reconnect granted the lease in the same millisecond the call captured, coverage fell 1ms short and an unneeded E2B setTimeout was issued. outlives() now takes the reference time, so acquisition measures from requestedAtMs with no second read. The session lease test froze nothing and slept on a real timer, so it only failed when the tick landed between the two reads. It now controls Date.now() to put the tick exactly there.
1 parent 9af89fc commit c98e3c7

6 files changed

Lines changed: 32 additions & 21 deletions

File tree

‎apps/sim/lib/execution/remote-sandbox/e2b-session.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ describe('E2B session lease', () => {
644644
const plane = controlPlane(5 * 60_000)
645645
const sandbox = await e2bProvider.findSessionSandbox?.('chat', { lifetimeMs: LEASE_MS })
646646
expect(plane.endAtMs).toBeGreaterThanOrEqual(Date.now() + LEASE_MS - 1000)
647-
expect(sandbox?.outlives?.(IDLE_MS)).toBe(true)
647+
expect(sandbox?.outlives?.(IDLE_MS, Date.now())).toBe(true)
648648
await sandbox?.extendLifetime?.(IDLE_MS)
649649
expect(plane.endAtMs).toBeGreaterThanOrEqual(Date.now() + IDLE_MS)
650650
expect(plane.requests).toBe(1)
@@ -656,7 +656,7 @@ describe('E2B session lease', () => {
656656
sessionKey: 'chat',
657657
lifetimeMs: LEASE_MS,
658658
})
659-
expect(sandbox.outlives?.(IDLE_MS)).toBe(true)
659+
expect(sandbox.outlives?.(IDLE_MS, Date.now())).toBe(true)
660660
await sandbox.extendLifetime?.(IDLE_MS)
661661
expect(plane.endAtMs).toBeGreaterThanOrEqual(Date.now() + LEASE_MS - 1000)
662662
expect(plane.requests).toBe(1)
@@ -673,7 +673,7 @@ describe('E2B session lease', () => {
673673
it('reads back and extends a lease it did not grant itself', async () => {
674674
const plane = controlPlane(5 * 60_000)
675675
const sandbox = await e2bProvider.findSessionSandbox?.('chat', {})
676-
expect(sandbox?.outlives?.(IDLE_MS)).toBe(false)
676+
expect(sandbox?.outlives?.(IDLE_MS, Date.now())).toBe(false)
677677
await sandbox?.extendLifetime?.(LEASE_MS)
678678
expect(plane.endAtMs).toBeGreaterThanOrEqual(Date.now() + LEASE_MS - 1000)
679679
expect(plane.requests).toBe(3)

‎apps/sim/lib/execution/remote-sandbox/e2b.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,17 +376,17 @@ class E2BSandboxHandle implements SandboxHandle {
376376
return this.sandbox.sandboxId
377377
}
378378

379-
outlives(lifetimeMs: number): boolean {
379+
outlives(lifetimeMs: number, fromMs: number): boolean {
380380
return (
381381
this.sessionDeadlineAtMs !== undefined &&
382-
this.sessionDeadlineAtMs >= Date.now() + e2bTimeoutMs(lifetimeMs)
382+
this.sessionDeadlineAtMs >= fromMs + e2bTimeoutMs(lifetimeMs)
383383
)
384384
}
385385

386386
async extendLifetime(lifetimeMs: number): Promise<void> {
387387
const timeoutMs = e2bTimeoutMs(lifetimeMs)
388388
if (this.sessionKey !== undefined) {
389-
if (this.outlives(lifetimeMs)) return
389+
if (this.outlives(lifetimeMs, Date.now())) return
390390
/** Session callers serialize updates so a short job cannot shorten another job's lease. */
391391
const info = await this.sandbox.getInfo()
392392
if (info.endAt.getTime() >= Date.now() + timeoutMs) return

‎apps/sim/lib/execution/remote-sandbox/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ async function leaseSandbox(
136136
session: status,
137137
release: async () => {
138138
// A deadline that already covers the idle window needs no serialized update.
139-
if (created.sandbox.outlives?.(SESSION_SANDBOX_IDLE_MS)) return
139+
if (created.sandbox.outlives?.(SESSION_SANDBOX_IDLE_MS, Date.now())) return
140140
// Cleanup failure cannot relabel a completed mutation as a failed execution.
141141
try {
142142
await withSandboxSessionLock(session.key, AbortSignal.timeout(30_000), async () => {

‎apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -772,26 +772,36 @@ describe('session sandbox lease', () => {
772772
})
773773

774774
it('trusts a lease granted by a slow reconnect for the whole call, then refreshes nothing', async () => {
775+
// The provider grants the lease in the same millisecond the call asked for it, then the
776+
// clock advances on every read: coverage judged from two separate readings falls short.
777+
let nowMs = Date.now()
778+
let advancing = false
779+
const clock = vi.spyOn(Date, 'now').mockImplementation(() => (advancing ? ++nowMs : nowMs))
775780
const { handle, calls } = fakeSandbox('sb-covered')
776781
let grantedUntilMs = 0
777-
handle.outlives = (lifetimeMs) => grantedUntilMs >= Date.now() + lifetimeMs
782+
handle.outlives = (lifetimeMs, fromMs) => grantedUntilMs >= fromMs + lifetimeMs
778783
mockFindSessionSandbox.mockImplementation(
779784
async (_key: string, options: { lifetimeMs?: number }) => {
780785
grantedUntilMs = Date.now() + (options.lifetimeMs ?? 0)
781-
await sleep(20)
786+
nowMs += 20
787+
advancing = true
782788
return handle
783789
}
784790
)
785791

786-
const result = await executeInSandbox({
787-
...CODE_REQUEST,
788-
sandboxKind: 'mothership',
789-
session: { key: 'mothership-chat:c3' },
790-
})
792+
try {
793+
const result = await executeInSandbox({
794+
...CODE_REQUEST,
795+
sandboxKind: 'mothership',
796+
session: { key: 'mothership-chat:c3' },
797+
})
791798

792-
expect(result.sandboxSession).toBe('reused')
793-
expect(grantedUntilMs).toBeGreaterThanOrEqual(Date.now() + 20 * 60_000)
794-
expect(calls.extendLifetime).toHaveLength(0)
799+
expect(result.sandboxSession).toBe('reused')
800+
expect(grantedUntilMs).toBeGreaterThanOrEqual(Date.now() + 20 * 60_000)
801+
expect(calls.extendLifetime).toHaveLength(0)
802+
} finally {
803+
clock.mockRestore()
804+
}
795805
})
796806

797807
it('does not rewrite an unchanged executable while earlier code can still use it', async () => {

‎apps/sim/lib/execution/remote-sandbox/session.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ export async function ensureSessionSandbox(args: {
6363
providerId: provider.id,
6464
sandboxId: created.sandbox.sandboxId,
6565
})
66-
// The budget is anchored before acquisition, so a lease granted by the lookup or create covers it.
67-
if (!created.sandbox.outlives?.(Math.max(0, lifetimeMs - (Date.now() - requestedAtMs))))
66+
// Measured from before acquisition: any lease the lookup or create granted starts no earlier.
67+
if (!created.sandbox.outlives?.(lifetimeMs, requestedAtMs))
6868
await created.sandbox.extendLifetime?.(lifetimeMs)
6969
signal.throwIfAborted()
7070
if (session.cli) {

‎apps/sim/lib/execution/remote-sandbox/types.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,10 @@ export interface SandboxHandle {
280280
extendLifetime?(lifetimeMs: number): Promise<void>
281281
/**
282282
* True when a deadline this handle already established keeps the sandbox alive for
283-
* `lifetimeMs` from now, so {@link extendLifetime} would make no provider request.
283+
* `lifetimeMs` counted from `fromMs`, so {@link extendLifetime} would make no provider
284+
* request. The caller supplies the clock reading the window is measured from.
284285
*/
285-
outlives?(lifetimeMs: number): boolean
286+
outlives?(lifetimeMs: number, fromMs: number): boolean
286287
/** Reads provider metadata without materializing the file contents. */
287288
getFileSize(path: string): Promise<number>
288289
readFile(path: string): Promise<string>

0 commit comments

Comments
 (0)