feat(lockfile): record the feed set per target, and each package's origin

[mobileoverlord]

Owner: the secure-feeds series (241 → 242 → 243 → 244). Both sessions push as mobileoverlord, so GitHub cannot tell you which one wrote a PR — this line can.

Records, per target, which feeds a build resolved against — and which feed each package came from.

Why

repo-snapshot pins the distro feed's content. Nothing equivalent exists for a third-party or on-disk feed, and packages are stored as a name-to-version map with no provenance — so with more than one feed the lock says a package is at some version and nothing about where it came from.

That matters because declaration order is a strict dnf priority override, which is the point of the feature: a locally built package shadows a released one at the same version. The lock could read hello-feed 1.0 while the bytes came from local-build on one machine and the public feed on another, and reordering distro.feeds changes what installs without changing anything the lock records.

What lands in the lock

"targets": {
  "qemux86-64": {
    "repo-snapshot": { "...": "unchanged" },
    "feeds": [
      { "name": "local-build", "position": 10, "url": "../feed-a", "digest": "c0f1033b16a2" },
      { "name": "avocado",     "position": 20, "url": "https://repo.avocadolinux.org/2026/next" },
      { "name": "vendor",      "position": 30, "url": "http://localhost:8080", "stages": ["sdk","ext"] }
    ]
  }
}

Three decisions worth stating, because each has a wrong version that looks reasonable:

Per target, not global. A resolved URL contains $target, and targets: can exclude a feed outright, so two targets in one project genuinely resolve different sets. One global list would record one target's sources against another's packages.

The source as configured, not the container's view. A loopback URL is rewritten to host.docker.internal for reachability, and a path: feed becomes a file:// path inside the read-only mount. Both are plumbing. Recording them would put an ephemeral port and an internal mount path into a file people commit and diff. There is a test asserting the two views differ and that the lock takes the configured one.

Written from the resolution funnel, not from each install command. My first attempt recorded it in rootfs install, and therefore recorded nothing at all for an sdk install — the rig caught it. This follows the snapshot pin, which records itself from utils::snapshot and saves immediately, for the same reason: it is the one place that always runs when feeds resolve. Once per target per invocation, and a failed write is reported rather than fatal, since a build should not stop because a provenance record could not be saved.

Other behaviour

• The set is replaced, not merged, so a feed dropped from the config leaves the lock rather than lingering.
• When it changes, the install says so, naming the old and new order. Reordering feeds silently changes which artifact a name and version resolve to, which is precisely the change nobody would otherwise notice.
• Built-ins are excluded: they are served by the .repo files baked into the SDK image and have a repoid glob rather than a URL, so recording one as a source would describe something that is not one. Their effect on resolution is already in the stamp projection.
• The canonical feed document is unchanged, and the new field is #[serde(skip)] specifically so no stamp hash moves and no cached sysroot is invalidated by this PR.

Format version

LOCKFILE_VERSION 7 → 8, additive. A v7 lockfile reads as v8 with an empty list, which has a test.

Per-package provenance

The feed set says which feeds were available. It does not say which one served a given package, and with ordering as a strict override that is the difference that matters. So the version map became a struct:

"rootfs": {
  "avocado-pkg-rootfs": "2026.9-r0.0",
  "hello-feed": { "version": "1.0-r0.0", "repo": "local-build" }
}

Additive by construction. LockedPackage serializes as a bare version string when the origin is unknown, so a single-feed project's lockfile is byte-identical to what it was before and only packages whose origin was actually resolved take the object form. It deserializes from either shape.

rpm cannot answer this — it records the package, not where it came from. dnf can, because it did the resolution and keeps the answer in the installroot's own history.sqlite, so the query is repoquery --installed --qf '%{name}|%{from_repo}' with --disablerepo="*": entirely local, no metadata fetch, and unable to fail because a feed is unreachable.

Three properties, each with a test:

