fix: make swift helper cache builds concurrent-safe#579
Conversation
|
23656b5 to
5793795
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23656b5c4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (Date.now() >= deadline) { | ||
| throw error; |
There was a problem hiding this comment.
Recover from stale cache locks before timing out
If a process holding this directory lock exits unexpectedly (e.g. SIGKILL, crash, machine restart), the .lock directory is never removed because cleanup only happens in ensureSwiftExecutable’s finally. Subsequent compiles for the same cache key will spin until timeoutMs and then rethrow EEXIST, effectively wedging that helper cache entry until someone manually deletes the lock directory. Please add stale-lock recovery (PID/mtime check or lock TTL) before throwing on timeout.
Useful? React with 👍 / 👎.
2b4a377 to
40fd2f6
Compare
40fd2f6 to
ad49824
Compare
Summary
Make Swift helper executable cache publication safe when multiple
agent-deviceprocesses compile the same cache key concurrently. Helper builds now coordinate through a cache-key lock, compile into a unique temporary executable, and let losing builders reuse the completed cache entry.Adds focused coverage for temp executable publishing and concurrent file-backed helper compiles.
Closes #570
Touched files: 2.
Validation
Verified with the focused Swift cache, recording overlay, and Swift recording-script suites; all passed. Also ran the repo quick check and formatter.
pnpm exec vitest run src/utils/__tests__/swift-cache.test.ts src/recording/__tests__/overlay.test.ts src/platforms/ios/__tests__/recording-scripts.test.tspnpm check:quickpnpm formatDocs/skills were not updated because this is an internal cache race fix with no CLI or workflow surface change.