Skip to content

chore: Move shared CLI error handling into common errors - #1483

Merged
hatayama merged 3 commits into
v3-betafrom
refactor/common-errors
Jul 4, 2026
Merged

chore: Move shared CLI error handling into common errors#1483
hatayama merged 3 commits into
v3-betafrom
refactor/common-errors

Conversation

@hatayama

@hatayama hatayama commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Shared CLI error envelopes and classifiers now live in a dedicated common/errors package.
  • Existing clicore callers keep working through compatibility wrappers.

User Impact

  • CLI error JSON and messages are unchanged.
  • This reduces the size and responsibility of clicore before later package splits.

Changes

  • Moved argument errors, busy status envelopes, error classification, transport error helpers, and their tests into common/errors.
  • Kept thin clicore aliases and wrappers for existing dispatcher and project-runner call sites.
  • Left tool-cache-specific unknown-command command collection in clicore to avoid reversing dependencies.

Verification

  • go test ./errors ./clicore ./unityipc
  • go test ./internal/projectrunner
  • go test ./internal/dispatcher
  • scripts/check-go-cli.sh

Review in cubic

Extract the shared CLI error envelope, classifier, busy status, argument error, and transport error code into common/errors while keeping clicore compatibility wrappers for existing callers.
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@hatayama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 93818084-0cbd-49e7-9885-b4bf9430b14f

📥 Commits

Reviewing files that changed from the base of the PR and between 6df3021 and 45a8fe0.

📒 Files selected for processing (5)
  • cli/common/clicore/error_aliases_test.go
  • cli/dispatcher/shared-inputs-stamp.json
  • cli/project-runner/shared-inputs-stamp.json
  • cli/release-automation/internal/automation/release_trigger_guard.go
  • scripts/stamp-release-inputs.sh
📝 Walkthrough

Walkthrough

This PR relocates error-handling types and helpers from the clicore package into a new clierrors package, introduces a firstNonEmpty string helper replacing FirstNonEmpty calls, moves UnityServerNotRespondingError accordingly, changes UnknownCommandError to accept an explicit availableCommands []string, and adds a clicore/error_aliases.go file re-exporting clierrors symbols with a ToolsCache-based wrapper.

Changes

clierrors extraction and clicore aliasing

Layer / File(s) Summary
Package rename to clierrors
cli/common/errors/argument_error.go, busy_status.go, error_editor_unresponsive.go, error_envelope.go, error_envelope_classification.go, error_envelope_test.go, transport_errors.go, transport_errors_test.go
Package declarations change from clicore to clierrors across all error-related files.
firstNonEmpty helper and call sites
cli/common/errors/string_helpers.go, argument_error.go, busy_status.go, error_envelope.go, error_envelope_classification.go
Adds firstNonEmpty(values ...string) string and updates several ProjectRoot/requestedToolName/command computations to use it instead of FirstNonEmpty.
UnityServerNotRespondingError relocation
cli/common/errors/server_not_responding.go, cli/common/clicore/server_not_responding.go
Adds the error type with Error()/Unwrap()/causeText() in clierrors, and simplifies the clicore file's imports, removing the old fmt-based implementation.
UnknownCommandError signature refactor
cli/common/errors/error_envelope.go, error_envelope_test.go
UnknownCommandError now takes availableCommands []string instead of a ToolsCache; the test updates its call and drops a now-relocated built-in-names test.
clicore error_aliases wrapper layer
cli/common/clicore/error_aliases.go, error_aliases_test.go
Adds re-exported ErrorCode*/ErrorPhase* constants, type aliases, wrapper functions delegating to clierrors, and an UnknownCommandError wrapper that derives available commands from NativeCommandNamesForCompletion() plus a ToolsCache via a new availableCommandNames helper.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant clicoreAliases as clicore.UnknownCommandError
  participant availableCommandNames
  participant ToolsCache
  participant clierrors as clierrors.UnknownCommandError

  Caller->>clicoreAliases: UnknownCommandError(command, cache, context)
  clicoreAliases->>availableCommandNames: availableCommandNames(cache)
  availableCommandNames->>ToolsCache: read Tools
  availableCommandNames-->>clicoreAliases: deduplicated command names
  clicoreAliases->>clierrors: UnknownCommandError(command, availableCommands, context)
  clierrors-->>clicoreAliases: CLIError with AvailableCommands details
  clicoreAliases-->>Caller: CLIError
