feat(errors): PRO-UX.1 + PRO-UX.2 — Pro CLI error UX bridge + root-cause fix#775
Merged
Conversation
…y PRO-UX.1] 5 top-5 codes mapped to EXISTING ErrorCategory (PERMISSION/NETWORK/ EXTERNAL_EXECUTOR — no invented categories). Warm G3 PT-BR userMessage + recovery arrays. Codes mirror license-server (no AIOX_ prefix). No collision with core registry. 7 tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ry PRO-UX.1][Story PRO-UX.2] - error-bridge: parseEnvelopeToAIOXError, 3-tier fallback (message_pt > registry.userMessage > server message), graceful on legacy/malformed - render-error: warm output + numbered steps + support_code + conditional support link + technical footer - recovery-actions: planRecoveryAction + OS-aware cache cleanup (PowerShell vs bash — fixes the bug that made Robert run bash in PowerShell) - 15 tests (bridge fallback, render anchor case, cross-platform) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ory PRO-UX.1] ROOT CAUSE of the anchor incident (Robert's opaque "HTTP 403"): _request read parsed.message / parsed.code at the ROOT, but the structured envelope nests them under `error`. So err.code was always undefined → the typed error branches (NOT_A_BUYER, SEAT_LIMIT_EXCEEDED, ...) in runProWizard were NEVER reached → fell through to the generic "HTTP <status>" message. Fix: read errorBody from parsed.error (nested) first, attach err.httpStatus + err.envelope for downstream renderError. Restores the existing i18n error branches. Preserves legacy root-shape compat. pro-setup suite 39/39 green (no regression). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-UX.1] IDS hook auto-registration for .aiox-core/core/errors/pro-error-registry.js. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
type: test
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds PRO error definitions and registry, parses license-server envelopes into canonical AIOXError, plans OS-aware recovery actions, renders warm CLI error messages, and updates registry/manifest metadata. ChangesPRO Error Handling & Recovery UX
Sequence DiagramsequenceDiagram
participant LicenseServer
participant ProSetup as Installer.ProSetup
participant ErrorBridge as parseEnvelopeToAIOXError
participant ProRegistry as PRO_ERROR_DEFINITIONS
participant RecoveryPlanner as planRecoveryAction
participant CliRenderer as renderError
LicenseServer->>ProSetup: HTTP response (JSON envelope)
ProSetup->>ProSetup: parse envelope (error.code, error.message, message_pt)
ProSetup->>ErrorBridge: call with parsed envelope + httpStatus
ErrorBridge->>ProRegistry: lookup code (Pro → default → unknown)
alt Known PRO code
ProRegistry-->>ErrorBridge: definition (category,severity,userMessage,recovery,exitCode,retryable)
else Unknown code
ErrorBridge->>ProRegistry: fallback to default/unknown definition
end
ErrorBridge-->>ProSetup: AIOXError (code, category, severity, recovery, metadata)
ProSetup->>CliRenderer: renderError(AIOXError)
CliRenderer->>RecoveryPlanner: planRecoveryAction(recovery_hint, { platform })
RecoveryPlanner-->>CliRenderer: action plan (wait/clean/contact/none)
CliRenderer-->>CliRenderer: format warm message + steps + support link + technical footer
CliRenderer-->>CLI: write to stderr
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 7
🧹 Nitpick comments (1)
tests/pro-cli/render-error.test.js (1)
1-2: ⚡ Quick winUse repository absolute imports instead of relative paths.
These imports violate the project import convention and should be switched to the repository’s absolute import style used in JS/TS files.
As per coding guidelines,
**/*.{js,jsx,ts,tsx}: 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 `@tests/pro-cli/render-error.test.js` around lines 1 - 2, The test imports use relative paths; update them to the repository's absolute import style by replacing the relative requires for parseEnvelopeToAIOXError and for renderError and SUPPORT_URL with the project’s absolute module paths used elsewhere (e.g., the package/module root import form) so parseEnvelopeToAIOXError, renderError and SUPPORT_URL are imported via the repo absolute import convention rather than '../../packages/...'.
🤖 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/core/errors/pro-error-registry.js:
- Around line 11-12: Replace the local relative requires for ErrorRegistry and
constants with the project's absolute import style: change the
require('./error-registry') and require('./constants') calls to the repository's
absolute module imports that export ErrorRegistry, ErrorCategory, and
ErrorSeverity, and ensure the top of pro-error-registry.js imports those same
symbols via the absolute paths used elsewhere in the core errors modules; also
update any other relative imports in this file to match the absolute-import
convention.
In `@packages/aiox-pro-cli/src/error-bridge.js`:
- Around line 8-9: Replace the two relative require calls in error-bridge.js
with the repository's absolute import paths: change the
require('../../../.aiox-core/core/errors') that provides AIOXError and
defaultErrorRegistry and the
require('../../../.aiox-core/core/errors/pro-error-registry') that provides
proErrorRegistry to use the project's configured absolute module paths (e.g.,
the root-level package/module identifiers used across the repo) so AIOXError,
defaultErrorRegistry, and proErrorRegistry are imported via absolute imports per
coding standards.
In `@packages/aiox-pro-cli/src/render-error.js`:
- Around line 20-39: renderError currently assumes err is an object and accesses
err.userMessage, err.message and err.code which throws for null/non-object
errors; modify renderError to first normalize the incoming err (e.g. if err is
falsy or not an object create a safe wrapper) and derive safe fallbacks for
err.userMessage, err.message and err.code (or use String(err) and a default code
like "UNKNOWN_ERROR") before using recovery, supportCode, recoveryHint,
httpStatus, and printing; ensure any uses of err.* are replaced with the
normalized values so the function never throws when passed a raw Error, string,
or null.
In `@packages/installer/src/wizard/pro-setup.js`:
- Around line 176-178: The error construction dereferences parsed.message and
parsed.code which will throw if parsed is null; update the logic in the error
creation block (the code that sets err.message and err.code using errorBody,
parsed, and res.statusCode) to guard parsed (e.g., use parsed && parsed.message
or optional chaining parsed?.message and parsed?.code) so you fall back to
errorBody.message or `HTTP ${res.statusCode}` without throwing when the server
returns JSON null; ensure you update both the message and code assignments that
reference parsed.
In `@tests/core/errors/pro-error-registry.test.js`:
- Around line 1-2: Replace the relative require(...) calls in this test with the
project’s absolute import paths: import proErrorRegistry and
PRO_ERROR_DEFINITIONS using the package/module absolute path that exposes
.aiox-core/core/errors/pro-error-registry, and import defaultErrorRegistry and
ErrorCategory from the absolute module that exposes .aiox-core/core/errors;
update the test file to use those absolute module specifiers while keeping the
same identifiers (proErrorRegistry, PRO_ERROR_DEFINITIONS, defaultErrorRegistry,
ErrorCategory).
In `@tests/pro-cli/error-bridge.test.js`:
- Line 1: The test imports parseEnvelopeToAIOXError using a relative path;
replace that relative require with the repository's absolute import style
(importing the same module that exports parseEnvelopeToAIOXError) so the test
uses the canonical package/module absolute path instead of ../../packages/...;
keep the symbol name parseEnvelopeToAIOXError unchanged and update only the
module specifier to the project's absolute import form.
In `@tests/pro-cli/recovery-actions.test.js`:
- Line 1: The test currently uses a relative require for recovery-actions;
change the import to use an absolute module path instead (replace
require('../../packages/aiox-pro-cli/src/recovery-actions') with the project’s
absolute module import for the recovery-actions module) so the same exported
symbols get imported (getCacheCleanCommands, planRecoveryAction) but via an
absolute path per the repo import conventions.
---
Nitpick comments:
In `@tests/pro-cli/render-error.test.js`:
- Around line 1-2: The test imports use relative paths; update them to the
repository's absolute import style by replacing the relative requires for
parseEnvelopeToAIOXError and for renderError and SUPPORT_URL with the project’s
absolute module paths used elsewhere (e.g., the package/module root import form)
so parseEnvelopeToAIOXError, renderError and SUPPORT_URL are imported via the
repo absolute import convention rather than '../../packages/...'.
🪄 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: 8a7470e8-981a-434a-b39f-d444d19f935e
📒 Files selected for processing (11)
.aiox-core/core/errors/pro-error-registry.js.aiox-core/data/entity-registry.yaml.aiox-core/install-manifest.yamlpackages/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.jstests/core/errors/pro-error-registry.test.jstests/pro-cli/error-bridge.test.jstests/pro-cli/recovery-actions.test.jstests/pro-cli/render-error.test.js
Comment on lines
+11
to
+12
| const { ErrorRegistry } = require('./error-registry'); | ||
| const { ErrorCategory, ErrorSeverity } = require('./constants'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use absolute imports for core error modules.
Please switch these require('./...') statements to the project’s absolute import form to align with repository standards.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: 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/core/errors/pro-error-registry.js around lines 11 - 12, Replace
the local relative requires for ErrorRegistry and constants with the project's
absolute import style: change the require('./error-registry') and
require('./constants') calls to the repository's absolute module imports that
export ErrorRegistry, ErrorCategory, and ErrorSeverity, and ensure the top of
pro-error-registry.js imports those same symbols via the absolute paths used
elsewhere in the core errors modules; also update any other relative imports in
this file to match the absolute-import convention.
Comment on lines
+8
to
+9
| const { AIOXError, defaultErrorRegistry } = require('../../../.aiox-core/core/errors'); | ||
| const { proErrorRegistry } = require('../../../.aiox-core/core/errors/pro-error-registry'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Switch to absolute imports in the bridge module.
Please replace these relative imports with absolute paths per repository standards.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: 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/aiox-pro-cli/src/error-bridge.js` around lines 8 - 9, Replace the
two relative require calls in error-bridge.js with the repository's absolute
import paths: change the require('../../../.aiox-core/core/errors') that
provides AIOXError and defaultErrorRegistry and the
require('../../../.aiox-core/core/errors/pro-error-registry') that provides
proErrorRegistry to use the project's configured absolute module paths (e.g.,
the root-level package/module identifiers used across the repo) so AIOXError,
defaultErrorRegistry, and proErrorRegistry are imported via absolute imports per
coding standards.
Comment on lines
+1
to
+2
| const { proErrorRegistry, PRO_ERROR_DEFINITIONS } = require('../../../.aiox-core/core/errors/pro-error-registry'); | ||
| const { defaultErrorRegistry, ErrorCategory } = require('../../../.aiox-core/core/errors'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use absolute imports in tests as well.
Convert these relative require(...) paths to absolute imports for consistency with the repository rule.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: 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 `@tests/core/errors/pro-error-registry.test.js` around lines 1 - 2, Replace the
relative require(...) calls in this test with the project’s absolute import
paths: import proErrorRegistry and PRO_ERROR_DEFINITIONS using the
package/module absolute path that exposes
.aiox-core/core/errors/pro-error-registry, and import defaultErrorRegistry and
ErrorCategory from the absolute module that exposes .aiox-core/core/errors;
update the test file to use those absolute module specifiers while keeping the
same identifiers (proErrorRegistry, PRO_ERROR_DEFINITIONS, defaultErrorRegistry,
ErrorCategory).
| @@ -0,0 +1,57 @@ | |||
| const { parseEnvelopeToAIOXError } = require('../../packages/aiox-pro-cli/src/error-bridge'); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use an absolute import path for the test subject.
Update this relative require to the repository’s absolute import style.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: 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 `@tests/pro-cli/error-bridge.test.js` at line 1, The test imports
parseEnvelopeToAIOXError using a relative path; replace that relative require
with the repository's absolute import style (importing the same module that
exports parseEnvelopeToAIOXError) so the test uses the canonical package/module
absolute path instead of ../../packages/...; keep the symbol name
parseEnvelopeToAIOXError unchanged and update only the module specifier to the
project's absolute import form.
| @@ -0,0 +1,45 @@ | |||
| const { getCacheCleanCommands, planRecoveryAction } = require('../../packages/aiox-pro-cli/src/recovery-actions'); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Adopt absolute import style in this test file.
Please replace the relative module path with an absolute import path.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: 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 `@tests/pro-cli/recovery-actions.test.js` at line 1, The test currently uses a
relative require for recovery-actions; change the import to use an absolute
module path instead (replace
require('../../packages/aiox-pro-cli/src/recovery-actions') with the project’s
absolute module import for the recovery-actions module) so the same exported
symbols get imported (getCacheCleanCommands, planRecoveryAction) but via an
absolute path per the repo import conventions.
The IDS hook's incremental update diverged from clean regen output (structural delta). Regenerated via populate-entity-registry.js. validate:registry-determinism now PASSES (821 entities). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ack [Story PRO-UX.2] Addresses 2 CodeRabbit Major findings on PR #775: - renderError: guard against null/non-object err (prints generic message) - pro-setup _request: guard parsed.message/parsed.code against parsed === null The 5 "absolute import" suggestions are NOT applied: CLI runtime modules (error-bridge, pro-error-registry) require relative paths — there is no runtime module alias for `aiox-core/*` outside Jest's moduleNameMapper. Same-dir `./` imports do not violate the no-`../../../` rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rafaelscosta
dismissed
coderabbitai[bot]’s stale review
Dismissed for release: actionable null/non-object guards and nested envelope parsing were fixed in commit 2df2f83; remaining absolute-import suggestions are not applicable because these CLI/runtime CommonJS modules do not have a production module alias outside Jest. CI, Pro Integration, installer smoke matrix, and focused local tests are green.
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
Implements STORY-PRO-UX.1 + STORY-PRO-UX.2 — the client half of the cross-repo error UX overhaul. Companion of server EPIC-PRO-16 (aios-license-server PR #12). Specs in PR #774.
What changed
.aiox-core/core/errors/pro-error-registry.js— 5 Pro codes extendingdefaultErrorRegistry, mapped to EXISTINGErrorCategory(PERMISSION/NETWORK/EXTERNAL_EXECUTOR — no invented categories). Warm G3 PT-BR + recovery arrays.packages/aiox-pro-cli/src/error-bridge.js—parseEnvelopeToAIOXErrorwith 3-tier fallback (message_pt > registry.userMessage > server message), graceful on legacy/malformed envelopes.packages/aiox-pro-cli/src/render-error.js— warm output + numbered steps + support_code + conditional support link + technical footer.packages/aiox-pro-cli/src/recovery-actions.js— OS-aware cache cleanup (PowerShell vs bash).🎯 ROOT CAUSE FIX (anchor incident)
packages/installer/src/wizard/pro-setup.js_requestreadparsed.message/parsed.codeat the root, but the structured envelope nests them undererror. Soerr.codewas alwaysundefined→ the typed error branches (NOT_A_BUYER,SEAT_LIMIT_EXCEEDED, ...) inrunProWizardwere never reached → fell through to the genericHTTP <status>message. This is why Robert saw "HTTP 403".Fix: read
errorBodyfromparsed.error(nested) first, attacherr.httpStatus+err.envelope. Restores the existing i18n error branches. Preserves legacy root-shape compat. pro-setup suite 39/39 green.Quality gates
eslintexit 0Architectural constraints honored
ErrorCategory— no new categoriesAIOXError+ErrorRegistry(EPIC-AIOX-ERROR-GOVERNANCE) — no parallel systemErrorCodes(noAIOX_prefix)Known follow-up (non-blocking)
The rich
renderErrorwire into the install flow vs the existingi18n.js(proSeatLimit etc — now functional thanks to the root-cause fix) is a design decision for @architect. Modules are ready + tested.Refs
Stories PRO-UX.1, PRO-UX.2. Companion specs PR #774. Server PR #12.
🤖 Generated with Claude Code
Summary by CodeRabbit