Repository navigation
Unify update feeds with channel-aware installer manifest - #414
Conversation
Give Application.Updates.IUpdateService a channel-aware CheckForUpdateAsync overload (default forwards to Stable so existing callers and fakes compile unchanged). UpdateService resolves manifest.json for Stable and manifest-preview.json for Preview and ignores prerelease manifests on Stable. Repoint ReleaseManifestUpdateService off the dead api.trackdub.com host to releases.trackdub.ai, removing the last api.trackdub.com reference. Covers C6 core half of the combined-priority work-split.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughUpdate checks now accept a channel and select a stable or preview manifest. Stable checks ignore prerelease manifests. The legacy release manifest service uses the new releases.trackdub.ai URL. Tests verify channel-based requests and stable prerelease handling. ChangesUpdate channels
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Caller
participant UpdateService
participant ManifestEndpoint
Caller->>UpdateService: CheckForUpdateAsync(currentVersion, channel)
UpdateService->>ManifestEndpoint: GET manifest for selected channel
ManifestEndpoint-->>UpdateService: Release manifest
alt Stable channel with prerelease manifest
UpdateService-->>Caller: No update and no error
else Other manifest cases
UpdateService-->>Caller: Continue version and download URL validation
end
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description provides a clear summary, test results, and follow-up notes, but it omits most required template sections, including the linked issue, scope checkboxes, architecture review, license/mo… | Update the description to include all template headings and required checklist items. Add the linked issue, scope confirmation, testing checklist and notes, architecture review, license/model impact, risk and rollback assessment, milestone … |
| Docstring Coverage | |
Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly summarizes the main change: unified update feeds with channel-aware installer manifests. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
Explanation
The description provides a clear summary, test results, and follow-up notes, but it omits most required template sections, including the linked issue, scope checkboxes, architecture review, license/model impact, risk and rollback, milestone notes, and agent notes.
Resolution
Update the description to include all template headings and required checklist items. Add the linked issue, scope confirmation, testing checklist and notes, architecture review, license/model impact, risk and rollback assessment, milestone notes, and agent notes. Mark each item as applicable or not applicable.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
✨ Simplify code
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
PR Summary by QodoAdd channel-aware installer update manifests and unify release hosts
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation successfully unifies update feeds with channel-aware manifests. The changes correctly add a channel-aware overload to IUpdateService, migrate from the deprecated api.trackdub.com to releases.trackdub.ai, and implement proper prerelease filtering for the Stable channel. The comprehensive test coverage validates all the new functionality including channel routing, prerelease handling, and manifest URL resolution. All 19 UpdateServiceTests pass as noted in the PR description. No blocking issues found.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df1cbb69df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return new Trackdub.Application.Updates.UpdateCheckResult(false, null, "Release manifest could not be parsed."); | ||
| } | ||
|
|
||
| if (channel == UpdateChannel.Stable && schema.IsPrerelease) |
There was a problem hiding this comment.
When the Preview manifest contains a normal SemVer prerelease such as 2.0.1-beta.1 (the format already exercised by CheckForUpdatesAsync_SemVerSuffix_ReturnsUpdateResult), this branch falls through to Version.TryParse(schema.LatestVersion), which rejects the suffix and returns “Version format in manifest is invalid.” Consequently, the newly added Preview channel cannot offer actual prerelease builds; compare prerelease-aware versions or normalize them before parsing.
Useful? React with 👍 / 👎.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
tonythethompson
left a comment
tonythethompson
left a comment
There was a problem hiding this comment.
One material note on Preview version parsing; otherwise the channel URL split and Stable prerelease gate look right.
| if (channel == UpdateChannel.Stable && schema.IsPrerelease) | ||
| { | ||
| logger.LogInformation( | ||
| $"Ignoring prerelease {schema.LatestVersion} on the Stable channel."); | ||
| return new Trackdub.Application.Updates.UpdateCheckResult(false, null, null); |
There was a problem hiding this comment.
Version.TryParse immediately below still rejects SemVer pre-release tags (2.0.1-beta.1, 2.1.0-preview.3). Preview manifests will almost always publish those forms, so a Preview check returns "Version format in manifest is invalid" instead of an update — this Stable IsPrerelease gate never gets a chance to help on the Preview path.
ReleaseManifestUpdateService already strips -/+ suffixes before comparing (SanitizeVersion). Mirror that here (or share a helper) for both schema.LatestVersion and currentVersion, and keep the full original string on ReleaseEntry. Worth a Preview test with a suffixed latest version; the new Preview URL test uses plain 2.0.0, so this gap is unguarded.
Residual: strip-and-compare still collapses 2.0.0-beta.1 vs 2.0.0-beta.2; fine to leave unless Preview bumps often within the same numeric triple.
|
The discovery worker independently confirmed my analysis: no confirmed defects in the changed code. The PR is a clean, well-tested, low-risk change. Per the skill, with no confirmed findings and no material verification notes, I report clean and do not submit an empty review. No noteworthy issues found. The PR cleanly adds a channel-aware |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
/q fix |
|
I'll implement the fix for the prerelease version parsing issue: Changes to be made: |
|
✅ Changes committed. The prerelease version parsing issue has been fixed by adding a |
Add StripVersionSuffix helper to remove SemVer prerelease suffixes (e.g., -beta.1, -preview.3) before parsing versions, allowing Preview channel manifests to use standard prerelease version formats. Keep full version strings in ReleaseEntry for display purposes. Add test coverage for Preview channel with prerelease versions.
C6 core half (combined-priority work-split).
Verification: UpdateServiceTests 19/19, Infrastructure.Tests 430/430, Application.Tests 1149/1149, Release -warnaserror clean on Application + Infrastructure.
Follow-ups (not in this PR): gated UpdateViewModel passes channel through after repin; release pipeline must publish manifest-preview.json.
Summary by CodeRabbit