feat(ampup): add ampup-gen-release-manifest binary #1381
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
LNSD
force-pushed
the
feat-ampup-gen-release-manifest
branch
from
3537048 to
e9d6547
Compare
LNSD
force-pushed
the
feat-ampup-gen-release-manifest
branch
2 times, most recently
from
7caf649 to
f2c03ba
Compare
| /// Note: GitHub API provides this field automatically. | ||
| /// If missing, asset cannot be included in manifest. | ||
| #[serde(default)] | ||
| pub digest: Option<String>, |
There was a problem hiding this comment.
Bug: GitHub API doesn't provide digest field for assets
The Asset struct expects a digest field from the GitHub Releases API, but the standard GitHub API response for release assets does not include this field. The existing github.rs in the same codebase also doesn't expect it. Since digest is Option<String> with #[serde(default)], every asset will have digest: None, causing generate_manifest to fail with MissingDigest error for every asset. The manifest generator will never successfully produce a manifest.
Additional Locations (1)
|
@LNSD related: Should we move the |
Makes sense to me 👍🏼 |
LNSD
force-pushed
the
feat-ampup-gen-release-manifest
branch
8 times, most recently
from
d14759f to
b229776
Compare
| // Validate digest field is present | ||
| let hash = asset.digest.as_ref().ok_or_else(|| Error::MissingDigest { | ||
| asset_name: asset.name.clone(), | ||
| })?; |
There was a problem hiding this comment.
Bug: No validation for empty digest string value
The digest validation only checks for None using as_ref().ok_or_else(), but doesn't validate that the digest is non-empty or follows the expected sha256:... format. If GitHub ever returns an empty string for the digest field, the manifest would be generated successfully with an empty hash value, causing integrity verification to fail when users try to install components.
LNSD
force-pushed
the
feat-ampup-gen-release-manifest
branch
from
b229776 to
0e5a950
Compare
Generate release manifests from GitHub releases to enable per-component installation, allowing users to install specific components instead of complete releases. - Create `ampup-gen-release-manifest` binary for manifest generation - Add shared manifest types to `ampup` library - Parse GitHub release assets into component-target structure - Integrate manifest generation into CI pipeline Signed-off-by: Lorenzo Delgado <lorenzo@edgeandnode.com>
LNSD
force-pushed
the
feat-ampup-gen-release-manifest
branch
from
0e5a950 to
047bd41
Compare
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 21
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| _ => Err(ParseAssetOsError(s.to_string())), | ||
| } | ||
| } |
There was a problem hiding this comment.
Bare windows OS not supported despite documentation claiming it
The documentation states that the standard 3-part asset naming format supports bare windows as an OS value (shown in the regex (linux|darwin|windows)). However, the AssetOs::from_str implementation only handles bare linux and darwin, not bare windows. Assets named like ampd-windows-x86_64 would silently fail to parse and be skipped as "non-binary assets" rather than being processed. Only the explicit 4-part formats windows-msvc-* and windows-gnu-* work correctly.
Generate release manifests from GitHub releases to enable per-component installation, allowing users to install specific components instead of complete releases.
ampup-gen-release-manifestbinary for manifest generationampuplibraryNote
Enables per-component installs by generating and publishing a distribution manifest derived from GitHub release assets.
ampup-gen-release-manifestbinary parses asset names into component/target entries, validates digests, and outputsmanifest.jsonampup::manifesttypes (Manifest,Component,Target,AssetNameparsing with OS/ABI/arch normalization) and docs atcrates/bin/ampup/docs/manifest.mdrelease-manifestjob runs the generator for tagged releases and uploadsrelease/manifest.jsonto the GitHub ReleaseCargo.toml, updatesampupdeps, and includes the crate in monitoringAMP_CRATESWritten by Cursor Bugbot for commit 047bd41. This will update automatically on new commits. Configure here.