feat: deliver desktop updates through Cloudflare R2 - #165
Merged
Conversation
Soulter
marked this pull request as ready for review
August 7, 2026 09:13
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/ci/publish_r2_release.py" line_range="28-46" />
<code_context>
+ mutable: bool = False
+
+
+def content_type_for(path: Path) -> str:
+ lower_name = path.name.lower()
+ if lower_name.endswith(".json"):
+ return "application/json"
+ if lower_name.endswith(".sig"):
+ return "text/plain; charset=utf-8"
+ if lower_name.endswith(".zip"):
+ return "application/zip"
+ if lower_name.endswith((".tar.gz", ".gz")):
+ return "application/gzip"
+ if lower_name.endswith(".exe"):
+ return "application/vnd.microsoft.portable-executable"
+ if lower_name.endswith(".deb"):
+ return "application/vnd.debian.binary-package"
+ if lower_name.endswith(".rpm"):
+ return "application/x-rpm"
+ if lower_name.endswith(".dmg"):
+ return "application/x-apple-diskimage"
+ return "application/octet-stream"
+
+
</code_context>
<issue_to_address>
**suggestion:** Add an explicit MIME type for AppImage artifacts instead of falling back to application/octet-stream.
Right now `.AppImage` files fall through to `application/octet-stream`. Please add an explicit mapping for them in `content_type_for` (e.g. `application/x-executable` or another appropriate type) so their content type is accurately advertised and better supported by tooling and intermediaries.
```suggestion
def content_type_for(path: Path) -> str:
lower_name = path.name.lower()
if lower_name.endswith(".json"):
return "application/json"
if lower_name.endswith(".sig"):
return "text/plain; charset=utf-8"
if lower_name.endswith(".zip"):
return "application/zip"
if lower_name.endswith((".tar.gz", ".gz")):
return "application/gzip"
if lower_name.endswith(".exe"):
return "application/vnd.microsoft.portable-executable"
if lower_name.endswith(".deb"):
return "application/vnd.debian.binary-package"
if lower_name.endswith(".rpm"):
return "application/x-rpm"
if lower_name.endswith(".dmg"):
return "application/x-apple-diskimage"
if lower_name.endswith(".appimage"):
# AppImage artifacts are self-contained Linux executables; use an explicit executable MIME type
return "application/x-executable"
return "application/octet-stream"
```
</issue_to_address>
### Comment 2
<location path="scripts/ci/test_publish_r2_release.py" line_range="156-102" />
<code_context>
+ "artifacts",
+ )
+
+ def test_build_upload_plan_rejects_missing_manifest_artifact(self):
+ with tempfile.TemporaryDirectory() as tmpdir:
+ root = Path(tmpdir)
+ (root / "other.exe").write_bytes(b"installer")
+ manifest = root / "latest-stable.json"
+ self.write_manifest(
+ manifest,
+ version="4.29.0",
+ channel="stable",
+ artifact_name="AstrBot.exe",
+ )
+
+ with self.assertRaisesRegex(ValueError, "missing from the upload set"):
+ MODULE.build_upload_plan(
+ root,
+ manifest,
+ "4.29.0",
+ "12345-1",
+ "stable",
+ "artifacts",
+ )
+
</code_context>
<issue_to_address>
**suggestion (testing):** Extend tests to cover invalid or non-HTTPS artifact URLs in the manifest
The remaining untested paths are the manifest URL validation branches (non-HTTPS, missing netloc, empty path/filename). Please add tests that construct manifests with values like `"http://..."`, `"/relative/path"`, and an `https` URL without a filename, and assert the corresponding `ValueError` messages. This will fully exercise the URL validation, including the enforcement that only HTTPS artifact URLs are accepted.
Suggested implementation:
```python
import json
from scripts.ci import publish_r2_release as MODULE
```
```python
def test_build_upload_plan_rejects_missing_manifest_artifact(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
(root / "other.exe").write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
with self.assertRaisesRegex(ValueError, "missing from the upload set"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_non_https_artifact_url(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# Non-HTTPS URL should be rejected
data["artifacts"][0]["url"] = "http://example.com/AstrBot.exe"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "HTTPS"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_artifact_url_missing_netloc(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# An HTTPS URL without a hostname/netloc should be rejected
data["artifacts"][0]["url"] = "https:///AstrBot.exe"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "netloc"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_artifact_url_without_filename(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# HTTPS URL without a filename in the path should be rejected
data["artifacts"][0]["url"] = "https://example.com/"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "filename"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
```
These tests assume the `ValueError` messages for the URL validation contain the substrings `"HTTPS"`, `"netloc"`, and `"filename"` respectively. If the actual messages differ (e.g., `"must use https"`, `"missing host"`, `"missing filename in URL"`), adjust the `assertRaisesRegex` patterns to match the real messages emitted by `build_upload_plan`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+28
to
+46
| def content_type_for(path: Path) -> str: | ||
| lower_name = path.name.lower() | ||
| if lower_name.endswith(".json"): | ||
| return "application/json" | ||
| if lower_name.endswith(".sig"): | ||
| return "text/plain; charset=utf-8" | ||
| if lower_name.endswith(".zip"): | ||
| return "application/zip" | ||
| if lower_name.endswith((".tar.gz", ".gz")): | ||
| return "application/gzip" | ||
| if lower_name.endswith(".exe"): | ||
| return "application/vnd.microsoft.portable-executable" | ||
| if lower_name.endswith(".deb"): | ||
| return "application/vnd.debian.binary-package" | ||
| if lower_name.endswith(".rpm"): | ||
| return "application/x-rpm" | ||
| if lower_name.endswith(".dmg"): | ||
| return "application/x-apple-diskimage" | ||
| return "application/octet-stream" |
There was a problem hiding this comment.
suggestion: Add an explicit MIME type for AppImage artifacts instead of falling back to application/octet-stream.
Right now .AppImage files fall through to application/octet-stream. Please add an explicit mapping for them in content_type_for (e.g. application/x-executable or another appropriate type) so their content type is accurately advertised and better supported by tooling and intermediaries.
Suggested change
| def content_type_for(path: Path) -> str: | |
| lower_name = path.name.lower() | |
| if lower_name.endswith(".json"): | |
| return "application/json" | |
| if lower_name.endswith(".sig"): | |
| return "text/plain; charset=utf-8" | |
| if lower_name.endswith(".zip"): | |
| return "application/zip" | |
| if lower_name.endswith((".tar.gz", ".gz")): | |
| return "application/gzip" | |
| if lower_name.endswith(".exe"): | |
| return "application/vnd.microsoft.portable-executable" | |
| if lower_name.endswith(".deb"): | |
| return "application/vnd.debian.binary-package" | |
| if lower_name.endswith(".rpm"): | |
| return "application/x-rpm" | |
| if lower_name.endswith(".dmg"): | |
| return "application/x-apple-diskimage" | |
| return "application/octet-stream" | |
| def content_type_for(path: Path) -> str: | |
| lower_name = path.name.lower() | |
| if lower_name.endswith(".json"): | |
| return "application/json" | |
| if lower_name.endswith(".sig"): | |
| return "text/plain; charset=utf-8" | |
| if lower_name.endswith(".zip"): | |
| return "application/zip" | |
| if lower_name.endswith((".tar.gz", ".gz")): | |
| return "application/gzip" | |
| if lower_name.endswith(".exe"): | |
| return "application/vnd.microsoft.portable-executable" | |
| if lower_name.endswith(".deb"): | |
| return "application/vnd.debian.binary-package" | |
| if lower_name.endswith(".rpm"): | |
| return "application/x-rpm" | |
| if lower_name.endswith(".dmg"): | |
| return "application/x-apple-diskimage" | |
| if lower_name.endswith(".appimage"): | |
| # AppImage artifacts are self-contained Linux executables; use an explicit executable MIME type | |
| return "application/x-executable" | |
| return "application/octet-stream" |
| "4.29.0", | ||
| "12345-1", | ||
| "stable", | ||
| "artifacts", |
There was a problem hiding this comment.
suggestion (testing): Extend tests to cover invalid or non-HTTPS artifact URLs in the manifest
The remaining untested paths are the manifest URL validation branches (non-HTTPS, missing netloc, empty path/filename). Please add tests that construct manifests with values like "http://...", "/relative/path", and an https URL without a filename, and assert the corresponding ValueError messages. This will fully exercise the URL validation, including the enforcement that only HTTPS artifact URLs are accepted.
Suggested implementation:
import json
from scripts.ci import publish_r2_release as MODULE def test_build_upload_plan_rejects_missing_manifest_artifact(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
(root / "other.exe").write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
with self.assertRaisesRegex(ValueError, "missing from the upload set"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_non_https_artifact_url(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# Non-HTTPS URL should be rejected
data["artifacts"][0]["url"] = "http://example.com/AstrBot.exe"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "HTTPS"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_artifact_url_missing_netloc(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# An HTTPS URL without a hostname/netloc should be rejected
data["artifacts"][0]["url"] = "https:///AstrBot.exe"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "netloc"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)
def test_build_upload_plan_rejects_artifact_url_without_filename(self):
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
artifact_path = root / "AstrBot.exe"
artifact_path.write_bytes(b"installer")
manifest = root / "latest-stable.json"
self.write_manifest(
manifest,
version="4.29.0",
channel="stable",
artifact_name="AstrBot.exe",
)
data = json.loads(manifest.read_text(encoding="utf-8"))
# HTTPS URL without a filename in the path should be rejected
data["artifacts"][0]["url"] = "https://example.com/"
manifest.write_text(json.dumps(data), encoding="utf-8")
with self.assertRaisesRegex(ValueError, "filename"):
MODULE.build_upload_plan(
root,
manifest,
"4.29.0",
"12345-1",
"stable",
"artifacts",
)These tests assume the ValueError messages for the URL validation contain the substrings "HTTPS", "netloc", and "filename" respectively. If the actual messages differ (e.g., "must use https", "missing host", "missing filename in URL"), adjust the assertRaisesRegex patterns to match the real messages emitted by build_upload_plan.
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.
What changed
releases.astrbot.app.app.tar.gzupdater artifacts@tauri-apps/cliin CI instead of compilingtauri-clion every runnerWhy
AstrBot Desktop already had a Tauri self-update bridge and GitHub Release manifests, but distribution still depended on GitHub assets, Linux AppImage publishing had been disabled, and macOS did not offer a normal drag-to-Applications installer. The release flow also promoted mutable manifests without first verifying every immutable updater object on the public download origin.
This change makes Desktop updates independent from the AstrBot Core update API and gives stable/nightly channels a dedicated, cacheable R2-backed delivery path.
User impact
releases.astrbot.appValidation
tomllibortomli(CI uses Python 3.12)hdiutil verify, mount, Applications symlink inspection, and detach passedRelease notes
The updater UI and native install/restart bridge already existed. This PR changes the delivery backend and completes the missing package/publishing paths rather than introducing a second update UI.
Summary by Sourcery
Route AstrBot Desktop updater manifests and artifacts through Cloudflare R2 while restoring cross-platform self-update packages and switching CI to the prebuilt Tauri CLI.
New Features:
Enhancements:
Documentation:
Tests: