feat!: consume the declarative index, drop the pkg_id requirement - #186
feat!: consume the declarative index, drop the pkg_id requirement#186QaidVoid wants to merge 25 commits into
Conversation
QaidVoid
changed the title
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis PR makes package identifiers optional across registry, database, core, operations, and CLI layers. It adds family-aware queries, versioned index parsing, binary and extra-file metadata, custom version comparison, safer extraction and removal handling, and removes package IDs from operation events and most output. ChangesPackage contracts and registry metadata
Database schema and repository operations
Archive extraction and installation
Operation events and CLI adaptation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RegistryIndex
participant MetadataRepository
participant PackageInstaller
participant EventHandler
RegistryIndex->>MetadataRepository: parse versioned index and import accepted packages
MetadataRepository->>PackageInstaller: resolve family-aware package metadata
PackageInstaller->>EventHandler: emit identifier-free operation events
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying soar-docs with
|
| Latest commit: |
cd0513e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://31044510.soar-docs.pages.dev |
| Branch Preview URL: | https://port-format.soar-docs.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
crates/soar-core/src/package/local.rs (1)
142-151: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the local package ID or remove its upstream generation.
from_pathstill acceptspkg_id_overrideand constructsself.pkg_id, butto_packageno longer copies it. Every local package therefore loses the generated or caller-supplied ID during conversion. Restore the optionalpkg_idassignment, or remove the field and override plumbing consistently if this is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-core/src/package/local.rs` around lines 142 - 151, Update the to_package conversion to preserve the local package ID by assigning the optional pkg_id from self.pkg_id, including generated or caller-supplied overrides. Keep the existing package fields unchanged and ensure the from_path pkg_id_override plumbing remains consistent.crates/soar-cli/src/download.rs (1)
153-161: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the filter arguments aligned in the repository-scoped lookup.
This call passes
(name, None, pkg_id, None, ...), while the all-repositories call passes(name, pkg_id, family, None, ...)to the same function. Afamily/name:repoquery therefore drops the family and shiftspkg_idinto a different filter slot, which can select the wrong variant or return no match. Passquery.pkg_id.as_deref()andquery.family.as_deref()in the same positions as the global lookup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-cli/src/download.rs` around lines 153 - 161, Update the repository-scoped MetadataRepository::find_filtered call to match the argument ordering used by the all-repositories lookup: pass query.pkg_id.as_deref() and query.family.as_deref() in their corresponding filter positions, preserving the remaining arguments.crates/soar-operations/src/update.rs (1)
138-147: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep variant identity when finding repository updates.
Line 140 no longer constrains the lookup by identifier or family. If a repository contains multiple
family/namevariants, this can select the newest version from a different family and install it over the current package. Extendfind_newer_versionand this call with the installed package’s family and optional identifier identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-operations/src/update.rs` around lines 138 - 147, Update MetadataRepository::find_newer_version and its call in the new_pkg lookup to accept and apply the installed package’s family and optional identifier, alongside the package name and version. Ensure repository updates only select newer versions matching the current package variant identity before converting and resolving the result.crates/soar-cli/src/install.rs (1)
244-251: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
family/namein--showqueries.Both candidate queries pass
Nonefor the family filter, sosoar install --show family/namedisplays candidates from every family instead of the requested one.
crates/soar-cli/src/install.rs#L244-L251: passquery.family.as_deref()as the family argument.crates/soar-cli/src/install.rs#L264-L271: pass the same family filter in the all-repositories branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-cli/src/install.rs` around lines 244 - 251, The candidate queries in install --show must honor the requested family filter. In both MetadataRepository::find_filtered calls in crates/soar-cli/src/install.rs at lines 244-251 and 264-271, pass query.family.as_deref() as the family argument instead of None.crates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/down.sql (1)
1-3: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDown migration doesn't restore
pkg_id NOT NULL.The paired up.sql rebuilds the table to relax
pkg_idto nullable (SQLite can'tALTER COLUMN). This down.sql only drops the index and deletes NULL rows — it never rebuilds the table to restore theNOT NULLconstraint, so rolling back leaves the schema in a mixed state that doesn't match the pre-migration schema.🔧 Suggested mirror-rebuild approach
DROP INDEX IF EXISTS packages_identity; DELETE FROM packages WHERE pkg_id IS NULL; CREATE TABLE packages_new ( id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, pkg_id TEXT NOT NULL COLLATE NOCASE, pkg_family TEXT COLLATE NOCASE, pkg_name TEXT NOT NULL COLLATE NOCASE, -- ... remaining columns unchanged from the pre-migration schema ); INSERT INTO packages_new SELECT id, pkg_id, pkg_family, pkg_name, /* ... */ FROM packages; DROP TABLE packages; ALTER TABLE packages_new RENAME TO packages;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/down.sql` around lines 1 - 3, Update the down migration to rebuild the packages table after removing NULL pkg_id rows, restoring the pre-migration schema with pkg_id defined as NOT NULL. Mirror the up migration’s table-rebuild approach, preserve all columns, constraints, indexes, and data, then replace the relaxed table with the rebuilt one.Source: Linters/SAST tools
crates/soar-core/src/package/install.rs (2)
271-279: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
package.pkg_familynever reaches the disambiguation queries.
has_pending_install/delete_pending_installs(and laterunlink_others/find_alternatesinrecord()) are only givenpkg_id/pkg_name/repo_name/version.self.package.pkg_familyis available but unused here — see the companion comment oncrates/soar-db/src/repository/core.rsfor the root cause and full impact.Also applies to: 306-329
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-core/src/package/install.rs` around lines 271 - 279, Update the pending-install checks in the install flow around has_pending_install and the related delete_pending_installs calls to pass self.package.pkg_family through to the repository APIs. Ensure the corresponding record() calls to unlink_others and find_alternates also receive the package family so all disambiguation queries use it.
808-839: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing pre-rename cleanup for existing destination entries.
This rename loop lacks the
to.exists()handling used in theextract_root(Lines 909-915) andnested_extract(Lines 976-982) blocks just below. On a resumed/retried install whereinstall_diralready contains a leftover file/directory from a prior attempt,fs::rename(&from, &to)will fail here (non-empty directory or existing file), aborting the install instead of overwriting stale leftovers the way the other two blocks do.🔧 Proposed fix
let from = entry.path(); let to = self.install_dir.join(entry.file_name()); + if to.exists() { + if to.is_dir() { + fs::remove_dir_all(&to).ok(); + } else { + fs::remove_file(&to).ok(); + } + } // Renaming a directory rewrites its `..`, so the directory🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-core/src/package/install.rs` around lines 808 - 839, Update the rename loop in the extracted-install block to remove any existing destination entry at `to` before calling `fs::rename(&from, &to)`, matching the cleanup behavior in the `extract_root` and `nested_extract` blocks. Preserve the existing contextual error handling and continue treating stale files or directories as replaceable leftovers.crates/soar-db/src/repository/core.rs (2)
327-471: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
pkg_familyisn't threaded through package-disambiguation logic. Both sites stem from the same gap:pkg_familywas added to distinguish variants oncepkg_idbecame optional, but it never reaches the queries/calls that decide which installed row is "the same package."
crates/soar-db/src/repository/core.rs#L327-L471: add apkg_family: Option<&str>parameter tofind_alternates,unlink_others,unlink_others_by_checksum,get_old_package_paths,delete_old_packages(and ideally the otherpkg_id-based lookups) and include it in the match predicate alongsidematch_pkg_id.crates/soar-core/src/package/install.rs#L271-L329: passself.package.pkg_family.as_deref()into thehas_pending_install,delete_pending_installs,unlink_others, andfind_alternatescalls once the repository signatures accept it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/src/repository/core.rs` around lines 327 - 471, Thread pkg_family through package disambiguation: in crates/soar-db/src/repository/core.rs lines 327-471, add Option<&str> parameters to find_alternates, unlink_others, unlink_others_by_checksum, get_old_package_paths, delete_old_packages, and other pkg_id-based lookups as applicable, incorporating it alongside match_pkg_id in predicates. In crates/soar-core/src/package/install.rs lines 271-329, pass self.package.pkg_family.as_deref() to has_pending_install, delete_pending_installs, unlink_others, and find_alternates; update all affected callers and preserve existing matching behavior.
327-346: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftPackage disambiguation queries don't consider the new
pkg_familycolumn.
find_alternates,unlink_others,unlink_others_by_checksum,get_old_package_paths,delete_old_packages,find_exact,find_by_pkg_id_and_repo,find_by_pkg_id_name_and_repo,record_installation,update_pkg_id,has_pending_install,delete_pending_installs, andlink_by_checksummatch rows using onlypkg_nameplus optionalpkg_id. With nullablepkg_idand the newpkg_familycolumn, unrelated families sharing apkg_namecan be treated as the same package;unlink_others/get_old_package_paths/delete_old_packagescan then unlink or delete an unrelated installation.Add
pkg_family: Option<&str>to these repository methods and threadself.package.pkg_familythrough the call sites incrates/soar-core/src/package/install.rs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/src/repository/core.rs` around lines 327 - 346, Add a pkg_family: Option<&str> parameter to find_alternates and the other listed repository methods, and include pkg_family in their package-matching filters alongside pkg_name/pkg_id. Update the corresponding calls in the install flow to pass self.package.pkg_family, preserving existing behavior while preventing operations from crossing package families.crates/soar-operations/src/apply.rs (1)
70-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass
pkg_idandpkg_familyin their declared positions.find_filteredexpects ID before family, but both calls provideNonefor ID and put the package ID in the family slot.
crates/soar-operations/src/apply.rs#L70-L78: passpkg.pkg_id.as_deref()as the second filter andNoneas the family filter.crates/soar-operations/src/switch.rs#L118-L126: passselected_package.pkg_id.as_deref()as the second filter andNoneas the family filter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-operations/src/apply.rs` around lines 70 - 78, Correct both MetadataRepository::find_filtered calls in crates/soar-operations/src/apply.rs lines 70-78 and crates/soar-operations/src/switch.rs lines 118-126: pass pkg.pkg_id.as_deref() or selected_package.pkg_id.as_deref() as the ID filter, followed by None for the package-family filter, preserving all other arguments.crates/soar-operations/src/install.rs (1)
266-293: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not use an absent ID as an unscoped variant filter.
Nonedisables thepkg_idpredicate, rather than selecting rows with a null ID.
crates/soar-operations/src/install.rs#L266-L293: whentarget_pkg_idis absent, constrain the metadata query by the selected name/family instead of querying every package.crates/soar-operations/src/install.rs#L306-L317: apply the same name/family fallback to installed-package lookup.crates/soar-operations/src/remove.rs#L96-L110: for an ID-less selected package, keep removal scoped to its name rather than resolving all repository packages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-operations/src/install.rs` around lines 266 - 293, The metadata queries currently treat an absent target_pkg_id as an unscoped filter; update the selected-package lookup in install.rs lines 266-293 and installed-package lookup in install.rs lines 306-317 to use the selected package name/family when the ID is absent, while retaining ID filtering when present. Apply the same name-scoped fallback to the removal lookup in remove.rs lines 96-110 so ID-less packages resolve only within their selected name.crates/soar-core/src/package/url.rs (1)
250-273: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the generated package ID for remote installs.
Both branches now default
Package::pkg_idtoNone, althoughUrlPackagealways derives an ID. Later URL status checks filter by that ID, so the existing install is not found and can be reinstalled repeatedly. Setpkg_id: Some(self.pkg_id.clone())in both literals.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-core/src/package/url.rs` around lines 250 - 273, Preserve the derived package ID when constructing the installed Package values in both branches of the UrlPackage conversion. Add pkg_id using Some(self.pkg_id.clone()) to each Package literal so URL status checks can find existing remote installations.crates/soar-db/src/repository/metadata.rs (1)
570-616: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftNullable
pkg_idmay defeat dedup on re-import viaon_conflict_do_nothing().The insert relies on
UNIQUE (pkg_id, pkg_name, version)(per the originalpackagescreate-table) to makeon_conflict_do_nothing()skip already-imported rows and returnOk(false). Under standard SQL/SQLite semantics,NULLis never equal toNULLfor UNIQUE-constraint purposes, so two imports of the samepkg_name+versionwithpkg_id = None(the common case for the new declarative index format this PR targets) will not conflict — each re-import/refresh will insert a duplicate row instead of being skipped, growing the table and confusing laterfind_by_name/find_filteredqueries.This may already be handled if
crates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/up.sqlredefines the unique index (e.g., unique on(pkg_name, version)alone, or on a coalesced expression), but that file isn't in this review batch.#!/bin/bash cat crates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/up.sql rg -n "UNIQUE" crates/soar-db/src/schema/metadata.rs crates/soar-db/migrations/metadata -g '*.sql'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/src/repository/metadata.rs` around lines 570 - 616, Ensure duplicate detection in the package insertion flow remains effective when pkg_id is None: update the migration/schema constraint or the insert logic around NewPackage and diesel::insert_into so repeated imports with the same pkg_name and version conflict and return Ok(false). Verify the existing uniqueness definition is adjusted without weakening deduplication for populated pkg_id values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/soar-cli/src/install.rs`:
- Around line 79-81: The ambiguity re-resolution currently loses the selected
package variant by querying only name:repo and ignoring the resolved result. In
crates/soar-cli/src/install.rs lines 79-81, preserve the selected candidate’s
full family-aware identity or install that candidate directly; apply the same
identity-preserving fix in the --show fallback at lines 202-204.
In `@crates/soar-cli/src/json2db.rs`:
- Around line 55-64: Update the json2db import flow around
MetadataRepository::import_packages so validation and import occur in a
temporary database or transaction rather than the existing output database. Only
atomically replace the output after imported is greater than zero; when all
packages are rejected, preserve the prior database unchanged.
In `@crates/soar-cli/src/update.rs`:
- Around line 29-32: Update the CLI messages using update_info and the
corresponding download and switch confirmation flows to include family and/or
repository context alongside pkg_name, preserving enough identity to distinguish
variants without restoring deprecated pkg_id syntax. Apply this to the update
preview in crates/soar-cli/src/update.rs lines 29-32, the update failure log in
crates/soar-cli/src/update.rs line 52, the download log in
crates/soar-cli/src/download.rs line 215, and the switch confirmation in
crates/soar-cli/src/use.rs line 60.
In `@crates/soar-cli/src/utils.rs`:
- Around line 136-143: Update the installed-package matching logic around
is_installed so it never treats a missing pkg.pkg_id() as an empty identity. Use
package name together with family/repository, or another stable resolved
identity, and revise the producer of the installed tuples to provide the same
identity fields so current and older installs match correctly.
In `@crates/soar-db/src/repository/metadata.rs`:
- Around line 442-473: Update find_newer_version to accept the installed package
family and filter candidates by both pkg_name and pkg_family, matching the
disambiguation behavior in find_filtered. Thread the new family argument through
all callers while preserving the existing version comparison and selection
logic.
In `@crates/soar-dl/src/download.rs`:
- Around line 345-349: Update the extraction flow around compak::extract_archive
so it always extracts into a dedicated temporary child directory rather than the
caller-owned extract_dir. On extraction failure, remove only that temporary
directory; preserve the existing caller-supplied directory and downloaded
archive, while retaining the current warning behavior.
In `@crates/soar-operations/src/install.rs`:
- Around line 834-849: The checksum-less directory suffix currently hashes only
pkg_name and version, causing distinct sources or variants to collide; update
the fallback hash input in the installation path logic to also include a stable
source identity, preferring pkg_id when available and otherwise the package URL
or GHCR reference, while preserving the existing 12-character suffix behavior.
In `@crates/soar-operations/src/list.rs`:
- Around line 60-77: Update the installed_pkgs construction in the list
filtering flow to merge duplicate (repo_name, pkg_name) records with logical OR
semantics, so any record with is_installed=true marks the package installed.
Replace the direct HashMap collect behavior with an aggregation that preserves
the existing key lookup used for entries.
In `@crates/soar-operations/src/search.rs`:
- Around line 58-67: The installed package aggregation in the `installed_pkgs`
construction must combine duplicate `(repo_name, pkg_name)` entries with logical
OR instead of allowing `collect()` to overwrite values arbitrarily. Update the
`into_par_iter` reduction/collection so any matching row with `is_installed ==
true` produces a true aggregate.
In `@crates/soar-operations/src/update.rs`:
- Around line 443-451: Update the install-report success tracking around the
succeeded set and the target cleanup loop to use a stable per-target identity
rather than pkg_name alone. Preserve that identity when recording installed
packages, key succeeded by it, and use the matching target identity for
membership checks so packages from different repositories, families, or variants
cannot share success status.
In `@crates/soar-package/src/formats/common.rs`:
- Around line 294-301: Update the no-pkg_id branch of the portable_dir_base
derivation around package.pkg_id() to include the package family alongside
package.pkg_name(), using the existing family-aware discriminator exposed by
PackageExt. Keep the current package-name-and-id format unchanged for packages
with pkg_id, while ensuring family-a/foo and family-b/foo produce distinct
portable directory bases.
In `@crates/soar-utils/src/version.rs`:
- Around line 30-50: Update compare_versions to detect when both inputs are
hash-only versions and return Ordering::Equal before segment comparison,
preventing distinct commit hashes from determining upgrade or downgrade
decisions. Reuse the existing hash-detection helper or add one near
compare_versions, and preserve normal semantic comparison for non-hash versions.
---
Outside diff comments:
In `@crates/soar-cli/src/download.rs`:
- Around line 153-161: Update the repository-scoped
MetadataRepository::find_filtered call to match the argument ordering used by
the all-repositories lookup: pass query.pkg_id.as_deref() and
query.family.as_deref() in their corresponding filter positions, preserving the
remaining arguments.
In `@crates/soar-cli/src/install.rs`:
- Around line 244-251: The candidate queries in install --show must honor the
requested family filter. In both MetadataRepository::find_filtered calls in
crates/soar-cli/src/install.rs at lines 244-251 and 264-271, pass
query.family.as_deref() as the family argument instead of None.
In `@crates/soar-core/src/package/install.rs`:
- Around line 271-279: Update the pending-install checks in the install flow
around has_pending_install and the related delete_pending_installs calls to pass
self.package.pkg_family through to the repository APIs. Ensure the corresponding
record() calls to unlink_others and find_alternates also receive the package
family so all disambiguation queries use it.
- Around line 808-839: Update the rename loop in the extracted-install block to
remove any existing destination entry at `to` before calling `fs::rename(&from,
&to)`, matching the cleanup behavior in the `extract_root` and `nested_extract`
blocks. Preserve the existing contextual error handling and continue treating
stale files or directories as replaceable leftovers.
In `@crates/soar-core/src/package/local.rs`:
- Around line 142-151: Update the to_package conversion to preserve the local
package ID by assigning the optional pkg_id from self.pkg_id, including
generated or caller-supplied overrides. Keep the existing package fields
unchanged and ensure the from_path pkg_id_override plumbing remains consistent.
In `@crates/soar-core/src/package/url.rs`:
- Around line 250-273: Preserve the derived package ID when constructing the
installed Package values in both branches of the UrlPackage conversion. Add
pkg_id using Some(self.pkg_id.clone()) to each Package literal so URL status
checks can find existing remote installations.
In
`@crates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/down.sql`:
- Around line 1-3: Update the down migration to rebuild the packages table after
removing NULL pkg_id rows, restoring the pre-migration schema with pkg_id
defined as NOT NULL. Mirror the up migration’s table-rebuild approach, preserve
all columns, constraints, indexes, and data, then replace the relaxed table with
the rebuilt one.
In `@crates/soar-db/src/repository/core.rs`:
- Around line 327-471: Thread pkg_family through package disambiguation: in
crates/soar-db/src/repository/core.rs lines 327-471, add Option<&str> parameters
to find_alternates, unlink_others, unlink_others_by_checksum,
get_old_package_paths, delete_old_packages, and other pkg_id-based lookups as
applicable, incorporating it alongside match_pkg_id in predicates. In
crates/soar-core/src/package/install.rs lines 271-329, pass
self.package.pkg_family.as_deref() to has_pending_install,
delete_pending_installs, unlink_others, and find_alternates; update all affected
callers and preserve existing matching behavior.
- Around line 327-346: Add a pkg_family: Option<&str> parameter to
find_alternates and the other listed repository methods, and include pkg_family
in their package-matching filters alongside pkg_name/pkg_id. Update the
corresponding calls in the install flow to pass self.package.pkg_family,
preserving existing behavior while preventing operations from crossing package
families.
In `@crates/soar-db/src/repository/metadata.rs`:
- Around line 570-616: Ensure duplicate detection in the package insertion flow
remains effective when pkg_id is None: update the migration/schema constraint or
the insert logic around NewPackage and diesel::insert_into so repeated imports
with the same pkg_name and version conflict and return Ok(false). Verify the
existing uniqueness definition is adjusted without weakening deduplication for
populated pkg_id values.
In `@crates/soar-operations/src/apply.rs`:
- Around line 70-78: Correct both MetadataRepository::find_filtered calls in
crates/soar-operations/src/apply.rs lines 70-78 and
crates/soar-operations/src/switch.rs lines 118-126: pass pkg.pkg_id.as_deref()
or selected_package.pkg_id.as_deref() as the ID filter, followed by None for the
package-family filter, preserving all other arguments.
In `@crates/soar-operations/src/install.rs`:
- Around line 266-293: The metadata queries currently treat an absent
target_pkg_id as an unscoped filter; update the selected-package lookup in
install.rs lines 266-293 and installed-package lookup in install.rs lines
306-317 to use the selected package name/family when the ID is absent, while
retaining ID filtering when present. Apply the same name-scoped fallback to the
removal lookup in remove.rs lines 96-110 so ID-less packages resolve only within
their selected name.
In `@crates/soar-operations/src/update.rs`:
- Around line 138-147: Update MetadataRepository::find_newer_version and its
call in the new_pkg lookup to accept and apply the installed package’s family
and optional identifier, alongside the package name and version. Ensure
repository updates only select newer versions matching the current package
variant identity before converting and resolving the result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 460f5f0c-292b-42f2-b8ad-be9ccfb6dff8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (64)
Cargo.tomlcrates/soar-cli/src/apply.rscrates/soar-cli/src/download.rscrates/soar-cli/src/health.rscrates/soar-cli/src/inspect.rscrates/soar-cli/src/install.rscrates/soar-cli/src/json2db.rscrates/soar-cli/src/list.rscrates/soar-cli/src/progress.rscrates/soar-cli/src/remove.rscrates/soar-cli/src/run.rscrates/soar-cli/src/update.rscrates/soar-cli/src/use.rscrates/soar-cli/src/utils.rscrates/soar-core/src/database/models.rscrates/soar-core/src/package/install.rscrates/soar-core/src/package/local.rscrates/soar-core/src/package/query.rscrates/soar-core/src/package/remove.rscrates/soar-core/src/package/update.rscrates/soar-core/src/package/url.rscrates/soar-db/migrations/core/2026-07-28-000000-0000_add_pkg_family/down.sqlcrates/soar-db/migrations/core/2026-07-28-000000-0000_add_pkg_family/up.sqlcrates/soar-db/migrations/core/2026-07-29-000000-0000_pkg_id_optional/down.sqlcrates/soar-db/migrations/core/2026-07-29-000000-0000_pkg_id_optional/up.sqlcrates/soar-db/migrations/metadata/2026-07-28-000000-0000_add_binaries/down.sqlcrates/soar-db/migrations/metadata/2026-07-28-000000-0000_add_binaries/up.sqlcrates/soar-db/migrations/metadata/2026-07-28-000001-0000_drop_webpage_tags/down.sqlcrates/soar-db/migrations/metadata/2026-07-28-000001-0000_drop_webpage_tags/up.sqlcrates/soar-db/migrations/metadata/2026-07-28-000002-0000_add_extra/down.sqlcrates/soar-db/migrations/metadata/2026-07-28-000002-0000_add_extra/up.sqlcrates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/down.sqlcrates/soar-db/migrations/metadata/2026-07-29-000000-0000_pkg_id_optional/up.sqlcrates/soar-db/src/migration.rscrates/soar-db/src/models/core.rscrates/soar-db/src/models/metadata.rscrates/soar-db/src/models/types.rscrates/soar-db/src/repository/core.rscrates/soar-db/src/repository/metadata.rscrates/soar-db/src/schema/core.rscrates/soar-db/src/schema/metadata.rscrates/soar-dl/src/download.rscrates/soar-events/src/event.rscrates/soar-operations/src/apply.rscrates/soar-operations/src/context.rscrates/soar-operations/src/health.rscrates/soar-operations/src/install.rscrates/soar-operations/src/list.rscrates/soar-operations/src/progress.rscrates/soar-operations/src/remove.rscrates/soar-operations/src/run.rscrates/soar-operations/src/search.rscrates/soar-operations/src/switch.rscrates/soar-operations/src/types.rscrates/soar-operations/src/update.rscrates/soar-operations/src/utils.rscrates/soar-package/src/formats/common.rscrates/soar-package/src/traits.rscrates/soar-registry/src/error.rscrates/soar-registry/src/lib.rscrates/soar-registry/src/metadata.rscrates/soar-registry/src/package.rscrates/soar-utils/src/lib.rscrates/soar-utils/src/version.rs
💤 Files with no reviewable changes (5)
- crates/soar-db/src/migration.rs
- crates/soar-operations/src/types.rs
- crates/soar-events/src/event.rs
- crates/soar-operations/src/health.rs
- crates/soar-operations/src/progress.rs
| "{}: {} -> {}", | ||
| Colored(Blue, &update_info.pkg_name), | ||
| Colored(Cyan, &update_info.pkg_id), | ||
| Colored(Red, &update_info.current_version), | ||
| Colored(Green, &update_info.new_version), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve variant and repository context when removing pkg_id from CLI messages.
pkg_name is not unique once family/name variants exist. These messages should retain family and/or repository context so users can distinguish operations without restoring the deprecated ID syntax.
crates/soar-cli/src/update.rs#L29-L32: include family/repository context in update previews.crates/soar-cli/src/update.rs#L52-L52: include the same context in failure logs.crates/soar-cli/src/download.rs#L215-L215: include family/repository context in download logs.crates/soar-cli/src/use.rs#L60-L60: include the selected family/repository in switch confirmations.
📍 Affects 3 files
crates/soar-cli/src/update.rs#L29-L32(this comment)crates/soar-cli/src/update.rs#L52-L52crates/soar-cli/src/download.rs#L215-L215crates/soar-cli/src/use.rs#L60-L60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/soar-cli/src/update.rs` around lines 29 - 32, Update the CLI messages
using update_info and the corresponding download and switch confirmation flows
to include family and/or repository context alongside pkg_name, preserving
enough identity to distinguish variants without restoring deprecated pkg_id
syntax. Apply this to the update preview in crates/soar-cli/src/update.rs lines
29-32, the update failure log in crates/soar-cli/src/update.rs line 52, the
download log in crates/soar-cli/src/download.rs line 215, and the switch
confirmation in crates/soar-cli/src/use.rs line 60.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql (1)
64-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude
pkg_familyin the identity index.Declarative packages use
pkg_familyas the identity instead ofpkg_id, so this index still groups different families with the samepkg_nameandversioninto the sameCOALESCE(pkg_id, '')bucket and makes those variants unrecoverable inserts that conflict with each other.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql` around lines 64 - 65, Update the packages_identity unique index to include pkg_family in the indexed identity columns, preserving the existing pkg_id, pkg_name, and version components so declarative packages from different families do not conflict.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql`:
- Line 2: Replace the unconditional NULL-ID deletion in the core downgrade
migration with a guard that aborts clearly when installed package rows have
pkg_id IS NULL, rather than deleting them. Apply the same pre-delete guard to
the metadata downgrade migration for NULL-ID metadata rows; update both
specified migration files accordingly.
---
Outside diff comments:
In
`@crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql`:
- Around line 64-65: Update the packages_identity unique index to include
pkg_family in the indexed identity columns, preserving the existing pkg_id,
pkg_name, and version components so declarative packages from different families
do not conflict.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 362731b8-f02d-4d4b-887a-046a68286552
📒 Files selected for processing (4)
crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sqlcrates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/up.sqlcrates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sqlcrates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/up.sql
| @@ -0,0 +1,31 @@ | |||
| -- Rows without an id cannot be represented once the column is required again. | |||
| DELETE FROM packages WHERE pkg_id IS NULL; | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refuse unsafe downgrade instead of deleting declarative package state.
pkg_id IS NULL is valid state in the new format; deleting it silently loses package tracking and can orphan installed artifacts.
crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql#L2-L2: abort the downgrade with a clear error when NULL-ID installed rows exist.crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql#L3-L3: apply the same guard before deleting NULL-ID metadata rows.
📍 Affects 2 files
crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql#L2-L2(this comment)crates/soar-db/migrations/metadata/2026-07-28-000000-0000_declarative_format/down.sql#L3-L3
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/soar-db/migrations/core/2026-07-28-000000-0000_declarative_format/down.sql`
at line 2, Replace the unconditional NULL-ID deletion in the core downgrade
migration with a guard that aborts clearly when installed package rows have
pkg_id IS NULL, rather than deleting them. Apply the same pre-delete guard to
the metadata downgrade migration for NULL-ID metadata rows; update both
specified migration files accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/soar-config/src/packages.rs`:
- Around line 337-349: Update PackageSpec::resolve so declarative installs no
longer copy defaults.install_patterns or opts.install_patterns into
ResolvedPackage.globs. Keep legacy install-pattern handling confined to the
OCI-specific path, and ensure declarative installation preserves the complete
package contents without applying these deprecated patterns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e368202-cd4f-448b-a4cf-5bc4e331be38
📒 Files selected for processing (9)
crates/soar-cli/src/health.rscrates/soar-config/src/config.rscrates/soar-config/src/packages.rscrates/soar-core/src/package/install.rscrates/soar-core/src/package/url.rscrates/soar-events/src/lib.rscrates/soar-operations/src/install.rscrates/soar-operations/src/progress.rscrates/soar-operations/src/utils.rs
💤 Files with no reviewable changes (1)
- crates/soar-events/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/soar-operations/src/utils.rs
- crates/soar-cli/src/health.rs
- crates/soar-operations/src/progress.rs
- crates/soar-operations/src/install.rs
- crates/soar-core/src/package/install.rs
| #[deprecated( | ||
| since = "0.13.0", | ||
| note = "only the OCI download path applies these; the declarative format does not use it" | ||
| )] | ||
| pub install_patterns: Option<Vec<String>>, | ||
| pub binary_only: bool, | ||
| pub arch_map: Option<HashMap<String, String>>, | ||
| } | ||
|
|
||
| impl PackageSpec { | ||
| /// Resolve the package specification with defaults applied. | ||
| // Still populated while the OCI path exists; see the field's deprecation. | ||
| #[allow(deprecated)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Prevent deprecated patterns from affecting declarative installs.
PackageSpec::resolve still copies defaults.install_patterns and opts.install_patterns into ResolvedPackage (Lines 383 and 426-429), and downstream installation receives them as globs. This contradicts the deprecation note that the declarative format does not use them and can silently omit files from installs. Restrict legacy pattern handling to the OCI-specific path, or document and test this compatibility behavior explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/soar-config/src/packages.rs` around lines 337 - 349, Update
PackageSpec::resolve so declarative installs no longer copy
defaults.install_patterns or opts.install_patterns into ResolvedPackage.globs.
Keep legacy install-pattern handling confined to the OCI-specific path, and
ensure declarative installation preserves the complete package contents without
applying these deprecated patterns.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/soar-cli/src/utils.rs (1)
148-154: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winShow package families wherever
pkg_idwas removed from identity output.Family is now the user-visible discriminator for variants, but these outputs render only
pkg_name:repo_name. Same-name variants can become indistinguishable.
crates/soar-cli/src/utils.rs#L148-L154: renderfamily/name:repoin the interactive selector.crates/soar-cli/src/utils.rs#L186-L187: include the family in target confirmation.crates/soar-cli/src/install.rs#L416-L418: include the family in the installation report.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-cli/src/utils.rs` around lines 148 - 154, Update the package identity formatting to include the family before name:repo so variants remain distinguishable. In crates/soar-cli/src/utils.rs lines 148-154, update the interactive selector output; also update lines 186-187 for target confirmation. In crates/soar-cli/src/install.rs lines 416-418, include the family in the installation report, preserving the existing formatting and markers.crates/soar-db/src/repository/core.rs (2)
338-357: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
find_alternatesmishandlesNonethrough SQL NULL comparison.The filter
pkg_id.is_null().or(pkg_id.ne(exclude_pkg_id))makescolumn != NULLone disjunct whenexclude_pkg_idisNone; in SQL this never matches, so non-null rows with the same package name are excluded from the alternates list. Use a Rust-side identity check, likerows_other_than, so only the exact samepkg_ididentity is omitted.🐛 Proposed fix: move the pkg_id inequality check to Rust-side comparison
pub fn find_alternates( conn: &mut SqliteConnection, pkg_name: &str, exclude_pkg_id: Option<&str>, exclude_version: &str, ) -> QueryResult<Vec<InstalledPackageWithPortable>> { let results: Vec<(Package, Option<PortablePackage>)> = packages::table .left_join(portable_package::table) .filter(packages::pkg_name.eq(pkg_name)) - .filter( - packages::pkg_id - .is_null() - .or(packages::pkg_id.ne(exclude_pkg_id)), - ) .filter(packages::version.ne(exclude_version)) .select((Package::as_select(), Option::<PortablePackage>::as_select())) .load(conn)?; - Ok(results.into_iter().map(Into::into).collect()) + Ok(results + .into_iter() + .filter(|(pkg, _)| pkg.pkg_id.as_deref() != exclude_pkg_id) + .map(Into::into) + .collect()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/src/repository/core.rs` around lines 338 - 357, Update find_alternates to avoid comparing packages::pkg_id with a nullable exclude_pkg_id in SQL. Load rows filtered by package name and version, then apply the existing rows_other_than-style Rust-side identity check so only the exact excluded pkg_id is omitted, while None excludes no package.
464-516: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winScope
unlink_othersto the installing repository.
rows_other_thanselects every row withpkg_nameacross all repositories. When a declarative install haspkg_id = NULL, another repository with the same package name/version can match the keep predicate here and be incorrectly left linked, or a differentpkg_idcan make the current install match a different repository’s stale row. Threadrepo_namethroughunlink_others/rows_other_thanfromPackageInstaller::record;record_installation,has_pending_install, anddelete_pending_installsalready scope this path by repository.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-db/src/repository/core.rs` around lines 464 - 516, The unlinking query currently considers matching package rows from every repository; scope it to the installing repository. Add a repo_name parameter to unlink_others and rows_other_than, filter rows by that repository before identity comparison, and pass the repository name from PackageInstaller::record while preserving the existing identity and update behavior.crates/soar-operations/src/install.rs (1)
248-276: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAmbiguity check no longer detects real ambiguity once
pkg_idis optional.
all_same_pkg_idcompares onlypkg_id. When the declarative index leavespkg_idNone(the whole point of this PR), every variant reads as "same" regardless ofpkg_family, so a name shared across two distinct families silently resolves to whichever variant happened to bevariants[0]instead of raisingResolveResult::Ambiguous. The fallback filter you added right below (267-276) then locks onto that arbitrarily-picked family for the rest of#allresolution.🐛 Proposed fix
if variants.len() > 1 { - let first_pkg_id = &variants[0].pkg_id; - let all_same_pkg_id = variants.iter().all(|v| v.pkg_id == *first_pkg_id); - if !all_same_pkg_id { + let first_identity = (&variants[0].pkg_id, &variants[0].pkg_family); + let all_same_identity = variants + .iter() + .all(|v| (&v.pkg_id, &v.pkg_family) == first_identity); + if !all_same_identity { return Ok(ResolveResult::Ambiguous(crate::AmbiguousPackage { query: query.name.clone().unwrap_or_default(), candidates: variants, })); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/soar-operations/src/install.rs` around lines 248 - 276, Update the ambiguity check in the resolution flow before target_pkg_id is selected so variants are considered equivalent only when both their pkg_id and pkg_family match. When pkg_id is absent, differing families must return ResolveResult::Ambiguous rather than selecting variants[0]; preserve the existing candidate construction and downstream name/id/family filter behavior for unambiguous results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/soar-cli/src/install.rs`:
- Around line 78-86: Update the specific_query construction for the selected
package: prefer the family/name:repo form, otherwise retain pkg_id using
name#pkg_id:repo, and fall back to name:repo only when both pkg_family and
pkg_id are absent. Apply the same fallback logic to the corresponding alternate
block noted in the comment.
In `@crates/soar-operations/src/remove.rs`:
- Around line 97-112: Preserve family identity for id-less package selection: in
crates/soar-operations/src/remove.rs lines 97-112, carry the selected package’s
pkg_family into the `#all` lookup and filter candidates by that family; in
crates/soar-cli/src/install.rs lines 348-352, require the selected package
family when choosing an existing installation. Keep id-based lookup behavior
unchanged.
---
Outside diff comments:
In `@crates/soar-cli/src/utils.rs`:
- Around line 148-154: Update the package identity formatting to include the
family before name:repo so variants remain distinguishable. In
crates/soar-cli/src/utils.rs lines 148-154, update the interactive selector
output; also update lines 186-187 for target confirmation. In
crates/soar-cli/src/install.rs lines 416-418, include the family in the
installation report, preserving the existing formatting and markers.
In `@crates/soar-db/src/repository/core.rs`:
- Around line 338-357: Update find_alternates to avoid comparing
packages::pkg_id with a nullable exclude_pkg_id in SQL. Load rows filtered by
package name and version, then apply the existing rows_other_than-style
Rust-side identity check so only the exact excluded pkg_id is omitted, while
None excludes no package.
- Around line 464-516: The unlinking query currently considers matching package
rows from every repository; scope it to the installing repository. Add a
repo_name parameter to unlink_others and rows_other_than, filter rows by that
repository before identity comparison, and pass the repository name from
PackageInstaller::record while preserving the existing identity and update
behavior.
In `@crates/soar-operations/src/install.rs`:
- Around line 248-276: Update the ambiguity check in the resolution flow before
target_pkg_id is selected so variants are considered equivalent only when both
their pkg_id and pkg_family match. When pkg_id is absent, differing families
must return ResolveResult::Ambiguous rather than selecting variants[0]; preserve
the existing candidate construction and downstream name/id/family filter
behavior for unambiguous results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b769e183-51cd-4c0a-a203-25ea7120cb9b
📒 Files selected for processing (22)
crates/soar-cli/src/download.rscrates/soar-cli/src/install.rscrates/soar-cli/src/json2db.rscrates/soar-cli/src/utils.rscrates/soar-core/src/database/models.rscrates/soar-core/src/package/install.rscrates/soar-core/src/package/local.rscrates/soar-core/src/package/update.rscrates/soar-db/src/repository/core.rscrates/soar-db/src/repository/metadata.rscrates/soar-dl/src/download.rscrates/soar-operations/src/apply.rscrates/soar-operations/src/install.rscrates/soar-operations/src/list.rscrates/soar-operations/src/remove.rscrates/soar-operations/src/search.rscrates/soar-operations/src/switch.rscrates/soar-operations/src/types.rscrates/soar-operations/src/update.rscrates/soar-package/src/formats/common.rscrates/soar-package/src/traits.rscrates/soar-utils/src/version.rs
🚧 Files skipped from review as they are similar to previous changes (14)
- crates/soar-core/src/package/update.rs
- crates/soar-package/src/formats/common.rs
- crates/soar-operations/src/switch.rs
- crates/soar-operations/src/search.rs
- crates/soar-package/src/traits.rs
- crates/soar-operations/src/types.rs
- crates/soar-dl/src/download.rs
- crates/soar-utils/src/version.rs
- crates/soar-cli/src/json2db.rs
- crates/soar-operations/src/list.rs
- crates/soar-operations/src/apply.rs
- crates/soar-core/src/database/models.rs
- crates/soar-core/src/package/install.rs
- crates/soar-db/src/repository/metadata.rs
| // Re-resolve with the specific selected package. The | ||
| // family has to come along, or a name ambiguous within one | ||
| // repository resolves right back to the same choice. | ||
| let specific_query = match &pkg.pkg_family { | ||
| Some(family) => { | ||
| format!("{}/{}:{}", family, pkg.pkg_name, pkg.repo_name) | ||
| } | ||
| None => format!("{}:{}", pkg.pkg_name, pkg.repo_name), | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Retain legacy pkg_id when no family is available.
For older indexes, a selected package may have pkg_family = None but a populated pkg_id. The None branch converts it to name:repo, which can become ambiguous or resolve to a different legacy variant despite the retained name#pkg_id syntax. Prefer family/name:repo, otherwise name#pkg_id:repo, and only use name:repo when both are absent.
Proposed identity fallback
- let specific_query = match &pkg.pkg_family {
- Some(family) => {
- format!("{}/{}:{}", family, pkg.pkg_name, pkg.repo_name)
- }
- None => format!("{}:{}", pkg.pkg_name, pkg.repo_name),
+ let specific_query = match (&pkg.pkg_family, &pkg.pkg_id) {
+ (Some(family), _) => {
+ format!("{}/{}:{}", family, pkg.pkg_name, pkg.repo_name)
+ }
+ (None, Some(pkg_id)) => {
+ format!("{}#{}:{}", pkg.pkg_name, pkg_id, pkg.repo_name)
+ }
+ (None, None) => format!("{}:{}", pkg.pkg_name, pkg.repo_name),
};Also applies to: 209-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/soar-cli/src/install.rs` around lines 78 - 86, Update the
specific_query construction for the selected package: prefer the
family/name:repo form, otherwise retain pkg_id using name#pkg_id:repo, and fall
back to name:repo only when both pkg_family and pkg_id are absent. Apply the
same fallback logic to the corresponding alternate block noted in the comment.
| let target_pkg_name = installed[0].pkg_name.clone(); | ||
| // Without an id, filtering on it alone selects every | ||
| // installed package in the repository, which would remove | ||
| // far more than was asked for. | ||
| let (name_filter, id_filter) = match target_pkg_id.as_deref() { | ||
| Some(id) => (None, Some(id)), | ||
| None => (Some(target_pkg_name.as_str()), None), | ||
| }; | ||
| // Find all packages with this identity | ||
| let all_installed: Vec<InstalledPackage> = diesel_db | ||
| .with_conn(|conn| { | ||
| CoreRepository::list_filtered( | ||
| conn, | ||
| query.repo_name.as_deref(), | ||
| None, | ||
| Some(&target_pkg_id), | ||
| name_filter, | ||
| id_filter, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve family identity in id-less package lookups.
Both paths treat a missing pkg_id as name-only identity, but declarative packages use pkg_family to distinguish variants. This can remove a different family or incorrectly skip an install.
crates/soar-operations/src/remove.rs#L97-L112: carry the selected family into the#alllookup and filter candidates by it.crates/soar-cli/src/install.rs#L348-L352: require the selected package family when choosing an existing installation.
📍 Affects 2 files
crates/soar-operations/src/remove.rs#L97-L112(this comment)crates/soar-cli/src/install.rs#L348-L352
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/soar-operations/src/remove.rs` around lines 97 - 112, Preserve family
identity for id-less package selection: in crates/soar-operations/src/remove.rs
lines 97-112, carry the selected package’s pkg_family into the `#all` lookup and
filter candidates by that family; in crates/soar-cli/src/install.rs lines
348-352, require the selected package family when choosing an existing
installation. Keep id-based lookup behavior unchanged.
1 participant
Teaches soar to install from the declarative, hash-pinned index that sbuilder now produces, and removes the assumption that every package carries a
pkg_id.Reading the new index
The index is published as
{format, packages}so a client can tell an index it cannot read from one that merely lacks a field. Anything without aformatfield is treated as the older format, so existing repositories keep working untouched.Packages now declare their contents rather than having them guessed:
binariesmaps a path inside the artifact to the name it is linked as, andextralists side files to install alongside it. This is what lets ripgrep shiprgwithout a naming convention to infer it from.pkg_id is optional
A repository whose names are already unique no longer has to invent ids, so
pkg_idis nullable throughout and nothing fabricates one on import. It no longer appears in human-readable output, though it is still carried as a field in structured output.Variants are selected by family instead. The query syntax is
family/name, and the oldname#pkg_idform still works but warns that it will be removed.Six migrations come with this. The installed-package table gains
pkg_family, the metadata table gainsbinariesandextraand drops the unused webpage tags, and both relax theirpkg_idcolumns. The metadata table also gains aCOALESCE(pkg_id, '')unique index, because SQLite treats NULLs as distinct and the plain index would have allowed duplicate rows once ids became optional.Version ordering
Comparison is segment-wise: a version splits into runs of digits and runs of non-digits, and matching runs compare numerically when both are numeric. This fixes
10sorting below9and1.10below1.9.Semver is deliberately not used. Most published versions carry a rebuild revision as
-N, which semver reads as a prerelease and ranks below the plain version, the opposite of what it means here. Plenty of real versions (1.05,r1287.fef2b38-1) are not valid semver at all.A version built from a commit hash has no recoverable order, so those compare equal unless identical. A repository wanting upgrades between snapshots has to publish something ordered.
Fixes found while testing this
.gzis no longer treated as a tarballCompatibility
Older clients cannot read the new index:
binariesandextrahave no equivalent in the old format, so this needs to land before the repository starts publishing it.Testing
Installed from a locally served index against an isolated config, covering a package with an unhashed licence, a package with a per-architecture hashed side file, upgrades from a pre-existing install with stale ids, and removal. Version comparison has unit tests covering dates, rebuild revisions, prereleases and commit hashes.
Summary by CodeRabbit
New Features
family/name@version:repo.Bug Fixes
Style
Chores
install_patternsis now deprecated (only the OCI download path applies it).