Skip to content

feat(bundles): reconcile exact component pins and version conflicts - #4844

Open
mnriem wants to merge 11 commits into
github:mainfrom
mnriem:mnriem-feat-4719-bundler-reconciliation
Open

mnriem wants to merge 11 commits into
github:mainfrom
mnriem:mnriem-feat-4719-bundler-reconciliation

Conversation

@mnriem

@mnriem mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Part of #4719. This finishes the bundler-specific exact-pin behavior on top of current main. Historical extension and preset release downloads already landed in #4753; this PR does not duplicate that work or the separately claimed bundle catalog release-history slice.

  • Reject incompatible pins across installed bundles, version drift in already installed components, unsafe shared refreshes, and duplicate component IDs within a bundle before making changes. Allow an exclusive owning bundle to repair drift with --refresh.
  • Install pinned workflows and steps through the winning catalog's exact-release selection rather than the advertised current release. Preserve the primitive installers' artifact checksum and declared-version checks.
  • Treat optional component source as the expected winning install-allowed catalog, not a URL or a catalog-priority override; verify it even when the component is already installed. Validate exact releases online, distinguishing malformed catalog metadata from an unreachable catalog.
  • Update bundle reference documentation and add positive and negative regression coverage. Existing primitive catalog histories and the pending bundle catalog-history work remain outside this PR.

Testing

  • Tested locally with uv run specify --help (ran uv run --extra test specify --help: passed; bundle is registered).
  • Ran existing tests with uv sync && uv run pytest (used this worktree's own virtualenv instead, as documented in AGENTS.md).
  • Tested with a sample project (if applicable) (automated project fixtures were tested; no separate manual sample-project walkthrough).

uv sync --extra test --quiet: passed. LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q: 9,755 passed, 19 skipped, 62 warnings; 9,774 tests collected.

.venv/bin/python -m pytest tests/specify_cli/bundles tests/specify_cli/workflows/test_catalog_versions.py tests/specify_cli/workflows/test_command_add.py tests/specify_cli/workflows/step/test_catalog_versions.py tests/specify_cli/workflows/step/test_command_add.py -q: 701 passed.

uvx ruff check src/specify_cli/bundles/{_commands.py,adapters.py,conflict.py,installer.py,manifest.py,primitives.py,references.py} tests/specify_cli/bundles/{helpers.py,test_command_validate.py,test_conflict.py,test_installer.py,test_primitives.py,test_references.py,test_validator.py} --output-format concise: passed. git diff --check: passed.

New regression cases were observed failing before implementation for cross-bundle conflicts, installed-version drift, historical workflow/step selection, source and malformed-release validation, and duplicate IDs; those cases pass afterward.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: GitHub Copilot (GPT-6 Sol, autonomous mode; reasoning effort not explicitly configured) authored the implementation, regression tests, and documentation and ran the checks listed above. This submission has not been represented as human-authored or line-by-line human-reviewed.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:37

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.

Copilot review overview

🟡 Changes recommended

Workflow and step releases are re-resolved after validation, and extension/preset catalog outages are incorrectly treated as validation failures.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds exact component-pin reconciliation and cross-bundle version conflict protection.

Changes:

  • Detects duplicate IDs, incompatible pins, installed drift, and unsafe refreshes.
  • Resolves pinned workflow/step releases and validates catalog sources.
  • Expands regression coverage and bundle documentation.

Testing results were reported in the PR but not independently rerun during review.

File Description
src/​specify_cli/​bundles/​_commands.py Surfaces version conflicts in bundle previews.
src/​specify_cli/​bundles/​adapters.py Adds explicit catalog-source validation.
src/​specify_cli/​bundles/​conflict.py Detects incompatible cross-bundle pins.
src/​specify_cli/​bundles/​installer.py Enforces installed pins before mutation.
src/​specify_cli/​bundles/​manifest.py Rejects duplicate component IDs.
src/​specify_cli/​bundles/​primitives.py Adds exact workflow/step catalog selection.
src/​specify_cli/​bundles/​references.py Validates exact local and catalog references.
tests/​specify_cli/​bundles/​helpers.py Extends the fake installer with version tracking.
tests/​specify_cli/​bundles/​test_command_validate.py Uses the actual bundled extension version.
tests/​specify_cli/​bundles/​test_conflict.py Covers incompatible and unknown pins.
tests/​specify_cli/​bundles/​test_installer.py Covers drift, sharing, and source checks.
tests/​specify_cli/​bundles/​test_primitives.py Covers exact-release primitive installation.
tests/​specify_cli/​bundles/​test_references.py Covers exact, source, malformed, and outage validation.
tests/​specify_cli/​bundles/​test_validator.py Covers duplicate component rejection.
docs/​reference/​bundles.md Documents exact pins, conflicts, and source semantics.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/bundles/primitives.py Outdated
Comment thread src/specify_cli/bundles/references.py Outdated
@muhammadumer-waheed

Copy link
Copy Markdown
Contributor

One regression I hit while working on the same change (#4846): a step pinned to the version its catalog currently advertises now always goes through workflow step add --version. That path calls validate_checksums(required=True), so a legacy single-version step entry with no sha256 (installable today) now fails with Step '…' needs SHA-256 digests for exactly [...]. I confirmed it against this branch with the real workflow_step_add. #4846 forwarded --version only when the pin selects a historical release (select_release returns the entry itself for the current one). Something similar here would keep legacy entries working.

Also worth confirming it's intended: a pinned component whose catalog entry advertises no version now fails with "no catalog release", whereas #4753 kept that case as "cannot enforce, install as resolved".

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 13:16
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the review in e84c0a0. Bundle-selected workflow and step records now reach the existing download/install paths directly, without a second catalog lookup; install-policy, ID/path preflights, digest/version checks, and step-refresh rollback remain enforced. Extension and preset catalog fetch failures now have distinct types, so online bundle validation warns on outages but rejects malformed catalogs or releases. Added failing-before/passing-after regressions for both findings, plus catalog-failure rollback and unsafe-ID preflight coverage.

Validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,768 passed, 19 skipped; focused bundle/reference regression suite — 71 passed. Ruff correctness/import-order checks on the touched paths and git diff --check passed.

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous mode, default reasoning settings). Copilot generated the implementation, regression tests, documentation update, and this review-round summary.

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.

Copilot review overview

🟡 Changes recommended

Catalog error classification and workflow path shadowing can produce incorrect validation or installation failures.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (2)

Comment thread src/specify_cli/extensions/__init__.py
Comment thread src/specify_cli/bundles/references.py Outdated
Comment thread src/specify_cli/workflows/command_add.py Outdated
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 13:51
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the three findings in de576dc. ID-targeted extension, workflow, and step lookups now stop at the first valid winning source and reject malformed higher-priority catalog metadata; untargeted search retains its existing skip-and-continue behavior. Workflow and step catalog readers distinguish fetch outages from malformed content and unsafe redirects with typed errors, so bundle validation warns only when verification is unavailable. The private preselected-workflow install path bypasses local-path disambiguation while retaining ID, project, symlink, release, and download checks.

The new regression cases reproduced the prior malformed-catalog and path-shadowing behavior before the fix. Validation: LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short — 9,784 passed, 19 skipped; affected bundle, extension, workflow, and step suites — 2,449 passed. Ruff correctness checks and git diff --check passed. Reviewer conversations remain open for the reviewer.

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous mode, default reasoning settings). Copilot generated the implementation, regression tests, and this review-round summary.

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.

