fix(wizard): handle plan limits, workspace scoping, and clean errors - #303
Open
vpetersson wants to merge 4 commits into
Open
fix(wizard): handle plan limits, workspace scoping, and clean errors#303vpetersson wants to merge 4 commits into
vpetersson wants to merge 4 commits into
Conversation
Fixes several wizard failures surfaced when onboarding a workspace whose token is scoped to a non-default workspace and hits a plan limit. - Bind contact-profile prefetch to the workspace the token can actually read: the auth screen now verifies the is_default_team-picked workspace and probes the rest on a 403, instead of silently treating the scope denial as "0 profiles" (which showed an empty picker and created new profiles in the wrong workspace). - Give product/component plan-limit failures a real recovery path on the Apply screen: reuse the single existing product & retry in place, or jump Back to the Pick-a-product / Components step — not a dead-end "retry". - get-or-create semantics for create_product and create_contact_profile so a retry after a partial apply (or a lost-response resubmit) reuses the existing resource instead of dying on DUPLICATE_NAME. - Detect plan limits via the backend's BILLING_LIMIT_EXCEEDED error code; tag PlanLimitError with the resource so the UI can tailor the CTA. - Strip HTTP status codes from every user-facing error string (apply banner/log, create-profile status, auth status); full codes still go to the debug log. - Rewrite British spellings to US English across the package and tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the onboarding Textual wizard against real-world sbomify API/workspace edge cases (scoped tokens, plan limits, and retry-after-partial-apply), while also improving user-facing error text by removing HTTP status-code noise.
Changes:
- Make wizard workspace/profile binding resilient to scoped tokens by probing for a workspace where contact profiles are readable, and binding profile operations to that workspace.
- Add plan-limit-specific recovery UX on the Apply screen (reuse existing product when possible; otherwise route Back to the relevant step) and implement get-or-create semantics for products/contact profiles to make retries idempotent.
- Strip HTTP status codes from user-visible error messages (while keeping richer details in logs), and normalize spelling to US English across code/tests.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_wizard_textual.py | Adds Textual pilot coverage for plan-limit recovery CTAs and back-routing behavior. |
| tests/test_wizard_state.py | Adds unit tests for workspace/profile resolution and status-code stripping; updates mocks for new product-create return shape. |
| tests/test_wizard_emitter.py | Updates wizard emitter tests for updated create_product mock return shape. |
| tests/test_upload_module.py | Renames test to US spelling (normalizes). |
| tests/test_sbomify_api.py | Adds/updates API-client tests for duplicate recovery, plan-limit typing, and contact-profile envelope handling. |
| tests/test_enrichment_module.py | US spelling updates in test names/comments/assert variables. |
| sbomify_action/spdx3.py | US spelling updates in comments/docstrings. |
| sbomify_action/sbomify_api.py | Implements plan-limit detection helper, product get-by-name + get-or-create semantics, and contact-profile envelope/duplicate recovery. |
| sbomify_action/exceptions.py | Extends PlanLimitError with a resource attribute for UI-specific recovery paths. |
| sbomify_action/enrichment.py | US spelling updates in comments/docstrings/locals. |
| sbomify_action/cli/wizard/widgets/stateful_radio.py | US spelling updates in docs/comments (color/colored). |
| sbomify_action/cli/wizard/widgets/pick_or_create.py | US spelling update in docstring. |
| sbomify_action/cli/wizard/styles.tcss | US spelling updates in stylesheet comments. |
| sbomify_action/cli/wizard/state.py | US spelling update in docstring. |
| sbomify_action/cli/wizard/screens/welcome.py | US spelling update in comment. |
| sbomify_action/cli/wizard/screens/review.py | US spelling update in docstring (“Color …”). |
| sbomify_action/cli/wizard/screens/done.py | US spelling updates in comments. |
| sbomify_action/cli/wizard/screens/create_profile.py | Escapes/cleans API errors for markup safety and strips status codes in UI error rendering; US spelling updates. |
| sbomify_action/cli/wizard/screens/configure_sbomify_json.py | US spelling update in subtitle. |
| sbomify_action/cli/wizard/screens/configure_sbom.py | US spelling updates and adjusts user notifications (“Canceled”). |
| sbomify_action/cli/wizard/screens/authenticate.py | Adds _resolve_profile_workspace probing logic and strips status codes/escapes markup in auth errors. |
| sbomify_action/cli/wizard/screens/apply.py | Adds plan-limit-aware UX, status-code stripping, and in-place retry using an existing product. |
| sbomify_action/cli/wizard/screens/_base.py | Introduces strip_status_codes() helper used by wizard screens. |
| sbomify_action/cli/wizard/io.py | US spelling updates in comments/docstrings. |
| sbomify_action/cli/wizard/apply.py | Consumes (product, was_created) create-product semantics for accurate logging/state. |
| sbomify_action/cli/main.py | US spelling update in comment/docstring. |
| sbomify_action/_generation/utils.py | US spelling update in comment. |
| sbomify_action/_enrichment/sources/pypi.py | US spelling update in comments. |
| sbomify_action/_enrichment/metadata.py | US spelling updates in dataclass field comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Keep SbomifyApiClient.create_product(name) -> dict backward-compatible (thin POST wrapper: returns the product, raises PlanLimitError on the plan limit, APIError otherwise). Move the DUPLICATE_NAME get-or-create recovery into a new get_or_create_product(name) -> (product, was_created) that the wizard uses — matching the module's "orchestration lives on top, not inside the endpoint wrapper" design and avoiding a breaking signature change for external callers. - Reword the workspace-scoping log line so it no longer trips the Opengrep python-logger-credential-disclosure heuristic; it only ever logged workspace keys, never a credential. - Update tests to the split API (create_product returns dict; get_or_create_product covers create / duplicate-recovery / not-found re-raise / plan-limit propagation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- strip_status_codes: anchor the regex to the exact shape _build_error emits — a real HTTP status (1xx–5xx) in brackets, followed by " - " or end-of-string. A bracketed number embedded in a (quoted) name like 'Widget [123]' is now left untouched instead of being mangled. - _resolve_profile_workspace: only switch workspaces on a genuine scope 403, never on a transient error. Added a typed ForbiddenError (403, subclass of APIError) raised by list_contact_profiles; the resolver probes other workspaces only on ForbiddenError and stays on the picked workspace (empty profiles, best-effort) for a 500/timeout — guessing a different workspace off a blip was the wrong-workspace binding this code exists to prevent. - Tests: ForbiddenError on 403, transient-error-stays-put, and the quoted-name / out-of-range-code strip cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Factor plan-limit message building into _plan_limit_message(resource, name, raw_detail), shared by create_product and create_component. It uses the backend detail only when it is actually a non-empty string; a list/dict detail falls back to a generic human sentence instead of being interpolated as a Python repr, and it never carries the HTTP status code. Fixes the component path, which previously interpolated a non-string detail and fell back to err_msg (which included [status]). - Test: BILLING_LIMIT_EXCEEDED 403 with a structured (list) detail stays clean and status-code-free. Co-Authored-By: Claude Opus 4.8 <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
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.
Important
Depends on sbomify/sbomify#1206 (backend token-scope enforcement on the workspaces API), now merged and validated on stage. The wizard's workspace-resolution fix here relies on
GET /api/v1/workspaces/returning only the token's bound workspace and on contact-profile writes being scope-gated; without #1206 deployed, a scoped token still binds/creates in the wrong workspace.Fixes several wizard failures surfaced when onboarding a workspace whose token is scoped to a non-default workspace and hits a plan limit. Companion to sbomify#1206, which fixed the backend token-scope leak this uncovered.
What was broken
Reproduced live against a scoped PAT (bound to a community-plan workspace, user belonging to many workspaces):
is_default_teamworkspace, but/api/v1/productsand/componentsresolve to the token's bound workspace. Contact-profile listing 403'd on the mismatched workspace and was silently swallowed as "0 profiles" — so the picker was empty and a profile created from the wizard landed in the wrong workspace.[403]message and only offered "return to Review and retry" — which re-fails identically.DUPLICATE_NAMEfor the product it had already created.Changes
_resolve_profile_workspace).create_productandcreate_contact_profile(recover fromDUPLICATE_NAMEby name lookup).BILLING_LIMIT_EXCEEDEDerror code; tagPlanLimitErrorwith the resource so the CTA can be tailored.Testing
uv run pytest— 2454 passed, 4 skipped.ruff check/ruff formatclean.🤖 Generated with Claude Code