fix: create the cache dir before symlinking the re-warm project at it - #64
Conversation
The re-warm broke the weekly run on its first outing, on pnpm 11: the cache directory is deleted two rows before update, and a manager whose `node_modules` row is a registry-free no-op — pnpm restores the tree from the lockfile copy inside it — never recreates it. The re-warm's `cache` symlink then points at nothing, and the warm-up install dies with ENOTDIR trying to mkdir its store through a dangling link. The smoke test missed it because pnpm 12 reaches the cache through absolute environment paths rather than the symlink, and Bun's re-downloading `node_modules` row recreates the directory — pnpm 11 is the manager that goes through the symlink AND skips the re-download. The directory is now created (idempotently) before the symlink, and the pnpm 11 chain runs clean end to end: its update lands at 1.8s against the 7.5s the cold-cache artifact used to charge it. Also: when every measuring job fails, the report job's samples directory never comes into existence, and merge-results died with a raw ENOENT scandir instead of its own explanation. A missing directory is the same situation as an empty one and now gets the same error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 11 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoPrevent dangling re-warm cache links and clarify missing results
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
The re-warm from #63 broke the first run after merge, on pnpm 11:
Why pnpm 11, and why the smoke test missed it
The cache directory is deleted two rows before
update, and a manager whosenode_modulesrow is a registry-free no-op — pnpm restores the tree from the lockfile copy insidenode_modules/.pnpm— never recreates it. The re-warm'scachesymlink then points at nothing, and the warm-up install dies trying to mkdircache/storethrough a dangling link.The pre-merge smoke test ran pnpm 12 and Bun: pnpm 12 reaches the cache through absolute environment paths (
PNPM_HOME,PNPM_CONFIG_CACHE_DIR), never touching the symlink, and Bun's re-downloadingnode_modulesrow recreates the directory as a side effect. pnpm 11 is the one manager that both goes through the symlink and skips the re-download — exactly the intersection the test didn't cover.Fix
Create
cwd/cache(idempotently) before symlinking at it. Verified by running pnpm 11 through the full real scenario chain against a local pnpr behind the emulated link: it completes, and its update lands at 1.8s against the 7.5s the cold-cache artifact used to charge it — the pnpm 11 column benefits from #63 just as pnpm 12 does.Also included: when every measuring job fails, the report job's samples directory never comes into existence, and
merge-resultsdied with a rawENOENTscandir instead of its own explanation. A missing directory is the same situation as an empty one and now produces the same "every measuring job must have failed" error.🤖 Generated with Claude Code