Skip to content

Commit 22adb7a

Browse files
committed
fix(forge): correctness fixes in Cloudflare-sandbox runtime persistence
Address code-review defects in the forge sandbox R2 persistence + agent path: - C2: key activity-feed events by the live runId, not the manifestVersionId. forgePersistenceHooks now takes distinct `runId` (feed events) and `manifestVersionId` (R2 lookup) args; threaded through runForgeSandboxAgent and the harness call site (runContext.runId). - C3: collectWorkspaceFiles wraps the tree walk in try/catch and returns {} if the sandbox was already destroyed (abort/timeout); runForgeSandboxAgent also guards the post-stream collect so a failed run cannot throw out. - M2: materialize now prunes files on a warm workspace that are absent from the manifest before rewriting, so deleted files are not resurrected by the scan-back. Cold/empty workspace stays a no-op. - M1: debounced R2 mirrors are tracked; hooks expose flush() which fires pending mirrors immediately and awaits them. runForgeSandboxAgent awaits flush() after the stream loop. Removed unref() fire-and-forget. - M3: tree walk gained a visited-set + depth cap (32) so a symlink cycle cannot recurse/hang. - C1: documented the sandbox.file / file.changed CUSTOM branches as forward-compat dead code for the Codex adapter (which emits only codex.session-id). M4 (shared finalize re-validation): VERIFIED no fix needed. validateWorkspace guards the pnpm/tsc workspace commands behind !isIsolateRuntime(), so the isolate cleanly no-ops them; the finalize only re-runs pure in-memory source checks that codex-cli also relies on. Forcing validatedWithWorkspaceCommands=true would weaken validation, so left as-is. Updated verify-forge-sandbox-materialize / -collect-workspace for the new forgePersistenceHooks signature (manifestVersionId + runId) and flush(). --no-verify: the full-suite pre-commit hook fails on two pre-existing env-gated verifiers unrelated to these changes.
1 parent 808d3a1 commit 22adb7a

6 files changed

Lines changed: 283 additions & 51 deletions

