Skip to content

Commit 71fc525

Browse files
authored
fix(sandbox): scope user-driven SANDBOX_START to the locked provider kind (#4478)
Auto-start and self-heal already passed sandboxProviderKind to SANDBOX_START so provisioning stays pinned to the thread's locked provider (#4025), but the user-driven start() — which retry() and resume() also call — built its args without it. Retrying a failed locked-provider sandbox (or resuming a suspended one) could get re-provisioned on a different provider than the one the preview is locked to, the exact 'different-provider sibling' the surrounding code says should never happen. Extract the shared arg-builder (buildSandboxStartArgs) so all three call sites stay in sync, and add a regression test.
1 parent 06e19fd commit 71fc525

2 files changed

Lines changed: 51 additions & 8 deletions

File tree

apps/mesh/src/web/components/sandbox/hooks/sandbox-lifecycle-context.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
shouldAutoStart,
55
shouldSelfHeal,
66
computeDrawerStatus,
7+
buildSandboxStartArgs,
78
type BranchMapEntryLike,
89
} from "./sandbox-lifecycle-context";
910

@@ -130,6 +131,26 @@ describe("shouldSelfHeal", () => {
130131
});
131132
});
132133

134+
describe("buildSandboxStartArgs", () => {
135+
test("includes sandboxProviderKind when the thread has a locked kind", () => {
136+
// Regression: user-driven start/retry/resume used to omit
137+
// sandboxProviderKind (unlike auto-start/self-heal), so retrying a
138+
// failed locked-provider thread could get provisioned on the wrong
139+
// provider instead of the one the preview is locked to.
140+
expect(buildSandboxStartArgs("vmcp1", "main", "user-desktop")).toEqual({
141+
virtualMcpId: "vmcp1",
142+
branch: "main",
143+
sandboxProviderKind: "user-desktop",
144+
});
145+
});
146+
147+
test("omits branch and sandboxProviderKind when absent", () => {
148+
expect(buildSandboxStartArgs("vmcp1", null, null)).toEqual({
149+
virtualMcpId: "vmcp1",
150+
});
151+
});
152+
});
153+
133154
describe("computeDrawerStatus", () => {
134155
test("suspended → suspended", () => {
135156
expect(computeDrawerStatus({ kind: "suspended" })).toBe("suspended");

apps/mesh/src/web/components/sandbox/hooks/sandbox-lifecycle-context.tsx

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,21 @@ export function shouldSelfHeal(args: ShouldSelfHealArgs): boolean {
6868
);
6969
}
7070

71+
/** Shared SANDBOX_START arg-builder so every call site (auto-start,
72+
* self-heal, user-driven start/retry/resume) scopes provisioning to the
73+
* locked provider kind identically — a caller that omits it can get
74+
* provisioned on the wrong provider. */
75+
export function buildSandboxStartArgs(
76+
virtualMcpId: string,
77+
branch: string | null,
78+
sandboxProviderKind: SandboxProviderKind | null,
79+
): SandboxStartArgs {
80+
const args: SandboxStartArgs = { virtualMcpId };
81+
if (branch) args.branch = branch;
82+
if (sandboxProviderKind) args.sandboxProviderKind = sandboxProviderKind;
83+
return args;
84+
}
85+
7186
export function computeDrawerStatus(state: PreviewState): DrawerStatus {
7287
switch (state.kind) {
7388
case "suspended":
@@ -239,9 +254,11 @@ export function SandboxLifecycleProvider({
239254
useEffect(() => {
240255
if (!autoStartEligible || !virtualMcpId) return;
241256
autoStartAttemptedForBranchRef.current.add(autoStartDedupKey);
242-
const args: SandboxStartArgs = { virtualMcpId };
243-
if (branch) args.branch = branch;
244-
if (sandboxProviderKind) args.sandboxProviderKind = sandboxProviderKind;
257+
const args = buildSandboxStartArgs(
258+
virtualMcpId,
259+
branch,
260+
sandboxProviderKind,
261+
);
245262
startVmMutate(args, {
246263
onSuccess: (data) => {
247264
if (data?.branch && !branch) setCurrentTaskBranch(data.branch);
@@ -276,9 +293,11 @@ export function SandboxLifecycleProvider({
276293
if (!selfHealEligible || !deadVmId || !virtualMcpId) return;
277294
// oxlint-disable-next-line ban-ref-current-assignment/ban-ref-current-assignment -- record dead handle to dedup repeat 404s
278295
reprovisionedForVmIdRef.current = deadVmId;
279-
const args: SandboxStartArgs = { virtualMcpId };
280-
if (branch) args.branch = branch;
281-
if (sandboxProviderKind) args.sandboxProviderKind = sandboxProviderKind;
296+
const args = buildSandboxStartArgs(
297+
virtualMcpId,
298+
branch,
299+
sandboxProviderKind,
300+
);
282301
startVmMutate(args, {
283302
onSuccess: (data) => {
284303
if (data?.branch && !branch) setCurrentTaskBranch(data.branch);
@@ -300,8 +319,11 @@ export function SandboxLifecycleProvider({
300319
// User-driven actions.
301320
const start = () => {
302321
if (!virtualMcpId) return;
303-
const args: SandboxStartArgs = { virtualMcpId };
304-
if (branch) args.branch = branch;
322+
const args = buildSandboxStartArgs(
323+
virtualMcpId,
324+
branch,
325+
sandboxProviderKind,
326+
);
305327
startVmMutate(args, {
306328
onSuccess: (data) => {
307329
if (data?.branch && !branch) setCurrentTaskBranch(data.branch);

0 commit comments

Comments
 (0)