Copilot review overview

🟡 Changes recommended

Catalog redirect and decoding failures remain incorrectly classified, allowing unsafe fallback or uncaught errors.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Wrap malformed UTF-8 catalog responses as validation errors

src/​specify_cli/​extensions/​__init__.py:4808

json.loads() on a bytes response can raise UnicodeDecodeError for malformed UTF-8, which is not a JSONDecodeError. That exception escapes the new catalog-validation path and can crash extension search/exact lookup instead of reporting malformed metadata. Wrap decoding failures in ExtensionCatalogValidationError too.

Comment thread src/specify_cli/extensions/__init__.py Outdated
Comment thread src/specify_cli/presets/_catalog.py
Reject unsafe extension and preset catalog redirects during exact-ID lookups and classify malformed extension encodings. Cover stacked and legacy fetch paths with before-and-after regressions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 14:19
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the latest catalog review in 6995bd7. Exact-ID extension and preset lookups now reject unsafe higher-priority catalog redirects instead of falling through; malformed extension UTF-8 is a catalog-validation error. Preset direct-fetch validation errors retain their existing type, while both stacked and legacy fetch paths classify redirect-policy failures as validation errors. Added 11 regression cases covering unsafe redirect callbacks, redirect-policy failures, and invalid encodings; these failures were reproduced before the fix.

Validation: affected catalog and bundle suites: 716 passed; full suite (LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q): 9,795 passed, 19 skipped. Configured Ruff security rules (S602/S604/S605) pass; general Ruff diagnostics are unchanged from the branch baseline (218 existing findings across the affected production files).

