Update self-update flow to use CDN release metadata#123
Merged
F16shen merged 1 commit intoAI-Shell-Team:mainfrom Apr 22, 2026
Merged
Update self-update flow to use CDN release metadata#123F16shen merged 1 commit intoAI-Shell-Team:mainfrom
F16shen merged 1 commit intoAI-Shell-Team:mainfrom
Conversation
Switch stable update checks from the GitHub latest release API to the CDN latest metadata endpoint so the update command follows the new release distribution path used by the installer. Update archive downloads to use the CDN bundle base URL and honor AISH_DOWNLOAD_BASE_URL, AISH_LATEST_URL, and the legacy AISH_REPO_URL override for compatibility with existing deployment setups. Refresh update manager tests to cover stable latest metadata parsing, CDN download URLs, and environment-based download base overrides. Validation: /home/lixin/workspace/aishell/aish/.venv/bin/python -m pytest tests/test_update_manager.py
Contributor
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
Contributor
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
📝 WalkthroughWalkthroughThe update_manager module transitions from GitHub API-based version checking to configurable CDN-based resolution. Environment variables now control the CDN download base URL and latest version metadata endpoint, replacing hardcoded GitHub constants. Version data is fetched as plain text from CDN rather than GitHub JSON API, with new validation logic introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/test_update_manager.py (2)
107-152:⚠️ Potential issue | 🟠 MajorMake the download tests deterministic across runners.
These assertions hard-code
linux-amd64, butdownload_release()builds the filename fromdetect_platform(). They will fail ondarwin/arm64runners, and the default-path case can also be skewed by ambientAISH_DOWNLOAD_BASE_URL/AISH_REPO_URLoverrides.Suggested fix pattern
-def test_download_release_success(mock_client_class, update_manager, tmp_path): +def test_download_release_success( + mock_client_class, update_manager, tmp_path, monkeypatch +): + monkeypatch.delenv("AISH_DOWNLOAD_BASE_URL", raising=False) + monkeypatch.delenv("AISH_REPO_URL", raising=False) + mock_response = Mock() mock_response.raise_for_status = Mock() mock_response.iter_bytes = Mock(return_value=[b"test data"]) @@ - with patch.object(update_manager, "client", mock_client_instance): + with ( + patch.object(update_manager, "client", mock_client_instance), + patch.object(update_manager, "detect_platform", return_value=("linux", "amd64")), + ): result = update_manager.download_release("v0.3.0", dest_dir=tmp_path)Use the same
detect_platform()stub in the override test as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_update_manager.py` around lines 107 - 152, The tests hard-code the platform string causing failures on non-linux-amd64 runners and can be influenced by ambient env vars; in the test function test_download_release_respects_download_base_override stub detect_platform() to return the same deterministic platform used in test_download_release (e.g., "linux-amd64"), and ensure AISH_DOWNLOAD_BASE_URL/AISH_REPO_URL are set/cleared via monkeypatch to known values before calling update_manager.download_release; keep the existing mocks (mock_client_instance.stream) and assert against the resulting stream_url as done currently.
38-55:⚠️ Potential issue | 🟠 MajorClear
AISH_*overrides in the default latest-version tests.These tests assert the default
/latestURL, butget_latest_release()now honorsAISH_LATEST_URL,AISH_DOWNLOAD_BASE_URL, andAISH_REPO_URL. If any of those are present in the runner environment, the assertions fail for the wrong reason.Suggested fix pattern
def test_get_latest_release_success( - mock_client_class, update_manager, mock_latest_version_text + mock_client_class, update_manager, mock_latest_version_text, monkeypatch ): + for var in ("AISH_LATEST_URL", "AISH_DOWNLOAD_BASE_URL", "AISH_REPO_URL"): + monkeypatch.delenv(var, raising=False) + mock_response = Mock() mock_response.raise_for_status = Mock() mock_response.text = mock_latest_version_textApply the same setup to the other default-path update-check tests in this block.
Also applies to: 71-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_update_manager.py` around lines 38 - 55, The test test_get_latest_release_success (and the other default-path update-check tests in the same block) fail when AISH_LATEST_URL, AISH_DOWNLOAD_BASE_URL or AISH_REPO_URL are set in the environment; update the tests to clear those env overrides before calling update_manager.get_latest_release() — e.g. use patch.dict(os.environ, {"AISH_LATEST_URL":"", "AISH_DOWNLOAD_BASE_URL":"", "AISH_REPO_URL":""}, clear=False) or monkeypatch.delenv for each key so the function uses its default "/latest" path; apply the same change to the other tests in the block (lines ~71-100) to ensure the mocked client.get assertion that URL endswith("/latest") is stable.src/aish/cli/update_manager.py (1)
135-153:⚠️ Potential issue | 🟠 MajorPick the newest version instead of returning
releases[0].When
include_pre_release=True, this path now trusts the first entry from GitHub’s releases list. That list can contain both stable and prerelease entries, so--pre-releasebecomes API-order dependent and can miss a newer beta if an older stable release happens to be first. Parse the tags and select the highest version before returning it.Suggested fix
if include_pre_release: # /releases/latest excludes pre-releases, use list endpoint instead response = self.client.get(GITHUB_API_LIST) response.raise_for_status() releases = response.json() - if not releases: - return None - data = releases[0] - tag_name = data.get("tag_name") - if not tag_name: + candidates = [] + for release in releases: + tag_name = release.get("tag_name") + if not tag_name or release.get("draft"): + continue + candidates.append((version.parse(tag_name.lstrip("v")), release)) + + if not candidates: return None + + _, data = max(candidates, key=lambda item: item[0]) + tag_name = data["tag_name"] return { "tag_name": tag_name, "name": data.get("name"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/aish/cli/update_manager.py` around lines 135 - 153, The code returning releases[0] when include_pre_release=True can miss the newest semantic version; in the method handling the GITHUB_API_LIST response (the branch using include_pre_release in update_manager.py), iterate all items in the releases list, extract each data.get("tag_name"), parse versions with packaging.version.parse (or pkg_resources.parse_version) to compare them, pick the release with the highest parsed version, and then return that release's fields ("tag_name","name","body","html_url","assets"); if no tag_name is parseable, fall back to the existing behavior (e.g., first non-empty tag_name) to avoid breaking the flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/aish/cli/update_manager.py`:
- Around line 135-153: The code returning releases[0] when
include_pre_release=True can miss the newest semantic version; in the method
handling the GITHUB_API_LIST response (the branch using include_pre_release in
update_manager.py), iterate all items in the releases list, extract each
data.get("tag_name"), parse versions with packaging.version.parse (or
pkg_resources.parse_version) to compare them, pick the release with the highest
parsed version, and then return that release's fields
("tag_name","name","body","html_url","assets"); if no tag_name is parseable,
fall back to the existing behavior (e.g., first non-empty tag_name) to avoid
breaking the flow.
In `@tests/test_update_manager.py`:
- Around line 107-152: The tests hard-code the platform string causing failures
on non-linux-amd64 runners and can be influenced by ambient env vars; in the
test function test_download_release_respects_download_base_override stub
detect_platform() to return the same deterministic platform used in
test_download_release (e.g., "linux-amd64"), and ensure
AISH_DOWNLOAD_BASE_URL/AISH_REPO_URL are set/cleared via monkeypatch to known
values before calling update_manager.download_release; keep the existing mocks
(mock_client_instance.stream) and assert against the resulting stream_url as
done currently.
- Around line 38-55: The test test_get_latest_release_success (and the other
default-path update-check tests in the same block) fail when AISH_LATEST_URL,
AISH_DOWNLOAD_BASE_URL or AISH_REPO_URL are set in the environment; update the
tests to clear those env overrides before calling
update_manager.get_latest_release() — e.g. use patch.dict(os.environ,
{"AISH_LATEST_URL":"", "AISH_DOWNLOAD_BASE_URL":"", "AISH_REPO_URL":""},
clear=False) or monkeypatch.delenv for each key so the function uses its default
"/latest" path; apply the same change to the other tests in the block (lines
~71-100) to ensure the mocked client.get assertion that URL endswith("/latest")
is stable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56fde727-fe1e-4e36-88e2-3e10a08f4c55
📒 Files selected for processing (2)
src/aish/cli/update_manager.pytests/test_update_manager.py
This was referenced Apr 22, 2026
Merged
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.
Summary
Switch the stable aish update flow to the CDN release metadata and download path used by the installer.
What changed
Why
The installer now resolves releases from the CDN, so the update command needs to follow the same distribution path. This keeps install and update behavior aligned and avoids depending on the old GitHub release download layout for stable updates.
Notes
Validation
Summary by CodeRabbit
Release Notes
New Features
AISH_DOWNLOAD_BASE_URL,AISH_REPO_URL,AISH_LATEST_URL) to customize CDN and metadata sources.Chores