Conversation
npm package structure (@taskdn/cli + 5 platform packages), publish-npm job in release workflow using OIDC trusted publishing, version bumping in prepare-release script, and fix repo URLs across the codebase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds npm publishing for the CLI: new GitHub Actions Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Plugin install flow now prefers npm over curl in Cowork environments. Added dual-install warning for Homebrew/npm coexistence on macOS. Updated release docs, CLI README, and plugin README for npm. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tdn-cli/scripts/install.sh (1)
4-16:⚠️ Potential issue | 🟡 MinorUpdate the usage comment to match the new repository.
The usage comment on line 5 still references
taskdn/taskdn, but theREPOvariable on line 16 now points todannysmith/taskdn. This inconsistency could confuse users who copy the command from the comment.📝 Proposed fix
# tdn-cli installer -# Usage: curl -fsSL https://github.com/taskdn/taskdn/releases/latest/download/install.sh | bash +# Usage: curl -fsSL https://github.com/dannysmith/taskdn/releases/latest/download/install.sh | bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-cli/scripts/install.sh` around lines 4 - 16, Update the usage comment's example curl command to match the REPO variable by replacing the old "taskdn/taskdn" reference with "dannysmith/taskdn" so the comment is consistent with INSTALL_DIR/REPO configuration; ensure the curl example mirrors the format used by VERSION/REPO (i.e., curl -fsSL https://github.com/dannysmith/taskdn/releases/latest/download/install.sh | bash) and keep the surrounding usage text intact.
🧹 Nitpick comments (3)
.github/workflows/release-cli.yml (2)
97-100: Consider addingpublish-npmas a dependency for thereleasejob.Currently,
publish-npmandreleaseboth depend only onbuildand run in parallel. If npm publishing fails, the GitHub release will still be created. Consider whether you want the release job to wait for successful npm publishing.♻️ Option to serialize jobs
release: name: Create GitHub Release - needs: build + needs: [build, publish-npm] runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-cli.yml around lines 97 - 100, The release workflow currently runs publish-npm and release in parallel (both depend only on build), so a failing npm publish won't block creating the GitHub release; change the release job's needs to include the publish-npm job (i.e., make release depend on publish-npm) so the release job waits for publish-npm to succeed before running; update the workflow YAML jobs section referencing the publish-npm and release job names accordingly.
123-161: Quote shell variables to prevent word splitting.Static analysis flags multiple SC2086 warnings. While the variables here are controlled, quoting prevents issues with unexpected input and follows shell best practices.
♻️ Proposed fix for shell quoting
publish_unix() { local target=$1 echo "Publishing `@taskdn/cli-`${target}@${VERSION}..." - tar -xzf ../artifacts/tdn-${target}.tar.gz -C npm/cli-${target}/ - chmod +x npm/cli-${target}/tdn + tar -xzf "../artifacts/tdn-${target}.tar.gz" -C "npm/cli-${target}/" + chmod +x "npm/cli-${target}/tdn" node -e " const fs = require('fs'); - const p = JSON.parse(fs.readFileSync('npm/cli-${target}/package.json')); - p.version = '${VERSION}'; - fs.writeFileSync('npm/cli-${target}/package.json', JSON.stringify(p, null, 2) + '\n'); + const p = JSON.parse(fs.readFileSync('npm/cli-${target}/package.json', 'utf8')); + p.version = '${VERSION}'; + fs.writeFileSync('npm/cli-${target}/package.json', JSON.stringify(p, null, 2) + '\n'); " - cd npm/cli-${target} + cd "npm/cli-${target}" npm publish --access public cd ../.. } publish_windows() { local target=$1 echo "Publishing `@taskdn/cli-`${target}@${VERSION}..." - unzip -o ../artifacts/tdn-${target}.zip -d npm/cli-${target}/ + unzip -o "../artifacts/tdn-${target}.zip" -d "npm/cli-${target}/" node -e " const fs = require('fs'); - const p = JSON.parse(fs.readFileSync('npm/cli-${target}/package.json')); - p.version = '${VERSION}'; - fs.writeFileSync('npm/cli-${target}/package.json', JSON.stringify(p, null, 2) + '\n'); + const p = JSON.parse(fs.readFileSync('npm/cli-${target}/package.json', 'utf8')); + p.version = '${VERSION}'; + fs.writeFileSync('npm/cli-${target}/package.json', JSON.stringify(p, null, 2) + '\n'); " - cd npm/cli-${target} + cd "npm/cli-${target}" npm publish --access public cd ../.. }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-cli.yml around lines 123 - 161, The shell functions publish_unix and publish_windows trigger SC2086 — quote all variable expansions to avoid word-splitting: change usages like ../artifacts/tdn-${target}.tar.gz, npm/cli-${target}/, npm/cli-${target}/tdn, npm/cli-${target}/package.json, cd npm/cli-${target and ../.. to use "${target}" and "${VERSION}" where appropriate (e.g., "../artifacts/tdn-${target}.tar.gz", "npm/cli-${target}/", "npm/cli-${target}/tdn", "npm/cli-${target}/package.json", cd "npm/cli-${target}"); apply the same quoting in publish_windows (zip path and unzip -d target dir) and inside the node -e file paths so all dynamic paths and the VERSION variable are double-quoted.tdn-cli/npm/cli/bin/tdn (1)
42-58: Consider preserving signal exit codes.When the spawned process is killed by a signal (e.g., SIGTERM, SIGINT),
result.statusisnullbutresult.signalcontains the signal name. The conventional Unix behavior is to exit with128 + signal_number. Currently, this falls through to exit code1.♻️ Proposed enhancement for signal handling
+const SIGNALS = { SIGHUP: 1, SIGINT: 2, SIGQUIT: 3, SIGTERM: 15 }; + const result = spawnSync(binPath, process.argv.slice(2), { stdio: "inherit", }); if (result.error) { if (result.error.code === "ENOENT") { console.error( `Binary not found at ${binPath}.\n` + `Try reinstalling: npm install -g `@taskdn/cli`` ); } else { throw result.error; } process.exit(1); } -process.exit(result.status ?? 1); +if (result.status !== null) { + process.exit(result.status); +} else if (result.signal && SIGNALS[result.signal]) { + process.exit(128 + SIGNALS[result.signal]); +} else { + process.exit(1); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-cli/npm/cli/bin/tdn` around lines 42 - 58, The code exits with 1 when the spawned process was terminated by a signal (result.status is null) instead of preserving the conventional exit code (128 + signal_number); update the post-spawn logic that references result and spawnSync to detect result.signal and, when present, resolve the signal name to its numeric value (e.g., using os.constants.signals[result.signal] or process.constants.os.signals) and call process.exit(128 + signalNumber) so signal-terminated child processes produce the correct exit code; keep the existing ENOENT handling and only apply this signal branch before falling back to result.status ?? 1.
🤖 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/release-cli.yml:
- Around line 157-161: The Windows artifact name in the release workflow is
mismatched: the build matrix uses target value "windows-x64" producing artifact
tdn-windows-x64.zip, but publish_windows is invoked with "win32-x64" which will
look for tdn-win32-x64.zip; fix by making the target and publish arguments
consistent—either change the publish_windows call (publish_windows windows-x64
win32-x64) so it extracts the actual tdn-windows-x64.zip and maps to the win32
npm package name, or rename the build matrix entry from "windows-x64" to
"win32-x64" and update any Windows-specific checks to use "win32-x64" so the
artifact and publish step match.
- Around line 107-111: Update the workflow to set registry-url in the
actions/setup-node@v4 step (add registry-url: 'https://registry.npmjs.org'
alongside node-version: 24.x and remove or correct the "Do NOT set registry-url"
comment) and update any npm publish invocations to include the --provenance flag
(e.g., npm publish --access public --provenance) so OIDC trusted publishing
works and provenance attestations are generated; keep id-token: write
permissions as-is for the OIDC token step.
---
Outside diff comments:
In `@tdn-cli/scripts/install.sh`:
- Around line 4-16: Update the usage comment's example curl command to match the
REPO variable by replacing the old "taskdn/taskdn" reference with
"dannysmith/taskdn" so the comment is consistent with INSTALL_DIR/REPO
configuration; ensure the curl example mirrors the format used by VERSION/REPO
(i.e., curl -fsSL
https://github.com/dannysmith/taskdn/releases/latest/download/install.sh | bash)
and keep the surrounding usage text intact.
---
Nitpick comments:
In @.github/workflows/release-cli.yml:
- Around line 97-100: The release workflow currently runs publish-npm and
release in parallel (both depend only on build), so a failing npm publish won't
block creating the GitHub release; change the release job's needs to include the
publish-npm job (i.e., make release depend on publish-npm) so the release job
waits for publish-npm to succeed before running; update the workflow YAML jobs
section referencing the publish-npm and release job names accordingly.
- Around line 123-161: The shell functions publish_unix and publish_windows
trigger SC2086 — quote all variable expansions to avoid word-splitting: change
usages like ../artifacts/tdn-${target}.tar.gz, npm/cli-${target}/,
npm/cli-${target}/tdn, npm/cli-${target}/package.json, cd npm/cli-${target and
../.. to use "${target}" and "${VERSION}" where appropriate (e.g.,
"../artifacts/tdn-${target}.tar.gz", "npm/cli-${target}/",
"npm/cli-${target}/tdn", "npm/cli-${target}/package.json", cd
"npm/cli-${target}"); apply the same quoting in publish_windows (zip path and
unzip -d target dir) and inside the node -e file paths so all dynamic paths and
the VERSION variable are double-quoted.
In `@tdn-cli/npm/cli/bin/tdn`:
- Around line 42-58: The code exits with 1 when the spawned process was
terminated by a signal (result.status is null) instead of preserving the
conventional exit code (128 + signal_number); update the post-spawn logic that
references result and spawnSync to detect result.signal and, when present,
resolve the signal name to its numeric value (e.g., using
os.constants.signals[result.signal] or process.constants.os.signals) and call
process.exit(128 + signalNumber) so signal-terminated child processes produce
the correct exit code; keep the existing ENOENT handling and only apply this
signal branch before falling back to result.status ?? 1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6786d2b5-24fa-42b3-918e-3df59345a3e8
⛔ Files ignored due to path filters (3)
docs/tasks-todo/task-x-npm-distribution.mdis excluded by!**/*.mdtdn-claude-plugin/commands/prime.mdis excluded by!**/*.mdtdn-claude-plugin/skills/task-management/cowork.mdis excluded by!**/*.md
📒 Files selected for processing (10)
.github/workflows/release-cli.ymltdn-cli/npm/cli-darwin-arm64/package.jsontdn-cli/npm/cli-darwin-x64/package.jsontdn-cli/npm/cli-linux-arm64/package.jsontdn-cli/npm/cli-linux-x64/package.jsontdn-cli/npm/cli-win32-x64/package.jsontdn-cli/npm/cli/bin/tdntdn-cli/npm/cli/package.jsontdn-cli/scripts/install.shtdn-cli/scripts/prepare-release.js
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 24.x | ||
| # Do NOT set registry-url — it breaks OIDC trusted publishing | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
npm OIDC trusted publishing GitHub Actions setup 2025
💡 Result:
To set up npm trusted publishing using OIDC with GitHub Actions (generally available since July 2025), follow these steps: 1. Ensure prerequisites: - npm CLI v11.5.1 or later (ideally latest, e.g., npm@latest). - Package must already exist on npmjs.com (publish initially with a token if needed). - GitHub-hosted runners (self-hosted not supported). - Node.js 20+ (preferably 24+). 2. Configure trusted publisher on npmjs.com: - Go to https://www.npmjs.com/package/YOUR_PACKAGE/settings (or /access tab). - In "Trusted Publisher" section, select GitHub Actions. - Enter: - Owner: GitHub username or organization (case-sensitive). - Repository: repo name (without owner, case-sensitive). - Workflow filename: e.g., publish.yml (full name with .yml/.yaml extension, must be in .github/workflows/). - Environment: Optional (e.g., 'npm' or 'production'; match workflow if used). - Save. Each package supports only one trusted publisher (editable). 3. Update your GitHub Actions workflow (.github/workflows/publish.yml example): name: Publish to npm on: push: tags: ['v*'] # Or release: types: [created] permissions: id-token: write # Required for OIDC contents: read jobs: publish: runs-on: ubuntu-latest environment: npm # Optional, match npm config steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: node-version: '24' registry-url: 'https://registry.npmjs.org' - run: npm ci # Use read-only token if needed for deps - run: npm install -g npm@latest # Ensure latest npm - run: npm run build # If needed - run: npm publish --provenance # Auto-generates provenance attestation Key notes: - No NPM_TOKEN secret needed; OIDC handles auth automatically. - id-token: write permission is critical for GitHub to issue OIDC token. - Workflow filename, repo, owner must match exactly npm config. - Provenance is automatic but add --provenance flag explicitly if issues. - Add "repository" field to package.json for verification. - CLI alternative: npm trust github YOUR_PACKAGE --file publish.yml --repo owner/repo --env npm. - Works with monorepos/tools like Lerna/Changesets (Lerna 7+ native support). This eliminates long-lived tokens, uses short-lived workflow-specific creds, and adds supply chain security via provenance.
Citations:
- 1: https://github.blog/changelog/2025-07-31-npm-trusted-publishing-with-oidc-is-generally-available/
- 2: https://npmdigest.com/guides/npm-trusted-publishing
- 3: https://docs.npmjs.com/trusted-publishers/
- 4: https://medium.com/@n.saarberg/trusted-publishing-with-github-oidc-668961051bf4
- 5: https://docs.npmjs.com/cli/v11/commands/npm-trust
- 6: https://www.mbgsec.com/_archive/2025-09-23-trusted-publishing-for-npm-packages-npm-docs/
- 7: https://philna.sh/blog/2026/01/28/trusted-publishing-npm/
🏁 Script executed:
cat .github/workflows/release-cli.ymlRepository: dannysmith/taskdn
Length of output: 7039
Set registry-url for npm OIDC trusted publishing and add --provenance flag.
The embedded comment "Do NOT set registry-url — it breaks OIDC trusted publishing" is incorrect. According to npm's official trusted publishing documentation (GA since July 2025), registry-url: 'https://registry.npmjs.org' is required in setup-node for OIDC-based publishing to work. Without it, npm publish will fail with a 401/403 error even though id-token: write permission is correctly set.
Additionally, add the --provenance flag to the npm publish commands to generate supply-chain provenance attestations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-cli.yml around lines 107 - 111, Update the
workflow to set registry-url in the actions/setup-node@v4 step (add
registry-url: 'https://registry.npmjs.org' alongside node-version: 24.x and
remove or correct the "Do NOT set registry-url" comment) and update any npm
publish invocations to include the --provenance flag (e.g., npm publish --access
public --provenance) so OIDC trusted publishing works and provenance
attestations are generated; keep id-token: write permissions as-is for the OIDC
token step.
Fix Windows artifact name mismatch in publish-npm workflow, add --provenance to npm publish, fix signal exit codes in wrapper script, fix stale URL in install.sh comment. Bump plugin version for npm install flow changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
Chores