AI disclosure: Prepared on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous). The code, tests, and this comment were AI-generated; no human line-by-line review is claimed.

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.

Copilot review overview

🔵 Needs a closer look

Targeted step lookup suppresses duplicate-ID errors, and deeply nested workflow or step catalogs can escape validation as raw exceptions.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Translate workflow catalog JSON recursion errors

src/​specify_cli/​workflows/​catalog/​_domain.py:616

The narrowed exception handling no longer translates RecursionError from json.loads (for a sufficiently nested, size-compliant catalog). Such malformed workflow catalogs now escape as raw exceptions instead of WorkflowCatalogValidationError. Include RecursionError among malformed-payload exceptions and add a nested-JSON regression case.

Medium severity Translate step catalog JSON recursion errors

src/​specify_cli/​workflows/​step/​catalog/​_domain.py:583

The narrowed exception handling no longer translates RecursionError from json.loads (for a sufficiently nested, size-compliant catalog). Such malformed step catalogs now escape as raw exceptions instead of StepCatalogValidationError. Include RecursionError among malformed-payload exceptions and add a nested-JSON regression case.

Medium severity Preserve duplicate-ID errors during targeted catalog lookup

src/​specify_cli/​workflows/​step/​catalog/​_domain.py:641

Targeted lookup now swallows the existing duplicate-ID error because it only re-raises StepCatalogValidationError. test_list_catalog_rejects_duplicate_step_ids therefore receives StepCatalogFetchError("All configured..."), and with multiple catalogs lookup can fall through to a lower-priority duplicate. Treat every non-fetch catalog error as malformed so precedence is preserved.

Classify JSON recursion during parsing or cache writes as malformed catalog data, recover from poisoned caches, and keep non-fetch step catalog errors blocking exact-ID lookup.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 14:49
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the previously missed catalog cases from review 5429853586 in 8a14af5. Workflow and step catalogs now classify deeply nested JSON as validation errors, including network parsing and cache serialization, and recover from malformed fresh or stale cache entries. Exact-ID step lookups no longer skip a non-fetch catalog error in favor of a lower-priority source. The existing duplicate-ID list test already passed before this round; a new regression covers a duplicate-ID error raised during fetching.

Before the fix, 7 of 8 new decoding/cache/precedence cases failed, and both cache-write cases raised raw RecursionError. Afterward, all 10 pass. Affected suites: 336 passed. Full suite (LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q): 9,805 passed, 19 skipped. Configured Ruff security checks pass; general Ruff diagnostics on touched files are unchanged from the branch baseline.

AI disclosure: Prepared on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous). The code, tests, and this comment were AI-generated; no human line-by-line review is claimed.

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.

Copilot review overview

🔵 Needs a closer look

Reference validation can approve bundled pin mismatches that installation rejects, and some unreachable extension catalogs are misclassified as fatal validation errors.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Validation accepts mismatched bundled presets that installation rejects

src/​specify_cli/​bundles/​references.py:38

A bundled preset with a different version only fails this local check, after which online validation can succeed against a catalog release. Installation does not have that fallback: _PresetKindManager always selects the bundled preset when source is absent and rejects the pin mismatch. This lets bundle validate approve a manifest that bundle install is guaranteed to reject; treat a mismatched bundled preset as a definitive resolution failure (or make installation fall back consistently).

This issue also appears on line 43 of the same file.

Medium severity Classify HTTPException as an unreachable extension catalog

src/​specify_cli/​extensions/​__init__.py:4814

Only URLError is classified as an unreachable extension catalog here. open_url() can also propagate http.client.HTTPException failures such as BadStatusLine, which are neither URLError nor OSError; these escape as generic exceptions and bundle validate reports a fatal “Catalog lookup failed” instead of the documented unreachable-catalog warning. Mirror the workflow/step fetchers by classifying HTTPException as ExtensionCatalogFetchError.

Reject unsourced bundled preset pins that the installer cannot satisfy, classify extension HTTP protocol failures as unreachable catalogs, and make deep-cache regressions deterministic across Python versions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 15:12
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the two “Previously missed” items in review 5430312635 with 2d91df2. Bundle validation now uses the installer’s own pin check when an unsourced bundled preset has a mismatched version; an explicitly selected catalog or a matching installed version remains valid. Extension HTTP protocol errors, including BadStatusLine, are classified as unreachable catalog fetches in both fetch paths, so online validation warns rather than reporting malformed metadata. Five new checker/fetch regression cases failed before the fix.