• Origins apply only to packages the lock already tracks. The query returns everything installed in the sysroot, transitive dependencies included, while the lock records what the config named.
• A version-only install does not erase an origin. A second install must not undo what the first established.
• Provenance moves no stamp hash. package_list_hash folds name and version and deliberately not the origin: a package's identity is its NEVRA, and folding provenance in would rebuild every project the first time origins are captured. A real change of origin already moves the hash through the feed projection.

Wired at every site that records versions — rootfs, runtime, the SDK's target sysroot, and extension installs, the stage most likely to draw from a second feed. The SDK's own sysroot has no installroot and is skipped by construction.

What this does not do

It cannot make an unpinned feed reproducible. A third-party repository that serves no snapshots can still change under you; this records the source and, for on-disk feeds, a content digest.

Testing

Unit tests for per-target recording, replacement, v7 compatibility, the configured-source invariant, the additive serialization, and the recorder's no-op paths. Verified end to end on the integration rig: the lock records the on-disk feed with its digest, the distro feed with its release version, a stage-scoped vendor feed at its own position, and hello-feed — published only to local-build — as coming from local-build.

Stacked on #243.

[Copilot]

🟡 Changes recommended

There are confirmed functional issues in lockfile version migration (v7→v8 loading) and in save/merge semantics that can resurrect dropped feed records under stale-writer scenarios.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds per-target feed provenance recording to avocado.lock so builds can later show which configured feeds (and in what priority order) a target resolved against, addressing ambiguity when multiple feeds can satisfy the same name/version.

Changes:

• Bumps lockfile format to v8 and introduces targets.<target>.feeds: Vec<LockedFeed> for per-target feed-set recording.
• Extends feed resolution to retain a “configured locator” (pre container-rewrite) and exposes ResolvedFeedSet::locked_feeds() to generate lockfile records.
• Records/replaces the feed set in the lock once per target per invocation, and emits a user-visible message when the recorded set changes.

File summaries

File	Description
src/utils/lockfile.rs	Adds v8 schema (LockedFeed, per-target feeds), merge semantics, and tests.
src/utils/feeds.rs	Tracks configured vs in-container locators and generates lockfile-ready feed records.
src/utils/config.rs	Records the resolved feed set into the lock during the per-invocation feed funnel and logs changes.

Review details

Suppressed comments (2)

src/utils/lockfile.rs:1136

• The merge logic for TargetLocks.feeds can resurrect a feed that a later install removed: a stale in-memory LockFile with a non-empty feeds list will keep its old list and overwrite the newer on-disk list during save()'s merge. This violates the intended "replace, not merge" semantics for feed sets.

            // Same rule as the snapshot: adopt the other side's record only when
            // this side has none. An install replaces the set wholesale, so a
            // merge must not resurrect feeds a later install dropped.
            if self_target.feeds.is_empty() {
                self_target.feeds = other_target.feeds;
            }

src/utils/lockfile.rs:340

• This doc comment says on-disk feeds are recorded as a file:// URL, but the implementation intentionally records the configured locator (e.g., ./feed) rather than the in-container mount path. Updating the comment will prevent future confusion about what ends up in the lock.

    /// The resolved URL, with `$releasever` and `$target` already expanded. A
    /// `file://` path for an on-disk feed, and the `connect://<org>/...`
    /// placeholder for a Connect feed — never the minted host, which changes per
    /// build and is not an input.

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

v7 lockfiles will not load/migrate correctly after the version bump due to missing v7 handling in the migration logic, breaking the stated v7→v8 compatibility.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/utils/lockfile.rs:49

• After bumping LOCKFILE_VERSION to 8, v7 lockfiles will parse successfully as the current struct but then fail migration: parse_and_migrate only treats versions 3/4/6 as “shape-compatible” and falls through to the legacy migration switch for v7, which returns "Unable to parse lock file format". This breaks the stated “v7 reads as v8” behavior and also prevents recording feeds into an existing v7 lock.

/// Version 7: Adds per-target `repo-snapshot`, the immutable channel snapshot
///            the target's packages were resolved against. Additive — v6
///            lockfiles read as v7 with `repo_snapshot: None` and behave
///            exactly as before (track the live channel head).
const LOCKFILE_VERSION: u32 = 8;

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There is a confirmed v7→v8 lockfile load regression (migration fallthrough) plus merge/clearing semantics for per-target feeds that can resurrect stale records and prevent proper replacement when feeds are removed/disabled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/utils/lockfile.rs:340

