Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This small change only canonicalizes a temporary test path so the symlinked-entrypoint test works on macOS. Production code, product defaults, and static-analysis configuration are unchanged. Notes:
You can add or adjust custom eligibility rules. Learn more. |
Mnigos
left a comment
There was a problem hiding this comment.
Can confirm on macOS: the entrypoint symlink test fails on main exactly as #9389 describes and passes with this change. Same /var to /private/var class I just ran into on the server side, resolving before asserting is the right call.
|
Note: GPT-6 on behalf of shivam (@shivamhwp). This branch conflicts with the newer symlink-support guard in main. Please rebase the realpath correction while keeping |
…ypoint The symlinked-entrypoint case builds its expected module URL from the raw temp path, but isEntrypoint compares against the realpath of the entry. On macOS the temp dir lives under /var, which is itself a symlink to /private/var, so the two strings never match and the test fails on every run. Linux CI is unaffected. Canonicalize the fixture path with realpathSync so the test only exercises the npm/npx symlink it is meant to cover. Fixes pingdotgg#9389
f629824 to
c962d18
Compare
|
Rebased onto main. The |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe symlinked-entrypoint test now resolves the real module path before creating ChangesEntrypoint symlink test
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~3 minutes Change: Other · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The test-only change resolves the macOS path mismatch while preserving Windows skipping behavior for unsupported symlink environments. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What Changed
apps/server/src/entrypoint.test.ts, "matches through a symlinked entrypoint": the expected module URL is now built fromrealpathSync(real)instead of the raw temp path. One line plus a comment.Why
Fixes #9389.
isEntrypointcompares the module URL against the realpath of the entry, which is what makes npm/npx-style symlinked entrypoints match. The fixture built its expected URL from the un-resolved temp path. On macOS the temp dir lives under/var, itself a symlink to/private/var, so the two strings never matched and the test failed on every run. Linux CI is unaffected, which is why this only shows up locally on macOS.The product code is correct. Canonicalizing the fixture path keeps the test focused on the symlink it is meant to exercise.
Before, on macOS:
After:
Server typecheck and lint are clean.
Checklist
Model and harness: Claude Fable 5.1 in Claude Code.
Note
Low Risk
Test-only change; entrypoint detection logic is untouched.
Overview
Fixes a macOS-only failure in
entrypoint.test.tsfor the symlinked npm/npx entrypoint case (#9389).The test now builds the expected
moduleUrlfromNodeFS.realpathSync(real)instead of the raw temp path, matching howisEntrypointcompares against the realpath ofentryPath. On macOS, temp dirs under/var(symlinked to/private/var) made the old expected string never match even though production behavior is correct.Adds a short comment explaining Node URL resolution and the
/varsymlink. No production code changes.Reviewed by Cursor Bugbot for commit f629824. Configure here.
Note
Resolve temp dir real path in
isEntrypointsymlinked-entrypoint testUpdates the test fixture's module URL in entrypoint.test.ts to use the filesystem-resolved real path of the target file. Node resolves module URLs to real paths, and macOS temp directories can be symlinked, which caused the previous assertion to fail.
Macroscope summarized f629824.
Summary by CodeRabbit