feat!: consume the declarative index, drop the pkg_id requirement - #186
Merged
Conversation
QaidVoid
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 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. ChangesDeclarative package metadata and identity
Installation and operations
CLI adaptation
Estimated code review effort: 5 (Critical) | ~120 minutes 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: |
a1634ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6000691c.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}
Contributor
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
Comment on lines
+29
to
32
| "{}: {} -> {}", | ||
| Colored(Blue, &update_info.pkg_name), | ||
| Colored(Cyan, &update_info.pkg_id), | ||
| Colored(Red, &update_info.current_version), | ||
| Colored(Green, &update_info.new_version), |
Contributor
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.
Contributor
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
Contributor
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
The original-repo lookup passed the installed id into the family parameter, and update matching dropped the id entirely, so either could resolve to a different package of the same name.
The uniqueness key omitted the family, so two variants publishing no id collided and the second was dropped on import.
A failed extraction was warned about and the package recorded as installed with nothing in it. Local files now detect the format first, so a plain binary is no longer run through the extractor.
An install directory carries its version, so links from the old one read as foreign and were left dangling. The bin/ fallback also no longer overrides a package that declares its own binaries.
Hashes compared equal to each other but segment-wise against a tag, so the ordering was intransitive and could panic sort_by.
An archive shipping read-only directories made the removal of a superseded version fail, and the failure was discarded, so the tree and its row stayed behind.
Requiring both to differ, and comparing against a NULL id, meant the alternate lookup returned the wrong set either way.
Scripted edits left five public items carrying the previous item's documentation, and one attribute on the wrong function.
A file listed twice is now copied rather than moved a second time, a failure part-way puts back what it took, an alias outside its target's directory gets a path that resolves, and the install marker is kept.
Two identity checks in one function disagreed, and the post-lock recheck could not narrow by family at all.
The cache key left out the repository its comment claimed to cover, and a laid-out binary that is the artifact itself is now checked against the published checksum before it is executed.
An untagged enum reports only that no variant matched, so the index shape is now told apart by its opening character.
The tree was re-walked for every binary mapping, and when several files answered to one name each overwrote the last silently.
Linking and unlinking each carried their own copy, so a directory added to one would have been missed by the other.
Both had stopped using the checksum they were named for, and one was byte-for-byte what unlink_others already did.
list and search carried the same key, index and predicate verbatim.
A NULL id made COUNT(DISTINCT) skip the row entirely, a raw projection could not load one, and an empty id from an index behaved as a real one.
Unanchored, a/b/c matched from the middle and silently dropped a.
find_exact and find_alternates take the family, and the lookups that cannot filter on it compare it themselves, preferring the installed row over whichever came first.
The ambiguity prompt rendered two candidates identically, rerun resolved by name alone, and the unique count folded them together.
It comes from metadata, so one holding .. would place the portable directory outside the portable root.
A fixed name let a second import delete the database the first was still building. Also documents a diagnostic code and corrects a field comment.
packages.toml gains 'family', which is what the declarative format publishes to tell identically-named projects apart. 'pkg_id' still parses and is marked deprecated, so every remaining read is flagged.
Dropping the old table cascades portable_package empty and leaves deferred violations a rename cannot clear, so foreign keys are turned off, which is only possible outside a transaction. A published metadata database is migrated on fetch; it was queried with whatever schema built it.
No repository ever published it and 'files' says more.
Walking one that does not exist failed the whole check.
Merged
github-actions Bot
pushed a commit
to Azathothas/soar
that referenced
this pull request
Aug 1, 2026
…kg_id requirement (pkgforge#186) ⌚
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Teaches soar to install from the declarative, hash-pinned index that sbuilder produces, and removes the assumption that every package carries a
pkg_id.Needs pkgforge/sbuilder#16 and pkgforge/soarpkgs#964.
Reading the new index
Published as
{format, packages}, so a client can tell an index it cannot read from one that merely lacks a field. Anything without aformatis treated as the older format, and existing repositories keep working untouched.Packages declare their contents rather than having them guessed.
filessays what comes out of the artifact and where it lands:tois a path inside the package directory, so where a file lands says what it is.bin/is a command,share/man/a manual page,share/*-completion*/a completion. An emptysourcemeans the artifact is itself the file, which is how a bare binary or an AppImage says so, andaliaslists other paths resolving to the same file, which is howdunstexposesdunstctlanddunstify.The layout is built in a staging directory and swapped in at the end, so a source that fails to resolve never leaves a package with its binary deleted. If nothing resolves, the artifact is left as it is rather than installing an empty package. Anything unlisted is pruned, which is how READMEs and changelogs stop being installed.
Linking beyond
binManual pages and completions only mean anything where the system looks for them, so they are linked out of the package: man pages beside the bin directory, completions into the per-shell directories, for the shells in
completions = [...], defaulting to those whose directory already exists. A destination holding something soar does not own is left alone, and removal takes back only links that resolve into the package being removed.soar healthgains a MANPATH row, shown once a package has installed a manual page. Worth knowing why: man-db discards everything it would derive whenMANPATHis set explicitly, which distributions do, so pages can be installed correctly and still be invisible.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. Variants are selected by family instead: the query syntax isfamily/name, andname#pkg_idstill works but warns.One migration per database. The installed-package table gains
pkg_family, the metadata table gainsfilesand drops three columns nothing read, and both relax theirpkg_idcolumns by rebuilding, since SQLite cannot drop NOT NULL in place. The metadata table also gains aCOALESCE(pkg_id, '')unique index, because SQLite treats NULLs as distinct and the plain index would have allowed a fresh duplicate row on every sync.Downgrading refuses rather than deleting installed rows that have no id. The metadata cache, being regenerable, just clears.
Version ordering
Comparison is segment-wise: a version splits into runs of digits and runs of non-digits, and matching runs compare numerically. 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, and plenty of real versions are not valid semver at all. Two commit hashes compare equal, so an arbitrary hash cannot read as an upgrade.Several versions are not several packages
A repository publishes every version it knows. Installing, running or listing a package by name means the newest, and
@versionpicks another and pins it againstupdate.Previously any two candidates were treated as ambiguous, the CLI re-resolved the chosen one by name, got the same ambiguity back, and exited silently. That affected
install,run, and 8 packages in soarpkgs. The chosen package is now used directly.listandsearchshow one row per package with the other versions named,18.18.1 (18.17.1), andqueryorders them newest first.Identity is repository, family, name and version
Everything that decides "is this the same package" now uses the whole identity. Previously the repository was missing from it, so installing the same name from a second repository did nothing, both variants stayed linked, and two repositories shipping byte-identical builds shared one directory, meaning removal of either destroyed the other's files. Package directories are now named
<name>-<version>-<hash>, readable at a glance and unique per install.runcaches per exact package too, and reuses that cache: previously it re-downloaded every time, because it looked for the artifact at a path the layout step had pruned.Fixes found while testing this
.gzis no longer treated as a tarballTesting
All 140 non-AppImage packages in soarpkgs install and are verified against what their recipe declares, every path, alias and bin link: 140 passing, 0 failing. Separately
sync,list,search,query,install,info,run,download,update,remove,use,json2dbandapplyare exercised end to end against a local index, including pinning, aliases and multi-version packages.Summary by CodeRabbit
New Features
Bug Fixes
Style