Skip to content

fix: preserve existing priority from registry when installing/refreshing bundles - #5331

Merged
mnriem merged 3 commits into
github:mainfrom
Yi-111-a:fix/bundle-priority-preservation
Nov 10, 2026
Merged

mnriem merged 3 commits into
github:mainfrom
Yi-111-a:fix/bundle-priority-preservation

Conversation

@Yi-111-a

@Yi-111-a Yi-111-a commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Description

Fix: preserve existing priority from registry when installing/refreshing bundles

When bundle install --refresh or bundle update is run, the
existing user-set priorities (via specify preset set-priority /
specify extension set-priority) were being reset to the bundle's
declared priority. This fix reads the existing priority from the
registry first, and only falls back to the component's declared
priority or DEFAULT_PRIORITY if no existing priority is found.

Testing

Manual testing showed that running bundle install --refresh after
setting a priority via specify extension set-priority now preserves
the user-set priority instead of resetting it to 10.

See the commit for details: fix/bundle-priority-preservation

…ing bundles

When  or  is run, the
existing user-set priorities (via  /
) were being reset to the bundle's
declared priority. This fix reads the existing priority from the
registry first, and only falls back to the component's declared
priority or DEFAULT_PRIORITY if no existing priority is found.

Fixes: github#5318
@Yi-111-a
Yi-111-a requested a review from mnriem as a code owner November 9, 2026 13:22
Copilot AI auto review requested due to automatic review settings November 9, 2026 13:22
@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Nov 9, 2026

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.

🟡 Changes recommended

Singular component-kind checks skip registry lookups for presets and extensions, so refresh still overwrites saved priorities, with no regression tests covering preservation.

3 open issues
What changed in this PR

This PR aims to preserve user-set preset and extension priorities when Specify CLI installs or refreshes bundles.

Changes:

  • Adds registry-priority lookups to both component installers.
  • Falls back to the bundle-declared priority or DEFAULT_PRIORITY when no saved priority exists.

Project tests were not run in this review environment.

AI-generated review by GitHub Copilot (model: unknown; automated mode) for the requesting contributor; analysis and wording are AI-generated.

File Description
src/​specify_cli/​bundles/​primitives.py Adds saved-priority selection for preset and extension installation paths.

Review effort: Balanced (auto)

Comment thread src/specify_cli/bundles/primitives.py Resolved
Comment thread src/specify_cli/bundles/primitives.py Resolved
Comment thread src/specify_cli/bundles/primitives.py Resolved
@mnriem

mnriem commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

ComponentRef.kind is "presets"/"extensions", so the singular checks
never read the registry. Add tests covering registry-priority preservation
on install/refresh and fallback to declared/default priority.
Copilot AI auto review requested due to automatic review settings November 9, 2026 16:49
@Yi-111-a

Yi-111-a commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — addressed the Copilot feedback in 60a3a1b:

  • The registry lookups now compare against the real plural kinds ("presets" / "extensions"), so a saved priority is actually read on install and refresh.
  • Added regression tests for both managers: a registry priority (37) survives install/refresh even when the bundle declares a different one, and without a saved priority we fall back to the declared value or DEFAULT_PRIORITY. The preservation tests fail on the previous commit and pass now.
  • Gave the existing _FakeManager test doubles an empty registry so they keep exercising the same paths.

pytest tests/specify_cli/bundles passes locally (1280 passed).

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.

🟡 Changes recommended

Unnormalized registry priorities can cause refresh/update failures for stored string or zero values, requiring normalization and a declared/default fallback in both bundle and extension paths.

1 open issue
3 resolved since last review

Review effort: Balanced (auto)

Comment thread src/specify_cli/bundles/primitives.py Resolved
@mnriem

mnriem commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

Registry data may hold strings, zero or booleans; normalize via
normalize_priority and fall back to the declared/default priority so
bundle refresh/update does not fail. Add regression tests for both
presets and extensions.
Copilot AI auto review requested due to automatic review settings November 10, 2026 04:47
@Yi-111-a

Copy link
Copy Markdown
Contributor Author

Thanks — addressed the second Copilot review in e83e024:

  • Stored registry priorities are now passed through normalize_priority in both the preset and extension paths, falling back to the declared priority (or DEFAULT_PRIORITY) when the saved value is a non-numeric string, zero, or a boolean. A numeric string like "37" is still honored as 37.
  • Added regression tests for both managers covering "37", "high", 0 and True; all 8 cases fail on the previous commit and pass now (tests/specify_cli/bundles/test_primitives.py: 109 passed).

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.

🟢 Approval recommended

No active review issues are reported for the priority-preservation change, although tests were not executed.

No open issues 🎉

1 resolved since last review

Review effort: Balanced (auto)

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.

🟢 Approval recommended

No active review issues are reported for the changes preserving registry-stored preset and extension priorities.

No open issues 🎉

Review effort: Balanced (auto)

@mnriem
mnriem merged commit 0730277 into github:main Nov 10, 2026
16 checks passed
@mnriem

mnriem commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants