feat(pro): EPIC-PRO-17 seat hygiene client and CLI [STORY-17.1-17.5-17.6]#799
Conversation
|
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:
WalkthroughChangesAdds authenticated Pro seat listing and release commands, propagates machine-ID provenance through installer licensing requests, improves structured error handling, and expands Claude integration namespace validation with allowlists and metrics. Pro Seats CLI
Installer Machine-ID Provenance
Claude Integration Governance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SeatsCLI as aiox pro seats
participant licenseApi
User->>SeatsCLI: invoke list or release
SeatsCLI->>SeatsCLI: resolve credentials and machine id
SeatsCLI->>licenseApi: listSeats or releaseSeat
licenseApi-->>SeatsCLI: return seat data or error
SeatsCLI-->>User: print result
sequenceDiagram
participant ProSetup as InlineLicenseClient
participant MachineId as Pro machine-id module
participant LicenseServer
ProSetup->>MachineId: generate machine id and source
MachineId-->>ProSetup: return machine id and provenance
ProSetup->>LicenseServer: send licensing request with machineIdSource
LicenseServer-->>ProSetup: return success or structured error envelope
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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 (13)
.aiox-core/cli/commands/pro/seats.js (1)
18-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate license-resolution logic with
index.js.
resolveLicensePathandloadModuleshere re-implement the same candidate-path search and dynamic-requirepattern already present in.aiox-core/cli/commands/pro/index.js(resolveLicensePath/loadLicenseModules). Any future change to path resolution or module list has to be kept in sync in two places.Consider extracting a shared helper (e.g.,
pro/license-module-loader.js) exportingresolveLicensePath()and a genericloadProLicenseModules(names)used by both files.🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 18 - 56, `seats.js` duplicates the same license path resolution and dynamic module loading logic already implemented in `index.js`, so the two implementations can drift apart. Refactor the shared path/module lookup into a common helper (for example, a license loader utility) that exports `resolveLicensePath()` and a generic `loadProLicenseModules(...)`, then have both `seats.js` and `index.js` call that shared helper instead of maintaining separate `resolveLicensePath` and `loadModules`/`loadLicenseModules` implementations..aiox-core/cli/commands/pro/index.js (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRelative import used for
createSeatsCommand.As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should "Use absolute imports instead of relative imports in all code," but line 26 imports the new module via a relative path (./seats). If the project has an established absolute-import mechanism for this CLI package (e.g. packageexports/path alias), please convert this to match; otherwise, please confirm how "absolute imports" should be applied in this CommonJS CLI context, since sibling-module relative requires are used elsewhere in this codebase.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 @.aiox-core/cli/commands/pro/index.js at line 26, The import for createSeatsCommand in the pro CLI index uses a sibling relative require, which conflicts with the absolute-import guideline. Update the pro command entrypoint to use the project’s established absolute import mechanism for this CLI package, or confirm and align with the codebase’s standard CommonJS approach if absolute resolution is not available. Keep the change localized around createSeatsCommand in the index module so it matches the rest of the package’s import style.Source: Coding guidelines
packages/aiox-pro-cli/src/error-bridge.js (2)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports. These cross-packagerequire('../../../.aiox-core/...')paths are fragile and should use package-level imports.🤖 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/aiox-pro-cli/src/error-bridge.js` around lines 8 - 9, The imports in error-bridge are using fragile relative paths instead of the required package-level absolute imports. Update the require statements that load AIOXError, defaultErrorRegistry, and proErrorRegistry so they resolve through the package import name/path used elsewhere in the repo, keeping the same symbols but removing the ../../../.aiox-core traversal.Source: Coding guidelines
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport the shared default error code instead of hardcoding it
packages/aiox-pro-cli/src/error-bridge.js:11should reuseDEFAULT_ERROR_CODEfrom the shared errors export surface so this value stays aligned with the canonical registry.🤖 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/aiox-pro-cli/src/error-bridge.js` at line 11, The default error code is hardcoded in error-bridge instead of using the shared canonical value. Update error-bridge.js to import DEFAULT_ERROR_CODE from the shared errors export surface and use that symbol in place of the local DEFAULT_CODE constant, so the bridge stays aligned with the central registry. Keep the change localized around the error-bridge module and preserve any existing error mapping logic.tests/pro-cli/error-bridge.test.js (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports.🤖 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 `@tests/pro-cli/error-bridge.test.js` at line 1, The test file currently imports parseEnvelopeToAIOXError using a relative path, which violates the absolute-import guideline for JS/TS files. Update the import in error-bridge.test.js to use the project’s absolute alias/module path for parseEnvelopeToAIOXError instead of ../../packages/aiox-pro-cli/src/error-bridge, keeping the same symbol name so the test continues to reference the same function.Source: Coding guidelines
.aiox-core/core/errors/pro-error-registry.js (1)
11-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports instead of relative imports. Theserequire('./error-registry')andrequire('./constants')calls use relative paths. Consider using package-level imports if the project supports them (e.g.,require('@aiox-core/core/errors/error-registry')).🤖 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 @.aiox-core/core/errors/pro-error-registry.js around lines 11 - 12, The imports in pro-error-registry.js are using relative require paths, which violates the absolute-import guideline. Update the top-level require statements in the module that defines ErrorRegistry, ErrorCategory, and ErrorSeverity to use the project’s package-level import aliases instead of './error-registry' and './constants', keeping the same exported symbols and module structure.Source: Coding guidelines
tests/core/errors/pro-error-registry.test.js (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports. Bothrequirecalls here use deep relative paths (../../../.aiox-core/...).🤖 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 `@tests/core/errors/pro-error-registry.test.js` around lines 1 - 2, The test file is using deep relative require paths, which violates the absolute-imports guideline. Update the imports in the proErrorRegistry test to use the project’s absolute module path for the `.aiox-core/core/errors/pro-error-registry` and `.aiox-core/core/errors` modules, and keep the same exported symbols (`proErrorRegistry`, `PRO_ERROR_DEFINITIONS`, `defaultErrorRegistry`, `ErrorCategory`) so the test behavior stays unchanged.Source: Coding guidelines
tests/pro-cli/render-error.test.js (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports.🤖 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 `@tests/pro-cli/render-error.test.js` around lines 1 - 2, The test file is using relative imports for parseEnvelopeToAIOXError and renderError/SUPPORT_URL, but the codebase guideline requires absolute imports in JS/TS files. Update the import statements in render-error.test.js to use the appropriate absolute package/module paths that match the existing project import convention, keeping the same symbols so the test continues to reference parseEnvelopeToAIOXError, renderError, and SUPPORT_URL correctly.Source: Coding guidelines
tests/pro-cli/recovery-actions.test.js (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports instead of relative imports.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}files should use absolute imports.🤖 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 `@tests/pro-cli/recovery-actions.test.js` at line 1, The test file is using a relative require for recovery-actions, but the guideline requires absolute imports in JS/TS files. Update the import in the recovery-actions test to use the repository’s absolute module path, keeping the same symbols getCacheCleanCommands and planRecoveryAction, and adjust any other similar relative imports in this file if present.Source: Coding guidelines
tests/license/license-crypto.test.js (1)
25-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports per coding guidelines.
The new
require('../../pro/license/machine-id-store')uses a relative path. As per coding guidelines,**/*.{js,jsx,ts,tsx}files should use absolute imports instead of relative imports in all code.♻️ Suggested fix
-const { - writePersistedMachineId, - _resetMachineIdStoreForTests, -} = require('../../pro/license/machine-id-store'); +const { + writePersistedMachineId, + _resetMachineIdStoreForTests, +} = require('`@aiox-squads/pro/license/machine-id-store`');🤖 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 `@tests/license/license-crypto.test.js` around lines 25 - 28, The test file is importing the license machine-id store with a relative path, which violates the absolute import guideline. Update the require in the license-crypto test to use the project’s absolute import path for machine-id-store, keeping the referenced symbols writePersistedMachineId and _resetMachineIdStoreForTests unchanged.Source: Coding guidelines
tests/license/machine-id-store.test.js (1)
7-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse absolute imports per coding guidelines.
Both import statements use relative paths (
../../pro/license/...). As per coding guidelines,**/*.{js,jsx,ts,tsx}files should use absolute imports instead of relative imports in all code.♻️ Suggested fix
-const { - readPersistedMachineId, - writePersistedMachineId, - isValidMachineId, - _resetMachineIdStoreForTests, - _PATHS, -} = require('../../pro/license/machine-id-store'); -const { - generateMachineId, - _resetMachineIdCacheForTests, -} = require('../../pro/license/license-crypto'); +const { + readPersistedMachineId, + writePersistedMachineId, + isValidMachineId, + _resetMachineIdStoreForTests, + _PATHS, +} = require('`@aiox-squads/pro/license/machine-id-store`'); +const { + generateMachineId, + _resetMachineIdCacheForTests, +} = require('`@aiox-squads/pro/license/license-crypto`');🤖 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 `@tests/license/machine-id-store.test.js` around lines 7 - 17, The imports in machine-id-store.test.js use relative paths, which violates the absolute-import guideline for JS/TS files. Update the require statements that load machine-id-store and license-crypto to use the project’s absolute import style instead of ../../ paths, keeping the same exported symbols like readPersistedMachineId, _resetMachineIdStoreForTests, generateMachineId, and _resetMachineIdCacheForTests.Source: Coding guidelines
tests/claude/subagent-governance.test.js (1)
10-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
expectedCoreNativeSubagentsis missing 3 agents present inALLOWED_NATIVE_SUBAGENTS.The allowlist in
validate-claude-integration.jspermits 13 native subagents (aiox-master,aiox-squad-creator,aiox-ux-design-expertare the extras), but this test expects only 10.mdfiles. If any of those 3 agents are added to.claude/agents, the validator would pass but this governance test would fail. Consider adding a comment documenting the relationship between the two lists, or deriving the expected list from the allowlist to keep them in sync.Also applies to: 46-46
🤖 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 `@tests/claude/subagent-governance.test.js` around lines 10 - 21, The expected native subagent list in the governance test is out of sync with the allowlist used by validate-claude-integration.js. Update expectedCoreNativeSubagents in the test to include the three additional native agents, or derive the expected filenames from ALLOWED_NATIVE_SUBAGENTS so the two lists stay aligned. Use the existing expectedCoreNativeSubagents constant and the validator’s allowlist as the main symbols to locate the change..aiox-core/infrastructure/scripts/validate-claude-integration.js (1)
110-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAgent-memory validation uses a prefix check while other validations use explicit allowlists.
The native agent, command, and skill checks all use
Set.has()for explicit membership testing, but the agent-memory check uses!entry.startsWith('aiox-'). This is more permissive — any directory starting withaiox-passes, even if it doesn't correspond to a real agent. Consider validating against the actual agent IDs or an explicit allowlist for consistency with the other checks.🤖 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 @.aiox-core/infrastructure/scripts/validate-claude-integration.js around lines 110 - 138, The agent-memory check in validate-claude-integration.js is using a broad prefix rule instead of explicit membership like the native agent, command, and skill checks. Update the agentMemoryEntries validation to use the same allowlist pattern as ALLOWED_NATIVE_SUBAGENTS, ALLOWED_CLAUDE_COMMAND_ENTRIES, and ALLOWED_CLAUDE_SKILL_ENTRIES, so only known agent-memory namespaces are accepted. Keep the existing error reporting structure, but base disallowedAgentMemoryEntries on the explicit allowed set rather than startsWith('aiox-').
🤖 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 @.aiox-core/cli/commands/pro/seats.js:
- Around line 89-156: `generateMachineId()` is currently called outside the
error-handling path in both `listSeatsAction` and `releaseSeatAction`, so
failures can bypass the friendly CLI error messages. Move the
`generateMachineId()` call into the existing `try` blocks (or wrap the entire
action body) in those functions so any machine-id read/write error is caught and
reported consistently with the `licenseApi` failures. While touching
`releaseSeatAction`, add the missing `return` immediately after the usage
error/`process.exit(1)` guard for consistency with the rest of the command flow.
- Around line 161-167: The authOptions CLI parser currently exposes a plaintext
--password flag, which is a credential-risk issue. Update the auth flow in
authOptions (and any downstream consumers in seats.js) to prefer --token or
environment-based auth, deprecate or remove --password, and if it must remain
for compatibility, emit a clear warning when cmd.option('--password <password>')
is used directly so callers are steered to safer alternatives.
- Around line 58-82: Update resolveAccessToken to accept licenseApi as an
argument instead of calling loadModules internally, and adjust its callers to
pass the already-loaded licenseApi through. Remove the dead ternary in the catch
block by using the actual error message handling you want without the redundant
instanceof check, and add an explicit return immediately after the
authentication failure process.exit(1) path so execution cannot continue into
licenseApi.login with missing credentials.
---
Nitpick comments:
In @.aiox-core/cli/commands/pro/index.js:
- Line 26: The import for createSeatsCommand in the pro CLI index uses a sibling
relative require, which conflicts with the absolute-import guideline. Update the
pro command entrypoint to use the project’s established absolute import
mechanism for this CLI package, or confirm and align with the codebase’s
standard CommonJS approach if absolute resolution is not available. Keep the
change localized around createSeatsCommand in the index module so it matches the
rest of the package’s import style.
In @.aiox-core/cli/commands/pro/seats.js:
- Around line 18-56: `seats.js` duplicates the same license path resolution and
dynamic module loading logic already implemented in `index.js`, so the two
implementations can drift apart. Refactor the shared path/module lookup into a
common helper (for example, a license loader utility) that exports
`resolveLicensePath()` and a generic `loadProLicenseModules(...)`, then have
both `seats.js` and `index.js` call that shared helper instead of maintaining
separate `resolveLicensePath` and `loadModules`/`loadLicenseModules`
implementations.
In @.aiox-core/core/errors/pro-error-registry.js:
- Around line 11-12: The imports in pro-error-registry.js are using relative
require paths, which violates the absolute-import guideline. Update the
top-level require statements in the module that defines ErrorRegistry,
ErrorCategory, and ErrorSeverity to use the project’s package-level import
aliases instead of './error-registry' and './constants', keeping the same
exported symbols and module structure.
In @.aiox-core/infrastructure/scripts/validate-claude-integration.js:
- Around line 110-138: The agent-memory check in validate-claude-integration.js
is using a broad prefix rule instead of explicit membership like the native
agent, command, and skill checks. Update the agentMemoryEntries validation to
use the same allowlist pattern as ALLOWED_NATIVE_SUBAGENTS,
ALLOWED_CLAUDE_COMMAND_ENTRIES, and ALLOWED_CLAUDE_SKILL_ENTRIES, so only known
agent-memory namespaces are accepted. Keep the existing error reporting
structure, but base disallowedAgentMemoryEntries on the explicit allowed set
rather than startsWith('aiox-').
In `@packages/aiox-pro-cli/src/error-bridge.js`:
- Around line 8-9: The imports in error-bridge are using fragile relative paths
instead of the required package-level absolute imports. Update the require
statements that load AIOXError, defaultErrorRegistry, and proErrorRegistry so
they resolve through the package import name/path used elsewhere in the repo,
keeping the same symbols but removing the ../../../.aiox-core traversal.
- Line 11: The default error code is hardcoded in error-bridge instead of using
the shared canonical value. Update error-bridge.js to import DEFAULT_ERROR_CODE
from the shared errors export surface and use that symbol in place of the local
DEFAULT_CODE constant, so the bridge stays aligned with the central registry.
Keep the change localized around the error-bridge module and preserve any
existing error mapping logic.
In `@tests/claude/subagent-governance.test.js`:
- Around line 10-21: The expected native subagent list in the governance test is
out of sync with the allowlist used by validate-claude-integration.js. Update
expectedCoreNativeSubagents in the test to include the three additional native
agents, or derive the expected filenames from ALLOWED_NATIVE_SUBAGENTS so the
two lists stay aligned. Use the existing expectedCoreNativeSubagents constant
and the validator’s allowlist as the main symbols to locate the change.
In `@tests/core/errors/pro-error-registry.test.js`:
- Around line 1-2: The test file is using deep relative require paths, which
violates the absolute-imports guideline. Update the imports in the
proErrorRegistry test to use the project’s absolute module path for the
`.aiox-core/core/errors/pro-error-registry` and `.aiox-core/core/errors`
modules, and keep the same exported symbols (`proErrorRegistry`,
`PRO_ERROR_DEFINITIONS`, `defaultErrorRegistry`, `ErrorCategory`) so the test
behavior stays unchanged.
In `@tests/license/license-crypto.test.js`:
- Around line 25-28: The test file is importing the license machine-id store
with a relative path, which violates the absolute import guideline. Update the
require in the license-crypto test to use the project’s absolute import path for
machine-id-store, keeping the referenced symbols writePersistedMachineId and
_resetMachineIdStoreForTests unchanged.
In `@tests/license/machine-id-store.test.js`:
- Around line 7-17: The imports in machine-id-store.test.js use relative paths,
which violates the absolute-import guideline for JS/TS files. Update the require
statements that load machine-id-store and license-crypto to use the project’s
absolute import style instead of ../../ paths, keeping the same exported symbols
like readPersistedMachineId, _resetMachineIdStoreForTests, generateMachineId,
and _resetMachineIdCacheForTests.
In `@tests/pro-cli/error-bridge.test.js`:
- Line 1: The test file currently imports parseEnvelopeToAIOXError using a
relative path, which violates the absolute-import guideline for JS/TS files.
Update the import in error-bridge.test.js to use the project’s absolute
alias/module path for parseEnvelopeToAIOXError instead of
../../packages/aiox-pro-cli/src/error-bridge, keeping the same symbol name so
the test continues to reference the same function.
In `@tests/pro-cli/recovery-actions.test.js`:
- Line 1: The test file is using a relative require for recovery-actions, but
the guideline requires absolute imports in JS/TS files. Update the import in the
recovery-actions test to use the repository’s absolute module path, keeping the
same symbols getCacheCleanCommands and planRecoveryAction, and adjust any other
similar relative imports in this file if present.
In `@tests/pro-cli/render-error.test.js`:
- Around line 1-2: The test file is using relative imports for
parseEnvelopeToAIOXError and renderError/SUPPORT_URL, but the codebase guideline
requires absolute imports in JS/TS files. Update the import statements in
render-error.test.js to use the appropriate absolute package/module paths that
match the existing project import convention, keeping the same symbols so the
test continues to reference parseEnvelopeToAIOXError, renderError, and
SUPPORT_URL correctly.
🪄 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: 2502f52a-90af-438c-b1d1-db6d3e4b762b
📒 Files selected for processing (57)
.aiox-core/cli/commands/pro/index.js.aiox-core/cli/commands/pro/seats.js.aiox-core/core/errors/pro-error-registry.js.aiox-core/data/entity-registry.yaml.aiox-core/infrastructure/scripts/validate-claude-integration.js.aiox-core/install-manifest.yaml.claude/agent-memory/oalanicolas/MEMORY.md.claude/agent-memory/pedro-valerio/MEMORY.md.claude/agent-memory/sop-extractor/MEMORY.md.claude/agent-memory/squad/MEMORY.md.claude/agents/brad-frost.md.claude/agents/copy-chief.md.claude/agents/cyber-chief.md.claude/agents/dan-mall.md.claude/agents/data-chief.md.claude/agents/dave-malouf.md.claude/agents/db-sage.md.claude/agents/design-chief.md.claude/agents/design-system.md.claude/agents/legal-chief.md.claude/agents/nano-banana-generator.md.claude/agents/oalanicolas.md.claude/agents/pedro-valerio.md.claude/agents/sop-extractor.md.claude/agents/squad-chief.md.claude/agents/squad.md.claude/agents/story-chief.md.claude/agents/tools-orchestrator.md.claude/agents/traffic-masters-chief.md.claude/commands/AIOX/scripts/agent-config-loader.js.claude/commands/cohort-squad/agents/cohort-manager.md.claude/commands/design-system/agents/brad-frost.md.claude/commands/design-system/agents/dan-mall.md.claude/commands/design-system/agents/dave-malouf.md.claude/commands/design-system/agents/design-chief.md.claude/commands/design-system/agents/nano-banana-generator.md.claude/skills/clone-mind.md.claude/skills/course-generation-workflow.md.claude/skills/enhance-workflow.md.claude/skills/ralph.md.claude/skills/squad.md.codex/skills/aiox-claude-mastery-chief/SKILL.mdeslint.config.jspackages/aiox-pro-cli/src/error-bridge.jspackages/aiox-pro-cli/src/recovery-actions.jspackages/aiox-pro-cli/src/render-error.jspackages/installer/src/wizard/pro-setup.jsprotests/claude/subagent-governance.test.jstests/core/errors/pro-error-registry.test.jstests/integration/codex-skills-sync.test.jstests/license/license-crypto.test.jstests/license/machine-id-store.test.jstests/pro-cli/error-bridge.test.jstests/pro-cli/recovery-actions.test.jstests/pro-cli/render-error.test.jstests/unit/validate-claude-integration.test.js
💤 Files with no reviewable changes (35)
- .claude/agents/sop-extractor.md
- .claude/skills/clone-mind.md
- .claude/agents/pedro-valerio.md
- .claude/skills/course-generation-workflow.md
- .claude/agent-memory/oalanicolas/MEMORY.md
- .claude/commands/cohort-squad/agents/cohort-manager.md
- .claude/commands/design-system/agents/design-chief.md
- .claude/agents/story-chief.md
- .codex/skills/aiox-claude-mastery-chief/SKILL.md
- .claude/agents/dave-malouf.md
- .claude/agent-memory/sop-extractor/MEMORY.md
- .claude/agent-memory/squad/MEMORY.md
- .claude/agents/design-chief.md
- .claude/skills/squad.md
- .claude/agents/cyber-chief.md
- .claude/commands/design-system/agents/nano-banana-generator.md
- .claude/agents/squad.md
- .claude/skills/ralph.md
- .claude/agents/nano-banana-generator.md
- .claude/commands/design-system/agents/dave-malouf.md
- .claude/agents/dan-mall.md
- .claude/skills/enhance-workflow.md
- .claude/commands/design-system/agents/dan-mall.md
- .claude/agents/db-sage.md
- .claude/commands/design-system/agents/brad-frost.md
- .claude/agents/design-system.md
- .claude/agents/tools-orchestrator.md
- .claude/agents/brad-frost.md
- .claude/agent-memory/pedro-valerio/MEMORY.md
- .claude/agents/copy-chief.md
- .claude/agents/legal-chief.md
- .claude/agents/traffic-masters-chief.md
- .claude/agents/squad-chief.md
- .claude/agents/oalanicolas.md
- .claude/agents/data-chief.md
…-17.5][STORY-17.6] - Bump @aiox-squads/pro submodule with persisted machine id and telemetry - Unify machine-id barrel across install wizard, activate, and artifact broker - Add aiox pro seats list|release self-service commands and license tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d20dcb0 to
299d730
Compare
github-actions
Bot
removed
area: agents
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.aiox-core/cli/commands/pro/seats.js (3)
161-167: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPlaintext
--passwordflag still unguarded — still present.
--passwordremains accepted as a CLI flag without any warning about shell-history/process-list exposure, same as previously flagged.As per path instructions, "Look for potential security vulnerabilities."
🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 161 - 167, Update authOptions to remove the plaintext --password CLI option, leaving only the supported safer authentication options such as --email and --token. Ensure command parsing no longer accepts or exposes passwords through this flag.Source: Path instructions
89-156: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
generateMachineId()still uncaught; missingreturnafter usage-error exit — still present.
generateMachineId()at lines 92 and 140 remains outside thetryblocks inlistSeatsAction/releaseSeatAction, so a persistence failure surfaces as a raw stack trace instead of the friendly error path.releaseSeatAction'sprocess.exit(1)at line 136 still isn't followed byreturn.As per path instructions, "Verify error handling is comprehensive."
🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 89 - 156, Wrap generateMachineId() within the existing try/catch flows of listSeatsAction and releaseSeatAction so persistence failures use the friendly error handling. In releaseSeatAction, return immediately after the usage-error process.exit(1) before resolving credentials or generating the machine ID.Source: Path instructions
58-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDead ternary, redundant module reload, missing
returnafterprocess.exit— still present.Same three issues from the earlier review round remain: the
instanceof AuthErrorternary at line 78 always resolves toerror.message;loadModules()is re-invoked here solely forlicenseApialthough every caller already has it;process.exit(1)at line 71 lacks a followingreturn.🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 58 - 82, Update resolveAccessToken to accept and reuse the caller-provided licenseApi instead of re-invoking loadModules(), remove the redundant AuthError-based ternary and use error.message directly, and add an explicit return immediately after the missing-credentials process.exit(1) path.
🧹 Nitpick comments (1)
.aiox-core/cli/commands/pro/index.js (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRelative import conflicts with absolute-import guideline.
require('./seats')is a relative import; the coding guideline requires absolute imports for JS files. Confirm whether this codebase has an established absolute-import/alias mechanism to apply here consistently.As per coding guidelines, "Use absolute imports."
🤖 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 @.aiox-core/cli/commands/pro/index.js at line 26, Update the import near createSeatsCommand to use the repository’s established absolute-import or alias mechanism instead of the relative './seats' path. Confirm the alias convention from neighboring imports or project configuration and preserve the existing createSeatsCommand usage.Source: Coding guidelines
🤖 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 @.aiox-core/cli/commands/pro/seats.js:
- Around line 84-121: Standardize listSeatsAction output to English: replace the
Portuguese machine marker and seat detail labels with English equivalents, and
update formatDate to use the locale convention used by the surrounding English
CLI output instead of hardcoding pt-BR. Keep the existing seat listing structure
and formatting behavior unchanged.
In @.aiox-core/infrastructure/scripts/validate-claude-integration.js:
- Around line 86-98: Update listTopLevelNames to include symlinked top-level
entries in its candidate set, resolving or explicitly recognizing Dirent
symlinks before the isGitIgnored allowlist check. Preserve the existing
projectRoot-relative filtering, name mapping, and sorting so disallowed symlink
namespaces remain visible to the governance validation and metrics.
---
Duplicate comments:
In @.aiox-core/cli/commands/pro/seats.js:
- Around line 161-167: Update authOptions to remove the plaintext --password CLI
option, leaving only the supported safer authentication options such as --email
and --token. Ensure command parsing no longer accepts or exposes passwords
through this flag.
- Around line 89-156: Wrap generateMachineId() within the existing try/catch
flows of listSeatsAction and releaseSeatAction so persistence failures use the
friendly error handling. In releaseSeatAction, return immediately after the
usage-error process.exit(1) before resolving credentials or generating the
machine ID.
- Around line 58-82: Update resolveAccessToken to accept and reuse the
caller-provided licenseApi instead of re-invoking loadModules(), remove the
redundant AuthError-based ternary and use error.message directly, and add an
explicit return immediately after the missing-credentials process.exit(1) path.
---
Nitpick comments:
In @.aiox-core/cli/commands/pro/index.js:
- Line 26: Update the import near createSeatsCommand to use the repository’s
established absolute-import or alias mechanism instead of the relative './seats'
path. Confirm the alias convention from neighboring imports or project
configuration and preserve the existing createSeatsCommand usage.
🪄 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: 2bca5d0b-b1d8-4ed1-8125-0f5411505e0d
📒 Files selected for processing (9)
.aiox-core/cli/commands/pro/index.js.aiox-core/cli/commands/pro/seats.js.aiox-core/infrastructure/scripts/validate-claude-integration.js.aiox-core/install-manifest.yamlpackages/installer/src/wizard/pro-setup.jsprotests/installer/pro-setup-auth.test.jstests/license/license-crypto.test.jstests/license/machine-id-store.test.js
💤 Files with no reviewable changes (1)
- pro
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/license/license-crypto.test.js
- packages/installer/src/wizard/pro-setup.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.aiox-core/cli/commands/pro/seats.js (3)
161-167: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPlaintext
--passwordflag still unguarded — still present.
--passwordremains accepted as a CLI flag without any warning about shell-history/process-list exposure, same as previously flagged.As per path instructions, "Look for potential security vulnerabilities."
🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 161 - 167, Update authOptions to remove the plaintext --password CLI option, leaving only the supported safer authentication options such as --email and --token. Ensure command parsing no longer accepts or exposes passwords through this flag.Source: Path instructions
89-156: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
generateMachineId()still uncaught; missingreturnafter usage-error exit — still present.
generateMachineId()at lines 92 and 140 remains outside thetryblocks inlistSeatsAction/releaseSeatAction, so a persistence failure surfaces as a raw stack trace instead of the friendly error path.releaseSeatAction'sprocess.exit(1)at line 136 still isn't followed byreturn.As per path instructions, "Verify error handling is comprehensive."
🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 89 - 156, Wrap generateMachineId() within the existing try/catch flows of listSeatsAction and releaseSeatAction so persistence failures use the friendly error handling. In releaseSeatAction, return immediately after the usage-error process.exit(1) before resolving credentials or generating the machine ID.Source: Path instructions
58-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDead ternary, redundant module reload, missing
returnafterprocess.exit— still present.Same three issues from the earlier review round remain: the
instanceof AuthErrorternary at line 78 always resolves toerror.message;loadModules()is re-invoked here solely forlicenseApialthough every caller already has it;process.exit(1)at line 71 lacks a followingreturn.🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 58 - 82, Update resolveAccessToken to accept and reuse the caller-provided licenseApi instead of re-invoking loadModules(), remove the redundant AuthError-based ternary and use error.message directly, and add an explicit return immediately after the missing-credentials process.exit(1) path.
🧹 Nitpick comments (1)
.aiox-core/cli/commands/pro/index.js (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRelative import conflicts with absolute-import guideline.
require('./seats')is a relative import; the coding guideline requires absolute imports for JS files. Confirm whether this codebase has an established absolute-import/alias mechanism to apply here consistently.As per coding guidelines, "Use absolute imports."
🤖 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 @.aiox-core/cli/commands/pro/index.js at line 26, Update the import near createSeatsCommand to use the repository’s established absolute-import or alias mechanism instead of the relative './seats' path. Confirm the alias convention from neighboring imports or project configuration and preserve the existing createSeatsCommand usage.Source: Coding guidelines
🤖 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 @.aiox-core/cli/commands/pro/seats.js:
- Around line 84-121: Standardize listSeatsAction output to English: replace the
Portuguese machine marker and seat detail labels with English equivalents, and
update formatDate to use the locale convention used by the surrounding English
CLI output instead of hardcoding pt-BR. Keep the existing seat listing structure
and formatting behavior unchanged.
In @.aiox-core/infrastructure/scripts/validate-claude-integration.js:
- Around line 86-98: Update listTopLevelNames to include symlinked top-level
entries in its candidate set, resolving or explicitly recognizing Dirent
symlinks before the isGitIgnored allowlist check. Preserve the existing
projectRoot-relative filtering, name mapping, and sorting so disallowed symlink
namespaces remain visible to the governance validation and metrics.
---
Duplicate comments:
In @.aiox-core/cli/commands/pro/seats.js:
- Around line 161-167: Update authOptions to remove the plaintext --password CLI
option, leaving only the supported safer authentication options such as --email
and --token. Ensure command parsing no longer accepts or exposes passwords
through this flag.
- Around line 89-156: Wrap generateMachineId() within the existing try/catch
flows of listSeatsAction and releaseSeatAction so persistence failures use the
friendly error handling. In releaseSeatAction, return immediately after the
usage-error process.exit(1) before resolving credentials or generating the
machine ID.
- Around line 58-82: Update resolveAccessToken to accept and reuse the
caller-provided licenseApi instead of re-invoking loadModules(), remove the
redundant AuthError-based ternary and use error.message directly, and add an
explicit return immediately after the missing-credentials process.exit(1) path.
---
Nitpick comments:
In @.aiox-core/cli/commands/pro/index.js:
- Line 26: Update the import near createSeatsCommand to use the repository’s
established absolute-import or alias mechanism instead of the relative './seats'
path. Confirm the alias convention from neighboring imports or project
configuration and preserve the existing createSeatsCommand usage.
🪄 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: 2bca5d0b-b1d8-4ed1-8125-0f5411505e0d
📒 Files selected for processing (9)
.aiox-core/cli/commands/pro/index.js.aiox-core/cli/commands/pro/seats.js.aiox-core/infrastructure/scripts/validate-claude-integration.js.aiox-core/install-manifest.yamlpackages/installer/src/wizard/pro-setup.jsprotests/installer/pro-setup-auth.test.jstests/license/license-crypto.test.jstests/license/machine-id-store.test.js
💤 Files with no reviewable changes (1)
- pro
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/license/license-crypto.test.js
- packages/installer/src/wizard/pro-setup.js
🛑 Comments failed to post (2)
.aiox-core/cli/commands/pro/seats.js (1)
84-121: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Inconsistent language in seat output — Portuguese labels mixed into English CLI text.
formatDatehardcodestoLocaleString('pt-BR'), and the seat detail lines use Portuguese labels ("esta máquina", "Nome:", "Versão:", "Ativado:", "Validado:") while the surrounding output ("AIOX Pro — Seats", "Used:", "Available:", all error strings) is English. This reads as an unintentional partial localization and will confuse most users of this command.🌐 Suggested fix
- const current = seat.isCurrentMachine ? ' (esta máquina)' : ''; + const current = seat.isCurrentMachine ? ' (this machine)' : ''; console.log(` • ${seat.machineIdMasked}${current}`); console.log(` ID: ${seat.id}`); if (seat.machineName) { - console.log(` Nome: ${seat.machineName}`); + console.log(` Name: ${seat.machineName}`); } if (seat.aiosVersion) { - console.log(` Versão: ${seat.aiosVersion}`); + console.log(` Version: ${seat.aiosVersion}`); } - console.log(` Ativado: ${formatDate(seat.activatedAt)}`); - console.log(` Validado: ${formatDate(seat.lastValidatedAt)}`); + console.log(` Activated: ${formatDate(seat.activatedAt)}`); + console.log(` Validated: ${formatDate(seat.lastValidatedAt)}`);function formatDate(dateStr) { if (!dateStr) return '—'; - return new Date(dateStr).toLocaleString('pt-BR'); + return new Date(dateStr).toLocaleString(); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function formatDate(dateStr) { if (!dateStr) return '—'; return new Date(dateStr).toLocaleString(); } async function listSeatsAction(options) { const { licenseApi, generateMachineId, AuthError } = loadModules(); const accessToken = await resolveAccessToken(options, AuthError); const machineId = generateMachineId(); try { const result = await licenseApi.listSeats(accessToken, machineId); console.log('\nAIOX Pro — Seats\n'); console.log(` Used: ${result.summary.used}/${result.summary.max}`); console.log(` Available: ${result.summary.available}`); console.log(''); if (!result.seats || result.seats.length === 0) { console.log(' No active seats.'); console.log(''); return; } for (const seat of result.seats) { const current = seat.isCurrentMachine ? ' (this machine)' : ''; console.log(` • ${seat.machineIdMasked}${current}`); console.log(` ID: ${seat.id}`); if (seat.machineName) { console.log(` Name: ${seat.machineName}`); } if (seat.aiosVersion) { console.log(` Version: ${seat.aiosVersion}`); } console.log(` Activated: ${formatDate(seat.activatedAt)}`); console.log(` Validated: ${formatDate(seat.lastValidatedAt)}`); console.log(''); }🤖 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 @.aiox-core/cli/commands/pro/seats.js around lines 84 - 121, Standardize listSeatsAction output to English: replace the Portuguese machine marker and seat detail labels with English equivalents, and update formatDate to use the locale convention used by the surrounding English CLI output instead of hardcoding pt-BR. Keep the existing seat listing structure and formatting behavior unchanged..aiox-core/infrastructure/scripts/validate-claude-integration.js (1)
86-98: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Symlinked top-level entries silently bypass the allowlist check.
entry.isDirectory() || entry.isFile()(line 89) excludes symlinks, sincefs.Direntreports both asfalsefor a symlink unless resolved. A disallowed namespace added as a symlink under.claude/commands,.claude/skills, or.claude/agent-memorywould be silently dropped fromcommandEntries/skillEntries/agentMemoryEntries— never flagged as disallowed and never counted in metrics, defeating the point of this governance check.🔧 Suggested fix
return fs.readdirSync(dirPath, { withFileTypes: true }) - .filter((entry) => entry.isDirectory() || entry.isFile()) + .filter((entry) => entry.isDirectory() || entry.isFile() || entry.isSymbolicLink())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function listTopLevelNames(dirPath, projectRoot) { if (!fs.existsSync(dirPath)) return []; return fs.readdirSync(dirPath, { withFileTypes: true }) .filter((entry) => entry.isDirectory() || entry.isFile() || entry.isSymbolicLink()) .filter((entry) => { if (!projectRoot) return true; const relativePath = path.relative(projectRoot, path.join(dirPath, entry.name)).split(path.sep).join('/'); return !isGitIgnored(projectRoot, relativePath); }) .map((entry) => entry.name) .sort(); }🧰 Tools
🪛 ast-grep (0.44.1)
[error] 91-91: An archive entry path (e.g. entry.path / entry.fileName / header.name) is joined to an output directory without validating that the resolved path stays inside that directory. A malicious archive can use "../" sequences to escape the extraction directory and overwrite arbitrary files (Zip Slip). Resolve the path and verify it starts with the normalized output directory, or strip traversal with path.basename, before writing the entry.
Context: path.join(dirPath, entry.name)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(zip-slip-archive-extraction-javascript)
🤖 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 @.aiox-core/infrastructure/scripts/validate-claude-integration.js around lines 86 - 98, Update listTopLevelNames to include symlinked top-level entries in its candidate set, resolving or explicitly recognizing Dirent symlinks before the isGitIgnored allowlist check. Preserve the existing projectRoot-relative filtering, name mapping, and sorting so disallowed symlink namespaces remain visible to the governance validation and metrics.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/cli/pro-seats.test.js (1)
5-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider expanding test coverage beyond help output.
The test only verifies
--passwordabsence in help text. Adding tests forlistSeatsActionandreleaseSeatAction(e.g., mockingloadModules,resolveAccessToken, andlicenseApicalls) would catch regressions in the core command logic.🤖 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 `@tests/cli/pro-seats.test.js` around lines 5 - 16, Expand the aiox pro seats CLI tests beyond help output by adding coverage for listSeatsAction and releaseSeatAction. Mock loadModules, resolveAccessToken, and the relevant licenseApi calls, then verify both actions invoke the expected dependencies and handle their results correctly.tests/unit/validate-claude-integration.test.js (1)
85-100: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider moving mock restoration to
afterEachfor robustness.
jest.restoreAllMocks()at line 98 runs inside the test body. If theexpectat line 96 fails, mocks are not restored and may leak into subsequent tests. Moving restoration to anafterEachhook ensures cleanup regardless of assertion outcomes.♻️ Suggested refactor
it('includes symbolic links when inspecting top-level Claude entries', () => { jest.spyOn(fs, 'existsSync').mockReturnValue(true); jest.spyOn(fs, 'readdirSync').mockReturnValue([ { name: 'linked-skill', isDirectory: () => false, isFile: () => false, isSymbolicLink: () => true, }, ]); expect(listTopLevelNames('/tmp/.claude/skills', null)).toEqual(['linked-skill']); - - jest.restoreAllMocks(); }); + + afterEach(() => { + jest.restoreAllMocks(); + });🤖 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 `@tests/unit/validate-claude-integration.test.js` around lines 85 - 100, Move Jest mock cleanup from the symbolic-link test body into an afterEach hook for the test suite, ensuring jest.restoreAllMocks() runs even when an assertion fails; remove the inline restoration from the test.
🤖 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 `@tests/cli/pro-seats.test.js`:
- Line 3: Replace the relative require in the pro-seats test with the project’s
configured absolute import alias for the createSeatsCommand module. Verify the
Jest or package alias configuration and use the established `@aiox-core-style`
mapping without changing the command usage.
---
Nitpick comments:
In `@tests/cli/pro-seats.test.js`:
- Around line 5-16: Expand the aiox pro seats CLI tests beyond help output by
adding coverage for listSeatsAction and releaseSeatAction. Mock loadModules,
resolveAccessToken, and the relevant licenseApi calls, then verify both actions
invoke the expected dependencies and handle their results correctly.
In `@tests/unit/validate-claude-integration.test.js`:
- Around line 85-100: Move Jest mock cleanup from the symbolic-link test body
into an afterEach hook for the test suite, ensuring jest.restoreAllMocks() runs
even when an assertion fails; remove the inline restoration from the test.
🪄 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: 0b546180-5bf4-420e-b20c-ff7202c3a97c
📒 Files selected for processing (5)
.aiox-core/cli/commands/pro/seats.js.aiox-core/infrastructure/scripts/validate-claude-integration.js.aiox-core/install-manifest.yamltests/cli/pro-seats.test.jstests/unit/validate-claude-integration.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- .aiox-core/install-manifest.yaml
- .aiox-core/infrastructure/scripts/validate-claude-integration.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cli/pro-seats.test.js (1)
50-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd error path test coverage for
listandreleaseactions.Both tests only verify the happy path. The source code handles
generateMachineIdfailure, missing authentication, and API rejections (all withprocess.exit(1)), but none of these paths are exercised. Adding a few error case tests would increase confidence in the error handling logic.♻️ Suggested error path tests
it('lists active seats using the authenticated machine identity', async () => { // ... existing test ... }); + it('exits when generateMachineId fails', async () => { + mockGenerateMachineId.mockReset().mockImplementation(() => { + throw new Error('disk error'); + }); + jest.spyOn(console, 'error').mockImplementation(() => {}); + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); + + const command = createSeatsCommand(); + const list = command.commands.find((c) => c.name() === 'list'); + await list.parseAsync(['--token', 'access-token'], { from: 'user' }); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockLicenseApi.listSeats).not.toHaveBeenCalled(); + }); + + it('exits when listSeats rejects', async () => { + mockLicenseApi.listSeats.mockRejectedValue(new Error('network error')); + jest.spyOn(console, 'error').mockImplementation(() => {}); + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); + + const command = createSeatsCommand(); + const list = command.commands.find((c) => c.name() === 'list'); + await list.parseAsync(['--token', 'access-token'], { from: 'user' }); + + expect(exitSpy).toHaveBeenCalledWith(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 `@tests/cli/pro-seats.test.js` around lines 50 - 82, Add error-path tests alongside the existing `list` and `release` command tests, covering `generateMachineId` failures, missing authentication, and `listSeats`/`releaseSeat` API rejections. Mock `process.exit` and assert each failure invokes it with status 1, while verifying the corresponding API is not called when authentication or machine-ID generation fails.
🤖 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.
Nitpick comments:
In `@tests/cli/pro-seats.test.js`:
- Around line 50-82: Add error-path tests alongside the existing `list` and
`release` command tests, covering `generateMachineId` failures, missing
authentication, and `listSeats`/`releaseSeat` API rejections. Mock
`process.exit` and assert each failure invokes it with status 1, while verifying
the corresponding API is not called when authentication or machine-ID generation
fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 539dd4a3-db7a-40d4-8603-61c409f35044
📒 Files selected for processing (2)
tests/cli/pro-seats.test.jstests/unit/validate-claude-integration.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/validate-claude-integration.test.js
1 participant
Summary
Implements the client half of EPIC-PRO-17 (Seat Hygiene & Machine ID Persistence):
Companion PRs
Tests
Deploy
Merge triggers semantic-release → @aiox-squads/core npm publish.
Summary by CodeRabbit
aiox pro seats listto view used/max seats and active seat activation details for the current machine.aiox pro seats release <activationId>to release a seat from the current machine.aiox pro seatshelp output (no plaintext password flag).