feat: make catalog add idempotent across all catalog families (#4505) - #4543
Open
mnriem wants to merge 1 commit into
Open
feat: make catalog add idempotent across all catalog families (#4505)#4543mnriem wants to merge 1 commit into
mnriem wants to merge 1 commit into
Conversation
…#4505) Rerunning `catalog add` for an already-configured entry now succeeds as a no-op instead of failing, so the command is safe inside re-runnable workflows. This implements the assessment's recommended Option A ("idempotent no-op for equivalent entries"): an identical rerun exits 0, while a request that matches an existing entry's identity but supplies different settings is rejected as a conflict rather than silently overwriting priority/install permissions. Applied consistently to all six families: - extension / preset — identity = catalog name - integration / workflow / step — identity = catalog URL - bundle — identity = source id or url The URL-identity class methods now return "added"/"unchanged" so the CLI can report the no-op, and bundle add_source returns (source, status). Updates duplicate-add tests to the new semantics, adds no-op + conflict coverage per family, and documents the behavior in the reference docs. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Supported YAML representations can produce false conflicts, and malformed bundle priorities can escape as uncaught exceptions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 3
New issues introduced by this change (5)
| Severity | Finding |
|---|---|
 |
src/specify_cli/bundler/commands_impl/catalog_config.py — Validate existing bundle entries before comparing priority View comment |
|
src/specify_cli/extensions/_commands.py — Compare normalized extension catalog settings View comment |
|
src/specify_cli/presets/_commands.py — Compare normalized preset catalog settings View comment |
 |
src/specify_cli/integrations/catalog.py — Test the explicit same-name no-op path View comment |
|
src/specify_cli/workflows/catalog.py — Test workflow reruns with the same explicit name View comment |
What changed in this PR
Makes all six catalog-add families idempotent while preserving conflicts for changed settings.
Changes:
- Returns successful no-ops for equivalent entries.
- Adds conflict handling and regression tests.
- Documents identity and equivalence rules.
| File | Description |
|---|---|
tests/unit/test_bundler_catalog_config.py |
Updates bundle API expectations. |
tests/test_workflows.py |
Tests workflow and step idempotency. |
tests/test_presets.py |
Tests preset no-op and conflict behavior. |
tests/test_extensions.py |
Tests extension no-op and conflicts. |
tests/integrations/test_integration_catalog.py |
Tests integration catalog statuses. |
tests/integrations/test_cli.py |
Tests integration CLI behavior. |
tests/contract/test_bundle_cli.py |
Adds bundle CLI contract tests. |
src/specify_cli/workflows/catalog.py |
Implements workflow and step no-ops. |
src/specify_cli/workflows/_commands.py |
Reports workflow catalog statuses. |
src/specify_cli/presets/_commands.py |
Implements preset equivalence checks. |
src/specify_cli/integrations/catalog.py |
Implements integration URL idempotency. |
src/specify_cli/integrations/_query_commands.py |
Reports integration catalog statuses. |
src/specify_cli/extensions/_commands.py |
Implements extension equivalence checks. |
src/specify_cli/commands/bundle/__init__.py |
Reports bundle catalog statuses. |
src/specify_cli/bundler/commands_impl/catalog_config.py |
Implements bundle source idempotency. |
docs/reference/workflows.md |
Documents workflow behavior. |
docs/reference/presets.md |
Documents preset behavior. |
docs/reference/integrations.md |
Documents integration behavior. |
docs/reference/extensions.md |
Documents extension behavior. |
docs/reference/bundles.md |
Documents bundle behavior. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+200
to
+206
| if ( | ||
| existing.get("id") == resolved_id | ||
| and existing.get("url") == url | ||
| and int(existing.get("priority", 0)) == desired["priority"] | ||
| and str(existing.get("install_policy", "")) == desired["install_policy"] | ||
| ): | ||
| return CatalogSource.from_dict(dict(existing), Scope.PROJECT), "unchanged" |
Comment on lines
+624
to
+627
| str(existing.get("url", "")) == url | ||
| and existing.get("priority") == priority | ||
| and bool(existing.get("install_allowed", False)) == install_allowed | ||
| and str(existing.get("description", "")) == description |
Comment on lines
+913
to
+916
| str(existing.get("url", "")) == url | ||
| and existing.get("priority") == priority | ||
| and bool(existing.get("install_allowed", False)) == install_allowed | ||
| and str(existing.get("description", "")) == description |
Comment on lines
+463
to
+465
| existing_name = str(cat.get("name", "")).strip() | ||
| if not requested_name or requested_name == existing_name: | ||
| return "unchanged" |
Comment on lines
+746
to
+748
| existing_name = str(cat.get("name", "")).strip() | ||
| if not requested_name or requested_name == existing_name: | ||
| return "unchanged" |
mnriem
added
the
triage-nice-to-have
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.
Summary
Addresses #4505. Today every
catalog addcommand fails when a matching entry already exists, which forces workflow authors to add their own duplicate-detection/remove-then-add logic around a command that may run repeatedly.This makes
catalog addidempotent, implementing the feature assessment's recommended Option A — "idempotent no-op for equivalent entries":catalog addis safe inside a re-runnable workflow.Scope — all six catalog families
specify extension catalog addspecify preset catalog addspecify integration catalog addspecify workflow catalog addspecify workflow step catalog addspecify bundle catalog addImplementation notes
IntegrationCatalog/WorkflowCatalog/StepCatalog.add_catalog) now return"added"/"unchanged"so the CLI can report the no-op; a same-URL add with a different--nameraises a conflict.bundler.commands_impl.catalog_config.add_sourcenow returns(CatalogSource, status).Tests & docs
extensions,presets,workflows,integrations,bundles).Family checklist
This pull request was authored autonomously by GitHub Copilot (model: Claude Opus 4.8) on behalf of @mnriem.