feat(uploads): add Uploader subsystem (file discovery + readiness) [M2]#93
Open
ben-miru wants to merge 25 commits into
Open
feat(uploads): add Uploader subsystem (file discovery + readiness) [M2]#93ben-miru wants to merge 25 commits into
ben-miru wants to merge 25 commits into
Conversation
ben-miru
pushed a commit
that referenced
this pull request
Jun 29, 2026
## Summary Adds SBOM (Software Bill of Materials) generation to the agent release pipeline using GoReleaser's native `syft` integration, mirroring the change already shipped in `cli-private` (#93). For every release, SPDX-JSON SBOMs are produced for the build artifacts and uploaded to the GitHub release automatically. This gives supply-chain transparency: downstream consumers and security tooling can enumerate dependencies for vulnerability response, license auditing, and provenance verification. It aligns the agent with the same supply-chain posture as the CLI and with regulatory expectations (US EO 14028, EU Cyber Resilience Act). ## Changes - **`build/.goreleaser.yaml`**: added an `sboms:` stanza (GoReleaser Pro) generating SPDX-JSON SBOMs — one entry for `artifacts: archive` (the tar.gz archives) and one for `artifacts: package` (the `.deb`). Relies on GoReleaser's syft + SPDX-JSON defaults; SBOM files (`{{ .ArtifactName }}.sbom.json`) are uploaded with the release automatically through the existing `release:` stanza. - **`build/Dockerfile.builder`**: added a checksum-verified `syft` install (anchore/syft, pinned `ARG SYFT_VERSION=1.46.0`), styled identically to the existing GoReleaser install — download the release tarball plus `syft_<ver>_checksums.txt`, verify with `sha256sum -c -`, extract to `/usr/local/bin`, and smoke-test with `syft version`. - **`build/Dockerfile`**: comment-only note that the builder image tag pin must be re-pinned once `.github/workflows/builder.yml` republishes the builder image with syft. The existing `a32d4c0` pin is left as-is. ##⚠️ Required follow-up / merge sequencing SBOM generation depends on `syft` being present in the builder image. This requires a specific sequence: 1. Merge this PR (adds the `syft` install to `Dockerfile.builder`). 2. Let `.github/workflows/builder.yml` republish `ghcr.io/mirurobotics/agent-builder:<short-sha>` so the published image actually contains `syft`. 3. Bump the `FROM ghcr.io/mirurobotics/agent-builder:<tag>` pin in `build/Dockerfile` to the new short-SHA tag of that republished image. **Until that re-pin lands, any release/snapshot build will fail at the SBOM step with `syft: command not found`** (the currently pinned builder image predates the syft install). Reviewers should confirm the builder-republish + re-pin sequence before relying on releases. ## Validation done - The `syft` install block (asset names, checksum verification, extraction) was reproduced on a host: both release assets returned HTTP 200, `sha256sum -c -` printed `syft_1.46.0_linux_amd64.tar.gz: OK`, and `syft version` reported `1.46.0`. - `build/.goreleaser.yaml` parses cleanly and the `sboms:` stanza deserializes to the two expected entries. - The Rust lint suite (`clippy`, `cargo fmt`, import-lint, `cargo-machete`, `cargo-diet`) is clean. `cargo audit` could not fetch the RustSec advisory DB in the sandbox (proxy returned HTTP 403) — environmental and unrelated to this change, which touches zero Rust code. - **Not run in sandbox** (no Docker / no GHCR access): the full Docker snapshot build and end-to-end SBOM emission. Deferred to CI post-merge, as the diff is strictly build-infra config. ## Scope note Strictly the `agent` repo's `build/` pipeline. No Rust source, no workflow files, and no other repos change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_012ttfWUipwm1fvHL2ZcZFzU)_ <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/mirurobotics/codesmith/agent/pr/92"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1785297775&installation_id=142588636&pr_number=92&repository=mirurobotics%2Fagent&return_to=https%3A%2F%2Fgithub.com%2Fmirurobotics%2Fagent%2Fpull%2F92&signature=a79e63480ff177d4b624f36e75bb8b7c2964db8e87eee46310d7d967c0c99e13"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> Co-authored-by: Claude <noreply@anthropic.com>
c65217a to
096b92a
Compare
ben-miru
added a commit
that referenced
this pull request
Jun 30, 2026
## Summary - Domain `Release` now carries `upload_rule_ids: Vec<UploadRuleID>`, populated from the server-side `upload_rules` expansion. `From<backend_client::Release>` is replaced with a `Release::from_backend(release, upload_rule_ids)` constructor, threaded through deserialize via `#[serde(default)]`. - Re-vendors `api/specs/backend/v04.yaml` from openapi `main` to pick up the new `getRelease` `expand` param (openapi #167) and regenerates models, adding a `ReleaseExpansion` model. - The get-release service now requests the expansion (`fetch_release` passes `&["upload_rules"]`) and projects the expanded rule ids onto the `Release`. If the backend fails to expand, it hard-errors (`ServiceErr::UploadRulesNotExpanded`) rather than silently caching an empty set that would poison the shared release store — mirroring the deployment-sync path. - Deployment sync (`store_expanded_release`) links the ids onto the stored `Release`, while still writing full rule bodies into the separate append-only `upload_rules` store. Split out of the in-progress uploads branch (#93) so it can be reviewed and merged independently; this should merge **before** #93. The uploader-feeding work (active-rule traversal + syncer push) intentionally stays on #93. Validation: `cargo check --workspace` clean; full test suite green (1349 passing); preflight clean; separate lint gate clean. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/mirurobotics/codesmith/agent/pr/101"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1785448593&installation_id=142588636&pr_number=101&repository=mirurobotics%2Fagent&return_to=https%3A%2F%2Fgithub.com%2Fmirurobotics%2Fagent%2Fpull%2F101&signature=7aee8f27130f17e52301df44d65c386ca00969467714b848d72e845d2612eb49"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a new `uploads` background worker that, for each cached UploadRule, glob-matches `source.glob` on a per-rule `poll_interval_secs` cadence and decides which matched files are quiescent (size+mtime stable for at least `stability_window_secs`) via a pure `decide_ready` seam run under spawn_blocking. Newly-ready files are emitted to a placeholder `info!` sink and tracked in-memory to avoid re-logging. Wire the worker into app/run.rs behind a default-on `enable_uploads_worker` flag with its JoinHandle tracked by the ShutdownManager, mirroring the poller/mqtt workers. Inject both `sleep_fn` and a `now_fn` clock for deterministic testing. Add the `glob = "0.3.2"` dependency. Scope is discovery + readiness only; digest/upload/confirm/ledger/ delete_policy are deferred to M3-M5. A finalization-marker HOOK is marked as a comment only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make `decide_ready` and `FileObservation` public and move the already-reported dedupe into `decide_ready` so it returns only NEWLY-ready files (recording each in `already_reported` exactly once). The `run_impl` placeholder `info!` sink now logs every returned file with no contains check, keeping the sink wired (no dead code). Behavior is unchanged; this exposes a pure, deterministic readiness/dedupe seam for unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add agent/tests/workers/uploads.rs exercising the pure `decide_ready` seam (glob match/miss + recursive + invalid glob; stability state machine with window reset; once-only dedupe; no-match) and the full `run` worker loop via a controllable test clock + SleepController (per-rule poll cadence, empty/no-rules idle, cache-error treated as empty). Add a reusable `mocks::clock::Clock` helper. Add the app/run.rs duplicate-handle test `register_handle_rejects_uploads_duplicates`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark M2.1-M2.7 done; record implementation surprises (decide_ready made a public dedupe-owning seam, glob locked at 0.3.3, spawn_blocking integration, write_if_absent signature, cache-error forced via shutdown, clock helper location), add the dedupe-into-decide_ready decision, and fill in Outcomes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add unit tests driving ShutdownManager::shutdown through both the registered and absent uploads_worker_handle paths. The new uploads worker wiring had dropped the app module coverage aggregate just below its covgate threshold (90.34% < 90.38%); these tests exercise the previously-uncovered shutdown_impl path and restore it to 90.76%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the import-linter auto-fix that collapses the two crate::mocks use statements into a single grouped import. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oyment Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add upload_rule_ids to the domain Release (mirroring Deployment.config_instance_ids) populated during acquisition, and change the upload-discovery worker to resolve its active rule set by traversal (Deployed deployment -> release -> upload_rule_ids -> rule bodies) instead of the whole append-only UploadRules store. No pruning: the store stays the append-only by-id body cache. - models::Release: add upload_rule_ids field, empty-Vec Default, serde default on the Deserialize path; replace From<backend_client::Release> with Release::from_backend(release, upload_rule_ids). - sync::store_expanded_release: extract upload rules first, derive ids, build the release via from_backend; bodies still written append-only. - workers::uploads: thread deployments + releases stores through run/ run_impl; add active_upload_rules traversal helper (union over Deployed deployments, dedupe by id, skip+debug missing ids). - app::init_uploads_worker: clone and pass the two extra stores. - services::release::get: adapt the fetch-by-id path to from_backend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- models::release: cover upload_rule_ids via optional_fields harness, defaults, and from_backend (populated + empty) cases. - sync: populates_release_upload_rule_ids asserts the domain release's upload_rule_ids from the expanded backend release. - workers::uploads: thread deployments+releases stores through the test harness (spawn_stores, seed_deployed); fix existing cadence/idle tests to seed a Deployed deployment+release; retarget the cache-error test to the deployments store; add mod active_set with 9 direct unit tests of active_upload_rules (resolved, stale-skip, no-deployed, missing-id, missing-release, union+dedupe, and three cache-error arms). - services::release::get: cover the Some(upload_rules) link branch. - server tests: add upload_rule_ids to exhaustive Release literals. Expose active_upload_rules as a pub test seam (mirrors decide_ready). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Satisfy clippy::cloned_ref_to_slice_refs under --all-targets. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark M1-M5 + V done; record the services/release/get.rs second From call site, server-test literal fixes, the active_upload_rules pub seam, the serde(default) decision, and final signatures/covgate/validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stability-window comment was attached to the unchanged size/mtime arm while describing the new/changed (re)start case. Swap the comments so each arm documents its actual behavior. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace four field-by-field assert_eq! calls in the release from_backend test with a single expected-struct comparison (the asserts linter flags field-by-field-assert), matching the defaults test idiom. Apply rustfmt to the active_upload_rules call sites in the uploads worker tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror the Syncer/poller split for the upload subsystem: - New agent/src/upload/ module: SingleThreadUploader actor owning the active rule set, per-file readiness state, and per-rule cadence; a cloneable Uploader handle (mpsc Command channel) and UploaderExt trait. FileObservation, ReadyFile, and the decide_ready seam move here; no storage dependency. - Relocate the active_upload_rules traversal to agent/src/sync/upload_rules.rs and have the Syncer push the active set to the Uploader after each deployments::sync (regardless of backend push outcome). - Rewrite agent/src/workers/uploads.rs as a thin timing-only worker that ticks uploader.scan() on a base interval, mirroring poller.rs. - Wire the Uploader into AppState (spawned before the Syncer, joined and shut down after it); add ServerErr <- UploadErr conversion. Behavior preserved; no M3+ (digest/PUT/confirm/ledger/delete_policy) and no pruning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… thin worker - agent/tests/upload/uploader.rs: ported pure decide_ready tests plus actor tests driving the Uploader handle (update_rules replaces the set; scan honors per-rule cadence via injected clock; readiness window; dedupe-once across scans; empty-set no-op). Adds a #[cfg(feature="test")] GetReportedCount inspector on the Uploader so readiness/cadence/dedupe are observable through the public handle without scraping logs. - agent/tests/sync/upload_rules.rs: relocated the active_upload_rules traversal tests (resolved, stale-skip, no-deployed, missing-id, missing-release, union/dedupe, three cache-error cases). - agent/tests/workers/uploads.rs: rewritten as a thin-worker test using a counting UploaderExt mock (scan-per-tick cadence + shutdown). - agent/tests/sync/syncer.rs: SyncerArgs gains the uploader handle; adds a push test asserting sync_impl computes deployed->release->rules and pushes the active set to the uploader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hold - agent/tests/upload/errors.rs: cover UploadErr From conversions and the impl_error! Error-trait arms for both variants. - agent/tests/upload/uploader.rs: add a shutdown test (stops the actor; later commands fail with a send error) exercising the shutdown path and send_command error mapping. - agent/tests/server/errors.rs: cover From<UploadErr> for ServerErr and the ServerErr::UploadErr Error-trait arms, restoring server module coverage above its threshold after the new variant was added. - Set agent/src/upload/.covgate to 89.00 (measured 89.29%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark M1-M4 complete; record impl-time surprises (ServerErr<-UploadErr wiring, GetReportedCount test inspector, apply no-op seeding for the push test) and decisions; add the completion retrospective. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rustfmt expands the single-line write_if_absent closure to a block; satisfies cargo fmt --check in the preflight gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
After openapi removed poll_interval_secs from the upload-rule contract (#100), the uploader can no longer read a per-rule interval. Collapse the per-rule cadence to the single global min_poll_interval_secs: every rule's next scan is one global interval out. Update the test fixtures (remove the poll_interval_secs arg from rule_with) and rewrite scan_honors_per_rule_cadence as scan_uses_global_cadence (both rules now come due together each tick). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The global-cadence refactor rewrote scan_honors_per_rule_cadence into scan_uses_global_cadence, which makes all rules due in the same scan and no longer exercises the "rule not yet due -> skip" branch in scan(). Add an actor test that scans within the cadence window and asserts the rule is skipped (nothing reported) until the interval elapses, restoring the upload module above its 89.00% coverage gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
096b92a to
97f1c2d
Compare
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
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.
Summary
M2 of the data-upload feature: an
Uploadersubsystem that discovers which on-device files are ready to upload. It is structured exactly like the existing Syncer/poller split — a statefulUploaderobject that owns the rules and logic, plus a thin timing-only worker that drives it — with the Syncer pushing the active rule set to the uploader. It does not upload anything yet: ready files are logged as a placeholder sink that M3 will replace with the mint → PUT → confirm pipeline.This branch was rebased onto
mainafter the upload-rule data model (#101) and thepoll_interval_secscontract removal (#100) landed. The now-redundant data-model / get-release / deployment-linkage changes were dropped in favor of main's canonical versions; the branch retains only its unique uploader work and consumesRelease.upload_rule_ids(added by #101) as an input.Architecture
Three pieces, mirroring how the Syncer is split from the poller:
Uploaderactor (agent/src/upload/) — a stateful mpsc Command actor (UpdateRules/Scan/Shutdown) wrapped by a cloneableUploaderhandle and anUploaderExttrait, mirroringSingleThreadSyncer/Syncer/SyncerExt. It owns, in memory: the active upload rule set, per-file readiness/observation state, a single global poll cadence, glob matching, thedecide_readystability logic, and the placeholder ready-file log sink. It has no storage dependency.uploadsworker (agent/src/workers/uploads.rs) — timing only, mirroringpoller: it ticks on a base interval and callsuploader.scan(). No rules, no logic.uploader.update_rules(...)after applying deployments each sync. The deployment → release → rules traversal lives inagent/src/sync/upload_rules.rs.Why: clean separation of concerns — the worker handles timing, the uploader handles rules + readiness logic, and the syncer decides which rules are active. The
Uploaderis spawned inAppState::initbefore the Syncer; its handle is injected intoSyncerArgsand into the worker.What it discovers
UploadRuleis matched against its absolutesource.glob(via theglobcrate; enumeration runs underspawn_blocking).poll_interval_secswas removed from the upload-rule contract (chore(api): sync vendored backend openapi spec and regen models #100), so every rule's next scan is now scheduled off a single globalmin_poll_interval_secs, owned by the uploader, rather than a per-rule interval.>= stability_window_secs(per-file observation state, reset on any change). The puredecide_readyseam makes readiness + dedupe deterministic to test.info!({file_path, file_modified_at}) and deduped in-memory so each ready file is reported once. M3 replaces this sink with mint → PUT → confirm.Rule sourcing
The active rule set is computed by traversal and pushed by the syncer on each sync: the currently-
Deployeddeployment(s) → theirrelease→ the release'supload_rule_ids→ rule bodies. The result is unioned acrossDeployeddeployments (normally a singleton; the union avoids dropping an outgoing release's files mid-redeploy) and deduped by id. No deployed deployment ⇒ empty set ⇒ the uploader idles.Release.upload_rule_ids— the domainReleasecarriesupload_rule_ids(added onmainby feat(release): carry upload-rule ids on domain Release #101, mirroringDeployment.config_instance_ids); this PR consumes it as the traversal input rather than adding it.update_rulespush (first-sync seeding). Rules from a since-replaced release are simply ignored.UploadRulesstore stays the append-only by-id body cache; it remains the syncer's body source keyed offupload_rule_ids, and is never pruned.Dependency
Adds
glob(workspace dep, locked 0.3.3) — a single absolute pattern per rule that both walks the filesystem and matches in one call.cargo machetesees it used; audit clean.Scope
Discovery + readiness only, with a placeholder log sink. Out of scope (M3–M5): sha256/digest,
POST /uploads, presignedPUT,POST confirm, the upload ledger, anddelete_policyenforcement. M3 (mint + PUT + confirm) replaces the log sink next.Testing
scripts/preflight.sh→ Preflight clean (import-linter, fmt, clippy-D warnings, cargo-diet, machete, audit, full suite — 1379 main + tools tests pass, all coverage thresholds met). The separate lint gate is also clean.update_rulesreplaces the set; the global cadence via injected clock (scan_uses_global_cadence, plusscan_skips_rule_until_cadence_elapsescovering the not-yet-due skip branch — replacing the old per-rulescan_honors_per_rule_cadence); the readiness/stability window; dedupe-once across scans; empty-set idle no-op (a#[cfg(feature="test")]GetReportedCountinspector makes readiness/cadence/dedupe observable through the public handle). Fixtures dropped the now-removedpoll_interval_secsarg fromrule_with.UploaderExtmock asserts onescan()per tick plus shutdown.active_upload_rulestraversal cases (resolved, stale-skip, no-deployed-idle, missing-id, missing-release, union/dedupe, cache-error arms) moved with the code intosync/upload_rules.sync_implcomputes deployed → release → rules and pushes the active set to the uploader.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.