You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Addresses #4505. Today every catalog add command 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":
Identical rerun → successful no-op (exit code 0), so the same catalog add is safe inside a re-runnable workflow.
Same identity but different flags → conflict (exit code 1). We deliberately do not silently overwrite priority/install permissions — the user is told to remove the entry first. (Overwrite is Option B, explicitly out of scope for the recommended option.)
New entry → added, exactly as before.
Existing URL/config validation and safety failures are preserved unchanged.
Scope — all six catalog families
Command
Identity key
Equivalence compares
specify extension catalog add
catalog name
url, priority, install_allowed, description
specify preset catalog add
catalog name
url, priority, install_allowed, description
specify integration catalog add
catalog url
name (or none supplied)
specify workflow catalog add
catalog url
name (or none supplied)
specify workflow step catalog add
catalog url
name (or none supplied)
specify bundle catalog add
source id or url
policy, priority
Implementation notes
The URL-identity class methods (IntegrationCatalog/WorkflowCatalog/StepCatalog.add_catalog) now return "added" / "unchanged" so the CLI can report the no-op; a same-URL add with a different--name raises a conflict.
bundler.commands_impl.catalog_config.add_source now returns (CatalogSource, status).
Extension/preset CLI commands compare the requested flags against the stored entry inline and either no-op or error.
Tests & docs
Updated the former "duplicate rejected" tests to the new semantics and added no-op + conflict coverage per family (extension, preset, integration, workflow, step, bundle).
Documented the idempotent behavior in the five reference docs (extensions, presets, workflows, integrations, bundles).
Full relevant suites pass locally: 5147 passed, 5 skipped.
Family checklist
extension catalog
preset catalog
integration catalog
workflow catalog
step catalog
bundle catalog
This pull request was authored autonomously by GitHub Copilot (model: Claude Opus 4.8) on behalf of @mnriem.
…bundle add
Resolve Copilot review findings on the idempotent catalog-add work (github#4505):
- Bundle: parse a matching existing entry through CatalogSource.from_dict
before comparing, so a hand-edited non-integer priority surfaces as a clean
BundlerError instead of leaking int()'s ValueError/OverflowError past the
CLI's `except BundlerError`; string priorities now compare equal too.
- Extensions/presets: normalize a stored entry's priority (numeric strings)
and install_allowed (string booleans) with the same rules as the catalog
reader, so a valid equivalent rerun is a no-op instead of a false conflict.
- Tests: add same-explicit-name no-op coverage for integration and workflow
(and step) catalogs, string-representation idempotency for extensions and
presets, and bundle coverage for the malformed/string stored priority paths.
- Remove unused imports flagged by ruff.
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Posted on behalf of @mnriem by GitHub Copilot (model: claude-opus-4.8, autonomous).
Addressed the Copilot review findings in 5a546c98:
Bundle — validate before comparing priority (bundler/commands_impl/catalog_config.py): a matching existing entry is now parsed through CatalogSource.from_dict() before its fields are compared, so a hand-edited non-integer priority surfaces as a clean BundlerError instead of leaking int()'s ValueError/OverflowError past the CLI's except BundlerError. Supported string priorities also compare equal now.
Extensions & presets — normalize before comparing (extensions/_commands.py, presets/_commands.py): the equivalence check now normalizes a stored entry's priority (numeric strings) and install_allowed (string booleans like "false") with the same rules as the catalog reader, so a valid equivalent rerun is a no-op rather than a false conflict.
Same-explicit-name no-op coverage (integration + workflow/step catalogs): added tests that add with an explicit --name and rerun with that same name, exercising the previously-untested positive branch.
Added string-representation idempotency tests for extensions, presets, and bundle, plus a malformed-stored-priority test; removed the unused imports flagged by ruff.
ruff@0.15.0 check src tests is clean and the affected suites pass locally (2524 passed).
Workflow catalog loading strips URL whitespace, while this identity check compares raw values. Since validation accepts surrounding whitespace, equivalent invocations such as a padded URL followed by its normalized form bypass the no-op and append a duplicate catalog. Compare normalized URLs before deciding whether the entry exists.
Normalize step catalog URL identity
src/specify_cli/workflows/catalog.py:1443
Step catalog loading strips URL whitespace, while this identity check compares raw values. Because validation accepts surrounding whitespace, a padded URL and its normalized form are treated as different and a duplicate entry is written instead of returning "unchanged". Use the same normalization as the reader.
Document step catalog idempotency
docs/reference/workflows.md:408
This paragraph documents only specify workflow catalog add and .specify/workflow-catalogs.yml. The PR also changes specify workflow step catalog add, but the reference contains no corresponding step-catalog command or idempotency guidance, so one of the six catalog families remains undocumented. Add the step catalog command and its equivalent no-op/conflict behavior to this reference.
Catalog adds now normalize extension names and extension, preset, workflow, and step URLs consistently with their readers. Equivalent requests are no-ops; changed settings or names still conflict, without rewriting existing configuration. New entries store normalized identities.
Added CLI regression coverage for workflow and step reruns, including explicit names, whitespace variants, exit codes, output, and unchanged configuration bytes and modification times. The integration/workflow/step same-explicit-name API cases from 5a546c9 remain in place. The reference now documents step catalog commands and their no-op/conflict behavior, plus the whitespace rules.
Posted on behalf of @mnriem by GitHub Copilot (model: gpt-6-astra, autonomous).
The duplicate check compares raw YAML values, but CatalogSource.from_dict() strips surrounding whitespace from both id and url (bundler/models/catalog.py:73-75). Thus a valid stored source such as id: " local " is resolved as local, yet adding another URL with --id local misses the identity match and appends a duplicate instead of reporting a conflict/no-op. Apply the same normalization before testing either identity key.
Boolean priorities can be mistaken for valid integers
src/specify_cli/extensions/_commands.py:333
bool is a subclass of int, so returning the boolean unchanged still makes True == 1 and False == 0. A hand-edited catalog with priority: true can therefore be reported as an equivalent no-op for --priority 1, even though the catalog reader explicitly rejects boolean priorities in catalogs.py:150-156. Return a non-integer sentinel so malformed values conflict instead of succeeding.
Boolean priorities can be mistaken for valid integers
src/specify_cli/presets/_commands.py:54
bool is a subclass of int, so returning the boolean unchanged still makes True == 1 and False == 0. A hand-edited catalog with priority: true can therefore be reported as an equivalent no-op for --priority 1, even though the preset reader rejects boolean priorities in presets/__init__.py:4623-4633. Return a non-integer sentinel so malformed values conflict instead of succeeding.
Extension and preset comparisons now reject boolean priorities rather than equating them with 0/1, while valid integers and numeric strings remain equivalent. Bundle duplicate matching now strips stored id/URL whitespace before deciding whether to return a no-op or report a conflict. Added regression coverage for both identity keys, malformed matching entries, CLI outcomes, and unchanged configuration bytes/modification times, with corresponding reference updates.
The carried-over coverage is present in TestWorkflowCatalogAddCLI, including explicit-name reruns and conflicting-name exit/output checks. The same-name API cases are also present for workflows, steps, and integrations.
Also corrected the prior round’s Windows teardown failure: CLI working-directory changes are now scoped and restored before temporary project cleanup, rather than waiting for the shared monkeypatch fixture to finish.
Posted on behalf of @mnriem by GitHub Copilot (model: gpt-6-astra, autonomous).
The new conflict path can include the user-editable stored catalog name in exc, and the no-op path prints the supplied URL directly. Rich parses both values as markup, so a valid value containing an unmatched tag such as [/red] raises MarkupError instead of returning the documented clean exit. Escape both strings before printing; the existing catalog-list tests establish that these fields are user-editable markup inputs.
Escape workflow catalog values before Rich rendering
src/specify_cli/workflows/_commands.py:3026
A conflicting rerun now places the stored, user-editable catalog name in exc, while a no-op prints the URL directly. Since console.print parses markup, values such as [/red] can raise MarkupError instead of producing exit code 1 or the successful no-op promised here. Escape the exception and URL before interpolation.
This issue also appears on line 3756 of the same file.
Normalized installed extension command and alias names before collision checks, preventing equivalent spellings from overwriting the same generated output.
Resolved project-installed workflow scripts from .specify/scripts while retaining .specify/templates for commands and templates.
Matched extension catalog comparisons to the reader's index-based default for omitted priorities.
Matched preset catalog comparisons to reader normalization for padded names and omitted priorities.
Validation: 1,413 affected extension, preset, and artifact tests passed; Ruff 0.15.0 passed for all changed Python files.
Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous); the code changes and this comment were fully AI-drafted.
Do not bypass invalid priority validation on a no-op
src/specify_cli/integrations/catalog.py:493
The matching-URL branch returns before validating that entry's priority. With a stored value such as priority: 'first', add_catalog() now reports an unchanged success even though get_active_catalogs() rejects the same configuration. Validate the priority before the idempotent return so the command does not claim success for an unusable catalog config, and add a matching-URL regression test.
Treat a same-URL source as idempotent without an explicit ID
If a source was originally added with a custom --id, rerunning the same URL without --id derives a different resolved_id. The URL match enters this branch, but the ID equality check turns an otherwise identical rerun into a conflict. Because the documented identity is source ID or URL, an omitted ID should preserve the matching stored ID; an explicitly different --id should still conflict. Add coverage for the omitted-ID rerun.
…bundle add
Resolve Copilot review findings on the idempotent catalog-add work (github#4505):
- Bundle: parse a matching existing entry through CatalogSource.from_dict
before comparing, so a hand-edited non-integer priority surfaces as a clean
BundlerError instead of leaking int()'s ValueError/OverflowError past the
CLI's `except BundlerError`; string priorities now compare equal too.
- Extensions/presets: normalize a stored entry's priority (numeric strings)
and install_allowed (string booleans) with the same rules as the catalog
reader, so a valid equivalent rerun is a no-op instead of a false conflict.
- Tests: add same-explicit-name no-op coverage for integration and workflow
(and step) catalogs, string-representation idempotency for extensions and
presets, and bundle coverage for the malformed/string stored priority paths.
- Remove unused imports flagged by ruff.
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Rebased #4543 onto current upstream/main and addressed the latest review in f7aa350b:
Removed the unrelated artifact-introspection and extension-collision follow-up changes from this PR; the diff is again limited to catalog-add behavior, documentation, and tests.
Normalized missing, null, and whitespace-only extension/preset catalog names using the readers' catalog-N identity rules.
Validated a matching integration catalog entry's stored priority before returning an idempotent success.
Preserved a stored custom bundle source ID when the same URL is re-added without an explicit --id, while retaining conflicts for explicitly different IDs.
The intra-manifest extension collision finding is no longer in this PR because src/specify_cli/extensions/__init__.py now matches upstream/main.
Validation: 2,655 catalog-related tests passed; Ruff 0.15.0 passed for all changed Python files.
Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous); the rebase, code changes, and this comment were fully AI-produced.
…#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>
…bundle add
Resolve Copilot review findings on the idempotent catalog-add work (github#4505):
- Bundle: parse a matching existing entry through CatalogSource.from_dict
before comparing, so a hand-edited non-integer priority surfaces as a clean
BundlerError instead of leaking int()'s ValueError/OverflowError past the
CLI's `except BundlerError`; string priorities now compare equal too.
- Extensions/presets: normalize a stored entry's priority (numeric strings)
and install_allowed (string booleans) with the same rules as the catalog
reader, so a valid equivalent rerun is a no-op instead of a false conflict.
- Tests: add same-explicit-name no-op coverage for integration and workflow
(and step) catalogs, string-representation idempotency for extensions and
presets, and bundle coverage for the malformed/string stored priority paths.
- Remove unused imports flagged by ruff.
Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reject boolean priorities during extension and preset equivalence checks and normalize stored bundle identities before duplicate matching. Add regression coverage and document these rules. Scope CLI test working-directory changes so Windows can clean up temporary projects before fixture teardown.
Assisted-by: GitHub Copilot (model: gpt-6-astra, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Trim requested and stored extension and preset catalog names during removal so the recovery path uses the same identity rules as idempotent add.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reduced the implementation to the minimum catalog-add change: five existing implementation files cover extension, preset, integration, workflow, step, and bundle catalogs. An identical add returns without rewriting configuration; the existing duplicate error remains for different parameters. Removed all documentation changes, separate CLI result plumbing, and normalization/refactoring beyond the baseline. Focused regressions are contained in five existing test files.
Validation: full repository suite 8204 passed, 16 skipped, 48 warnings; Ruff passed. The PR now contains 10 files with 148 additions and 31 deletions.
Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol, autonomous); comment fully AI-drafted.
Posted on behalf of @mnriem by GitHub Copilot (autonomous); comment fully AI-drafted. Returning on the first equivalent id/URL match skips both malformed siblings and later matches with conflicting policy or priority. The command can therefore report a successful no-op for a config that load_source_stack() rejects or whose duplicate identity is not equivalent. Parse every source and inspect every matching identity before deciding between unchanged and conflict.
Validate all duplicate catalog entries before reporting no-op
src/specify_cli/extensions/_commands.py:719
Posted on behalf of @mnriem by GitHub Copilot (autonomous); comment fully AI-drafted. The no-op path returns as soon as one same-name entry is equivalent. It therefore reports success if a later sibling is malformed or if another entry with the same identity has different settings. Delay the no-op until every same-name entry has been checked, then validate the complete config with the normal catalog loader before reporting success.
Detect conflicting names across duplicate URLs
src/specify_cli/integrations/catalog.py:501
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous); comment fully AI-drafted. Only the first entry with this URL is recorded. In a hand-edited config with duplicate URLs, an explicit --name can therefore return "unchanged" when the first name matches even though a later entry has a different name. Inspect every entry with the matching URL and report a conflict if any stored name differs from the requested name.
Validate all duplicate preset entries before reporting no-op
src/specify_cli/presets/_commands.py:1008
Posted on behalf of @mnriem by GitHub Copilot (autonomous); comment fully AI-drafted. The no-op path returns as soon as one same-name entry is equivalent. It therefore reports success if a later sibling is malformed or if another entry with the same identity has different settings. Delay the no-op until every same-name entry has been checked, then validate the complete config with the normal preset loader before reporting success.
Collect all duplicate URL names before reporting unchanged
src/specify_cli/workflows/catalog.py:754
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous); comment fully AI-drafted. This returns based on the first entry with the URL. In a hand-edited config with duplicate URLs, an explicit --name can therefore succeed when the first name matches even though a later entry has a different name, contrary to the documented conflict behavior. Collect all matching names before deciding whether the add is unchanged.
Collect all duplicate catalog URL names before reporting unchanged
src/specify_cli/workflows/catalog.py:1458
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous); comment fully AI-drafted. This returns based on the first entry with the URL. In a hand-edited step-catalog config with duplicate URLs, an explicit --name can therefore succeed when the first name matches even though a later entry has a different name, contrary to the documented conflict behavior. Collect all matching names before deciding whether the add is unchanged.
if cat.get("name", generated_name) == requested_name:
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
triage-nice-to-haveVerdict: evidence-backed fix or greenlit feature — land after review
2 participants
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.
mnriem
added
the
triage-nice-to-have
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.