Skip to content

CLI template issues: invalid engine constraints, missing CI workflow, and failing architecture checks#245

Merged
GeraBart merged 1 commit intocyberfabric:developfrom
Mitriyweb:cli-template-issues
Mar 30, 2026
Merged

CLI template issues: invalid engine constraints, missing CI workflow, and failing architecture checks#245
GeraBart merged 1 commit intocyberfabric:developfrom
Mitriyweb:cli-template-issues

Conversation

@Mitriyweb
Copy link
Copy Markdown
Collaborator

@Mitriyweb Mitriyweb commented Mar 27, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Generated projects now include a pre-configured GitHub Actions CI workflow that automatically uses the Node.js version from .nvmrc
    • Improved architecture validation for newly scaffolded projects with Node version-aware checks
  • Chores

    • Updated minimum Node.js version requirement to 24.14.0

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2cf97751-423f-4aab-96d6-975e5ee4fb14

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Mitriyweb Mitriyweb changed the title CLI template issues: invalid engine constraints, missing CI workflow,… CLI template issues: invalid engine constraints, missing CI workflow, and failing architecture checks Mar 27, 2026
@Mitriyweb Mitriyweb force-pushed the cli-template-issues branch from 86c5723 to ed7329a Compare March 27, 2026 16:27
@Mitriyweb Mitriyweb requested a review from GeraBart March 28, 2026 06:55
@Mitriyweb Mitriyweb force-pushed the cli-template-issues branch 6 times, most recently from 105e922 to 05e1564 Compare March 28, 2026 12:59
Comment thread architecture/features/feature-cli-scaffolding-quality/FEATURE.md Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@architecture/DECOMPOSITION.md`:
- Around line 602-627: Update the decomposition metadata to include the new
feature: increment the total features count on Line 25 from 11 to 12 and add
cpt-frontx-feature-cli-scaffolding-quality to the Section 3 dependency
list/graph so it appears as a dependent node (ensure it links to its declared
dependency cpt-frontx-feature-cli-tooling and is represented in any bullet lists
or graph markup used in Section 3). Ensure the feature ID
cpt-frontx-feature-cli-scaffolding-quality appears in the summary/dependency
graph and that any counts or indexes elsewhere that enumerate features are
adjusted accordingly.

In `@packages/cli/src/generators/project.ts`:
- Around line 414-416: The CI workflow file was added to ROOT_CONFIG_FILES but
transformPackageManagerText is changing "npm ci" into non-deterministic
installer commands (e.g., "pnpm install" / "yarn install"); add a CI-specific
transformation that returns the lockfile-enforcing equivalents (pnpm install
--frozen-lockfile, yarn install --immutable, keep npm ci) instead of the generic
transformPackageManagerText result, implement it as a new helper (e.g.,
transformPackageManagerTextForCI or packageManagerInstallCommandCI) and update
the codepath that processes '.github/workflows/ci.yml' so it calls this
CI-specific function rather than transformPackageManagerText.

In `@packages/cli/template-sources/project/scripts/test-architecture.ts`:
- Around line 119-124: The Node version gate in test-architecture.ts currently
checks nodeVersion >= 25 which skips the arch:deps check for projects using
.nvmrc 24.x; change the condition to nodeVersion >= 24 so the checks.push({
command: runScriptCommand(packageManager, 'arch:deps'), description: 'Dependency
rules' }) branch runs for Node 24, keeping the log(...) fallback only for older
Node versions; update the numeric comparison where nodeVersion is derived from
process.versions.node.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6655c37-f8de-441b-8a7a-87bd3dc80912

📥 Commits

Reviewing files that changed from the base of the PR and between f88fd38 and 05e1564.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • architecture/DECOMPOSITION.md
  • architecture/features/feature-cli-scaffolding-quality/FEATURE.md
  • package.json
  • packages/cli/scripts/copy-templates.ts
  • packages/cli/scripts/e2e-pr-smoke.mjs
  • packages/cli/src/core/packageManager.ts
  • packages/cli/src/generators/project.ts
  • packages/cli/template-sources/project/configs/_github/workflows/ci.yml
  • packages/cli/template-sources/project/scripts/test-architecture.ts

Comment thread architecture/DECOMPOSITION.md Outdated
Comment thread packages/cli/src/generators/project.ts Outdated
Comment thread packages/cli/template-sources/project/scripts/test-architecture.ts Outdated
@GeraBart
Copy link
Copy Markdown
Collaborator

GeraBart commented Mar 30, 2026

Review Summary

Verdict: REQUEST CHANGES — 4 blocking issues found.

1. Remove the new FEATURE document (architecture)

This PR is bug fixes + a small template enhancement, not a new feature. The scope belongs in the existing cpt-frontx-feature-cli-tooling — add CI workflow generation as a new algorithm/DoD there. Remove feature-cli-scaffolding-quality/FEATURE.md and revert the DECOMPOSITION renumbering.

2. npm ci loses lockfile enforcement (correctness)

transformPackageManagerText replaces npm ci with pnpm install / yarn install (via getInstallCommand). These don't enforce the lockfile. CI equivalents are pnpm install --frozen-lockfile and yarn install --immutable. Add a getCiInstallCommand helper or fix the existing replacement.

3. Node version gate >= 25 should be >= 24 (correctness)

Template .nvmrc is 24.14.0 (LTS). Gate at >= 25 means arch:deps is silently skipped on every generated project. Change to >= 24.

4. "hai3 create" references remain (naming)

As flagged earlier — Overview (1.1) and Actor Flow step 1 still say hai3 create. Replace with frontx create.

Recommended (non-blocking)

  • Rename "HAI3 Architecture Validation""FrontX Architecture Validation" in test-architecture.ts:144
  • Add unit tests for new transformPackageManagerText cache-key branches
  • Comment explaining why generate:colors was removed from standalone checks

@Mitriyweb Mitriyweb force-pushed the cli-template-issues branch 3 times, most recently from 47c2822 to 8b374e2 Compare March 30, 2026 10:31
@GeraBart
Copy link
Copy Markdown
Collaborator

Re-review: ✅ APPROVE

All 4 previously raised blocking issues are resolved:

Issue Resolution
Remove FEATURE document FEATURE.md deleted, DECOMPOSITION reverted
npm ci lockfile enforcement getCiInstallCommand added with --frozen-lockfile / --immutable
Node gate >=25>=24 ✅ Fixed, uses Number.parseInt
hai3 create references ✅ Fixed in presets/index.ts and types.ts; HAI3→FrontX in test-architecture.ts

No new blocking issues found. CI template actions (@v6) are valid. Code logic is sound.

Recommended (non-blocking)

  1. Add unit tests for getCiInstallCommand (3 branches: npm/pnpm/yarn)
  2. Add unit tests for new transformPackageManagerText cache-key regex branches
  3. Comment explaining why generate:colors was removed from standalone checks

Full review: .prs/245/review.md

… and failing architecture checks

Signed-off-by: Mitriyweb <mitriyweb@gmail.com>
@Mitriyweb Mitriyweb force-pushed the cli-template-issues branch from 8b374e2 to 8545c72 Compare March 30, 2026 13:48
@sonarqubecloud
Copy link
Copy Markdown

@GeraBart GeraBart merged commit a13dfd9 into cyberfabric:develop Mar 30, 2026
12 checks passed
@Mitriyweb Mitriyweb deleted the cli-template-issues branch March 31, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants