fix(server): stop tying codex text-generation temp files to the caller's scope - #5406
Conversation
…r's scope
runCodexJson wrote its schema and output temp files with
makeTempFileScoped, leaking a Scope requirement to the ambient fiber
context. When generation runs from a background fiber whose launching
request scope has already closed — the v2 worktree branch rename fork —
the finalizer registered on the closed scope fires immediately, reaping
the temp directory between creation and write. Every branch name
generation then failed with ENOENT ("Failed to write temp file") and
threads kept their temporary t3code/<hash> branches, while title
generation kept working because the durable effect worker's scope stays
live.
Use unscoped makeTempFile and remove the per-file temp directories in
runCodexJson's existing explicit cleanup instead, so generation is
self-contained no matter which fiber calls it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…rtway Review follow-up: with the temp files no longer scope-managed, a failure between allocations leaked what was already created — the schema temp directory survived if its own write failed or if the output allocation failed, since Effect.ensuring(cleanup) is only installed after both exist. Each allocation now removes its own directory on failure, and the output allocation removes the schema directory on the way out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ApprovabilityVerdict: Approved 0a5b8c1 Straightforward bug fix changing temp file management from scope-based to manual cleanup, addressing an issue where background fibers outlive their launching scope. The author is the primary contributor to this code, and the fix includes a test case. You can customize Macroscope's approvability policy. Learn more. |
…r's scope (#5406) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Stacked on #2829. Follow-up to #5309 — explains and fixes why branch renaming still didn't work in real builds even though #5309's rename path executed.
Diagnosis (from a live Alpha build containing #5176 + #5309)
Titles generated fine, branches stayed
t3code/<hash>. Traces showed the rename fork running and failing in 22ms:runCodexJsonwrites its--output-schema/--output-last-messagetemp files withmakeTempFileScoped, whoseScoperequirement leaks to the ambient fiber context (swallowed by the genericS["DecodingServices"]in the return annotation, so it never typechecked as missing). The branch rename fork inherits its context from thelaunchThreadRPC request, whose scope has long since closed by the time the fork runs — and registering a finalizer on a closed scope runs it immediately, deleting the temp directory between creation and write. Hence ENOENT on every branch generation, with #5309's deliberate fallback keeping the temp name.Title generation kept working because #5176 runs it on the durable effect worker, whose ambient scope stays live — which is exactly why the symptom split (titles ✓ / branches ✗).
Fix
Use unscoped
makeTempFileand remove the per-file temp directories inrunCodexJson's existing explicitEffect.ensuring(cleanup)instead. Generation is now self-contained regardless of which fiber calls it, and the temp directories (previously left behind by the file-only unlink whenever the scope path wasn't exercised) are cleaned up too.Testing
generateBranchNamesucceeds with an explicitly closed ambientScopeprovided — fails with the ENOENT error before the fix, passes after.CodexTextGenerationsuite: 18 passed.t3.🤖 Generated with Claude Code
Note
Medium Risk
Changes temp-file lifecycle for all Codex JSON generation paths; behavior is intentional but touches shared CLI orchestration used by commits, PRs, branches, and titles.
Overview
Fixes background
generateBranchNamefailures (ENOENT on temp files) when Codex CLI schema/output temps were created viamakeTempFileScoped, so a closed ambientScopefrom the launching RPC could reap the directory before the write finished.writeTempFilenow uses unscopedmakeTempFilewith explicit cleanup:removeTempFileDiron write failure, andrunCodexJson’s existingEffect.ensuringremoves the full temp directories (not single-file unlinks) on success or failure.Adds a regression test that
generateBranchNamesucceeds whenScope.Scopeis already closed.Reviewed by Cursor Bugbot for commit 0a5b8c1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix codex text-generation temp files to be independent of the caller's scope
writeTempFileinCodexTextGeneration.tsswitches frommakeTempFileScopedtomakeTempFile, removing the dependency on an ambientScopebeing open.removeTempFileDirhelper cleans up the entire temp directory (recursively) on both success and error paths, replacing per-file unlinks that could leave directories behind.generateBranchNameworks correctly even when the ambient scope is already closed.Macroscope summarized 0a5b8c1.