Skip to content

Commit f6587ea

Browse files
committed
fix(sandbox): initialize workbench history before honoring a stop during creation
A chat workbench created while the caller stopped kept running for the next call to reuse, but the abort check ran before its history was initialized. The reused machine then had no history, so it could never be certified and every later code result in that chat was withheld.
1 parent ac604b7 commit f6587ea

2 files changed

Lines changed: 77 additions & 25 deletions

File tree

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

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* The persistent workbench's input history is what lets a scratch file reach the model. This runs
33
* the real code boundary against the real history script in a disposable Redis, so a mount the
44
* caller could not classify has to leave the machine uncertified. Only the sandbox provider is a
5-
* stand-in: it hands back an existing machine that executes nothing.
5+
* stand-in: it hands back a machine that executes nothing.
66
*
77
* Set TEST_REDIS_URL to an isolated local Redis service.
88
*/
@@ -14,14 +14,23 @@ import {
1414
import { generateShortId } from '@sim/utils/id'
1515
import { afterAll, describe, expect, it, vi } from 'vitest'
1616

17-
const { redisUrl, inheritedRedisUrl, mockFindSessionSandbox } = await vi.hoisted(async () => {
18-
const { readTestRedisUrl } = await import('@sim/db/testing/test-infrastructure')
19-
const url = readTestRedisUrl()
20-
const inheritedRedisUrl = process.env.REDIS_URL
21-
/** The real Redis module reads this at import. */
22-
if (url) process.env.REDIS_URL = url
23-
return { redisUrl: url, inheritedRedisUrl, mockFindSessionSandbox: vi.fn() }
24-
})
17+
const { redisUrl, inheritedRedisUrl, mockCreate, mockFindSessionSandbox } = await vi.hoisted(
18+
async () => {
19+
const { readTestRedisUrl } = await import('@sim/db/testing/test-infrastructure')
20+
const url = readTestRedisUrl()
21+
const inheritedRedisUrl = process.env.REDIS_URL
22+
/** The real Redis module reads this at import. */
23+
if (url) process.env.REDIS_URL = url
24+
return {
25+
redisUrl: url,
26+
inheritedRedisUrl,
27+
mockCreate: vi.fn<SandboxProvider['create']>(async () => {
28+
throw new Error('The certification fixture only reuses an existing machine')
29+
}),
30+
mockFindSessionSandbox: vi.fn<NonNullable<SandboxProvider['findSessionSandbox']>>(),
31+
}
32+
}
33+
)
2534

2635
vi.mock('@/lib/execution/remote-sandbox/provider', () => remoteSandboxProviderMock)
2736
vi.mock('@/lib/execution/remote-sandbox/resolve', () => ({
@@ -51,9 +60,7 @@ remoteSandboxProviderMockFns.mockResolveProvider.mockImplementation(
5160
id: 'e2b',
5261
dependencyStrategy: 'prebuilt',
5362
resolveLifetimeMs: (ms: number) => ms,
54-
create: async () => {
55-
throw new Error('The certification fixture only reuses an existing machine')
56-
},
63+
create: mockCreate,
5764
findSessionSandbox: mockFindSessionSandbox,
5865
}) satisfies SandboxProvider
5966
)
@@ -77,6 +84,22 @@ function machine(sandboxId: string): SandboxHandle {
7784
/** Machine-history keys this suite created, so cleanup never touches another suite's state. */
7885
const createdKeys: string[] = []
7986

87+
function trackHistoryKey(key: string, sandboxId: string) {
88+
createdKeys.push(
89+
`mothership:workbench-provenance:v2:${createHash('sha256')
90+
.update(JSON.stringify([key, 'e2b', sandboxId]))
91+
.digest('hex')}`
92+
)
93+
}
94+
95+
function run(kind: 'code' | 'shell', request: Parameters<typeof executeInSandbox>[0]) {
96+
return observeSandboxSessionInputs(
97+
() => true,
98+
() =>
99+
kind === 'code' ? executeInSandbox(request) : executeShellInSandbox({ ...request, envs: {} })
100+
)
101+
}
102+
80103
afterAll(async () => {
81104
if (redisUrl && createdKeys.length) {
82105
const redis = getRedisClient()
@@ -99,11 +122,7 @@ describe.skipIf(!redisUrl)('workbench certification at the code boundary', () =>
99122
const sandboxId = `machine-${generateShortId(12)}`
100123
const key = `certification-${generateShortId(12)}`
101124
const identity = { providerId: 'e2b', sandboxId } as const
102-
createdKeys.push(
103-
`mothership:workbench-provenance:v2:${createHash('sha256')
104-
.update(JSON.stringify([key, identity.providerId, sandboxId]))
105-
.digest('hex')}`
106-
)
125+
trackHistoryKey(key, sandboxId)
107126
mockFindSessionSandbox.mockResolvedValue(machine(sandboxId))
108127
await initializeSessionFileProvenance(key, identity)
109128
expect(await isSessionFileProvenanceClean(key, identity)).toBe(true)
@@ -114,15 +133,46 @@ describe.skipIf(!redisUrl)('workbench certification at the code boundary', () =>
114133
timeoutMs: 30_000,
115134
session: { key, ...(unprovenanced ? { unprovenancedInputs: true } : {}) },
116135
}
117-
const execution = observeSandboxSessionInputs(
118-
() => true,
119-
() =>
120-
kind === 'code'
121-
? executeInSandbox(request)
122-
: executeShellInSandbox({ ...request, envs: {} })
123-
)
136+
const execution = run(kind, request)
124137
if (unprovenanced) await expect(execution).rejects.toThrow('Workbench output withheld')
125138
else await expect(execution).resolves.toMatchObject({ sandboxId })
126139
expect(await isSessionFileProvenanceClean(key, identity)).toBe(!unprovenanced)
127140
})
141+
142+
/**
143+
* The provider finishes creating a session machine even when the caller stops mid-create, and
144+
* the next call reuses that machine. Its history must already exist then: a machine reused with
145+
* no history can never be certified, so every later result in the chat would be withheld.
146+
*/
147+
it.each(['code', 'shell'] as const)(
148+
'%s certifies a workbench whose creation outlived a stopped caller',
149+
async (kind) => {
150+
const sandboxId = `machine-${generateShortId(12)}`
151+
const key = `stopped-create-${generateShortId(12)}`
152+
trackHistoryKey(key, sandboxId)
153+
mockCreate.mockClear()
154+
const controller = new AbortController()
155+
mockFindSessionSandbox.mockResolvedValueOnce(null)
156+
mockCreate.mockImplementationOnce(async () => {
157+
controller.abort(new Error('User Stop'))
158+
return machine(sandboxId)
159+
})
160+
const request = {
161+
code: 'print(1)',
162+
language: CodeLanguage.Python,
163+
timeoutMs: 30_000,
164+
session: { key },
165+
}
166+
167+
await expect(run(kind, { ...request, signal: controller.signal })).rejects.toThrow(
168+
'User Stop'
169+
)
170+
expect(mockCreate).toHaveBeenCalledTimes(1)
171+
172+
mockFindSessionSandbox.mockResolvedValue(machine(sandboxId))
173+
await expect(run(kind, request)).resolves.toMatchObject({ sandboxId })
174+
expect(mockCreate).toHaveBeenCalledTimes(1)
175+
expect(await isSessionFileProvenanceClean(key, { providerId: 'e2b', sandboxId })).toBe(true)
176+
}
177+
)
128178
})

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,14 @@ export async function ensureSessionSandbox(args: {
8383
true,
8484
provider
8585
)
86-
signal.throwIfAborted()
86+
// Before the abort check: a stopped caller still leaves this machine for the next call to
87+
// reuse, and a reused machine without history can never be certified.
8788
if (!existing)
8889
await initializeSessionFileProvenance(session.key, {
8990
providerId: provider.id,
9091
sandboxId: created.sandbox.sandboxId,
9192
})
93+
signal.throwIfAborted()
9294
// Measured from before acquisition: any lease the lookup or create granted starts no earlier.
9395
if (!created.sandbox.outlives?.(lifetimeMs, requestedAtMs))
9496
await created.sandbox.extendLifetime?.(lifetimeMs)

0 commit comments

Comments
 (0)