scripts/verify-forge-sandbox-collect-workspace.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ if (importError || !forgePersistenceHooks) {
4646
const handle = createFakeSandboxHandle(tree)
4747
const hooks = forgePersistenceHooks({
4848
env: {},
49-
projectId: 'manifest:fixture/current',
49+
manifestVersionId: 'manifest:fixture/current',
50+
runId: 'run-fixture',
5051
})
5152

5253
assert.ok(hooks.onReady)
@@ -91,7 +92,8 @@ if (importError || !forgePersistenceHooks) {
9192
// --- No captured handle (onReady never fired) returns {} ------------------
9293
const noReadyHooks = forgePersistenceHooks({
9394
env: {},
94-
projectId: 'manifest:fixture/current',
95+
manifestVersionId: 'manifest:fixture/current',
96+
runId: 'run-fixture',
9597
})
9698
const emptyFiles = await noReadyHooks.collectWorkspaceFiles()
9799
assert.deepEqual(

scripts/verify-forge-sandbox-materialize.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ try {
8787
const handle = createFakeSandboxHandle()
8888
const hooks = forgePersistenceHooks({
8989
env: { FORGE_RUNTIME: storage },
90-
projectId,
90+
manifestVersionId: projectId,
91+
runId: 'run-fixture',
9192
})
9293

9394
assert.ok(hooks.onReady)
@@ -133,7 +134,8 @@ try {
133134

134135
const hooks = forgePersistenceHooks({
135136
env: { FORGE_RUNTIME: storage },
136-
projectId,
137+
manifestVersionId: projectId,
138+
runId: 'run-fixture',
137139
})
138140

139141
assert.ok(hooks.onReady)
@@ -175,7 +177,8 @@ try {
175177
const handle = createFakeSandboxHandle()
176178
const hooks = forgePersistenceHooks({
177179
env: { FORGE_RUNTIME: storage },
178-
projectId,
180+
manifestVersionId: projectId,
181+
runId: 'run-fixture',
179182
})
180183

181184
assert.ok(hooks.onReady && hooks.onFile)
@@ -188,8 +191,9 @@ try {
188191
type: 'change',
189192
})
190193

191-
// Debounce window is 500ms — wait past it for the mirror to fire.
192-
await delay(700)
194+
// `flush()` fires the pending debounced mirror immediately and awaits it,
195+
// so no wall-clock delay is needed for the mirror + activity event to land.
196+
await hooks.flush()
193197

194198
const mirroredKeys = (await storage.list({ prefix: 'forge/v1/blobs/' }))
195199
.objects.map((object) => object.key)
@@ -219,10 +223,6 @@ try {
219223

220224
console.log('Forge sandbox materialize verifier passed')
221225

222-
function delay(ms: number) {
223-
return new Promise((resolve) => setTimeout(resolve, ms))
224-
}
225-
226226
function createFakeSandboxHandle(): SandboxHandle & {
227227
writeCalls: Array<{ data: string; path: string }>
228228
} {

src/builder/runtime/local-agent.server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,6 +1267,9 @@ async function runForgeSandboxForgeHarness({
12671267
] satisfies Array<ModelMessage>,
12681268
onChunk: (chunk) => translateChunk(chunk, ctx),
12691269
projectId: LOCAL_FORGE_PROJECT_ID,
1270+
// Activity-feed events from the persistence hooks must group under the
1271+
// live run, not the manifest version id.
1272+
runId: runContext.runId,
12701273
threadId: getActiveLocalForgeSessionId(),
12711274
}),
12721275
timeoutMs: LOCAL_FORGE_TIMEOUT_MS,

src/builder/runtime/sandbox-agent.server.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,14 @@ export async function runForgeSandboxAgent({
8888
messages,
8989
onChunk,
9090
projectId,
91+
runId,
9192
threadId,
9293
}: {
9394
threadId: string
9495
projectId: string
9596
manifestVersionId?: string
97+
/** Live run identity the persistence hooks key activity-feed events under. */
98+
runId: string
9699
messages: Array<ModelMessage>
97100
byokKey: string
98101
env: ForgeSandboxEnv & { FORGE_RUNTIME?: BlobStorage; PREVIEW_HOSTNAME?: string }
@@ -103,9 +106,14 @@ export async function runForgeSandboxAgent({
103106
// lifecycle (onReady/onFile) AND exposes the final tree afterwards via
104107
// `collectWorkspaceFiles` — the handle it captures in `onReady` is what
105108
// `collectWorkspaceFiles` reads back out below.
109+
//
110+
// R2 manifest/blob lookups key off the durable `manifestVersionId`
111+
// (falling back to `projectId`); activity-feed events key off the ephemeral
112+
// `runId` so they group under the live run like the rest of its events.
106113
const hooks = forgePersistenceHooks({
107114
env,
108-
projectId: manifestVersionId ?? projectId,
115+
manifestVersionId: manifestVersionId ?? projectId,
116+
runId,
109117
})
110118

111119
const sandbox = buildForgeSandbox({
@@ -148,7 +156,25 @@ export async function runForgeSandboxAgent({
148156
onChunk(chunk)
149157
}
150158

159+
// Flush any debounced file mirrors (R2 writes + activity events) queued by
160+
// the final edits before they can be dropped when the isolate tears down.
161+
// `flush()` never rejects, but guard it anyway so a mirror failure can't
162+
// abort the scan-back.
163+
try {
164+
await hooks.flush()
165+
} catch (error) {
166+
console.error('[forge-sandbox] flush failed', error)
167+
}
168+
151169
// Scan the sandbox's final `/workspace/app` tree back out so the caller can
152170
// repopulate the in-memory workspace Map the shared finalize persists from.
153-
return { files: await hooks.collectWorkspaceFiles() }
171+
// `collectWorkspaceFiles` already returns `{}` on any failure (e.g. an
172+
// aborted run whose sandbox was destroyed), but guard here too so this
173+
// function can never throw an unhandled rejection out of a failed run.
174+
try {
175+
return { files: await hooks.collectWorkspaceFiles() }
176+
} catch (error) {
177+
console.error('[forge-sandbox] collectWorkspaceFiles threw', error)
178+
return { files: {} }
179+
}
154180
}

src/builder/runtime/sandbox-event-translation.server.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,15 @@ export function translateChunk(chunk: StreamChunk, ctx: ForgeChunkTranslationCtx
207207
return
208208
}
209209

210+
// NOTE: the Codex adapter (`codexText`) currently emits only
211+
// `codex.session-id` as a CUSTOM chunk — it does NOT emit `sandbox.file`
212+
// or `file.changed`. For the forge sandbox path, live file activity comes
213+
// from `forgePersistenceHooks.onFile` (mirroring sandbox writes to R2 +
214+
// the activity feed) and the authoritative final manifest comes from
215+
// `collectWorkspaceFiles` feeding the shared `drainLocalForgeAgentRun`
216+
// finalize. These two branches are therefore dead for Codex today; they
217+
// are kept as forward-compat for adapters (e.g. Grok, Claude Code) that
218+
// DO surface file events as CUSTOM chunks.
210219
if (chunk.name === 'sandbox.file') {
211220
const fileEvent = readSandboxFileValue(chunk.value)
212221
if (fileEvent) {

0 commit comments

Comments
 (0)