fix: resolve next binary via createRequire and config from cwd#21
fix: resolve next binary via createRequire and config from cwd#21
Conversation
…path When installed via npx/bunx or as a dependency, next gets hoisted and node_modules/.bin/next doesn't exist inside the chronicle package dir. Use createRequire + require.resolve to find next/dist/bin/next via Node's module resolution which handles hoisting automatically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allows placing chronicle.yaml in project root when using chronicle as a library dependency, while still supporting it inside content dir. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates how the Chronicle CLI resolves and invokes the Next.js CLI (ESM-aware), adds port and content directory options to serve/start commands, and centralizes chronicle.yaml resolution into a shared resolver used by CLI and library config loaders. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/chronicle/src/cli/utils/config.ts (1)
19-25: Code duplication withlib/config.ts.This
resolveConfigPathimplementation duplicates the logic inpackages/chronicle/src/lib/config.ts:14-23, with slight semantic differences: the CLI version takescontentDiras a parameter while the lib version reads fromprocess.env.CHRONICLE_CONTENT_DIRdirectly. Consider extracting a shared utility to avoid drift between these two implementations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/cli/utils/config.ts` around lines 19 - 25, The resolveConfigPath implementation in CLI (resolveConfigPath) duplicates logic from lib/config.ts; extract a shared utility (e.g., findChronicleConfig) into a common module that accepts an optional contentDir parameter (and falls back to process.env.CHRONICLE_CONTENT_DIR when undefined) and replace both resolveConfigPath and the code in packages/chronicle/src/lib/config.ts to call this shared function so behavior is centralized and avoids drift.packages/chronicle/src/cli/commands/dev.ts (1)
5-11: Consider extracting shared Next.js CLI resolution.The
createRequire+require.resolve('next/dist/bin/next')pattern is duplicated acrossdev.ts,build.ts,start.ts, andserve.ts. Consider extracting this to a shared utility (e.g., in@/cli/utils) to reduce duplication and centralize the resolution logic.♻️ Example shared utility
// In `@/cli/utils/next.ts` import { createRequire } from 'module' const require = createRequire(import.meta.url) export const nextCli = require.resolve('next/dist/bin/next')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/cli/commands/dev.ts` around lines 5 - 11, The Next.js CLI resolution (createRequire + require.resolve('next/dist/bin/next')) is duplicated in dev.ts (and also in build.ts, start.ts, serve.ts); extract it into a shared utility (e.g., add a new export nextCli in "@/cli/utils/next" or "@/cli/utils") that performs createRequire(import.meta.url) and resolves 'next/dist/bin/next', then import and use that exported nextCli symbol in dev.ts, build.ts, start.ts and serve.ts to remove duplication and centralize resolution logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chronicle/src/cli/utils/config.ts`:
- Around line 30-31: The error message built when the configPath check fails
currently uses comma-separated console.log arguments which yields awkward
spacing; update the code inside the configPath falsy branch (the if
(!configPath) block) to build a single formatted string — e.g., use a template
literal combining process.cwd() and contentDir — and pass that single string
into chalk.red so the entire message is colored and properly formatted (for
example: console.log(chalk.red(`Error: chronicle.yaml not found in
${process.cwd()} or ${contentDir}`))).
---
Nitpick comments:
In `@packages/chronicle/src/cli/commands/dev.ts`:
- Around line 5-11: The Next.js CLI resolution (createRequire +
require.resolve('next/dist/bin/next')) is duplicated in dev.ts (and also in
build.ts, start.ts, serve.ts); extract it into a shared utility (e.g., add a new
export nextCli in "@/cli/utils/next" or "@/cli/utils") that performs
createRequire(import.meta.url) and resolves 'next/dist/bin/next', then import
and use that exported nextCli symbol in dev.ts, build.ts, start.ts and serve.ts
to remove duplication and centralize resolution logic.
In `@packages/chronicle/src/cli/utils/config.ts`:
- Around line 19-25: The resolveConfigPath implementation in CLI
(resolveConfigPath) duplicates logic from lib/config.ts; extract a shared
utility (e.g., findChronicleConfig) into a common module that accepts an
optional contentDir parameter (and falls back to
process.env.CHRONICLE_CONTENT_DIR when undefined) and replace both
resolveConfigPath and the code in packages/chronicle/src/lib/config.ts to call
this shared function so behavior is centralized and avoids drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 69d15e4f-0518-4c4a-9966-8782bed0715d
📒 Files selected for processing (7)
packages/chronicle/bin/chronicle.jspackages/chronicle/src/cli/commands/build.tspackages/chronicle/src/cli/commands/dev.tspackages/chronicle/src/cli/commands/serve.tspackages/chronicle/src/cli/commands/start.tspackages/chronicle/src/cli/utils/config.tspackages/chronicle/src/lib/config.ts
| if (!configPath) { | ||
| console.log(chalk.red('Error: chronicle.yaml not found in'), process.cwd(), 'or', contentDir) |
There was a problem hiding this comment.
Error message formatting produces unintended output.
Using comma-separated arguments in console.log will print values separated by spaces without proper formatting. The message will read awkwardly (e.g., Error: chronicle.yaml not found in /path/cwd or /path/content).
🛠️ Proposed fix using template literal
if (!configPath) {
- console.log(chalk.red('Error: chronicle.yaml not found in'), process.cwd(), 'or', contentDir)
+ console.log(chalk.red(`Error: chronicle.yaml not found in '${process.cwd()}' or '${contentDir}'`))
console.log(chalk.gray(`Run 'chronicle init' to create one`))
process.exit(1)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!configPath) { | |
| console.log(chalk.red('Error: chronicle.yaml not found in'), process.cwd(), 'or', contentDir) | |
| if (!configPath) { | |
| console.log(chalk.red(`Error: chronicle.yaml not found in '${process.cwd()}' or '${contentDir}'`)) | |
| console.log(chalk.gray(`Run 'chronicle init' to create one`)) | |
| process.exit(1) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chronicle/src/cli/utils/config.ts` around lines 30 - 31, The error
message built when the configPath check fails currently uses comma-separated
console.log arguments which yields awkward spacing; update the code inside the
configPath falsy branch (the if (!configPath) block) to build a single formatted
string — e.g., use a template literal combining process.cwd() and contentDir —
and pass that single string into chalk.red so the entire message is colored and
properly formatted (for example: console.log(chalk.red(`Error: chronicle.yaml
not found in ${process.cwd()} or ${contentDir}`))).
Publishes canary npm release on every PR commit with version format 0.1.0-canary.<short-sha> using --tag canary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/canary.yml:
- Around line 3-5: The workflow is triggered on pull_request which exposes repo
secrets only for same-repo PRs, so guard the publish path: modify the publish
job or the specific publish step to run only for same-repo PRs by adding an if
condition that compares the PR head repo to the base/repository (e.g. if:
github.event.pull_request.head.repo.full_name ==
github.event.pull_request.base.repo.full_name) or gate the whole job similarly;
alternatively, keep pull_request but change only the publish step/job to use
that same-repo conditional so forked PRs won’t attempt publishing.
- Around line 29-38: The current canary flow uses SHORT_SHA and sets VERSION
then always runs bun publish, which will fail on reruns for the same commit;
modify the job so that before running bun publish it checks the registry for the
computed VERSION (e.g., with npm view <package>@$VERSION or npm info
<package>@$VERSION) and if that version already exists skip the publish step
(echo/skipped) instead of attempting bun publish, keeping the existing VERSION
and SHORT_SHA logic and only invoking bun publish when the registry check
indicates the version is not present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 905e6000-e347-4126-8bd8-894e3b928992
📒 Files selected for processing (1)
.github/workflows/canary.yml
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub Actions, are repository secrets available to workflows triggered by the pull_request event from forked repositories?
💡 Result:
No—by default, repository secrets are not exposed to workflows triggered by the pull_request event when the PR comes from a fork (the workflow runs with minimal privileges and a read-only token). [1]
For private/internal repositories, an admin can explicitly enable a setting to “Send secrets to workflows from pull requests”, which would make secrets available to forked-PR workflows. [2]
Guard the publish path for forked PRs.
On pull_request, repository secrets are not exposed to workflows coming from forks, so the publish step will fail for external contributors and make the PR look broken. Gate this job or at least the publish step to same-repo PRs only.
Suggested change
jobs:
canary-release:
name: Publish canary to npm
+ if: github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/canary.yml around lines 3 - 5, The workflow is triggered
on pull_request which exposes repo secrets only for same-repo PRs, so guard the
publish path: modify the publish job or the specific publish step to run only
for same-repo PRs by adding an if condition that compares the PR head repo to
the base/repository (e.g. if: github.event.pull_request.head.repo.full_name ==
github.event.pull_request.base.repo.full_name) or gate the whole job similarly;
alternatively, keep pull_request but change only the publish step/job to use
that same-repo conditional so forked PRs won’t attempt publishing.
| - name: Set canary version | ||
| run: | | ||
| SHORT_SHA=$(echo "${{ github.event.pull_request.head.sha }}" | cut -c1-7) | ||
| VERSION=$(jq -r .version package.json)-canary.${SHORT_SHA} | ||
| jq --arg v "$VERSION" '.version = $v' package.json > package.tmp.json | ||
| mv package.tmp.json package.json | ||
| echo "Published version: $VERSION" | ||
|
|
||
| - name: Publish | ||
| run: bun publish --tag canary --access public |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Can npm publish publish the same package version more than once, and what happens if a CI workflow retries publishing an already-existing version?
💡 Result:
No. The npm registry treats the (package name, version) pair as immutable:
npm publishfails if that name+version already exists in the target registry. Once a version is published, that exact name+version can never be used again, even if it was later removed vianpm unpublish. [1]- Attempting to “publish over” an existing version is rejected by the registry (historically shown as HTTP 403 Forbidden). [2]
If a CI workflow retries npm publish for a version that already got published, the retry will not overwrite anything; it will simply error and exit non‑zero (commonly surfaced as “cannot publish over previously published version” / publish conflict). The already-published tarball/version remains unchanged. [1] [2]
Sources: [1] npm-publish manual page (npm publish fails if name+version exists; cannot ever reuse) • [2] npm blog (“No More npm publish -f”, republishing rejected/403)
Make reruns idempotent for the same commit.
The canary version is derived only from the PR head SHA, so any rerun for the same commit will attempt to republish the exact same npm version. npm rejects republishing any previously published version—the retry will fail immediately. This prevents recovery from transient publish failures without pushing a new commit. Either skip publish when the version already exists or include a rerun-safe suffix.
Suggested change
- name: Set canary version
run: |
+ PACKAGE_NAME=$(jq -r .name package.json)
SHORT_SHA=$(echo "${{ github.event.pull_request.head.sha }}" | cut -c1-7)
VERSION=$(jq -r .version package.json)-canary.${SHORT_SHA}
jq --arg v "$VERSION" '.version = $v' package.json > package.tmp.json
mv package.tmp.json package.json
+ echo "PACKAGE_NAME=$PACKAGE_NAME" >> "$GITHUB_ENV"
+ echo "VERSION=$VERSION" >> "$GITHUB_ENV"
echo "Published version: $VERSION"
- name: Publish
- run: bun publish --tag canary --access public
+ run: |
+ if npm view "${PACKAGE_NAME}@${VERSION}" version >/dev/null 2>&1; then
+ echo "Canary ${VERSION} already exists; skipping publish."
+ exit 0
+ fi
+ bun publish --tag canary --access public
env:
NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/canary.yml around lines 29 - 38, The current canary flow
uses SHORT_SHA and sets VERSION then always runs bun publish, which will fail on
reruns for the same commit; modify the job so that before running bun publish it
checks the registry for the computed VERSION (e.g., with npm view
<package>@$VERSION or npm info <package>@$VERSION) and if that version already
exists skip the publish step (echo/skipped) instead of attempting bun publish,
keeping the existing VERSION and SHORT_SHA logic and only invoking bun publish
when the registry check indicates the version is not present.
1 participant
Summary
node_modules/.bin/nextpath withcreateRequire+require.resolve('next/dist/bin/next')to handle hoisted dependencies in npx/bunx/library installschronicle.yamlfrom cwd first, then content dir — allows placing config in project root when using chronicle as a dependencyTest plan
chronicle devworks locally via global aliasnpm pack+ tarball install in temp project🤖 Generated with Claude Code