I also corrected the prior round’s deeply nested cache tests: Ubuntu/Python 3.14 parsed the fixture where Python 3.13 raised RecursionError. The tests now induce the cache decode failure deterministically. Affected suites on Python 3.13: 590 passed; reference and bundle-validation suites on Python 3.14: 71 passed. Full Python 3.13 suite (LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q): 9,813 passed, 19 skipped. New PR CI is pending.

AI disclosure: Prepared on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous). The code, tests, and this comment were AI-generated; no human line-by-line review is claimed.

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.

Copilot review overview

🟡 Changes recommended

Missing catalog collection keys can bypass higher-priority validation, and bundled extension validation can approve an installation that must fail.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Require workflows collection in catalog metadata

src/​specify_cli/​workflows/​catalog/​_domain.py:536

Requiring workflows only when the key happens to be present lets {} (or a schema-only response) count as a valid empty high-priority catalog. A targeted lookup then falls through to a lower-priority source, even though malformed higher-priority metadata is supposed to block that bypass. Require the workflows collection key with a dict/list value, and cover the missing-key payload as a negative case.

Medium severity Require steps collection in catalog metadata

src/​specify_cli/​workflows/​step/​catalog/​_domain.py:499

Requiring steps only when the key happens to be present lets {} (or a schema-only response) count as a valid empty high-priority catalog. A targeted lookup then falls through to a lower-priority source, even though malformed higher-priority metadata is supposed to block that bypass. Require the steps collection key with a dict/list value, and cover the missing-key payload as a negative case.

Comment thread src/specify_cli/bundles/references.py Outdated
Require workflow and step collections in fetched or cached catalogs before exact-ID lookup may fall through. Align bundle validation with bundled-first extension installs while retaining explicit catalog sources and matching installed versions.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 18:23
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5430642733 in f127aff. Workflow and step catalogs now require their respective collection keys with dict/list values; a missing collection in a higher-priority fetched or cached catalog blocks exact-ID fallback. Valid empty collections still allow lower-priority lookup. Bundle validation now applies the installer’s bundled-version check to extensions as well as presets: an unsourced mismatched bundled pin fails online and offline even when a matching catalog release exists, while an explicit catalog source opts into that release. Updated the bundle reference documentation and kept the earlier deep-nesting tests exercising parsing and cache writes with otherwise valid collections.

Before the fix, 12 selected cases failed; afterward, the affected suites passed (875 passed on Python 3.13; 157 passed on Python 3.14). Full worktree suite (LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short): 9,830 passed, 19 skipped (9,849 collected). Ruff checks on changed tests and the configured security rules passed. PR CI is pending.

AI disclosure: Prepared on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous). The code, tests, documentation, and this comment were AI-generated; no human line-by-line review is claimed.

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.

Copilot review overview

🔵 Needs a closer look

List-form workflow catalogs can silently select the last of multiple duplicate workflow IDs.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject duplicate workflow IDs in list-form catalogs

src/​specify_cli/​workflows/​catalog/​_domain.py:714

List-form workflow catalogs still accept the same workflow ID more than once: each occurrence overwrites merged[wf_id], and this targeted lookup then returns the last record. That makes an ambiguous/malformed catalog silently control which exact release gets installed, while the equivalent step-catalog path rejects duplicate IDs. Track IDs within each source and raise WorkflowCatalogValidationError on a duplicate before returning the winning entry.

Treat duplicate workflow IDs within one list-form catalog as invalid during exact-ID lookup and search, while retaining priority-based resolution across distinct sources.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 19:37
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the “Previously missed” item in review 5432942769 in 20c61cc. List-form workflow catalogs now reject duplicate workflow IDs within the same source before an exact-ID lookup returns, and search rejects the same ambiguity. An ID shared across distinct catalogs remains valid and is resolved by catalog priority. Updated the workflow catalog documentation.

Before the fix, four duplicate-ID regression cases failed while the distinct-source control passed; afterward, all five passed. Affected workflow/step/bundle tests: 263 passed. Full worktree suite (LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --tb=short): 9,835 passed, 19 skipped (9,854 collected). Ruff checks on changed tests and the configured security rules passed. PR CI is pending.

AI disclosure: Prepared on behalf of @mnriem by GitHub Copilot (GPT-6 Sol, autonomous). The code, tests, documentation, and this comment were AI-generated; no human line-by-line review is claimed.

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.

Copilot review overview

🔵 Needs a closer look

Partial catalog outages are incorrectly reported as definitive missing references instead of unverifiable references.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Catalog lookup masks unreachable sources as missing references

src/​specify_cli/​bundles/​references.py:83

