fix(integrations): catch OverflowError on a 'priority: .inf' in add/remove#3589
Open
chuenchen309 wants to merge 1 commit into
Open
fix(integrations): catch OverflowError on a 'priority: .inf' in add/remove#3589chuenchen309 wants to merge 1 commit into
chuenchen309 wants to merge 1 commit into
Conversation
…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>
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.
Description
IntegrationCatalog.add_catalogandremove_catalogre-validate the existing catalog entries' priorities inline (separately from the inherited base loader). Both didint(raw_priority)underexcept (TypeError, ValueError), so apriority: .inf(which YAML loads asfloat('inf')) raisedOverflowError, which neither handler catches:add_catalogleaked a raw traceback instead of the documentedIntegrationValidationError, andremove_catalogcrashed while building the display order.This adds
OverflowErrorto both handlers:add_catalognow raisesIntegrationValidationError(fail-fast on a corrupt sibling entry, as the surrounding comment intends).remove_catalogfalls 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 ownadd_catalog. TheIntegrationCatalog.add_catalog/remove_catalogmethods 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.py— 108 passed (addedtest_add_catalog_rejects_inf_priority_in_existing_entryandtest_remove_catalog_tolerates_inf_priority; both fail onmainwithOverflowErrorand pass with this change).uvx ruff check src/clean.uv run pytest(ran the integration-catalog module; the full suite is a slow integration run).AI Disclosure
This PR was authored by an AI coding agent (Claude Code) running on this account: the AI noticed that #3525/#3526 left the
IntegrationCatalogadd/remove priority parsing unpatched, reproduced theOverflowError, 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.