chore(cli): define error actionability taxonomy#5829
Open
jgoux wants to merge 6 commits into
Open
Conversation
…rations Defines the CLI error actionability taxonomy for KPI reporting (CLI-1560): error kinds, categories, suggestion types, and the Q2 strict-recovery / repeat-error / internal-unknown-rate metric definitions. Classification is declared where each error is defined: every error class in apps/cli exposes a CliErrorActionabilityDeclaration under the ErrorActionabilityId symbol, enforced by an exhaustiveness test that fails when any error class lacks a declaration. Errors from workspace packages (stack, config, process-compose) and effect cli/http are classified by a small structural adapter, exhaustiveness-checked against those packages' sources. Broad wrapper errors gain structured discriminants instead of message sniffing: StackBuildError.reason, DockerPullError.daemonDown, LegacyDockerRunError.reason/daemonDown, and a typed upgradeSuggested prop threaded from legacySuggestUpgrade through the plan-gated sso, branches, and vanity-subdomains errors. No telemetry capture changes: cli_command_executed still emits only existing fields until CLI-1561 wires classification into failure events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@c9af260751871ee82f3854e0e53894931ddad892Preview package for commit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afd73d0c8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Coverage: the drift guard now also enforces declarations on plain `extends Error` classes, and the external adapters cover the full effect cli parser error set (MissingArgument, DuplicateOption, InvalidValue), UserError unwrapping, and PlatformError/BadArgument with reason-based classification. Classification fixes from a construction-site audit of all ~420 classes: local filesystem failures mislabeled as config/input errors now classify as permission; CLI-internal declarative script failures move to internal_bug; transport failures conflated with user errors in LegacyDbQueryExecError, LegacyDeclarativeApplyError, and LegacyDomainsCnameError are split via typed discriminants; paused projects get the new open_dashboard suggestion type; and LegacyDbConfigIpv6Error's suggestion now matches the link command the CLI actually prints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68442e6671
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Coverage scan now recognizes any local `*Error("Tag")` factory (e.g. the
next login shell's LoginError), closing the drift-guard gap for NoTtyError
and LoginFailedError, which now declare auth classifications.
- classifyCliErrorActionability recurses into a StackError's preserved
tagged cause before falling back, and uses own-property adapter lookups so
sanitized tags like "constructor" cannot hit Object.prototype.
- DockerPullError.daemonDown now also covers docker spawn failures (binary
missing), not just daemon-down output.
- DeleteFunctionUnexpectedStatusError, LegacyDbQueryUnexpectedStatusError,
and LegacyLinkAuthTokenError thread the response status and classify via
statusCodeActionability, so 401s land in auth and 5xx in api_status.
- Paused projects get a dedicated project_paused category instead of
polluting plan_limit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11232cb803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ures - classifyCliErrorActionability recurses into the classifiable preserved cause of stack wrapper errors (asset-preparation StackBuildError, DownloadError, StackError), so a daemon-down DockerPullError or a local filesystem PlatformError is no longer reported as a registry/network failure. Recursion is guarded: an unclassifiable cause keeps the wrapper's own bucket. - statusCodeActionability and the HttpClientError adapter classify 403 as user-actionable account permission (after the entitlement-gate branch, so confirmed plan gates stay plan_limit). - packages/api joins the adapter coverage scan; SupabaseApiConfigError classifies as auth/set_env_var. - LegacyStorageAuthTokenError branches on its stored status like its link twin, so api-keys 5xx responses stop counting as user login problems. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ror-actionability-colocated
The coverage guard correctly failed on the develop merge: the stop/status port and config-validation refactor introduced new error classes. Each now declares its actionability from its construction sites (workdir flags → provide_flags, config.toml content → invalid_config, docker CLI wrapper failures → docker_not_running, missing/unhealthy containers → start-stack remediation), LegacyContainerRuntimeNotFoundError is exported for guard verification, and packages/config's InvalidRemoteProjectIdError joins the external adapters as invalid_config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Defines the CLI error actionability taxonomy for KPI reporting (CLI-1560). Replaces #5775 with an architecture where classification lives on the error types themselves instead of a centralized tag registry.
What
apps/cli/src/shared/telemetry/error-actionability.ts):error_kind,error_category,suggestion_typeenums, the sanitizedCliErrorActionabilitycontract, and the Q2 metric definitions (strict recovery, repeat error, internal/unknown bug rate).apps/cli(391 classes) declares its own classification via a getter under theErrorActionabilityIdsymbol, next to the message/suggestion it already carries. Instance-dependent cases branch on typed fields (statusCodeActionability(this.status, { upgradeSuggested })), never on message text.TaggedErrorinapps/cli/srcand fails if one lacks a declaration (or isn't exported for verification), and checks that every error tag inpackages/{stack,config,process-compose}has an external adapter. A productionunknownclassification now means "genuinely unforeseen failure", not registry drift.StackBuildError.reason(invalid_config/asset_preparation),DockerPullError.daemonDownandLegacyDockerRunError.reason/daemonDown(detected at the docker boundary where the output is produced), and a typedupgradeSuggestedprop threaded fromlegacySuggestUpgradeinto the plan-gated sso/branches/vanity-subdomains errors — noObject.definePropertyside-channel, no sniffing user-facing English for "upgrade"/"billing"/"quota".Why the different architecture vs #5775
The centralized
actionabilityByTagmap re-stated, far from each definition, facts each error already knows about itself. It covered 229 of 407 tags after 169 review-comment rounds, and any new error silently fell through tounknown— inflating the exact KPI numerator this project is meant to measure. With co-location the classification is reviewed in the same diff that introduces an error, and the coverage test turns "we forgot to classify" into a CI failure instead of a data-quality bug.Scope notes
cli_command_executedstill emits only existing fields. Wiring classification into failure events (and command-name attribution for pre-handler failures) is CLI-1561.has_suggestion/suggestion_typefrom the code path that renders suggestions, so telemetry can never disagree with what the user saw.🤖 Generated with Claude Code