None is not proof that the component is absent: the catalog lookup methods suppress individual fetch failures whenever another catalog was readable. For example, if a readable higher-priority catalog lacks this ID and a lower catalog is unreachable, current is None, so online bundle validation reports a definitive missing reference instead of the documented “catalog unreachable” warning. Preserve partial-fetch status (or raise the family’s fetch error when a targeted lookup exhausts catalogs after any unreachable source) so this helper can return the unreachable state; add a regression case covering one readable non-match plus one unreachable catalog.

Preserve uncertainty when an exact bundle reference is absent from readable catalogs but another configured catalog could not be fetched. Retain winning-source and untargeted search behavior; cover all four catalog families and CLI validation.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 20:27
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the previously missed partial-catalog-outage finding in review 5433755265 in commit dd2e3be. Targeted extension, preset, workflow, and step lookups now preserve fetch uncertainty when no catalog matches the requested ID and another configured source was unreachable, so bundle validation warns rather than reporting definitive absence. Winning readable matches and untargeted search behavior are unchanged; the preset versions command also reports its fetch failure rather than claiming no release exists.

Regression evidence: eight exact-pin partial-outage cases failed before the fix and pass afterward. The expanded reference tests cover both outage orders, readable-source absence, and matches; a CLI validation test checks the warning and successful exit. LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --disable-warnings --tb=short passed (9,856 passed, 19 skipped). The PR CI checks are still running at the time of this comment.

AI disclosure: Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol), autonomous mode, authored the code, tests, documentation, commit, and this summary; no human review or testing is claimed.

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.

Copilot review overview

🟡 Changes recommended

Conflict detection loses pins for independently installed components because records contain ownership rather than all bundle requirements.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread src/specify_cli/bundles/conflict.py
Keep independently installed component pins separate from bundle-owned contributions. Reject incompatible later installs, retain requirements on remove, and preserve legacy records' known pins.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 21:41

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.

Copilot review overview

🟡 Changes recommended

Required-only components are not considered during shared unpinned install and refresh protection.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)

Comment thread src/specify_cli/bundles/installer.py
@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5434259010 in ffdeac0. Installed-bundle records now persist required pins separately from contributed/owned components. Conflict checks use every requirement, including a compatible independent install skipped by the bundle, while attribution and removal remain ownership-based. Existing records fall back to their known contributed pins until reinstalled. Added failing-before/passing-after lifecycle, retention, serialization, corruption, and legacy-record regressions; all bundle tests passed (547), and the full suite passed (9,869 passed, 19 skipped) with LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --disable-warnings --tb=short. Platform pytest CI jobs were still running when this summary was posted; Ruff, markdownlint, and analyses passed.

AI disclosure: Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol), autonomous mode, authored the implementation, tests, documentation, commit, and this comment. No human review or testing is claimed.

Preserve ownership attribution while consulting all recorded pinned requirements in preflight. Reject unpinned installs or refreshes that may replace a required version and allow no-op sharing of a matching installation.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 21:51

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.

Copilot review overview

🔵 Needs a closer look

The cross-family catalog changes conflict with issue #4719’s required dedicated-PR boundaries.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Split catalog-family lookup changes from the bundler PR

src/​specify_cli/​workflows/​catalog/​_domain.py:663

Issue #4719 explicitly requires the bundler PR to consume catalog-family exact-version APIs and keeps workflow, step, extension, and preset behavior in separate area PRs. This change alters the core targeted-lookup/error semantics used by unqualified workflow info/add (with parallel changes in the other three catalog families), so this PR is no longer the dedicated bundler slice described by the issue. Please split these catalog-family changes into their respective area/foundation PRs and have the bundler consume the landed contract.

@mnriem

mnriem commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed review 5434931275 in commit 5e1141e. Preflight now reads pinned requirements from other bundles even when those bundles did not contribute/own the component. An unpinned install cannot introduce a missing component or accept a drifted version under another bundle’s pin, and an unpinned refresh cannot change a component it owns under such a pin. A matching already-installed component may still be shared or skipped on refresh without adoption. Ownership attribution and removal remain separate.

Regression evidence: three unpinned install/refresh cases failed before the fix and pass afterward; a matching no-op sharing case remains allowed. All 553 bundle tests passed. LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q --disable-warnings --tb=short passed (9,873 passed, 19 skipped). Ruff and Linux CI checks passed; macOS/Windows CI pytest jobs were still running at the time of this comment.

AI disclosure: Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol), autonomous mode, authored the implementation, tests, documentation, commit, and this summary. No human review or testing is claimed.

This branch has not been deployed

No deployments
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