Skip to content

fix: resolve --file absolute paths in clerk env pull#270

Merged
rafa-thayto merged 2 commits into
mainfrom
dull-shingle
May 11, 2026
Merged

fix: resolve --file absolute paths in clerk env pull#270
rafa-thayto merged 2 commits into
mainfrom
dull-shingle

Conversation

@rafa-thayto
Copy link
Copy Markdown
Contributor

@rafa-thayto rafa-thayto commented May 11, 2026

Summary

  • Fixes clerk env pull --file <path> prints success but writes no file (exits 1) when framework cannot be detected #269. clerk env pull --file <absolute path> no longer silently writes under the project cwd; it writes exactly where the user asked.
  • Root cause: resolveTargetFile called path.join(cwd, flag), which doesn't treat an absolute second arg as a reset. --file /Users/u/clerk-dev.env from /Users/u/Developer/some-dir produced /Users/u/Developer/some-dir/Users/u/clerk-dev.env. Bun.write auto-creates parent dirs, so the wrong write succeeded and the success log (which printed options.file verbatim) was misleading. Swapped to path.resolve — absolute paths are honored, relative paths still resolve against cwd.

Test plan

  • Added pull.test.ts > "writes --file to an absolute path outside cwd" — file lands at the absolute path, not nested under cwd; success log shows the resolved path.
  • Added pull.test.ts > "writes --file with no package.json present (framework detection fails)" — reproduces the exact scenario from the issue (non-framework dir + absolute --file) and verifies keys land at the requested path.
  • Existing 26 env-pull tests still green (28/28 pass).
  • bun run format / bun run lint / bun run typecheck / bun run test all pass locally (97/97).
  • Reviewer to spot-check clerk env pull --file /tmp/foo.env from a fresh dir against the prebuilt binary once CI publishes the preview build.

`resolveTargetFile` used `path.join(cwd, flag)`, which doesn't reset on
an absolute argument. `--file /Users/u/clerk-dev.env` from a project
cwd silently landed at `<cwd>/Users/u/clerk-dev.env` while the success
message still claimed it was written at the user's path. Swap to
`path.resolve` so absolute paths are honored and relative paths still
resolve against cwd.

Fixes #269
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 11, 2026

🦋 Changeset detected

Latest commit: 7578b86

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
clerk Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Review Change Stack

Warning

Rate limit exceeded

@rafa-thayto has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 44 minutes and 29 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c863c98-5532-43b1-86ba-63f6b66cf56a

📥 Commits

Reviewing files that changed from the base of the PR and between 5a5b2ea and 7578b86.

📒 Files selected for processing (2)
  • packages/cli-core/src/commands/env/pull.test.ts
  • packages/cli-core/src/commands/env/pull.ts
📝 Walkthrough

Walkthrough

This PR fixes a critical bug in clerk env pull --file <path> where absolute file paths were being incorrectly nested under the current working directory instead of being written to their specified locations. The fix changes path resolution from path.join(cwd, flag) to path.resolve(cwd, flag), which correctly preserves absolute paths while still anchoring relative paths to the working directory. Display path formatting is simplified to use basename(). Test coverage is added for absolute path handling both with and without package.json present. Documentation is updated to clarify the expected behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing --file absolute path resolution in clerk env pull command.
Linked Issues check ✅ Passed The PR fully addresses issue #269 by switching from path.join() to path.resolve() to honor absolute paths, adds comprehensive tests, and verifies all existing tests pass.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the absolute file path resolution bug and documenting the fix; no unrelated modifications are present.
Description check ✅ Passed The pull request description clearly explains the bug fix, root cause, implementation details, and test coverage related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rafa-thayto
Copy link
Copy Markdown
Contributor Author

!snapshot

Two cleanup passes on the env-pull test file:

- `pull.test.ts:397` checked `Bun.file(join(tempDir, absoluteTarget)).exists()`
  to assert "nothing was written inside cwd". But `path.join` with an absolute
  second arg concatenates rather than nests (the exact bug the source fix
  addresses), so the assertion was vacuous. Replaced with a check that the
  default fallback `.env.local` was not created inside cwd — a meaningful
  invariant that --file fully overrides auto-detection.
- Renamed the second test to describe what it actually asserts (absolute
  path with no package.json) instead of "framework detection fails", and
  dropped the unnecessary `app`/`instance` flags so it differs from T1 only
  in the missing package.json.
- Trimmed two paraphrase comments in the same test.
- Shortened the WHY comment in `resolveTargetFile` to one line.
@github-actions
Copy link
Copy Markdown
Contributor

Snapshot published

npm install -g clerk@1.2.1-snapshot.5a5b2ea
Package Version
clerk 1.2.1-snapshot.5a5b2ea

Published from 5a5b2ea

@rafa-thayto rafa-thayto merged commit 187384d into main May 11, 2026
10 checks passed
@rafa-thayto rafa-thayto deleted the dull-shingle branch May 11, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clerk env pull --file <path> prints success but writes no file (exits 1) when framework cannot be detected

2 participants