feat(enterprise): add Pro upgrade pipeline#704
Merged
rafaelscosta merged 2 commits intomainfrom May 8, 2026
Merged
Conversation
rafaelscosta
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
area: agents
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
WalkthroughAdds a full Enterprise upgrade subsystem: manifest schema and loader, detection of Core/Pro/Enterprise installs, plan generation and CLI, transactional upgrader with backups and rollback, utilities, unit tests, and an end-to-end smoke test. ChangesEnterprise Upgrade System
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as bin/aiox.js
participant Plan as enterprise-upgrade-plan
participant Upgrader as enterprise-upgrader
participant FS as filesystem
User->>CLI: aiox enterprise upgrade --enterprise-source ...
CLI->>Plan: runEnterpriseUpgradeCli(argv)
Plan-->>User: plan (dry-run) or writes plan file
User->>CLI: aiox enterprise upgrade --apply --enterprise-source ...
CLI->>Upgrader: applyEnterpriseUpgrade(plan, options)
Upgrader->>FS: expand source, backup files, write files, write execution manifest
User->>CLI: aiox enterprise upgrade rollback --manifest ...
CLI->>Upgrader: rollbackEnterpriseUpgrade(manifest)
Upgrader->>FS: restore backed-up files, write rollback manifest
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/installer/tests/unit/enterprise/enterprise-upgrade-plan.test.js (1)
8-12: 💤 Low valueSame absolute-import guideline applies here.
The same relative-import note flagged on
enterprise-upgrader.test.jsapplies —'../../../src/enterprise/enterprise-upgrade-plan'can be replaced with'@aiox-squads/core/installer/enterprise-upgrade-plan'per the project's coding guideline. Defer until convention is confirmed across the test tree.As per coding guidelines: "Use absolute imports instead of relative imports in all code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/installer/tests/unit/enterprise/enterprise-upgrade-plan.test.js` around lines 8 - 12, Replace the relative import of '../../../src/enterprise/enterprise-upgrade-plan' with the project-standard absolute module import '@aiox-squads/core/installer/enterprise-upgrade-plan' so tests import buildEnterpriseUpgradePlan, runEnterpriseUpgradeCli, and writeEnterpriseUpgradePlan via the absolute path; update the require statement in enterprise-upgrade-plan.test.js to use that module name to comply with the absolute-import guideline.bin/aiox.js (1)
78-79: 💤 Low valueOptional: harmonize
--targetmention across help blocks.The top USAGE block (lines 78-79) shows
--dry-run --enterprise-source <path>while the ENTERPRISE section (lines 116-117) consistently leads with--target .. Since--targetdefaults to cwd this isn't incorrect, but a one-line tweak in USAGE would prevent users from missing the flag.Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bin/aiox.js` around lines 78 - 79, Update the USAGE help string that currently reads "npx aiox-core@latest enterprise upgrade --dry-run --enterprise-source <path>" to include the --target flag (e.g., "--target <path|.>") or note its default, matching the ENTERPRISE section which leads with "--target ."; locate and edit the string in the help generation for the enterprise upgrade command (the literal command line shown) so both USAGE and the ENTERPRISE section consistently mention --target to avoid confusion.packages/installer/src/enterprise/enterprise-upgrader.js (1)
265-297: 💤 Low value
sha256File(destPath)called twice on the merged result inapplyMergeYamlLine 289 and line 295 both call
sha256File(destPath)after the YAML write; nothing changes between the two calls so the results are always identical. Cache the hash from the first call.♻️ Proposed fix
- executionManifest.merged.push({ + const mergedHash = sha256File(destPath); + executionManifest.merged.push({ path: relativePath, policy: 'merge-yaml', - sha256: sha256File(destPath), + sha256: mergedHash, backedUp: existed, }); executionManifest.validated.push({ path: relativePath, type: 'exists', - sha256: sha256File(destPath), + sha256: mergedHash, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/installer/src/enterprise/enterprise-upgrader.js` around lines 265 - 297, applyMergeYaml currently calls sha256File(destPath) twice after writeYaml; compute the hash once and reuse it to avoid redundant work. After writeYaml(destPath, merged) call sha256File(destPath) one time (e.g., const sha = sha256File(destPath)) and replace both sha256File(destPath) occurrences in executionManifest.merged and executionManifest.validated with that cached variable; keep the rest of applyMergeYaml logic unchanged.packages/installer/src/enterprise/enterprise-rollback.js (1)
7-7: ⚡ Quick winExtract
sha256Fileto a shared utility to break the cross-module coupling
enterprise-rollback.jsimports a single hashing utility fromenterprise-upgrader.js.enterprise-upgrader.jsin turn imports fromenterprise-upgrade-plan.js, which dynamicallyrequiresenterprise-rollback.jsat runtime. This creates a fragile load-order dependency: any future top-level import ofenterprise-rollbackinsideenterprise-upgrade-planwould form a real circular dependency. Movingsha256File(and optionallynowStamp) to anenterprise-utils.jsfile removes this risk with minimal churn.♻️ Proposed refactor
New file
packages/installer/src/enterprise/enterprise-utils.js:'use strict'; const crypto = require('crypto'); const fs = require('fs-extra'); function sha256File(filePath) { return crypto.createHash('sha256').update(fs.readFileSync(filePath)).digest('hex'); } module.exports = { sha256File };Then in
enterprise-rollback.js:-const { sha256File } = require('./enterprise-upgrader'); +const { sha256File } = require('./enterprise-utils');And in
enterprise-upgrader.js:-function sha256File(filePath) { ... } +const { sha256File } = require('./enterprise-utils');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/installer/src/enterprise/enterprise-rollback.js` at line 7, The import of sha256File in enterprise-rollback.js creates a fragile load-order coupling with enterprise-upgrader.js/enterprise-upgrade-plan.js; extract sha256File (and optionally nowStamp) into a new shared module named enterprise-utils.js that exports sha256File, then update enterprise-rollback.js and enterprise-upgrader.js to require './enterprise-utils' instead of requiring across modules; ensure the new sha256File implementation uses crypto and fs-extra as in the proposal and remove the original sha256File definition/import from enterprise-upgrader.js so the circular load dependency is eliminated.scripts/e2e/pro-to-enterprise-upgrade-smoke.js (1)
148-148: 💤 Low value
asynconmain()is unnecessaryNo
awaitexpression exists anywhere in the function body; all operations are synchronous. Theasyncmodifier only adds a Promise wrapper that the.catch()at line 227 consumes. A plain synchronous function with a top-leveltry/catchwould be clearer.♻️ Proposed refactor
-async function main() { +function main() { try { ... } finally { cleanup(); } } -main().catch((error) => { +try { + main(); +} catch (error) { console.error(`[pro-enterprise-e2e] FAIL: ${error.message}`); process.exit(1); -}); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/e2e/pro-to-enterprise-upgrade-smoke.js` at line 148, The function declaration async function main() has no await usages; remove the async modifier (make it function main()) and convert the top-level .catch() invocation into a plain try/catch around the synchronous call: call main() inside try { main(); } catch (err) { /* reuse existing error handling logic */ } so that error handling remains but the unnecessary Promise wrapper is eliminated; update references to main() and the existing .catch() chain accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/installer/src/enterprise/enterprise-upgrade-plan.js`:
- Around line 203-233: parseEnterpriseUpgradeArgs currently assigns argv[index +
1] to flags that expect values without checking that a value exists or isn't
another flag; update the parsing for '--target', '--enterprise-source',
'--manifest', '--plan', and '--format' so that before setting
options.targetDir/options.enterpriseSource/options.manifestPath/options.planPath/options.format
you verify argv[index + 1] exists and does not start with '-' (i.e., is not
another flag); if the guard fails, throw an EnterpriseUpgradeError indicating a
missing value for that specific option instead of consuming the next flag as the
value.
- Line 44: The warning string is stale and is always added by buildWarnings
(called from buildEnterpriseUpgradePlan and applyEnterpriseUpgrade), so update
buildWarnings to only push "Dry-run only: apply, rollback, and validation policy
are implemented in later slices." when the invocation is actually a dry-run (or
when a flag like options.mode === 'dry-run' or options.dryRun is true); locate
and modify buildWarnings (and any caller that builds options passed into it,
e.g., buildEnterpriseUpgradePlan and applyEnterpriseUpgrade) so they pass the
current mode/flag and the warning is conditionally appended rather than
unconditionally pushed.
In `@packages/installer/src/enterprise/enterprise-upgrader.js`:
- Around line 44-63: mergeDeep currently returns target when either side is an
array, which silently drops enterprise array values; change array handling in
mergeDeep to perform a union merge when both source and target are arrays
(preserve existing target items and append any source items not already
present), e.g., detect Array.isArray(source) && Array.isArray(target) and return
a deduplicated concatenation that keeps target order, while keeping the existing
behavior for when only one side is an array (return the defined side); update
mergeDeep (function name) so array-merge happens before the early return that
currently returns target unconditionally.
---
Nitpick comments:
In `@bin/aiox.js`:
- Around line 78-79: Update the USAGE help string that currently reads "npx
aiox-core@latest enterprise upgrade --dry-run --enterprise-source <path>" to
include the --target flag (e.g., "--target <path|.>") or note its default,
matching the ENTERPRISE section which leads with "--target ."; locate and edit
the string in the help generation for the enterprise upgrade command (the
literal command line shown) so both USAGE and the ENTERPRISE section
consistently mention --target to avoid confusion.
In `@packages/installer/src/enterprise/enterprise-rollback.js`:
- Line 7: The import of sha256File in enterprise-rollback.js creates a fragile
load-order coupling with enterprise-upgrader.js/enterprise-upgrade-plan.js;
extract sha256File (and optionally nowStamp) into a new shared module named
enterprise-utils.js that exports sha256File, then update enterprise-rollback.js
and enterprise-upgrader.js to require './enterprise-utils' instead of requiring
across modules; ensure the new sha256File implementation uses crypto and
fs-extra as in the proposal and remove the original sha256File definition/import
from enterprise-upgrader.js so the circular load dependency is eliminated.
In `@packages/installer/src/enterprise/enterprise-upgrader.js`:
- Around line 265-297: applyMergeYaml currently calls sha256File(destPath) twice
after writeYaml; compute the hash once and reuse it to avoid redundant work.
After writeYaml(destPath, merged) call sha256File(destPath) one time (e.g.,
const sha = sha256File(destPath)) and replace both sha256File(destPath)
occurrences in executionManifest.merged and executionManifest.validated with
that cached variable; keep the rest of applyMergeYaml logic unchanged.
In `@packages/installer/tests/unit/enterprise/enterprise-upgrade-plan.test.js`:
- Around line 8-12: Replace the relative import of
'../../../src/enterprise/enterprise-upgrade-plan' with the project-standard
absolute module import '@aiox-squads/core/installer/enterprise-upgrade-plan' so
tests import buildEnterpriseUpgradePlan, runEnterpriseUpgradeCli, and
writeEnterpriseUpgradePlan via the absolute path; update the require statement
in enterprise-upgrade-plan.test.js to use that module name to comply with the
absolute-import guideline.
In `@scripts/e2e/pro-to-enterprise-upgrade-smoke.js`:
- Line 148: The function declaration async function main() has no await usages;
remove the async modifier (make it function main()) and convert the top-level
.catch() invocation into a plain try/catch around the synchronous call: call
main() inside try { main(); } catch (err) { /* reuse existing error handling
logic */ } so that error handling remains but the unnecessary Promise wrapper is
eliminated; update references to main() and the existing .catch() chain
accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dca13969-1bf6-4232-8d4b-5e2d1f586898
📒 Files selected for processing (14)
bin/aiox.jspackage.jsonpackages/installer/src/enterprise/enterprise-detector.jspackages/installer/src/enterprise/enterprise-errors.jspackages/installer/src/enterprise/enterprise-manifest-loader.jspackages/installer/src/enterprise/enterprise-rollback.jspackages/installer/src/enterprise/enterprise-upgrade-manifest.schema.jsonpackages/installer/src/enterprise/enterprise-upgrade-manifest.yamlpackages/installer/src/enterprise/enterprise-upgrade-plan.jspackages/installer/src/enterprise/enterprise-upgrader.jspackages/installer/tests/unit/enterprise/enterprise-manifest-loader.test.jspackages/installer/tests/unit/enterprise/enterprise-upgrade-plan.test.jspackages/installer/tests/unit/enterprise/enterprise-upgrader.test.jsscripts/e2e/pro-to-enterprise-upgrade-smoke.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
aiox enterprise upgradedry-run/apply/rollback CLI for Core + Pro to Enterprise migrations.Boundary
Validation
npm run test:e2e:pro-enterprise-upgradenpm test -- packages/installer/tests/unit/enterprise --runInBandnpm run typechecknpm run lint(0 errors; 3 existing warnings intests/ide-sync/kimi-transformer.test.js)npm test -- --runInBandcompleted with one timing threshold failure intests/unit/squad/squad-loader.test.js(126ms vs 100ms); isolated rerun passed (32/32). Jest held an open handle after summary and was terminated after output was captured.Summary by CodeRabbit
New Features
enterprise upgradeCLI to plan and apply migrations from Pro to Enterprise, with--dry-run/--apply,--enterprise-source, and--planoutput.Documentation
Tests