Loading

Possibly related PRs

  • hatayama/unity-cli-loop#1312: Adds shared transport-error predicate helpers that the aliasing layer now wraps in place of local implementations.
  • hatayama/unity-cli-loop#1339: Introduces the Unity startup/process-exit timeout error codes that are re-exported by the new error_aliases.go.
  • hatayama/unity-cli-loop#1460: Updates the unknown-command registry to source names from NativeCommandNamesForCompletion(), matching the availableCommandNames logic added here.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main refactor: moving shared CLI error handling into common/errors.
Description check ✅ Passed The description matches the changeset and clearly describes the package move, wrappers, and unchanged behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/common-errors

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

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cli/common/clicore/error_aliases.go (1)

87-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Duplicate command-name dedup logic.

This dedup-and-append logic (native names + cache.Tools names) mirrors printCommandNames in cli/dispatcher/internal/dispatcher/completion.go. Consider extracting a shared helper (e.g., in a common package) to avoid two independent implementations drifting apart.

♻️ Sketch of shared helper
-func availableCommandNames(cache ToolsCache) []string {
-	seen := map[string]bool{}
-	names := []string{}
-	for _, name := range NativeCommandNamesForCompletion() {
-		seen[name] = true
-		names = append(names, name)
-	}
-	for _, tool := range cache.Tools {
-		if seen[tool.Name] {
-			continue
-		}
-		seen[tool.Name] = true
-		names = append(names, tool.Name)
-	}
-	return names
-}
+func availableCommandNames(cache ToolsCache) []string {
+	return dedupedCommandNames(NativeCommandNamesForCompletion(), cache.Tools)
+}
🤖 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 `@cli/common/clicore/error_aliases.go` around lines 87 - 102, The command-name
dedup-and-append logic in availableCommandNames is duplicated elsewhere, so
extract a shared helper for building the unique ordered command-name list and
reuse it here and in printCommandNames. Keep the behavior identical for
NativeCommandNamesForCompletion plus cache.Tools, and place the helper in a
common package or shared internal location so both callers stay in sync.
🤖 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 `@cli/common/clicore/error_aliases.go`:
- Around line 87-102: The command-name dedup-and-append logic in
availableCommandNames is duplicated elsewhere, so extract a shared helper for
building the unique ordered command-name list and reuse it here and in
printCommandNames. Keep the behavior identical for
NativeCommandNamesForCompletion plus cache.Tools, and place the helper in a
common package or shared internal location so both callers stay in sync.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fe8070f2-e9fd-4ed0-a18c-9fa15294f7ec

📥 Commits

Reviewing files that changed from the base of the PR and between 149436f and 6df3021.

📒 Files selected for processing (13)
  • cli/common/clicore/error_aliases.go
  • cli/common/clicore/error_aliases_test.go
  • cli/common/clicore/server_not_responding.go
  • cli/common/errors/argument_error.go
  • cli/common/errors/busy_status.go
  • cli/common/errors/error_editor_unresponsive.go
  • cli/common/errors/error_envelope.go
  • cli/common/errors/error_envelope_classification.go
  • cli/common/errors/error_envelope_test.go
  • cli/common/errors/server_not_responding.go
  • cli/common/errors/string_helpers.go
  • cli/common/errors/transport_errors.go
  • cli/common/errors/transport_errors_test.go

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found and verified against the latest diff

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread cli/common/clicore/error_aliases_test.go
@hatayama
hatayama merged commit ab93bbd into v3-beta Jul 4, 2026
10 checks passed
@hatayama
hatayama deleted the refactor/common-errors branch July 4, 2026 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant