chore: update GitHub Actions workflows to include additional branch triggers and add compute-next-patch script for versioning #203
Conversation
…riggers and add compute-next-patch script for versioning
📝 WalkthroughWalkthroughAdds and removes multiple CI workflows, introduces Nx cache composite actions, updates push/release flows, adds a PR-scoped Verdaccio registry workflow, a cherry-pick automation workflow, and a versioning script with tests; also updates nx.json caching defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Default as Default Branch
participant PR as Original PR
GH->>Repo: on PR closed (merged to release/* or next/*) fetch PR info and merge_commit_sha
GH->>Repo: sanitize cherry-pick branch name, determine default branch
GH->>Default: create/reset cherry-pick branch from default
GH->>Repo: git cherry-pick (use -m 1 for merge commits)
alt success
GH->>Repo: commit attribution, push branch
GH->>GH: create PR targeting default, add labels
GH->>PR: post success comment
else conflict
GH->>Repo: git cherry-pick --abort
GH->>GH: create manual-cherry-pick issue (assign author if possible)
GH->>PR: post conflict comment with issue link
end
sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Node as Node/Yarn
participant Verd as Verdaccio
participant Ngrok as ngrok
participant PR as Pull Request
GH->>Repo: workflow_dispatch(pr_number) or validate PR
GH->>Repo: checkout PR branch, capture SHAs
GH->>Node: setup Node (from .nvmrc), compute PR-scoped versions, build packages
GH->>Verd: start Verdaccio, create CI user
GH->>Ngrok: start tunnel, obtain public URL
GH->>Verd: publish packages under tag pr-<PR>
GH->>GH: upload registry-info artifact
GH->>PR: post comment with .npmrc and install instructions
GH->>Verd: keep registry alive until timeout (health checks/restarts)
GH->>PR: post shutdown comment on timeout/cancel
sequenceDiagram
participant Trigger as Manual or PR trigger
participant Repo as Repository
participant Prepare as Prepare job
participant Publish as Publish job (matrix)
participant Npm as npm registry
participant Finalize as Finalize job
Trigger->>Prepare: start workflow (dispatch or release/** PR)
Prepare->>Repo: checkout, compute version (possibly using compute-next-patch), build artifacts, upload artifacts, emit matrix
Publish->>Publish: per-project download artifact, setup Node, publish to npm with tag
Publish->>Finalize: aggregate per-project summaries
Finalize->>Repo: create & push vVERSION tag
Finalize->>GH: create GitHub Release and final summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @.github/workflows/cherry-pick-prompt.yml:
- Around line 18-26: The workflow’s conditional should guard against forked PRs
by adding a repo-origin check to the existing if expression in
.github/workflows/cherry-pick-prompt.yml; update the if that currently uses
github.event.pull_request.* to also require that the PR head repo is the same
repo (e.g. add a clause using github.event.pull_request.head.repo.full_name ==
github.repository) or require github.event.pull_request.head.repo.fork == false
so the job won’t run for forked PRs where GITHUB_TOKEN may be read-only.
- Around line 51-56: After computing and sanitizing CHERRY_BRANCH (from
ORIGINAL_BRANCH) run git check-ref-format --branch "$CHERRY_BRANCH" and if it
returns non-zero print a clear error (including the invalid CHERRY_BRANCH value)
to stderr and exit non‑zero to fail early; place this validation immediately
after the CHERRY_BRANCH sanitization and before writing to GITHUB_OUTPUT so we
never output an invalid ref.
- Around line 70-90: The script currently runs git cherry-pick "$MERGE_SHA"
--no-commit which fails for merge commits; detect if MERGE_SHA is a merge by
checking parent count (e.g., using git rev-list --parents -n1 "$MERGE_SHA" and
counting parents) and if it has multiple parents run git cherry-pick -m 1
"$MERGE_SHA" --no-commit instead; update the cherry-pick invocation around the
MERGE_SHA usage so merge commits use -m 1 while non-merge commits keep the
existing --no-commit behavior.
In @.github/workflows/create-release-branch.yml:
- Around line 75-78: Add a validation step for the BASE="${{ inputs.base_branch
}}" value to ensure it is not an option-style value (doesn't start with '-') and
that it resolves to a real git ref: check the string for a leading '-' and fail
fast if present, then run a git ref verification command (e.g., git rev-parse
--verify or git show-ref --verify) against the supplied BASE and exit with a
clear error if the ref is invalid; apply the same validation where BASE is
set/used later (the other block at lines 116-118).
- Around line 130-134: The LAST_TAG assignment pipeline can fail under set -euo
pipefail because grep exits non-zero when no match and that causes the entire
subshell to fail before the trailing || echo "" runs; update the pipeline so the
grep step never returns non-zero (for example wrap grep with a command group
that falls back to true: use git tag --list "v*" --sort=-version:refname | {
grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' || true; } | head -n1 || echo ""), ensuring
LAST_TAG receives an empty string when no tag exists without breaking the step.
In @.github/workflows/pr-testing-registry.yml:
- Around line 71-105: The Verdaccio config created in the "Create Verdaccio
config" step currently allows anonymous publish/unpublish for package scopes
"@frontmcp/*", "frontmcp" and the catch-all "**"; change those package blocks so
publish and unpublish require authenticated users (remove $anonymous and set
publish: $authenticated or a specific group, and set unpublish: false or
restrict to admin) and ensure proxy packages keep access as needed; update the
"Create Verdaccio config" here and the similar block at lines 192-221, and add
an auth setup step before any publish action that creates a htpasswd user (or
configures token/auth) so CI authenticates before publishing.
- Around line 129-136: The workflow step "Setup ngrok" hardcodes the
distribution codename as "buster" when adding the ngrok apt source; change the
echo line that writes to /etc/apt/sources.list.d/ngrok.list to use dynamic
distribution detection by replacing "buster" with $(lsb_release -cs) so it
becomes: echo "deb https://ngrok-agent.s3.amazonaws.com $(lsb_release -cs) main"
| sudo tee /etc/apt/sources.list.d/ngrok.list, then run apt-get update/install
as before.
In @.github/workflows/publish-release.yml:
- Around line 162-175: The matrix output can be corrupted by multiline (pretty)
JSON when writing to GITHUB_OUTPUT; update the assignments that set
PROJECTS_JSON so they produce compact single-line JSON before writing to
GITHUB_OUTPUT (e.g., replace the calls that set PROJECTS_JSON from `npx nx show
projects ... --json` with a compacting command such as piping to `jq -c .` or
using a Node one-liner to JSON.stringify, and apply the same compacting to both
the synchronized-tag and fallback branches) and then write that single-line
PROJECTS_JSON to GITHUB_OUTPUT via the existing echo line.
- Around line 253-255: The workflow step titled "Update npm CLI for OIDC"
currently installs npm@latest; change this to pin a known-good version that
supports OIDC (e.g., replace the run command installing npm@latest with a
specific version like npm@11.5.1 or a newer stable release) so the line becomes
a deterministic install (or optionally make the version a workflow
input/variable), ensuring the publish step uses a tested npm CLI that supports
OIDC.
- Around line 176-185: The release workflow updates package versions via the npx
nx release version step but passes --git-commit=false so the bumped source is
never committed, causing tags to point to pre-bump commits; change the npx nx
release version invocation in the prepare job to allow committing (remove or set
--git-commit to true), then add an explicit git push of the release branch after
the commit (e.g., git add/commit if needed and git push origin <branch>), and in
the finalize/tagging step ensure you fetch/pull the latest branch (e.g., git
fetch origin <branch> and git checkout <branch> or git pull) before creating the
git tag so the tag created in the finalize job matches the committed
version-bumped source; apply the same fix to the existing tagging steps
referenced around lines 305-312.
In @.github/workflows/trigger-docs-update.yml:
- Around line 81-85: The workflow uses github.event.repository.default_branch
unconditionally to set DEFAULT_BRANCH/DEFAULT_REF and then runs git fetch origin
"$DEFAULT_BRANCH", which breaks for workflow_dispatch where that context is
missing; update the script that sets DEFAULT_BRANCH (and consequently
DEFAULT_REF) to detect github.event_name == "workflow_dispatch" and fall back to
a safe default like "main" (or use vars.DEFAULT_BRANCH with a default) before
calling git fetch origin "$DEFAULT_BRANCH" so the fetch will not fail when run
manually.
🧹 Nitpick comments (5)
scripts/compute-next-patch.test.mjs (2)
34-46: Consider cleanup for temporary test repositories.The
initRepo()function creates temporary directories but doesn't clean them up. While this is typically fine in CI environments (ephemeral), it could lead to disk space issues during local development.♻️ Suggested enhancement
Consider returning a cleanup function or using
test.after()hooks to remove temporary directories:async function initRepo() { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'compute-next-patch-')); git(dir, ['init']); git(dir, ['config', 'user.email', 'test@example.com']); git(dir, ['config', 'user.name', 'test']); await fs.writeFile(path.join(dir, 'README.md'), 'test\n'); git(dir, ['add', '.']); git(dir, ['commit', '-m', 'init']); - return dir; + return { + dir, + cleanup: async () => await fs.rm(dir, { recursive: true, force: true }) + }; }Then update tests to call cleanup in a finally block or use test hooks.
48-83: Good test coverage for main scenarios.The test suite covers the critical paths:
- Stable version increment
- RC pre-release auto-increment
- Error handling for invalid inputs
Consider adding tests for:
- First version (no existing tags)
- Beta release type
- Explicit pre-release number (not auto-increment)
- Invalid release line format
- Multiple stable versions to ensure proper max selection
.github/workflows/pr-testing-registry.yml (1)
192-220: Prefer discovering publishable packages dynamically (avoid hard-coded lists).Hard-coded arrays will drift as libs/plugins change; consider enumerating
**/dist/package.jsonand publishing those paths..github/workflows/create-release-branch.yml (1)
20-23: Droppull-requests: writeif unused.This workflow no longer creates/updates PRs; least-privilege suggests removing it.
.github/workflows/trigger-docs-update.yml (1)
189-199: Add defensive check before reading response file.Lines 193 and 197 attempt to
cat /tmp/response.jsonwithout verifying the file exists. Whilecurl -oshould create it, network failures or other issues could result in missing files.🛡️ Proposed defensive fix
if [ "$HTTP_STATUS" -eq 204 ]; then echo "Successfully triggered docs update in $DOCS_REPO" elif [ "$HTTP_STATUS" -eq 404 ]; then echo "::error::Repository $DOCS_REPO not found or PAT doesn't have access" - cat /tmp/response.json + [ -f /tmp/response.json ] && cat /tmp/response.json || echo "No response body available" exit 1 else echo "::error::Failed to trigger docs update. HTTP status: $HTTP_STATUS" - cat /tmp/response.json + [ -f /tmp/response.json ] && cat /tmp/response.json || echo "No response body available" exit 1 fi
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/cherry-pick-prompt.yml.github/workflows/codex-mintlify-docs.yml.github/workflows/create-release-branch.yml.github/workflows/package-e2e.yml.github/workflows/pr-testing-registry.yml.github/workflows/publish-alpha.yml.github/workflows/publish-on-next-close.yml.github/workflows/publish-release.yml.github/workflows/push.yml.github/workflows/trigger-docs-update.yml.github/workflows/update-draft-docs.ymlscripts/compute-next-patch.mjsscripts/compute-next-patch.test.mjs
💤 Files with no reviewable changes (4)
- .github/workflows/update-draft-docs.yml
- .github/workflows/publish-on-next-close.yml
- .github/workflows/codex-mintlify-docs.yml
- .github/workflows/publish-alpha.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Package E2E (Verdaccio)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
.github/workflows/push.yml (1)
3-9: LGTM! Branch filtering aligns with release strategy.The explicit branch filtering appropriately limits CI execution to main development, feature (next/**), release, and hotfix branches, reducing unnecessary workflow runs.
.github/workflows/package-e2e.yml (1)
11-11: LGTM! Consistent branch coverage for E2E testing.Adding
release/**to both push and pull_request triggers ensures package validation runs during the release workflow, catching potential packaging issues before publication.Also applies to: 16-16
scripts/compute-next-patch.mjs (1)
1-112: LGTM! Well-structured versioning script with comprehensive validation.The script correctly handles:
- Stable, rc, and beta release types
- Automatic patch increment based on git tags
- Pre-release number auto-increment and manual specification
- Edge cases (no tags, no stable tags, invalid inputs)
The validation is thorough and error messages are clear. The logic for determining the next version is sound.
.github/workflows/create-release-branch.yml (1)
32-41: Input validation looks good.Numeric major/minor checks are a solid guardrail for deterministic branch naming/versioning.
.github/workflows/publish-release.yml (1)
242-273: Review comment is unfounded; artifact paths are correctly configured.The workflow correctly handles artifact extraction. When
actions/download-artifact@v4downloads a single artifact by name without an explicitpathparameter, it extracts directly to the workspace root preserving internal structure. The upload step uses multiple glob patterns (libs/*/distandplugins/*/dist), which upload-artifact resolves by using the least-common-ancestor (workspace root) as the artifact root, preserving the full directory hierarchy:libs/{projectname}/distandplugins/{projectname}/dist. The publish step's path checks forlibs/$PROJECT/distandplugins/$PROJECT/distcorrectly match this extracted structure. No path resolution issues exist here.Likely an incorrect or invalid review comment.
.github/workflows/trigger-docs-update.yml (6)
1-24: LGTM! Workflow triggers and permissions are correctly configured.The combination of automated PR merge triggers and manual dispatch provides flexibility, and the read-only permissions are appropriate since external repo access uses a PAT.
26-33: LGTM! Job condition correctly filters events.The condition properly ensures the workflow runs only for manual triggers or merged PRs to release branches.
117-142: LGTM! Change summary generation is well-structured.The git diff command properly filters relevant files, excludes tests, and has appropriate fallbacks for empty results.
164-187: LGTM! Repository dispatch payload is well-structured.The client_payload includes comprehensive metadata for traceability and proper documentation updates in the target repository.
201-212: LGTM! Summary output provides excellent visibility.The formatted table in the GitHub step summary clearly shows all relevant information about the triggered docs update.
158-160: Code properly implements repository_dispatch event—target repository configuration requires external verification.The workflow correctly sets up the
repository_dispatchtrigger (line 168) with event typedocs-updateand includes comprehensive error handling (HTTP 204 for success, 404 for missing repo, and generic error handling). TheDOCS_REPOvariable properly defaults toagentfront/frontmcp-docswhile allowing override viavars.DOCS_REPO.To complete implementation, verify that the target
agentfront/frontmcp-docsrepository (or the configuredDOCS_REPOalternative) has a workflow configured to handlerepository_dispatchevents withevent_type: docs-update.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/actions/nx-cache-restore/action.yml:
- Line 41: Replace the pinned action reference "actions/cache/restore@v4" with
the recommended stable release "actions/cache@v5" in the workflow action step
(the uses: line), and update any step inputs or call pattern to match the v5
action signature if needed (verify step keys and outputs used elsewhere after
changing the uses line).
In @.github/workflows/push.yml:
- Around line 52-53: The GitHub Actions cache steps (e.g., the "Restore Nx
cache" action and corresponding "Save Nx cache" steps) are generating identical
keys across parallel jobs causing collisions; update the cache actions to accept
and use a job-specific identifier (e.g., a cache-suffix input like lint, build,
unit-tests, e2e-<project>, coverage) so each job computes a unique key, and/or
remove save steps from non-build jobs so only the build job writes the cache
(keep restore-only in others); locate the steps named "Restore Nx cache" and
"Save Nx cache" and either add a cache-suffix input to their usage or centralize
saving to the build job (or implement a dedicated cache consolidation job) to
avoid last-write-wins races.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/nx-cache-restore/action.yml.github/actions/nx-cache-save/action.yml.github/workflows/push.ymlnx.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
Applied to files:
nx.json.github/workflows/push.yml
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Applies to **/*.test.{ts,tsx} : Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Package E2E (Verdaccio)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
nx.json (1)
81-89: LGTM! Cache configuration aligns with Nx best practices.The new cache configurations for
@nx/esbuild:esbuildandbuildtargets follow the same pattern as the existing@nx/js:tscconfiguration. The use ofproductioninputs ensures that only production-relevant files trigger cache invalidation, which is appropriate for build targets..github/actions/nx-cache-restore/action.yml (1)
18-37: Cache key computation logic is well-structured.The fallback chain for restore-keys is well-designed:
- Exact match on PR/branch + lockfile hash
- Any cache for the same PR/branch
- Cache from main branch
- Any cache with the prefix
This provides good balance between cache hit rate and correctness.
.github/actions/nx-cache-save/action.yml (2)
13-23: Cache key computation is consistent with restore action.The key computation logic correctly matches the restore action, ensuring that saved caches can be retrieved by the restore action. Both use the same PR/branch + lockfile hash pattern.
26-26: No action required. Verification confirms thatactions/cache/save@v4is available and correctly matches theactions/cache/restore@v4version used in the corresponding restore action. Both use v4, which is the latest stable major version..github/workflows/push.yml (3)
3-9: Branch trigger configuration looks appropriate.The branch filters align with a typical Git flow strategy, triggering on main, next branches, release branches, and hotfix branches. This provides good coverage for CI/CD while avoiding unnecessary runs on feature branches.
98-112: Good failure handling pattern with cache reset and retry.The build job correctly handles cache-related failures:
- Uses
continue-on-errorto capture failures without stopping- Resets Nx cache on failure with
nx reset- Retries the build without cache corruption
- Saves cache even on failure with
if: always()This pattern prevents poisoned cache from blocking builds while still benefiting from cache when it works.
213-224: E2E test retry logic mirrors build pattern effectively.The E2E tests apply the same defensive cache handling as the build job, which is appropriate given that E2E tests can be sensitive to cached state inconsistencies. The matrix strategy combined with retry logic provides good resilience.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/push.yml (1)
11-13: Concurrency group may cancel other workflows on the same branch.
group: ci-${{ github.ref }}can collide with other workflows that also useci-${{ github.ref }}(concurrency groups are repo-wide), causing cross-workflow cancellations. Prefer namespacing with${{ github.workflow }}.Proposed fix
concurrency: - group: ci-${{ github.ref }} + group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true
🤖 Fix all issues with AI agents
In @.github/workflows/cherry-pick-prompt.yml:
- Around line 75-76: The workflow currently uses git checkout -b
"$CHERRY_BRANCH" which will fail if that branch already exists; update the step
to detect an existing branch (e.g., git rev-parse --verify
"refs/heads/$CHERRY_BRANCH" or git show-ref --verify --quiet
"refs/heads/$CHERRY_BRANCH") and then either git checkout "$CHERRY_BRANCH" if it
exists or delete and recreate it (git branch -D "$CHERRY_BRANCH" && git checkout
-b "$CHERRY_BRANCH") depending on desired behavior, ensuring all git commands
run with proper error handling so the job continues predictably.
- Around line 194-195: The workflow currently uses --assignee "$PR_AUTHOR" which
can fail if the PR author cannot be assigned; change this to make assignment
optional by removing the direct --assignee invocation and adding a separate step
that attempts assignment but does not fail the job, e.g. run gh issue edit
"$ISSUE_URL" --add-assignee "$PR_AUTHOR" 2>/dev/null || echo "Could not assign
to $PR_AUTHOR, skipping assignment" (or mark the step continue-on-error: true);
update the step that used --assignee "$PR_AUTHOR" to only set labels/other
fields and move the optional assignment into the new tolerant step.
In @.github/workflows/create-release-branch.yml:
- Around line 68-96: Read the GitHub Action inputs into safe shell variables
before any use and validate them first (especially BASE) to prevent
option/command injection; update the "Determine branch and version" step (ids:
branch, variables: MAJOR, MINOR, BASE, BRANCH, VERSION, RELEASE_LINE) to accept
inputs via the step's env block (so they are not interpolated into the shell),
perform the BASE validation immediately upon start, and only then construct and
echo BRANCH, VERSION, RELEASE_LINE and write to GITHUB_OUTPUT, ensuring every
variable is quoted when used.
- Around line 31-40: The "Validate inputs" step currently interpolates inputs
directly into the shell which enables command injection; change it to read
inputs via environment variables (e.g., set MAJOR_VERSION and MINOR_VERSION from
inputs) and perform validation against those env vars (use the same numeric
regex checks on $MAJOR_VERSION and $MINOR_VERSION), ensuring all variable
expansions are quoted in error messages and commands; update the step labeled
"Validate inputs" to reference these env names instead of using `${{
inputs.major_version }}`/`${{ inputs.minor_version }}` inline.
In @.github/workflows/pr-testing-registry.yml:
- Around line 378-383: The Verdaccio restart block should clean up stale PID
info and ensure the port is free before restarting: check for an existing PID
file (e.g., /tmp/verdaccio-pr/verdaccio.pid) and if present verify the process
is alive and kill it if stale, remove the PID file; then test port 4873 (using
lsof/ss or netstat) and wait/retry until it is free before launching Verdaccio;
after starting Verdaccio (verdaccio --config /tmp/verdaccio-pr/config.yaml
--listen 4873 &), capture its PID and write it to the PID file and redirect logs
appropriately so future restarts can use the PID and avoid orphaned processes or
port conflicts.
- Around line 197-200: The npm auth step "Authenticate with Verdaccio" is using
npm property _authToken with a base64-encoded "username:password" string which
is incorrect; change the npm config key from //_localhost:4873/:_authToken to
//_localhost:4873/:_auth and keep the base64-encoded credentials (echo -n
'ci-publisher:ci-publisher-pass' | base64) so Verdaccio receives basic auth via
_auth rather than a bearer token.
In @.github/workflows/publish-release.yml:
- Around line 231-294: The workflow currently claims OIDC support but installs
npm@10.9.2 which lacks OIDC publishing; update the "Update npm CLI for OIDC"
step by installing npm@11.5.1 or newer (replace the npm install -g npm@10.9.2
invocation) and, if you intend OIDC, ensure the "Publish ${{ matrix.project }}"
step uses the appropriate OIDC flags (e.g., --provenance) and remove any
leftover token-based env; alternatively, if you intend token auth, keep
npm@10.9.2 but remove the OIDC comment and explicitly provide NPM_TOKEN /
NODE_AUTH_TOKEN to the "Publish ${{ matrix.project }}" step so the
authentication method and versions align.
In @.github/workflows/push.yml:
- Around line 93-105: The retry step labeled "Retry build without cache" still
runs the same command (npx nx affected -t build) and will continue to use Nx
caching; modify that step to explicitly disable Nx cache by either adding the
CLI flag --skip-nx-cache to the command or setting the environment variable
NX_SKIP_NX_CACHE=true for that step, and keep the preceding "Reset Nx cache on
failure" step as-is (it can remain) while ensuring the retry step references the
original build step outcome (steps.build.outcome) as the condition.
🧹 Nitpick comments (10)
.github/workflows/create-release-branch.yml (2)
113-131: Consider removing redundantgit fetchcall.Line 122 performs
git fetch origin --tagswhich was already executed on line 104 in the previous step. Since both steps run sequentially, the second fetch is redundant.♻️ Proposed optimization
- name: Create release branch shell: bash run: | set -euo pipefail BRANCH="${{ steps.branch.outputs.branch }}" BASE="${{ steps.branch.outputs.base }}" # Validate BASE is a real git ref - git fetch origin --tags if ! git rev-parse --verify "origin/$BASE" >/dev/null 2>&1; then echo "::error::Invalid base_branch: '$BASE' does not exist on remote" exit 1 fi git fetch origin "$BASE"
143-147: Redundant fallback handling in tag extraction.The
|| trueinside the command group already preventsgrepfrom failing when there are no matches, so the outer|| echo ""onhead -n1is unnecessary (head won't fail on empty input).♻️ Proposed simplification
LAST_TAG=$( git tag --list "v*" --sort=-version:refname | { grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' || true - } | head -n1 || echo "" + } | head -n1 ).github/workflows/cherry-pick-prompt.yml (5)
91-91: Quote the variable expansion to prevent word splitting.The
$CHERRY_PICK_ARGSvariable should be quoted to prevent unexpected word splitting, especially when it contains flags like-m 1 --no-commit.♻️ Proposed fix
- if git cherry-pick $CHERRY_PICK_ARGS "$MERGE_SHA"; then + if git cherry-pick ${CHERRY_PICK_ARGS} "$MERGE_SHA"; thenNote: In this case, we want word splitting for the flags, so the current approach is actually correct. However, consider using an array for cleaner handling:
CHERRY_PICK_ARGS=(--no-commit) if [ "$PARENT_COUNT" -gt 1 ]; then CHERRY_PICK_ARGS=(-m 1 --no-commit) fi if git cherry-pick "${CHERRY_PICK_ARGS[@]}" "$MERGE_SHA"; then
95-103: Sanitize PR title in commit message to prevent formatting issues.The
$PR_TITLEvariable comes from user input and could contain special characters or newlines that might break the commit message formatting. Consider sanitizing it before use.♻️ Proposed fix
+ # Sanitize PR title for commit message + SANITIZED_TITLE=$(echo "$PR_TITLE" | tr '\n' ' ' | tr -s ' ') + # Create commit with proper attribution git commit -m "$(cat <<EOF - Cherry-pick: $PR_TITLE + Cherry-pick: $SANITIZED_TITLE Cherry-picked from #$PR_NUMBER (merged to $RELEASE_BRANCH) Original commit: $MERGE_SHA
156-156: Remove or declare the unused output variable.The
pr_urlis written to$GITHUB_OUTPUTbut is not declared in the step'soutputssection, making it inaccessible to subsequent steps or jobs. Either declare it in the outputs or remove this line if it's not needed.♻️ Proposed fix
If you want to use this output, add it to the step outputs:
- name: Push branch and create PR + id: create_pr if: steps.prepare.outputs.conflict == 'false' + outputs: + pr_url: ${{ steps.create_pr.outputs.pr_url }}Otherwise, remove the line:
echo "Created cherry-pick PR: $PR_URL" - echo "pr_url=$PR_URL" >> "$GITHUB_OUTPUT"
218-219: Include merge commit handling in manual cherry-pick instructions.The manual cherry-pick instructions don't mention the
-m 1flag that may be needed for merge commits (as detected in the automatic cherry-pick). This could cause confusion when users follow the instructions.♻️ Proposed fix
# Cherry-pick the commit - git cherry-pick $MERGE_SHA + # For merge commits, you may need to use: git cherry-pick -m 1 $MERGE_SHA + git cherry-pick $MERGE_SHA
125-125: Consider adding retry logic for GitHub CLI operations.The workflow uses
ghcommands for creating PRs, issues, and comments. These API calls could fail due to rate limiting or transient network issues. Consider adding retry logic to make the workflow more resilient.♻️ Example retry wrapper
Add a retry function at the beginning of each step that uses
gh:retry_gh_command() { local max_attempts=3 local attempt=1 while [ $attempt -le $max_attempts ]; do if "$@"; then return 0 fi echo "Attempt $attempt failed, retrying..." sleep $((attempt * 2)) attempt=$((attempt + 1)) done echo "Command failed after $max_attempts attempts" return 1 } # Then use it like: retry_gh_command gh pr create --base "$DEFAULT_BRANCH" ...Also applies to: 128-153, 190-237
.github/workflows/push.yml (1)
52-54: Nx cache restore is added, but most jobs never save—likely leaving performance on the table.
If your composite actions are based onactions/cache, restoring without saving means new/updated.nx/cachestate from lint/tests/coverage won’t persist to future workflow runs (build currently saves, but the other jobs don’t). Consider addingnx-cache-savewithif: always()at the end oflint,unit-tests,e2e-tests, andcoveragejobs (or document why build-only save is sufficient).Also applies to: 146-148, 199-201, 246-248
.github/workflows/pr-testing-registry.yml (2)
108-109: Consider using system package for htpasswd.Installing
htpasswdvia npm is less common. Consider using the system package for better reliability:♻️ Alternative installation approach
- npm install -g htpasswd - htpasswd -cb /tmp/verdaccio-pr/htpasswd ci-publisher ci-publisher-pass + sudo apt-get install -y apache2-utils + htpasswd -cb /tmp/verdaccio-pr/htpasswd ci-publisher ci-publisher-pass
202-231: Consider tracking and reporting publish failures.The current error handling with
|| echo "Warning: publish failed for $pkg"continues on error, which may hide failures from users. Consider tracking failed packages and reporting them in the PR comment.📦 Improved error tracking
- name: Publish packages to local Verdaccio run: | VERSION="${{ steps.version.outputs.version }}" PR_NUM="${{ inputs.pr_number }}" + FAILED_PACKAGES=() # Publish libs packages LIBS_PACKAGES=(di utils uipack sdk ui adapters testing cli plugins) for pkg in "${LIBS_PACKAGES[@]}"; do if [ -d "libs/$pkg/dist" ]; then echo "Publishing @frontmcp/$pkg@$VERSION..." - npm publish "libs/$pkg/dist" \ + if ! npm publish "libs/$pkg/dist" \ --registry http://localhost:4873 \ --access public \ - --tag "pr-$PR_NUM" || echo "Warning: publish failed for $pkg" + --tag "pr-$PR_NUM"; then + echo "Warning: publish failed for $pkg" + FAILED_PACKAGES+=("@frontmcp/$pkg") + fi fi done # Publish plugin packages PLUGIN_PACKAGES=(plugin-approval plugin-cache plugin-codecall plugin-dashboard plugin-remember) for pkg in "${PLUGIN_PACKAGES[@]}"; do if [ -d "plugins/$pkg/dist" ]; then echo "Publishing @frontmcp/$pkg@$VERSION..." - npm publish "plugins/$pkg/dist" \ + if ! npm publish "plugins/$pkg/dist" \ --registry http://localhost:4873 \ --access public \ - --tag "pr-$PR_NUM" || echo "Warning: publish failed for $pkg" + --tag "pr-$PR_NUM"; then + echo "Warning: publish failed for $pkg" + FAILED_PACKAGES+=("@frontmcp/$pkg") + fi fi done - echo "All packages published to local Verdaccio" + if [ ${#FAILED_PACKAGES[@]} -eq 0 ]; then + echo "All packages published successfully" + else + echo "Failed to publish: ${FAILED_PACKAGES[*]}" + echo "PUBLISH_FAILURES=${FAILED_PACKAGES[*]}" >> "$GITHUB_ENV" + fiThen update the PR comment step to include failure information if any.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/cherry-pick-prompt.yml.github/workflows/create-release-branch.yml.github/workflows/pr-testing-registry.yml.github/workflows/publish-release.yml.github/workflows/push.yml.github/workflows/trigger-docs-update.ymlscripts/compute-next-patch.mjsscripts/compute-next-patch.test.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/trigger-docs-update.yml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Applies to **/*.test.{ts,tsx} : Use Jest for testing with 95%+ coverage requirement and 100% test pass rate
Applied to files:
scripts/compute-next-patch.test.mjs
📚 Learning: 2026-01-04T14:35:18.366Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.366Z
Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
Applied to files:
scripts/compute-next-patch.test.mjs
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Package E2E (Verdaccio)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (31)
.github/workflows/create-release-branch.yml (5)
98-111: LGTM!Good approach using
git ls-remoteto verify the branch doesn't already exist on the remote before attempting creation. The error messaging is clear.
165-172: LGTM!Correctly uses
nx release versionwith--git-tag=false --git-commit=falseto update package versions without creating commits, allowing the workflow to bundle all changes into a single release preparation commit later. This is consistent with the Nx-based monorepo setup.
174-181: LGTM!Good approach to create a marker file for tracking the base SHA. This enables downstream workflows to calculate proper diffs without needing to infer the base.
183-207: LGTM!Good defensive programming: checking for script existence before execution and properly guarding docs archival to only run when transitioning to a new minor version.
247-262: LGTM!Well-structured summary with a clear table of properties and actionable next steps. Good use of GitHub step summary for visibility.
.github/workflows/push.yml (2)
3-9: Branch filters look good; double-check you’re not unintentionally dropping desired push CI coverage.
Now onlymain,next/**,release/**,hotfix/**will trigger “On Push” (e.g., feature branches won’t). If that’s intentional, LGTM.
44-48: All action versions are valid and current.
actions/setup-node@v6(latest: v6.1.0),codecov/codecov-action@v5, andnrwl/nx-set-shas@v4(latest: v4.4.0) all exist and are appropriate for their respective use cases. The v4 action is compatible with Nx 18, 19, and 20.If supply-chain hardening is a priority, SHA pinning is worth considering as an optional hardening measure, but it is not required.
Likely an incorrect or invalid review comment.
.github/workflows/pr-testing-registry.yml (6)
1-34: LGTM! Well-structured workflow configuration.The workflow dispatch inputs, permissions, and timeout settings are appropriately configured for a long-running registry service.
36-69: LGTM! Solid validation and setup steps.The PR validation, checkout strategy, and dependency installation follow best practices for reproducible builds.
111-131: LGTM! Proper service startup with health checks.The Verdaccio startup sequence includes appropriate health checks and error handling.
133-177: LGTM! Excellent ngrok setup with proper validation.The dynamic distro detection, secret validation, and retry logic with clear error messages are well-implemented.
179-195: LGTM! Appropriate versioning strategy for PR testing.The PR-specific version format and Nx commands follow best practices for monorepo builds.
233-436: LGTM! Comprehensive documentation and cleanup.The artifact creation, PR comments, step summary, and cleanup steps are well-implemented with appropriate conditional execution and clear messaging.
scripts/compute-next-patch.mjs (7)
1-13: LGTM!Clear documentation with helpful usage examples covering all supported scenarios.
15-23: LGTM!Good security practice using
escapeRegExpto prevent regex injection when building patterns from user input.
25-50: LGTM!Comprehensive input validation with clear error messages. The format validation and release-type constraints are correctly implemented.
52-63: LGTM!Git tag fetching with appropriate error handling. The descending version sort and empty-tag fallback are correctly implemented.
65-79: LGTM!Correct logic for determining the next patch number. Efficiently finds the highest stable tag and increments appropriately.
81-118: LGTM!Pre-release handling logic is well-structured with proper validation and auto-increment behavior. The version-specific filtering ensures correct pre-release numbering.
120-121: LGTM!Correct use of
process.stdout.writewithout a newline for shell script compatibility.scripts/compute-next-patch.test.mjs (4)
1-25: LGTM!Proper test setup with centralized cleanup in the
afterhook. Good practice for managing temporary test resources.
27-61: LGTM!Well-structured helper functions for test isolation. The
initRepohelper properly initializes Git repositories for each test scenario.
63-147: LGTM!Comprehensive test coverage for stable, rc, and beta release scenarios. Tests validate both happy paths and error conditions with appropriate assertions.
148-154: LGTM!Good coverage of the missing-argument error case with proper usage message validation.
.github/workflows/publish-release.yml (7)
1-43: LGTM!Well-structured workflow triggers and appropriate permissions. The
cancel-in-progress: falsesetting is correct for release workflows to prevent interrupting publishes.
48-86: LGTM!Proper job setup with correct triggering conditions and environment configuration. Using
.nvmrcand frozen lockfile ensures consistency.
87-114: LGTM!Robust branch validation ensures the workflow only runs from properly formatted release branches. Clear error messaging helps with troubleshooting.
115-161: LGTM!Correct integration with the
compute-next-patch.mjsscript. The duplicate tag check prevents accidental re-releases, and NPM tag derivation aligns with semantic versioning conventions.
299-368: LGTM!Proper finalization with Git tag creation and GitHub release. The conditional prerelease notice and generated release notes enhance the release experience.
374-395: LGTM!Helpful dry-run summary for validating release configuration without publishing. Clear messaging prevents confusion.
162-226: The E2E test step will fail ondemo-e2e-transport-recreationVerification found that 18 of 19 E2E projects have a
testtarget, butdemo-e2e-transport-recreationis missing it. The workflow step:npx nx run-many -t test --projects='demo-e2e-*' --parallel=3will fail attempting to run tests on a project without the target.
Add a
testtarget toapps/e2e/demo-e2e-transport-recreation/project.jsonor exclude it from the test pattern (e.g.,--projects='demo-e2e-*' --exclude='demo-e2e-transport-recreation').⛔ Skipped due to learnings
Learnt from: CR Repo: agentfront/frontmcp PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-06T17:16:04.304Z Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`Learnt from: CR Repo: agentfront/frontmcp PR: 0 File: libs/uipack/CLAUDE.md:0-0 Timestamp: 2026-01-04T14:35:18.366Z Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/push.yml:
- Around line 94-109: The retry step "Retry build without cache" currently
invokes `npx nx affected -t build --skip-nx-cache`, which prevents writing task
results back into `.nx/cache` after the earlier `npx nx reset`; remove the
`--skip-nx-cache` flag so the retry uses the cache normally and repopulates
`.nx/cache` for the later "Save Nx cache" step, i.e., change the retry command
to `npx nx affected -t build` and keep the conditional on `steps.build.outcome
== 'failure'` intact.
In @.github/workflows/trigger-docs-update.yml:
- Around line 157-163: The git diff invocation assigning CHANGED_FILES is using
three-dot syntax (git diff "$DIFF_BASE"...HEAD) which computes changes from the
merge base; change that invocation to use two dots (git diff "$DIFF_BASE..HEAD
--name-only -- ...") so the diff is computed directly from the specified
DIFF_BASE commit, ensuring CHANGED_FILES contains the intended commit-range
changes; update the CHANGED_FILES assignment accordingly in the workflow.
🧹 Nitpick comments (7)
.github/workflows/publish-release.yml (2)
30-34: Workflow triggered on any closed PR to release branches may cause unintended runs.The
pull_request: types: [closed]trigger will fire for both merged and unmerged (closed without merge) PRs. While thepreparejob has anifcondition checkinggithub.event.pull_request.merged == true, if someone adds a job without that condition, it would run on any closed PR.Consider adding the merge check at the workflow level or adding a comment to document this requirement for future maintainers.
182-190: Direct push to release branch without force-push protection check.The workflow commits and pushes directly to HEAD on the release branch. If the branch has been updated since checkout (e.g., another workflow run), this push could fail or cause conflicts.
Consider adding a check or using
git pull --rebasebefore pushing, or ensure branch protection rules prevent concurrent runs (which the concurrency setting partially addresses).Suggested improvement
- name: Commit version bump run: | git config user.name "agentfront[bot]" git config user.email "agentfront[bot]@users.noreply.github.com" + # Ensure we have latest changes + git pull origin HEAD --rebase || true if [ -n "$(git status --porcelain)" ]; then git add -A git commit -m "chore(release): bump version to ${{ steps.version.outputs.version }}" git push origin HEAD fi.github/workflows/cherry-pick-prompt.yml (2)
54-62: Branch name truncation may create collisions.The branch name is truncated to 100 characters with
head -c 100, but this could create collisions if multiple branches share the same 100-character prefix. Consider appending a short hash or the PR number to ensure uniqueness.Suggested improvement
# Create cherry-pick branch name # Sanitize branch name (remove invalid characters, truncate if needed) - CHERRY_BRANCH="${ORIGINAL_BRANCH}-cherry-pick-main" - CHERRY_BRANCH=$(echo "$CHERRY_BRANCH" | sed 's/[^a-zA-Z0-9._/-]/-/g' | head -c 100) + CHERRY_BRANCH="${ORIGINAL_BRANCH}-cherry-pick-main" + CHERRY_BRANCH=$(echo "$CHERRY_BRANCH" | sed 's/[^a-zA-Z0-9._/-]/-/g' | head -c 80) + # Append PR number to ensure uniqueness + CHERRY_BRANCH="${CHERRY_BRANCH}-${PR_NUMBER}"
22-27: Condition logic is correct but consider documenting edge cases.The condition correctly filters for merged PRs from non-fork heads targeting release/next branches. Consider adding a comment about why fork PRs are excluded (read-only GITHUB_TOKEN) for future maintainers.
.github/workflows/pr-testing-registry.yml (2)
107-109: Hardcoded credentials for Verdaccio are exposed in workflow logs.The username and password
ci-publisher:ci-publisher-passare visible in the workflow file and logs. While this is a temporary registry, consider using randomly generated credentials for each run.Suggested improvement
# Create CI user for publishing - npm install -g htpasswd - htpasswd -cb /tmp/verdaccio-pr/htpasswd ci-publisher ci-publisher-pass + npm install -g htpasswd + CI_PASSWORD=$(openssl rand -base64 32) + htpasswd -cb /tmp/verdaccio-pr/htpasswd ci-publisher "$CI_PASSWORD" + echo "CI_PASSWORD=$CI_PASSWORD" >> "$GITHUB_ENV"Then update line 200 to use
$CI_PASSWORD:- npm set //localhost:4873/:_auth "$(echo -n 'ci-publisher:ci-publisher-pass' | base64)" + npm set //localhost:4873/:_auth "$(echo -n "ci-publisher:$CI_PASSWORD" | base64)"
210-241: Hardcoded package lists may become stale.The
LIBS_PACKAGESandPLUGIN_PACKAGESarrays are hardcoded. Unlike thepublish-release.ymlworkflow which dynamically discovers packages usingnx show projects, this list requires manual maintenance.Consider using the same dynamic discovery approach for consistency.
Suggested approach
- name: Publish packages to local Verdaccio run: | VERSION="${{ steps.version.outputs.version }}" PR_NUM="${{ inputs.pr_number }}" FAILED_PACKAGES=() PUBLISHED_PACKAGES=() - # Publish libs packages - LIBS_PACKAGES=(di utils uipack sdk ui adapters testing cli plugins) - for pkg in "${LIBS_PACKAGES[@]}"; do + # Discover and publish all library packages + for dist_dir in libs/*/dist plugins/*/dist; do + if [ -d "$dist_dir" ]; then + pkg=$(basename "$(dirname "$dist_dir")") + echo "Publishing @frontmcp/$pkg@$VERSION..." + if npm publish "$dist_dir" \ + --registry http://localhost:4873 \ + --access public \ + --tag "pr-$PR_NUM"; then + PUBLISHED_PACKAGES+=("@frontmcp/$pkg") + else + echo "Warning: publish failed for $pkg" + FAILED_PACKAGES+=("@frontmcp/$pkg") + fi + fi + done.github/workflows/push.yml (1)
52-54: Nx cache restore usage is consistent; consider whether tests/coverage should also save cache (optional).
Restoring before Nx commands in each job is consistent and should improve hit rates across runs. If you want maximum benefit, consider also saving cache at the end ofunit-tests,e2e-tests, and/orcoveragejobs (depending on whether those targets materially populate the cache in your Nx version/config).Also applies to: 87-89, 146-148, 199-201, 246-248
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/cherry-pick-prompt.yml.github/workflows/create-release-branch.yml.github/workflows/pr-testing-registry.yml.github/workflows/publish-release.yml.github/workflows/push.yml.github/workflows/trigger-docs-update.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Package E2E (Verdaccio)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (18)
.github/workflows/publish-release.yml (4)
316-330: LGTM!The tag creation and push logic is well-structured. Fetching and checking out the latest branch state before tagging ensures the tag points to the correct commit including the version bump.
48-66: Well-structured job outputs for downstream jobs.The prepare job cleanly exposes all necessary outputs (version, npm_tag, is_prerelease, release_type, branch, release_line, matrix) for the downstream publish and finalize jobs. This promotes good separation of concerns.
267-285: The review comment is incorrect. OIDC/trusted publishing does not require NODE_AUTH_TOKEN or --provenance flag.With npm CLI ≥ 11.5.1, the workflow is correctly configured for OIDC-based trusted publishing:
- The
id-token: writepermission enables OIDC token mintingsetup-nodewithregistry-urlconfigures the registry- npm automatically handles authentication and provenance generation
No explicit
NODE_AUTH_TOKENenvironment variable is needed—npm will use OIDC tokens automatically. Similarly, the--provenanceflag is not required; npm includes provenance attestations by default in the trusted publishing flow.Likely an incorrect or invalid review comment.
197-201: > Likely an incorrect or invalid review comment..github/workflows/cherry-pick-prompt.yml (2)
99-117: LGTM!The cherry-pick logic correctly handles both regular and merge commits. Using
--no-commitfollowed by a custom commit withCo-Authored-Byattribution maintains proper authorship tracking. The conflict detection and abort handling is robust.
185-251: Well-designed conflict handling with actionable instructions.The issue creation for conflicts includes comprehensive manual cherry-pick instructions, making it easy for developers to resolve. The separation of issue creation from assignment with
continue-on-erroron assignment is a good defensive pattern..github/workflows/trigger-docs-update.yml (3)
62-76: LGTM!The version extraction from package.json uses Node.js for reliable JSON parsing with proper error handling. The validation checks ensure the workflow fails fast with clear error messages if the version cannot be determined.
195-230: LGTM!The repository_dispatch implementation is well-structured with proper HTTP status handling. The payload includes comprehensive context for the external docs workflow. Error cases (404, other failures) provide helpful diagnostic output.
108-113: Good fallback handling for workflow_dispatch context.Using
${{ vars.DEFAULT_BRANCH || 'main' }}provides a sensible fallback when repository context is limited during workflow_dispatch. This is a defensive pattern that prevents failures in edge cases..github/workflows/pr-testing-registry.yml (2)
443-462: LGTM!The cleanup step correctly uses
always()to ensure resources are cleaned up regardless of job status. The cleanup handles both Verdaccio and ngrok processes, and removes temporary files.
133-140: ngrok installation uses dynamic distro detection—good practice.Using
$(lsb_release -cs)instead of hardcoding "buster" ensures compatibility across different Ubuntu versions. This is a robust approach for long-term maintainability..github/workflows/create-release-branch.yml (5)
81-85: Good security practice: option injection prevention.Checking that
BASEdoesn't start with-prevents potential command injection via malformed input. This is a thoughtful security measure.
101-133: LGTM!The branch creation logic has proper guards: checking for existing branches, validating the base branch exists on the remote, and using
origin/$BASEfor checkout. The two-step validation (input format + remote existence) is thorough.
176-183: Nice integration with trigger-docs-update workflow.Creating the
.release-docs-basemarker file with the base branch SHA enables incremental docs updates. This allowstrigger-docs-update.ymlto compute proper diffs for documentation changes specific to this release branch.
145-149: LGTM!The tag filtering logic correctly excludes pre-release versions (rc/beta) when determining the last stable release. The command group with
|| trueproperly handles the case where no matching tags exist, maintaining compatibility withset -euo pipefail.
204-209: Graceful handling of optional archive script.Checking for the existence of
archive-and-publish-docs.mjsbefore attempting to run it allows the workflow to function even if the script hasn't been added yet. This is good for incremental adoption..github/workflows/push.yml (2)
3-9: Branch trigger expansion looks correct; sanity-check intended release flow.
Addingnext/**,release/**,hotfix/**is straightforward. Just confirm you don’t also need tag triggers (e.g.,v*) for your release/publish process, since this workflow is now strictly branch-push driven.
12-12: Concurrency grouping is good (per-branch serialization).
group: ${{ github.workflow }}-${{ github.ref }}should prevent overlapping runs on the same branch while allowing parallelism across branches.
…prove diff syntax
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/push.yml (1)
93-117: Artifact upload may fail or upload incomplete artifacts if retry fails.The retry build step (line 102-104) doesn't have
continue-on-error, so if it fails, the job should fail. However, the artifact upload step (lines 110-117) has no condition and will attempt to run regardless of the build outcome.Consider adding a success condition to prevent uploading potentially incomplete build artifacts:
♻️ Suggested fix
- name: Upload build artifacts + if: success() uses: actions/upload-artifact@v4 with: name: dist.github/workflows/trigger-docs-update.yml (3)
62-71: Consider usingjqfor JSON parsing instead of Node.js.While ubuntu-latest runners have Node.js pre-installed, using
jqfor JSON parsing in shell scripts is more idiomatic and doesn't rely on implicit tool availability. This also makes the workflow more explicit about its dependencies.♻️ Proposed refactor using jq
- VERSION=$(node -e " - try { - const pkg = require('./package.json'); - if (!pkg.version) throw new Error('No version field'); - console.log(pkg.version); - } catch (e) { - console.error('Failed to read version:', e.message); - process.exit(1); - } - ") + VERSION=$(jq -r '.version // empty' ./package.json) if [ -z "$VERSION" ]; then echo "::error::Failed to extract version from package.json" exit 1 fi
121-134: Consider more defensive marker file parsing.The
tr -d ' \n\r'approach works but could be more explicit. Usinghead -1to read only the first line before trimming would be clearer and more defensive against unexpected file content.♻️ Proposed refactor
if [ -f "$MARKER_FILE" ]; then - MARKER_SHA=$(tr -d ' \n\r' < "$MARKER_FILE") + MARKER_SHA=$(head -1 "$MARKER_FILE" | tr -d '[:space:]') if git rev-parse --verify --quiet "${MARKER_SHA}^{commit}" >/dev/null; then
79-87: Version parsing assumes standard semver format.The current parsing logic works for standard semver versions (e.g., "0.7.1" or "0.7.1-beta.1") but may produce unexpected results for non-standard formats like "1.0" (one dot) or "1" (no dots). Consider adding validation to ensure the version string contains at least one dot before parsing.
🔍 More robust version parsing
# Extract minor version (e.g., 0.7 from 0.7.1 or 0.7.1-beta.1) # Handle pre-release versions by stripping the pre-release suffix first BASE_VERSION=$(echo "$VERSION" | sed 's/-.*$//') + + # Validate format before parsing + if ! echo "$BASE_VERSION" | grep -qE '^[0-9]+\.[0-9]+(\.[0-9]+)?$'; then + echo "::error::Invalid semver format: $VERSION" + exit 1 + fi + VERSION_MINOR=$(echo "$BASE_VERSION" | awk -F. '{print $1"."$2}') if [ -z "$VERSION_MINOR" ]; then
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/push.yml.github/workflows/trigger-docs-update.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
📚 Learning: 2026-01-06T17:16:04.304Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T17:16:04.304Z
Learning: Nx-based monorepo build system - use commands like `nx build sdk`, `nx test ast-guard`, `nx run-many -t test`
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Package E2E (Verdaccio)
🔇 Additional comments (8)
.github/workflows/push.yml (7)
3-9: LGTM!The expanded branch triggers appropriately cover the typical Git flow patterns for main, feature previews (
next/**), releases, and hotfixes.
11-13: LGTM!Including
github.workflowin the concurrency group prevents unrelated workflows from cancelling each other on the same ref.
52-54: LGTM!Restoring Nx cache before linting improves performance. Not saving cache here is appropriate since linting doesn't produce new cacheable artifacts.
146-148: LGTM!Cache restore is correctly placed after artifact download, ensuring build outputs are available.
199-216: LGTM!The cache restore and retry pattern for E2E tests is consistent with the build job approach. Running without cache save in this matrix job is appropriate since the build job handles cache persistence.
246-248: LGTM!Cache restore in the coverage job is appropriate for improving performance of the coverage collection steps.
218-223: Verify: Coverage runs only onmaindespite expanded triggers.The workflow now triggers on
main,next/**,release/**, andhotfix/**, but the coverage job's condition (line 223) restricts it torefs/heads/mainonly. This is likely intentional, but confirm that skipping coverage on release/hotfix branches is the desired behavior..github/workflows/trigger-docs-update.yml (1)
1-244: Well-structured workflow with robust error handling.The workflow demonstrates good practices with comprehensive error handling (
set -euo pipefail), clear separation of concerns across steps, proper handling of both manual and automated triggers, and detailed logging. The external repository dispatch mechanism is implemented correctly with appropriate HTTP status code handling.
Summary by CodeRabbit
New Features
Chores
Removed
✏️ Tip: You can customize this high-level summary in your review settings.