Skip to content

fix(wizard): handle plan limits, workspace scoping, and clean errors - #303

Open
vpetersson wants to merge 4 commits into
masterfrom
fix/wizard-plan-limit-workspace-scoping
Open

fix(wizard): handle plan limits, workspace scoping, and clean errors#303
vpetersson wants to merge 4 commits into
masterfrom
fix/wizard-plan-limit-workspace-scoping

Conversation

@vpetersson

@vpetersson vpetersson commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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):

  • Wrong workspace binding. The auth screen picked the user's is_default_team workspace, but /api/v1/products and /components resolve 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.
  • Plan-limit dead end. Creating a product past the plan limit failed with a raw [403] message and only offered "return to Review and retry" — which re-fails identically.
  • Retry couldn't recover. A retry after a partial apply died on DUPLICATE_NAME for the product it had already created.
  • Status codes leaked into user-facing error text.

Changes

  • Bind contact-profile prefetch to the workspace the token can actually read: verify the picked workspace, probe the rest on a 403 (_resolve_profile_workspace).
  • Give 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.
  • get-or-create semantics for create_product and create_contact_profile (recover from DUPLICATE_NAME by name lookup).
  • Detect plan limits via the backend BILLING_LIMIT_EXCEEDED error code; tag PlanLimitError with the resource so the CTA can be tailored.
  • Strip HTTP status codes from every user-facing error string; full codes still go to the debug log.
  • Rewrite British spellings to US English across the package and tests.

Testing

  • uv run pytest — 2454 passed, 4 skipped.
  • ruff check / ruff format clean.
  • New coverage: product/contact-profile duplicate recovery, plan-limit typing + clean messages, workspace probing (readable / forbidden / none-readable), status-code stripping, and Apply-screen plan-limit reuse + Back-routing (Textual pilot).

🤖 Generated with Claude Code

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>
Copilot AI review requested due to automatic review settings July 23, 2026 12:54
Comment thread sbomify_action/cli/wizard/screens/authenticate.py Fixed

Copilot AI 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.

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.

Comment thread sbomify_action/sbomify_api.py Outdated
- 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>
Copilot AI review requested due to automatic review settings July 23, 2026 13:06

Copilot AI 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.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.

Comment thread sbomify_action/cli/wizard/screens/_base.py Outdated
Comment thread sbomify_action/cli/wizard/screens/authenticate.py
- 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>
Copilot AI review requested due to automatic review settings July 23, 2026 13:17

Copilot AI 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.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Comment thread sbomify_action/sbomify_api.py Outdated
- 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>
Copilot AI review requested due to automatic review settings July 23, 2026 13:23

Copilot AI 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.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

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.

3 participants