fix: bmad tea instal version#89
Conversation
🤖 Augment PR SummarySummary: Updates the TEA release pipeline to publish to npm and keep release metadata versions in sync. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe pull request enables NPM package publishing by adding version metadata synchronization across configuration files, implementing release-metadata validation checks, configuring npm registry setup in CI workflows, and establishing distribution-tag-based publishing with appropriate documentation and test script updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
228-234:⚠️ Potential issue | 🟡 MinorGrep check will also match
"private": false, slightly misleading.
grep '"private"' package.jsonmatches both"private": trueand"private": false. Since the currentpackage.jsonhas the field removed, this works today, but the snippet as documentation for "package must not be markedprivate: true" is imprecise. Consider narrowing the pattern so the success/failure branches map cleanly to the stated invariant.💡 Tighter check
grep -E '"private"\s*:\s*true' package.json && echo '❌ package.json is marked "private": true' || echo '✅ "private": true is not set'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 228 - 234, The grep check in the README currently uses grep '"private"' which also matches '"private": false'; update the snippet that checks package.json to search specifically for a true value (e.g., use a regex matching '"private"\s*:\s*true') so the success/failure messages align with the stated invariant that package must not be marked "private": true; modify the existing grep '"private"' command and its echo branches accordingly so the check prints an explicit failure when "private": true is present and a clear success otherwise..github/workflows/manual-release.yaml (1)
106-191:⚠️ Potential issue | 🟠 MajorOrdering:
npm publishruns before the git tag is created/pushed.The current step order is: commit → verify npm creds → publish to npm → generate release notes → create/push tag → push branch → create GitHub Release. If the
Create and push tagstep fails after publish succeeds (for example transient auth/push issues, or a pre-existing tag mismatch), the npm registry will havev${new_version}with no corresponding git tag or GitHub Release — and npm only allows unpublish within 72 hours.Safer ordering is: commit → create tag locally → push tag → publish → push branch → GitHub Release. That way any failure before
npm publishleaves nothing on the registry to clean up, and a post-publish failure still has the immutable tag for traceability.🔁 Suggested reorder (move tag creation/push above publish)
- name: Commit version bump run: | git add package.json package-lock.json .claude-plugin/marketplace.json git commit -m "release: bump to v${{ steps.version.outputs.new_version }}" - name: Create and push tag run: | if git rev-parse "v${{ steps.version.outputs.new_version }}" >/dev/null 2>&1; then echo "Tag v${{ steps.version.outputs.new_version }} already exists, skipping tag creation" else git tag -a "v${{ steps.version.outputs.new_version }}" -m "Release v${{ steps.version.outputs.new_version }}" git push origin "v${{ steps.version.outputs.new_version }}" fi - name: Verify npm credentials env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} run: | if [ -z "$NODE_AUTH_TOKEN" ]; then echo "NPM_TOKEN secret is not configured." >&2 exit 1 fi - name: Publish to npm env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} run: | if [ "${{ steps.version.outputs.npm_tag }}" = "latest" ]; then npm publish else npm publish --tag "${{ steps.version.outputs.npm_tag }}" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/manual-release.yaml around lines 106 - 191, Publish happens before the git tag is created/pushed which can leave an npm release without a corresponding git tag; move the "Create and push tag" step to run before the "Publish to npm" step so the tag for v${{ steps.version.outputs.new_version }} is created and pushed (or detected as existing) prior to running the "Publish to npm" step that uses ${{ steps.version.outputs.npm_tag }} and ${{ steps.version.outputs.new_version }}; ensure the "Create and push tag" logic (git tag -a "v${{ steps.version.outputs.new_version }}" ... and git push origin "v${{ steps.version.outputs.new_version }}") is placed immediately after the commit/version bump and before the publish step, and keep the npm credentials verification (NODE_AUTH_TOKEN) before publishing.
🧹 Nitpick comments (3)
.github/workflows/manual-release.yaml (2)
41-45: Release workflow runs a thinner test suite thannpm test.This step only runs
validate:schemas,format:check, andlint. It skipstest:schemas,test:install,test:knowledge,test:tea-workflow-descriptions, andlint:md— all of which are part ofnpm testand PR CI. On a mergedmainthis is normally covered byquality.yaml, but a manually triggered release from a non-PR branch (e.g. a hotfix branch with no PR) can reachnpm publishwithout those checks.Consider replacing these three commands with
npm testso the release gate is identical to what PR CI enforces.♻️ Suggested change
- name: Run tests and validation run: npm test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/manual-release.yaml around lines 41 - 45, The "Run tests and validation" job currently only executes the three commands validate:schemas, format:check, and lint, which omits other scripts run by npm test (test:schemas, test:install, test:knowledge, test:tea-workflow-descriptions, lint:md); update the step named "Run tests and validation" to run the single command npm test instead of the three-line block so the manual-release workflow enforces the same test gate as PR CI and prevents publishing from branches that skip those checks.
97-104: Small improvement: fail earlier and without exposing the secret presence via env.The
Verify npm credentialsstep does exactly what the name says, but it runs after bump/sync/commit. If the secret is missing, you'll have already mutated the working tree. Consider moving this check up next toInstall dependenciesso operators don't have to inspect and discard a dirty working copy on missing-token failures. Nit-level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/manual-release.yaml around lines 97 - 104, Move the "Verify npm credentials" step (the step that uses env NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}) to run immediately after the "Install dependencies" step and before any bump/sync/commit steps so the workflow fails early without mutating the working tree; keep the check logic the same but relocate the step placement in the job sequence to run prior to any commits or working-tree changes.test/test-release-metadata.js (1)
20-22: Wrap file reads/JSON parse to surface a friendly failure.If any of the three files is missing or contains invalid JSON, the script throws an unhandled
ENOENT/SyntaxErrorstack trace instead of the nicely formatted "Release metadata validation failed" output that the rest of the file is designed to produce. Low severity, but this script is the user-visible failure mode for a bad release, so the message quality matters.As per coding guidelines: "CLI tooling code. Check for: missing error handling on fs operations".
♻️ Suggested safe-read helper
function readJson(filePath, label) { try { return JSON.parse(fs.readFileSync(filePath, 'utf8')); } catch (error) { console.error(`Release metadata validation failed: unable to read ${label} at ${filePath}\n${error.message}`); process.exit(1); } } const packageJson = readJson(packageJsonPath, 'package.json'); const packageLock = readJson(packageLockPath, 'package-lock.json'); const marketplace = readJson(marketplacePath, '.claude-plugin/marketplace.json'); const marketplacePlugin = (marketplace.plugins || []).find((plugin) => plugin && plugin.name === packageJson.name);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test-release-metadata.js` around lines 20 - 22, Wrap the three direct file reads/JSON.parse calls in a small safe helper (e.g., function readJson(filePath, label)) that tries fs.readFileSync + JSON.parse, catches errors, prints a formatted message like "Release metadata validation failed: unable to read {label} at {filePath}\n{error.message}" to stderr, and calls process.exit(1); then replace the assignments to packageJson, packageLock, and marketplace with calls to readJson(packageJsonPath, 'package.json'), readJson(packageLockPath, 'package-lock.json'), and readJson(marketplacePath, '.claude-plugin/marketplace.json') respectively so marketplacePlugin lookup continues to use the validated packageJson and marketplace objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/manual-release.yaml:
- Around line 106-191: Publish happens before the git tag is created/pushed
which can leave an npm release without a corresponding git tag; move the "Create
and push tag" step to run before the "Publish to npm" step so the tag for v${{
steps.version.outputs.new_version }} is created and pushed (or detected as
existing) prior to running the "Publish to npm" step that uses ${{
steps.version.outputs.npm_tag }} and ${{
steps.version.outputs.new_version }}; ensure the "Create and push tag" logic
(git tag -a "v${{ steps.version.outputs.new_version }}" ... and git push origin
"v${{ steps.version.outputs.new_version }}") is placed immediately after the
commit/version bump and before the publish step, and keep the npm credentials
verification (NODE_AUTH_TOKEN) before publishing.
In `@README.md`:
- Around line 228-234: The grep check in the README currently uses grep
'"private"' which also matches '"private": false'; update the snippet that
checks package.json to search specifically for a true value (e.g., use a regex
matching '"private"\s*:\s*true') so the success/failure messages align with the
stated invariant that package must not be marked "private": true; modify the
existing grep '"private"' command and its echo branches accordingly so the check
prints an explicit failure when "private": true is present and a clear success
otherwise.
---
Nitpick comments:
In @.github/workflows/manual-release.yaml:
- Around line 41-45: The "Run tests and validation" job currently only executes
the three commands validate:schemas, format:check, and lint, which omits other
scripts run by npm test (test:schemas, test:install, test:knowledge,
test:tea-workflow-descriptions, lint:md); update the step named "Run tests and
validation" to run the single command npm test instead of the three-line block
so the manual-release workflow enforces the same test gate as PR CI and prevents
publishing from branches that skip those checks.
- Around line 97-104: Move the "Verify npm credentials" step (the step that uses
env NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}) to run immediately after the
"Install dependencies" step and before any bump/sync/commit steps so the
workflow fails early without mutating the working tree; keep the check logic the
same but relocate the step placement in the job sequence to run prior to any
commits or working-tree changes.
In `@test/test-release-metadata.js`:
- Around line 20-22: Wrap the three direct file reads/JSON.parse calls in a
small safe helper (e.g., function readJson(filePath, label)) that tries
fs.readFileSync + JSON.parse, catches errors, prints a formatted message like
"Release metadata validation failed: unable to read {label} at
{filePath}\n{error.message}" to stderr, and calls process.exit(1); then replace
the assignments to packageJson, packageLock, and marketplace with calls to
readJson(packageJsonPath, 'package.json'), readJson(packageLockPath,
'package-lock.json'), and readJson(marketplacePath,
'.claude-plugin/marketplace.json') respectively so marketplacePlugin lookup
continues to use the validated packageJson and marketplace objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3218e5e7-c2de-4b7b-b802-ea5487b57189
📒 Files selected for processing (6)
.claude-plugin/marketplace.json.github/workflows/manual-release.yaml.github/workflows/quality.yamlREADME.mdpackage.jsontest/test-release-metadata.js
1 participant
No description provided.