feat: align aiox-pro naming, sync, and update flow#625
Open
rafaelscosta wants to merge 15 commits intomainfrom
Open
feat: align aiox-pro naming, sync, and update flow#625rafaelscosta wants to merge 15 commits intomainfrom
rafaelscosta wants to merge 15 commits intomainfrom
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an automated "Sync Pro Submodule Fallback" GitHub Actions workflow that resolves a target commit from the upstream Pro repo (or uses an input SHA), compares it with this repo's Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Trigger as "Cron / Manual Trigger"
participant Runner as "GH Actions Runner"
participant Upstream as "aiox-pro (Git)"
participant Repo as "aiox-core (repo with `pro` submodule)"
participant GH as "GitHub API / gh CLI"
Trigger->>Runner: start workflow
Runner->>Upstream: resolve upstream URL (from .gitmodules or fallback)
Runner->>Upstream: determine target SHA (input or upstream main)
Runner->>Repo: read current `pro` submodule SHA
Runner->>Runner: compare SHAs
alt SHAs differ
Runner->>Repo: remove existing `pro` dir
Runner->>Upstream: clone/fetch target commit (authenticated if token)
Runner->>Repo: checkout target commit in `pro`
Runner->>Repo: stage submodule pointer, commit to bot/sync-pro-submodule
Runner->>GH: force-push branch
Runner->>GH: create or update PR with prior/target SHAs and trigger info
else SHAs match
Runner->>Runner: exit no-op
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sync-pro-submodule.yml:
- Around line 22-24: The workflow sets PRO_REPO_URL to a different repository
than the submodule actually tracks, causing TARGET_SHA resolution to use the
wrong remote; change the logic so TARGET_SHA is resolved from the submodule's
actual remote (either update PRO_REPO_URL to match the submodule remote declared
in .gitmodules or compute the remote at runtime via git -C pro remote get-url
origin) and use that derived URL when fetching/checkout the submodule; update
every place that currently relies on PRO_REPO_URL/TARGET_SHA (the job steps that
set TARGET_SHA and the subsequent fetch/checkout steps) to use the corrected
remote resolution.
- Around line 26-30: The checkout step using actions/checkout@v4 currently omits
authentication and will fail when fetching the private SynkraAI/aios-pro
submodule; update the Checkout aiox-core with submodules step to pass token: ${{
secrets.PRO_SUBMODULE_TOKEN }} to actions/checkout (keeping fetch-depth: 0 and
submodules: recursive) and add a short comment noting that PRO_SUBMODULE_TOKEN
must be configured in repository secrets so the later jobs ("Resolve upstream
target" and "Update pro submodule pointer") can access the private submodule.
In `@docs/stories/epic-123/STORY-123.1-pro-sync-automation.md`:
- Around line 41-48: The markdown in STORY-123.1-pro-sync-automation.md contains
workstation-local absolute file links that must be replaced: change each
/Users/rafaelcosta/... entry to repository-safe references—use
repository-relative links for files that live in aiox-core (e.g.,
docs/stories/epic-123/STORY-123.1-pro-sync-automation.md,
.github/workflows/sync-pro-submodule.yml, .github/workflows/sync-aiox-core.yml)
and use plain inline code references (e.g., `package.json`, `squads/README.md`,
`scripts/validate-publish-surface.js`) or canonical cross-repo URLs for files
that live in the aios-pro repo (.github/workflows/ci.yml,
.github/workflows/publish.yml); update those link targets in the document so no
/Users/... paths remain.
🪄 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: 00f14639-db01-4347-97be-74e732f72d82
📒 Files selected for processing (2)
.github/workflows/sync-pro-submodule.ymldocs/stories/epic-123/STORY-123.1-pro-sync-automation.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sync-pro-submodule.yml:
- Around line 64-73: The workflow currently may continue with an empty
TARGET_SHA when git ls-remote returns nothing; after the TARGET_SHA resolution
(the block setting TARGET_SHA and the git ls-remote call), add a validation
check that TARGET_SHA is non-empty and if empty write a clear error to stderr
(or use echo "::error::..."), and exit non-zero (e.g., exit 1) to fail fast;
likewise validate CURRENT_SHA (the git ls-tree result) and fail with an
explanatory message if it’s empty before writing outputs like "target_sha" and
"current_sha" to GITHUB_OUTPUT.
- Around line 26-32: The core checkout should not depend on PRO_SUBMODULE_TOKEN;
update the actions/checkout step that currently sets token: ${{
secrets.PRO_SUBMODULE_TOKEN || github.token }} to always use github.token so the
core-repo push/auth is decoupled from the pro remote PAT, and change the
upstream resolution step that currently uses repo_url to use auth_url instead so
upstream SHA resolution is performed with authenticated URL; locate the
actions/checkout step (uses: actions/checkout@v4) and the step referencing
repo_url/auth_url and replace token usage and repo_url with auth_url
respectively.
🪄 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: e14ca179-470b-4ba5-be64-e5b54ce0c56d
📒 Files selected for processing (2)
.github/workflows/sync-pro-submodule.ymldocs/stories/epic-123/STORY-123.1-pro-sync-automation.md
…stack/pro [Story 122.1] - pro-detector: add npm package resolution with canonical/fallback priority - pro CLI commands: resolve license path from both scopes - pro-setup wizard: loadProModule tries both npm scopes - pro-scaffolder: resolve source dir from both npm scopes - aiox-pro-cli: isProInstalled checks both scopes - i18n: update user-facing messages to active package name - CI: update publish workflow to active package name - tests: update pro-detector tests for new API (21/21 passing) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Add pro-updater.js: detect installed Pro, query npm, check compat, update, re-scaffold - Register 'aiox pro update' subcommand with --check, --dry-run, --force, --include-core, --skip-scaffold - Add 'aiox update --include-pro' to update core + Pro in one command - License validation before update (skip on --check/--dry-run) - Detects package manager (npm/pnpm/yarn/bun) automatically - Re-scaffolds Pro assets (squads, skills, configs) after package update - Formatted CLI output with update summary Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
rafaelscosta
changed the title
Collaborator
Author
|
Atualização de contexto para review:
Neste ponto, a PR ficou alinhada com o rename para |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
docs/stories/epic-123/STORY-123.2-aiox-pro-alignment-and-update-flow.md (1)
44-44: Consider moving account-specific npm access details out of the story file.This line is highly time-sensitive and person-specific; it may age quickly and create maintenance noise in long-lived docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/stories/epic-123/STORY-123.2-aiox-pro-alignment-and-update-flow.md` at line 44, The sentence "Estado do npm em 2026-04-11: a conta local `rafaelscosta` tem administração no org `@aios-fullstack`, mas não possui acesso ao org `@aiox-fullstack`..." is account- and time-specific and should be removed from the public story; replace it with a generic note such as "Verify package publication permissions with the npm org owners" or move the detailed account-specific info to an internal/team-only document (e.g., a private ops/or permissions checklist) and add a short pointer in the story to that private doc; update the story file (STORY-123.2-aiox-pro-alignment-and-update-flow.md) to use the generic sentence or pointer and ensure no personal account names or dates remain.bin/utils/pro-detector.js (1)
95-107: Only treatMODULE_NOT_FOUNDas “not installed”.These broad catches also suppress real initialization errors from the resolved Pro module and silently fall back to another source. That can hide a broken npm install and load stale code from
pro/instead of surfacing the actual failure.packages/installer/src/wizard/pro-setup.js:334-346already uses the narrower pattern.Possible fix
+function tryRequire(requestPath) { + try { + return require(requestPath); + } catch (error) { + if ( + error?.code === 'MODULE_NOT_FOUND' + && typeof error.message === 'string' + && error.message.includes(requestPath) + ) { + return null; + } + throw error; + } +} + function loadProModule(moduleName) { // 1. Try npm package const npmPro = resolveNpmProPackage(); if (npmPro) { - try { - return require(path.join(npmPro.packagePath, moduleName)); - } catch { /* fall through */ } + const loaded = tryRequire(path.join(npmPro.packagePath, moduleName)); + if (loaded) return loaded; } // 2. Try submodule if (fs.existsSync(PRO_PACKAGE_PATH)) { - try { - return require(path.join(PRO_DIR, moduleName)); - } catch { /* not available */ } + const loaded = tryRequire(path.join(PRO_DIR, moduleName)); + if (loaded) return loaded; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/utils/pro-detector.js` around lines 95 - 107, The try/catch blocks around require(...) in pro-detector.js are too broad and mask real errors; update both catches (the one after using resolveNpmProPackage()/npmPro and the one after checking PRO_PACKAGE_PATH/PRO_DIR) to only swallow errors whose code === 'MODULE_NOT_FOUND' and rethrow any other error so initialization issues surface; locate the require calls that use npmPro.packagePath and path.join(PRO_DIR, moduleName) and change the catch logic to inspect err.code and only return/fall-through for MODULE_NOT_FOUND, otherwise throw the caught error.tests/pro/pro-updater.test.js (1)
185-190: Add a scoped-peer fixture here too.The updater now detects
@synkra/aiox-core, but the registry fixtures in this suite still only publishpeerDependencies['aiox-core']. A dedicated scoped-peer case would catch compatibility regressions for the renamed package.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/pro/pro-updater.test.js` around lines 185 - 190, The registry fixture used by mockRegistryResponse currently sets peerDependencies['aiox-core'] only; add a scoped-peer case so the mock also includes '@synkra/aiox-core' (e.g. peerDependencies: { 'aiox-core': '>=5.0.0', '@synkra/aiox-core': '>=5.0.0' } or add a separate mockRegistryResponse call that publishes '@synkra/aiox-core' with the same version), so the pro-updater tests exercise detection of the renamed scoped package; update the mockRegistryResponse invocation near the existing one in the pro-updater.test.js suite to include the scoped package.tests/pro/pro-detector.test.js (2)
209-217: Cover the new npm-first branch in the version/info tests.
getProVersion()andgetProInfo()now prefer the installed npm package, but the assertions here still only exercise the submodule path. A regression in the new primary branch would leave this suite green.Also applies to: 253-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/pro/pro-detector.test.js` around lines 209 - 217, The tests only exercise the submodule path; update the test(s) around getProVersion() and getProInfo() (including the other block at lines 253-273) to also cover the npm-first branch by mocking the installed package/package.json lookup (simulate the node_modules resolution used by getProVersion/getProInfo, e.g., mock whatever check your functions use—existsSync/require.resolve/path.join—to return true for the npm package path and return a package.json with the expected version/info), then assert that getProVersion() and getProInfo() prefer and return the npm-installed package values instead of the submodule values.
36-39: Reset thefsmock implementations, not just call history.
jest.clearAllMocks()keeps the previousfs.existsSync/fs.readFileSyncimplementations alive, so these cwd/tmp-dir cases can become order-dependent.jest.resetAllMocks()or explicitmockReset()calls would keep each test isolated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/pro/pro-detector.test.js` around lines 36 - 39, The tests use beforeEach with jest.clearAllMocks(), which leaves mocked implementations like fs.existsSync and fs.readFileSync intact and causes order-dependent failures; update the beforeEach to fully reset mocks (e.g., use jest.resetAllMocks() instead of jest.clearAllMocks()) or explicitly call mockReset() on the fs mocks (reset fs.existsSync and fs.readFileSync) so each beforeEach starting at process.chdir(originalCwd) runs with fresh mock implementations and no stale state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.aiox-core/cli/commands/pro/index.js:
- Around line 717-760: The call to await updatePro(...) can throw and is not
wrapped, so add a try/catch around the updatePro invocation (the block that
requires proUpdaterPath and calls updatePro with the options object and
onProgress) to catch runtime/validation errors; in the catch log a user-friendly
error via console.error (include error.message) and call process.exit(1), and
only call formatUpdateResult(result) and inspect result.success if updatePro
completed successfully (i.e., move those uses into the try block or guard them
by checking result exists).
In @.aiox-core/core/pro/pro-updater.js:
- Around line 363-369: In the no-update re-scaffold branch, replace the raw
projectRoot with the already-validated resolvedProjectRoot when calling
applyScaffoldStep so the code uses the normalized/verified path; update the call
that sets scaffolded (the applyScaffoldStep invocation) to pass
resolvedProjectRoot (not projectRoot) to ensure callers validated relative paths
cannot scaffold into a different path.
- Around line 386-393: The compatibility check only looks up
latest.peerDependencies['aiox-core'] so it misses scoped peer keys like
'@synkra/aiox-core'; update the logic in the block around
coreVersion/requiredCore (where requiredCore is currently assigned from
latest.peerDependencies?.['aiox-core']) to also check for
latest.peerDependencies?.['@synkra/aiox-core'] (e.g. choose the scoped value if
present or fallback to the unscoped one) before calling satisfiesPeer; keep the
rest of the flow the same (use includeCoreUpdate, satisfiesPeer, and set
result.error/result.actions when incompatible).
In `@bin/aiox.js`:
- Around line 347-370: The catch block for the pro update currently swallows
errors and only logs a warning, causing --include-pro to appear successful;
update the error handling around the require(proUpdaterPath) / updatePro(...)
call so that any thrown proError is logged with full details and then causes a
non-zero exit (e.g., call process.exit(1) or rethrow) when updateArgs includes
'--include-pro'; modify the catch that references proError to both console.error
the error (message and stack) and fail fast instead of continuing.
In `@docs/guides/pro/install-gate-setup.md`:
- Around line 56-60: The "Alternativa (instalacao manual)" section incorrectly
repeats the primary installer command ("npx aiox-pro install") and should either
be removed or replaced with a true manual-install fallback; update the
heading/text titled "Alternativa (instalacao manual)" to provide explicit manual
steps (for example: install the package via the package manager, pin a version,
and any required post-install configuration or flags), or delete the redundant
subsection so only the main install command remains; locate the section by the
heading "Alternativa (instalacao manual)" and the duplicate "npx aiox-pro
install" line and change the content to actual manual install guidance (package
install/pin and config steps) or remove it entirely.
In `@docs/stories/epic-123/STORY-123.1-pro-sync-automation.md`:
- Around line 34-39: The runbook lists the expected secret as
AIOX_CORE_SYNC_TOKEN but the referenced fallback workflow actually uses
PRO_SUBMODULE_TOKEN; update the document text so the "Segredo esperado no
`aiox-pro`" entry either names the correct primary secret and mentions
`PRO_SUBMODULE_TOKEN` as the documented fallback, or replace
AIOX_CORE_SYNC_TOKEN with PRO_SUBMODULE_TOKEN if that is the canonical name,
ensuring both `AIOX_CORE_SYNC_TOKEN` and `PRO_SUBMODULE_TOKEN` references in
this section match the authentication used by the fallback workflow.
In `@packages/installer/src/wizard/pro-setup.js`:
- Around line 1224-1237: The current selection of proSourceDir uses
npmProCandidates.find(p => fs.existsSync(p)) which can pick a stale/partial
package; update the selection logic so each candidate from npmProCandidates is
validated for both a package.json and the required 'squads' directory before
being chosen (e.g., check fs.existsSync(path.join(p, 'package.json')) &&
fs.existsSync(path.join(p, 'squads'))); keep the existing bundledProDir check
(fs.existsSync(bundledProDir) && fs.existsSync(path.join(bundledProDir,
'squads'))) and only fall back to a validated npmProCandidates entry when
bundledProDir is not valid, assigning the result to proSourceDir.
---
Nitpick comments:
In `@bin/utils/pro-detector.js`:
- Around line 95-107: The try/catch blocks around require(...) in
pro-detector.js are too broad and mask real errors; update both catches (the one
after using resolveNpmProPackage()/npmPro and the one after checking
PRO_PACKAGE_PATH/PRO_DIR) to only swallow errors whose code ===
'MODULE_NOT_FOUND' and rethrow any other error so initialization issues surface;
locate the require calls that use npmPro.packagePath and path.join(PRO_DIR,
moduleName) and change the catch logic to inspect err.code and only
return/fall-through for MODULE_NOT_FOUND, otherwise throw the caught error.
In `@docs/stories/epic-123/STORY-123.2-aiox-pro-alignment-and-update-flow.md`:
- Line 44: The sentence "Estado do npm em 2026-04-11: a conta local
`rafaelscosta` tem administração no org `@aios-fullstack`, mas não possui acesso
ao org `@aiox-fullstack`..." is account- and time-specific and should be removed
from the public story; replace it with a generic note such as "Verify package
publication permissions with the npm org owners" or move the detailed
account-specific info to an internal/team-only document (e.g., a private ops/or
permissions checklist) and add a short pointer in the story to that private doc;
update the story file (STORY-123.2-aiox-pro-alignment-and-update-flow.md) to use
the generic sentence or pointer and ensure no personal account names or dates
remain.
In `@tests/pro/pro-detector.test.js`:
- Around line 209-217: The tests only exercise the submodule path; update the
test(s) around getProVersion() and getProInfo() (including the other block at
lines 253-273) to also cover the npm-first branch by mocking the installed
package/package.json lookup (simulate the node_modules resolution used by
getProVersion/getProInfo, e.g., mock whatever check your functions
use—existsSync/require.resolve/path.join—to return true for the npm package path
and return a package.json with the expected version/info), then assert that
getProVersion() and getProInfo() prefer and return the npm-installed package
values instead of the submodule values.
- Around line 36-39: The tests use beforeEach with jest.clearAllMocks(), which
leaves mocked implementations like fs.existsSync and fs.readFileSync intact and
causes order-dependent failures; update the beforeEach to fully reset mocks
(e.g., use jest.resetAllMocks() instead of jest.clearAllMocks()) or explicitly
call mockReset() on the fs mocks (reset fs.existsSync and fs.readFileSync) so
each beforeEach starting at process.chdir(originalCwd) runs with fresh mock
implementations and no stale state.
In `@tests/pro/pro-updater.test.js`:
- Around line 185-190: The registry fixture used by mockRegistryResponse
currently sets peerDependencies['aiox-core'] only; add a scoped-peer case so the
mock also includes '@synkra/aiox-core' (e.g. peerDependencies: { 'aiox-core':
'>=5.0.0', '@synkra/aiox-core': '>=5.0.0' } or add a separate
mockRegistryResponse call that publishes '@synkra/aiox-core' with the same
version), so the pro-updater tests exercise detection of the renamed scoped
package; update the mockRegistryResponse invocation near the existing one in the
pro-updater.test.js suite to include the scoped package.
🪄 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: feb2c613-9176-4eeb-92b1-2429528a7a27
📒 Files selected for processing (20)
.aiox-core/cli/commands/pro/index.js.aiox-core/core/pro/pro-updater.js.aiox-core/install-manifest.yaml.github/workflows/publish-pro.yml.github/workflows/sync-pro-submodule.ymlREADME.en.mdREADME.mdbin/aiox.jsbin/utils/pro-detector.jsdocs/guides/pro/install-gate-setup.mddocs/stories/epic-123/STORY-123.1-pro-sync-automation.mddocs/stories/epic-123/STORY-123.2-aiox-pro-alignment-and-update-flow.mdpackage.jsonpackages/aiox-pro-cli/bin/aiox-pro.jspackages/installer/src/pro/pro-scaffolder.jspackages/installer/src/wizard/i18n.jspackages/installer/src/wizard/pro-setup.jsprotests/pro/pro-detector.test.jstests/pro/pro-updater.test.js
✅ Files skipped from review due to trivial changes (6)
- .github/workflows/publish-pro.yml
- pro
- README.md
- packages/installer/src/pro/pro-scaffolder.js
- packages/installer/src/wizard/i18n.js
- .github/workflows/sync-pro-submodule.yml
rafaelscosta
dismissed
coderabbitai[bot]’s stale review
Review superseded: all CodeRabbit threads resolved and latest CodeRabbit checks passed.
vegasandre
approved these changes
Apr 12, 2026
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
2 participants
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-corewith the renamedSynkraAI/aiox-prorepository across workflows, docs, and theprosubmodule@aiox-fullstack/proas the canonical package name with fallback support for@aios-fullstack/proaiox pro updateflow so Pro can refresh from npm without reinstallingaiox-coreValidation
npm run lintnpm run typechecknpx jest tests/pro/pro-detector.test.js tests/pro/pro-updater.test.js tests/installer/pro-scaffolder.test.js --runInBand --no-coveragegit diff --checkExternal dependency
SynkraAI/aiox-pro#7merged tomain@aiox-fullstackorg, so canonical publication remains dependent on npm org permissions even though the code path already supports itSummary by CodeRabbit
New Features
Documentation
Tests