Skip to content

fix(integrations): catch OverflowError on a 'priority: .inf' in add/remove#3589

Open
chuenchen309 wants to merge 1 commit into
github:mainfrom
chuenchen309:fix/integration-catalog-inf-priority
Open

fix(integrations): catch OverflowError on a 'priority: .inf' in add/remove#3589
chuenchen309 wants to merge 1 commit into
github:mainfrom
chuenchen309:fix/integration-catalog-inf-priority

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

Description

IntegrationCatalog.add_catalog and remove_catalog re-validate the existing catalog entries' priorities inline (separately from the inherited base loader). Both did int(raw_priority) under except (TypeError, ValueError), so a priority: .inf (which YAML loads as float('inf')) raised OverflowError, which neither handler catches:

>>> from specify_cli.integrations.catalog import IntegrationCatalog
>>> # .specify/integration-catalogs.yml contains an entry with `priority: .inf`
>>> IntegrationCatalog(project_root).add_catalog("https://new.example.com/catalog.json")
OverflowError: cannot convert float infinity to integer   # not IntegrationValidationError

add_catalog leaked a raw traceback instead of the documented IntegrationValidationError, and remove_catalog crashed while building the display order.

This adds OverflowError to both handlers:

  • add_catalog now raises IntegrationValidationError (fail-fast on a corrupt sibling entry, as the surrounding comment intends).
  • remove_catalog falls back to positional order, like the other non-integer priorities there.

Relationship to the earlier fixes: #3525 fixed the base CatalogStackBase._load_catalog_config (which extensions/integrations inherit for the load path) and the preset loader; #3526 fixed the workflow/step loaders including their own add_catalog. The IntegrationCatalog.add_catalog/remove_catalog methods have their own inline priority parsing that neither of those touched, so this is the remaining sibling. catch (TypeError, ValueError, OverflowError) here mirrors both.

Testing

  • uv run pytest tests/integrations/test_integration_catalog.py108 passed (added test_add_catalog_rejects_inf_priority_in_existing_entry and test_remove_catalog_tolerates_inf_priority; both fail on main with OverflowError and pass with this change).
  • uvx ruff check src/ clean.
  • Full uv run pytest (ran the integration-catalog module; the full suite is a slow integration run).

AI Disclosure

  • I did not use AI assistance for this contribution.
  • I did use AI assistance for this contribution.

This PR was authored by an AI coding agent (Claude Code) running on this account: the AI noticed that #3525/#3526 left the IntegrationCatalog add/remove priority parsing unpatched, reproduced the OverflowError, wrote the fix and the two regression tests, and wrote this description. The account holder reviews every change and is accountable for it. The fail-before/pass-after tests are real and re-runnable from the diff. If this isn't the kind of contribution you want, say so and I'll close it.

…emove

IntegrationCatalog.add_catalog and remove_catalog re-validate the
existing catalog entries' priorities inline, separately from the base
loader. Both did `int(raw_priority)` under `except (TypeError,
ValueError)`, so a `priority: .inf` (float('inf')) raised OverflowError:
add_catalog leaked a raw traceback instead of IntegrationValidationError,
and remove_catalog crashed while building the display order.

Add OverflowError to both handlers, matching the base loader (github#3525) and
the workflow/step loaders (github#3526). add_catalog now raises
IntegrationValidationError; remove_catalog falls back to positional order
like the other non-integer priorities.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chuenchen309
chuenchen309 requested a review from mnriem as a code owner July 19, 2026 03:09
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