Slim update feed storage#354
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR extends the TelegramSearchBot update system to support optional full-package fallback metadata in the update catalog, cumulative package generation via touched-file snapshot planning, and runtime downloads from catalog-provided URLs with ZIP archive support. ChangesFull-Package Metadata and Cumulative Snapshot-Based Planning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
PR Check ReportSummary
Test Results
Code Quality
Test Artifacts
LinksThis report is auto-generated by GitHub Actions |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
TelegramSearchBot.Common/Model/Update/UpdateCatalog.cs (1)
14-14: ⚡ Quick winConsider making
FullPackageSizenullable for consistency.All other optional full-package metadata properties (lines 10-13) are nullable, but
FullPackageSizedefaults to 0. This creates ambiguity: you cannot distinguish between "size not provided" and "actual 0-byte file". Making itlong?would maintain consistency with the other optional fields and eliminate the ambiguity.♻️ Proposed fix
- public long FullPackageSize { get; init; } + public long? FullPackageSize { get; init; }You'll also need to update the corresponding usage in
Program.csline 580 and the workflow integration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TelegramSearchBot.Common/Model/Update/UpdateCatalog.cs` at line 14, Change the FullPackageSize property to a nullable long (long?) on the UpdateCatalog record to match the other optional full-package metadata fields, then adjust all usages that assume a non-null value (notably the code that reads FullPackageSize in Program.cs and any workflow/serialization code) to handle null explicitly—either by checking HasValue, using a null-coalescing default where appropriate, or altering logic to treat null as "unknown" rather than zero; update any tests or integration points expecting a default 0 accordingly..github/workflows/push.yml (1)
290-290: ⚡ Quick winClarify error messages for anchor download failures.
Lines 290 and 300 say "refusing to publish a cumulative-free update feed", but this wording is misleading. If the anchor download fails, the generated feed will still contain other entries—it simply won't include the cumulative package from this specific anchor version.
Consider rephrasing to: "refusing to publish without cumulative package from anchor {anchorVersion}" for clarity.
♻️ Proposed fix
gh release download $anchorTag --repo ${{ github.repository }} --pattern "TelegramSearchBot-win-x64-full-*.zip" --dir artifacts\anchor-release if ($LASTEXITCODE -ne 0) { - throw "Failed to download cumulative update anchor $anchorTag; refusing to publish a cumulative-free update feed." + throw "Failed to download cumulative update anchor $anchorTag; refusing to publish without cumulative package from anchor." } $zipFile = Get-ChildItem artifacts\anchor-release -Filter "TelegramSearchBot-win-x64-full-*.zip" | Select-Object -First 1 if ($zipFile) { Expand-Archive -Path $zipFile.FullName -DestinationPath .\artifacts\anchor-standalone -Force Add-Content -Path $env:GITHUB_ENV -Value "ANCHOR_VERSION=$anchorVersion" -Encoding utf8 Add-Content -Path $env:GITHUB_ENV -Value "ANCHOR_STANDALONE_DIR=artifacts\anchor-standalone" -Encoding utf8 Write-Host "Extracted cumulative update anchor from $($zipFile.Name)." } else { - throw "No anchor full zip found; refusing to publish a cumulative-free update feed." + throw "No anchor full zip found; refusing to publish without cumulative package from anchor." }Also applies to: 300-300
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/push.yml at line 290, Update the thrown error strings that reference the anchor download failure to make them specific about the missing cumulative package: replace occurrences that say "refusing to publish a cumulative-free update feed." with wording like "refusing to publish without cumulative package from anchor {anchorTag}" (or {anchorVersion} if available) so the message clearly indicates the cumulative package for the given anchor is missing; adjust both the throw at the location referencing anchorTag and the similar throw at the other occurrence to use the new phrasing while keeping the anchor identifier interpolated.TelegramSearchBot.UpdateBuilder/Program.cs (1)
590-590: 💤 Low valueConsider improving the usage help formatting.
The usage string on line 590 is extremely long and difficult to read. Consider breaking it into multiple lines or a bulleted list for better maintainability and user readability.
♻️ Example improved formatting
public static void PrintUsage() { Console.WriteLine("Usage:"); - Console.WriteLine( - " TelegramSearchBot.UpdateBuilder --source-dir <dir> --output-dir <dir> --target-version <version> --min-source-version <version> [--prev-source-dir <dir> --prev-version <version>] [--existing-catalog <path>] [--anchor-version <version> --anchor-source-dir <dir>] [--base-cumulative-package <path>] [--cumulative-source-package-dir <dir>] [--full-package-url <url> --full-package-name <name> --full-package-checksum <sha512> --full-package-size <bytes>] [--updater-url <url>] [--package-base-url <url>]"); + Console.WriteLine(" TelegramSearchBot.UpdateBuilder"); + Console.WriteLine(" --source-dir <dir>"); + Console.WriteLine(" --output-dir <dir>"); + Console.WriteLine(" --target-version <version>"); + Console.WriteLine(" --min-source-version <version>"); + Console.WriteLine(" [--prev-source-dir <dir> --prev-version <version>]"); + Console.WriteLine(" [--existing-catalog <path>]"); + Console.WriteLine(" [--anchor-version <version> --anchor-source-dir <dir>]"); + Console.WriteLine(" [--base-cumulative-package <path>]"); + Console.WriteLine(" [--cumulative-source-package-dir <dir>]"); + Console.WriteLine(" [--full-package-url <url> --full-package-name <name>"); + Console.WriteLine(" --full-package-checksum <sha512> --full-package-size <bytes>]"); + Console.WriteLine(" [--updater-url <url>]"); + Console.WriteLine(" [--package-base-url <url>]"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TelegramSearchBot.UpdateBuilder/Program.cs` at line 590, The long usage string in Program.cs (the literal passed as the help/usage text near the top-level usage message) is hard to read; split it into a multi-line verbatim string or construct it from an array of shorter lines joined with Environment.NewLine (or use StringBuilder) so each option appears on its own line or as a bulleted list; update the code that currently contains the single long literal (the usage string shown in Program.cs) to use the new multi-line format so the help output and source remain readable and maintainable.TelegramSearchBot/Service/Update/SelfUpdateBootstrap.Windows.cs (1)
877-895: 💤 Low valueConsider validating that at least one of
PackageUrlorPackagePathis provided.The current implementation relies on a fallback at line 887 when the filename cannot be derived. While this works, explicitly validating that
entry.PackageUrlorentry.PackagePathis non-empty earlier in the method would make the contract clearer and catch malformed catalog entries sooner.♻️ Optional early validation
private static string GetPackageCacheFileName(UpdateCatalogEntry entry) { + if (string.IsNullOrWhiteSpace(entry.PackageUrl) && string.IsNullOrWhiteSpace(entry.PackagePath)) + { + throw new InvalidDataException("Catalog entry must have either PackageUrl or PackagePath."); + } + var pathOrUrl = string.IsNullOrWhiteSpace(entry.PackageUrl) ? entry.PackagePath : entry.PackageUrl;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@TelegramSearchBot/Service/Update/SelfUpdateBootstrap.Windows.cs` around lines 877 - 895, GetPackageCacheFileName currently proceeds when both UpdateCatalogEntry.PackageUrl and PackagePath are empty and only falls back to a default filename later; add an explicit early validation in GetPackageCacheFileName to check that at least one of entry.PackageUrl or entry.PackagePath is not null/whitespace, and if both are empty throw an ArgumentException (or ArgumentNullException) mentioning the invalid UpdateCatalogEntry so callers get immediate, clear feedback; keep the existing fallback logic for filename derivation and continue to use IsZipPackage to decide default extension if you still need it after validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@TelegramSearchBot.UpdateBuilder/Program.cs`:
- Around line 555-561: The code silently ignores long.TryParse on
fullPackageSizeText, so fullPackageSize can be 0 even when parsing failed;
update the parsing logic around fullPackageSizeText/fullPackageSize and
fullPackageUrl so that you check the TryParse return value and handle failures:
if fullPackageUrl is provided require a successful long.TryParse (using the
returned bool) and either log/throw an error or exit with a clear message,
otherwise handle the missing/invalid size explicitly (e.g., set an optional flag
or default only when intended). Locate the parsing call where
values.TryGetValue("--full-package-size", out var fullPackageSizeText) and the
subsequent _ = long.TryParse(...) and replace with a validated parse that reacts
to failure.
In `@TelegramSearchBot/Service/Update/SelfUpdateBootstrap.Windows.cs`:
- Around line 648-653: The IsZipPackage method risks a NullReferenceException by
calling updateEntry.PackagePath.EndsWith(...) without a null check; update the
logic in IsZipPackage to guard PackagePath (and optionally PackageUrl) with
null/empty checks before calling EndsWith, e.g., check
!string.IsNullOrEmpty(updateEntry.PackagePath) prior to EndsWith and similarly
handle updateEntry.PackageUrl (or use the null-conditional pattern already used
for PackageUrl) so the method safely returns whether the entry is a ZIP by
inspecting PackageFormat, PackagePath, and PackageUrl.
---
Nitpick comments:
In @.github/workflows/push.yml:
- Line 290: Update the thrown error strings that reference the anchor download
failure to make them specific about the missing cumulative package: replace
occurrences that say "refusing to publish a cumulative-free update feed." with
wording like "refusing to publish without cumulative package from anchor
{anchorTag}" (or {anchorVersion} if available) so the message clearly indicates
the cumulative package for the given anchor is missing; adjust both the throw at
the location referencing anchorTag and the similar throw at the other occurrence
to use the new phrasing while keeping the anchor identifier interpolated.
In `@TelegramSearchBot.Common/Model/Update/UpdateCatalog.cs`:
- Line 14: Change the FullPackageSize property to a nullable long (long?) on the
UpdateCatalog record to match the other optional full-package metadata fields,
then adjust all usages that assume a non-null value (notably the code that reads
FullPackageSize in Program.cs and any workflow/serialization code) to handle
null explicitly—either by checking HasValue, using a null-coalescing default
where appropriate, or altering logic to treat null as "unknown" rather than
zero; update any tests or integration points expecting a default 0 accordingly.
In `@TelegramSearchBot.UpdateBuilder/Program.cs`:
- Line 590: The long usage string in Program.cs (the literal passed as the
help/usage text near the top-level usage message) is hard to read; split it into
a multi-line verbatim string or construct it from an array of shorter lines
joined with Environment.NewLine (or use StringBuilder) so each option appears on
its own line or as a bulleted list; update the code that currently contains the
single long literal (the usage string shown in Program.cs) to use the new
multi-line format so the help output and source remain readable and
maintainable.
In `@TelegramSearchBot/Service/Update/SelfUpdateBootstrap.Windows.cs`:
- Around line 877-895: GetPackageCacheFileName currently proceeds when both
UpdateCatalogEntry.PackageUrl and PackagePath are empty and only falls back to a
default filename later; add an explicit early validation in
GetPackageCacheFileName to check that at least one of entry.PackageUrl or
entry.PackagePath is not null/whitespace, and if both are empty throw an
ArgumentException (or ArgumentNullException) mentioning the invalid
UpdateCatalogEntry so callers get immediate, clear feedback; keep the existing
fallback logic for filename derivation and continue to use IsZipPackage to
decide default extension if you still need it after validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77b0a5c6-24a0-467a-b93a-3fd09a866cd0
📒 Files selected for processing (14)
.github/workflows/push.ymlDocs/Build_and_Test_Guide.mdREADME.mdTelegramSearchBot.Common/Model/Update/UpdateCatalog.csTelegramSearchBot.Common/Model/Update/UpdateCatalogEntry.csTelegramSearchBot.Common/Model/Update/UpdateManifest.csTelegramSearchBot.Common/Model/Update/UpdatePackageFormats.csTelegramSearchBot.Test/ModerUpdateIntegrationTests.csTelegramSearchBot.Test/Service/Update/SelfUpdateBootstrapTests.csTelegramSearchBot.Test/Service/Update/UpdateBuilderTests.csTelegramSearchBot.UpdateBuilder/Program.csTelegramSearchBot/Service/Update/SelfUpdateBootstrap.Windows.csexternal/moder-update/src/Moder.Update/Models/UpdateCatalogEntry.csexternal/moder-update/src/Moder.Update/Models/UpdateManifest.cs
Owner
Author
|
Addressed the CodeRabbit findings in e0651ba: validated --full-package-size parsing, guarded zip/package path handling, made full package size nullable, clarified anchor errors, and added regression tests for the parser and zip detection cases. |
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
Tests
Update|FullyQualifiedNameModerUpdateIntegrationTests"Summary by CodeRabbit
New Features
Chores