• LockedFeed::url’s doc comment says on-disk feeds are recorded as a file:// URL, but ResolvedFeedSet::locked_feeds() records the configured locator (e.g., "./feed") specifically to avoid container mount paths. This mismatch can confuse consumers of the lock format.

    /// The source as configured, not the container's view of it: a `url:` with
    /// `$releasever` and `$target` expanded but WITHOUT the loopback rewrite, a
    /// `path:` exactly as written in the config, and the `connect://<org>/...`
    /// placeholder for a Connect feed — never the minted host, which changes per

src/utils/lockfile.rs:1894

• With LOCKFILE_VERSION = 8, loading a v7 lockfile should succeed via LockFile::load()/parse_and_migrate(). Right now parse_and_migrate() only bumps versions 3/4/6 when the struct parse succeeds, so a v7 file parses but then falls through to the JSON migration match (which doesn’t handle 7) and fails. This test currently bypasses that path by using serde_json::from_str directly, so it won’t catch the regression.

            .collect();
        assert_eq!(names, vec!["a"], "dropped feed must not linger");
    }

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Important “recording failed / feeds changed” notices are currently emitted with output that can be suppressed under TUI/JSON modes, undermining the PR’s stated “reported rather than fatal” behavior.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/config.rs:4065

• These lock-recording failures are printed with print_info, which is suppressed when the TUI or --json output is active, so the user may never see that the feed provenance was not recorded (contradicting the “reported rather than fatal” behavior described in the PR). Use a notice function that cannot be swallowed, e.g. print_warning_above (and emits a JSON warning event).

This issue also appears in the following locations of the same file:

• line 4083
• line 4093

src/utils/config.rs:4087

• The “feeds changed” notice is currently emitted via print_info, which is suppressed under the TUI and in --json mode; this makes the reorder/change warning easy to miss even though it’s a behavior change the user is supposed to notice. Consider using print_warning_above so it is always surfaced (and becomes a JSON warning event).

        crate::utils::output::print_info(
            &format!(
                "feeds for '{target}' changed since the lock was written: [{}] -> [{}]",
                names(previous),
                names(&locked)

src/utils/config.rs:4097

• Like the earlier read failure, a write failure here uses print_info, which is suppressed when the TUI or --json output is active; users may not learn that the lock was not updated. Use a non-swallowable notice (e.g. print_warning_above).

    if let Err(e) = lock.save(project_root) {
        crate::utils::output::print_info(
            &format!("could not record the feed set in the lock: {e}"),
            crate::utils::output::OutputLevel::Normal,
        );

• Files reviewed: 3/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new feed-recording logic in record_feed_set_in_lock will repeatedly re-save the lockfile for no-feeds projects and can miss emitting a “feeds changed” message for empty→non-empty transitions, which conflicts with the intended observable behavior and adds unnecessary churn.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 11/12 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A new dnf ... command is constructed for bash -c with an unescaped installroot path, which can allow shell injection/breakout and should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 11/12 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It changes lockfile format and recording paths across multiple install flows (feeds resolution, lock merging, dnf origin querying), so it warrants final human review despite strong test coverage.

Review details

• Files reviewed: 11/12 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new dnf-based origin query is missing the same RPM env-var unsetting used by installroot rpm queries, which can lead to incorrect/no provenance under the container entrypoint’s exported RPM config.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 11/12 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The feature is implemented additively with strong migration and behavior tests; only a minor test robustness nit was identified.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/config.rs:6649

• The test asserts the lock file is not rewritten by comparing mtime, but the 20ms sleep can be shorter than the filesystem timestamp resolution (commonly 1s), which can hide a rewrite and let regressions slip through. Use a longer delay (>= 1s) so a rewrite would reliably change modified() on coarse-resolution filesystems.

• Files reviewed: 11/12 changed files
• Comments generated: 0 new
• Review effort level: Lite