cheapnud commented

Owner

WithVelopackUpdates gains an accessToken overload passed through to the Gitea/GitHub source, so private repositories can serve auto-updates. Null keeps the feed anonymous. Version 3.4.0.

cheapnud added 1 commit

2026-08-22 15:14:42 +00:00

Support private update feeds with an access token

Claude PR Review / AI Code Review (pull_request) Failing after 11m57s

Details

Build & Test / Build and Test (pull_request) Failing after 11m58s

Details

904bd7b6e9

cheapnud force-pushed f/private-update-feeds from 904bd7b6e9

Claude PR Review / AI Code Review (pull_request) Failing after 11m57s

Details

Build & Test / Build and Test (pull_request) Failing after 11m58s

Details

to b8435e77c7

Claude PR Review / AI Code Review (pull_request) Successful in 31s

Details

Build & Test / Build and Test (pull_request) Failing after 2m27s

Details

2026-08-22 15:36:36 +00:00

Compare

forgejo-actions commented

2026-08-22 15:37:08 +00:00

🤖 Claude AI Code Review

Code Review: Support private update feeds with an access token

1. Summary

This PR adds an accessToken overload to WithVelopackUpdates, threading a token through to the GithubSource/GiteaSource so private repositories can serve auto-updates. It also includes an unrelated but valid CI fix (reverting the SDK from .NET 11-preview back to .NET 10) and bumps the version to 3.4.0. The changes are small, focused, and well-documented.

2. Code Quality

Strengths:

Clean use of overloading — the original WithVelopackUpdates(repoUrl, autoCheck) delegates to the new overload, avoiding duplication.
The string.IsNullOrWhiteSpace(accessToken) ? null : accessToken normalization is a nice touch — it defends against empty/whitespace tokens producing malformed auth requests.
Excellent documentation discipline: XML docs, features.md, and release notes all consistently emphasize the "token is a secret" guidance.
Host-based source detection (retained) and the comments explaining why are good.

Minor observations:

The delegating overload passes autoCheck positionally: WithVelopackUpdates(repoUrl, accessToken: null, autoCheck). This works but mixing named and positional args reads slightly oddly. Consider WithVelopackUpdates(repoUrl, accessToken: null, autoCheck: autoCheck) for clarity.

3. Potential Issues

Overload resolution ambiguity (worth verifying):
Both overloads have autoCheck with a default value:

WithVelopackUpdates(string repoUrl, bool autoCheck = true)
WithVelopackUpdates(string repoUrl, string? accessToken, bool autoCheck = true)

A call like WithVelopackUpdates("url", false) still resolves unambiguously to the bool overload (better match than string?), so existing call sites are safe. But WithVelopackUpdates("url", null) would now bind to the new overload — this is the intended behavior, just flag it as a subtle behavioral surface. No breaking change for existing callers.

Token in memory / logging (low risk, worth a glance):

The token is stored on _options.UpdateAccessToken for the process lifetime. Confirm CheapAvaloniaBlazorOptions is not serialized anywhere (e.g., diagnostic dumps, logging of options objects) that would leak the token. If options are ever logged, the token should be redacted.
Confirm the URL/token isn't logged in UpdateService error paths (not visible in this diff, but the catch block referenced at line 104 should be checked).

Documentation accuracy:
The old doc line "Private update feeds are not supported yet" is correctly removed. Good — no stale claims left behind.

CI change scope:
The .forgejo/workflows/build.yml SDK revert is unrelated to the feature. It's a legitimate fix with a clear explanatory comment, but bundling an infra fix into a feature PR slightly muddies the changelog. Not blocking.

4. Recommendations

Verify options aren't logged/serialized — add [JsonIgnore] or similar if UpdateAccessToken could ever end up in serialized diagnostics, and consider overriding ToString() redaction if the options object is logged.
Consider a Func<string?>/callback for token retrieval (future enhancement, not blocking) — private feed tokens sometimes expire. A pull-based accessor would let consumers refresh tokens without rebuilding the host. For now the static string is fine given Velopack's API shape.
Minor readability: use autoCheck: autoCheck in the delegating call.
Optional: split the CI SDK fix into its own commit/PR for a cleaner history (or just note it explicitly in the PR description, which it currently isn't).
Sanity check that GithubSource/GiteaSource treat a null token identically to the previous behavior — since the third arg (false) is preserved, this should be a pure superset. Worth a quick confirmation against the Velopack version in use.

5. Verdict

APPROVE (with minor suggestions)

This is a well-scoped, well-documented change that maintains backward compatibility. The token normalization and clear secret-handling guidance show good attention to detail. The only follow-ups are defensive checks around whether the options object could leak the token via logging/serialization — worth confirming before merge, but not blocking given the token is only passed to the Velopack source constructor in the visible code path.

📊 Tokens: 4196 input + 1597 output | 💰 Cost: ~$0.0609
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review # Code Review: Support private update feeds with an access token ## 1. Summary This PR adds an `accessToken` overload to `WithVelopackUpdates`, threading a token through to the `GithubSource`/`GiteaSource` so private repositories can serve auto-updates. It also includes an unrelated but valid CI fix (reverting the SDK from .NET 11-preview back to .NET 10) and bumps the version to 3.4.0. The changes are small, focused, and well-documented. ## 2. Code Quality **Strengths:** - Clean use of overloading — the original `WithVelopackUpdates(repoUrl, autoCheck)` delegates to the new overload, avoiding duplication. - The `string.IsNullOrWhiteSpace(accessToken) ? null : accessToken` normalization is a nice touch — it defends against empty/whitespace tokens producing malformed auth requests. - Excellent documentation discipline: XML docs, `features.md`, and release notes all consistently emphasize the "token is a secret" guidance. - Host-based source detection (retained) and the comments explaining *why* are good. **Minor observations:** - The delegating overload passes `autoCheck` positionally: `WithVelopackUpdates(repoUrl, accessToken: null, autoCheck)`. This works but mixing named and positional args reads slightly oddly. Consider `WithVelopackUpdates(repoUrl, accessToken: null, autoCheck: autoCheck)` for clarity. ## 3. Potential Issues **Overload resolution ambiguity (worth verifying):** Both overloads have `autoCheck` with a default value: ```csharp WithVelopackUpdates(string repoUrl, bool autoCheck = true) WithVelopackUpdates(string repoUrl, string? accessToken, bool autoCheck = true) ``` A call like `WithVelopackUpdates("url", false)` still resolves unambiguously to the `bool` overload (better match than `string?`), so existing call sites are safe. But `WithVelopackUpdates("url", null)` would now bind to the new overload — this is the *intended* behavior, just flag it as a subtle behavioral surface. No breaking change for existing callers. **Token in memory / logging (low risk, worth a glance):** - The token is stored on `_options.UpdateAccessToken` for the process lifetime. Confirm `CheapAvaloniaBlazorOptions` is not serialized anywhere (e.g., diagnostic dumps, logging of options objects) that would leak the token. If options are ever logged, the token should be redacted. - Confirm the URL/token isn't logged in `UpdateService` error paths (not visible in this diff, but the `catch` block referenced at line 104 should be checked). **Documentation accuracy:** The old doc line "*Private update feeds are not supported yet*" is correctly removed. Good — no stale claims left behind. **CI change scope:** The `.forgejo/workflows/build.yml` SDK revert is unrelated to the feature. It's a legitimate fix with a clear explanatory comment, but bundling an infra fix into a feature PR slightly muddies the changelog. Not blocking. ## 4. Recommendations 1. **Verify options aren't logged/serialized** — add `[JsonIgnore]` or similar if `UpdateAccessToken` could ever end up in serialized diagnostics, and consider overriding `ToString()` redaction if the options object is logged. 2. **Consider a `Func<string?>`/callback for token retrieval** (future enhancement, not blocking) — private feed tokens sometimes expire. A pull-based accessor would let consumers refresh tokens without rebuilding the host. For now the static string is fine given Velopack's API shape. 3. **Minor readability**: use `autoCheck: autoCheck` in the delegating call. 4. **Optional**: split the CI SDK fix into its own commit/PR for a cleaner history (or just note it explicitly in the PR description, which it currently isn't). 5. **Sanity check** that `GithubSource`/`GiteaSource` treat a `null` token identically to the previous behavior — since the third arg (`false`) is preserved, this should be a pure superset. Worth a quick confirmation against the Velopack version in use. ## 5. Verdict **APPROVE** (with minor suggestions) This is a well-scoped, well-documented change that maintains backward compatibility. The token normalization and clear secret-handling guidance show good attention to detail. The only follow-ups are defensive checks around whether the options object could leak the token via logging/serialization — worth confirming before merge, but not blocking given the token is only passed to the Velopack source constructor in the visible code path. --- 📊 **Tokens**: 4196 input + 1597 output | 💰 **Cost**: ~$0.0609 *Generated by Claude Opus 4.8 via Gitea Actions*

cheapnud added 1 commit

2026-08-22 15:42:17 +00:00

Install the .NET 10 SDK the projects actually target

Claude PR Review / AI Code Review (pull_request) Successful in 30s

Details

Build & Test / Build and Test (pull_request) Successful in 2m59s

Details

b2cb014c97

cheapnud merged commit 09f945f733 into master

2026-08-22 15:45:45 +00:00

cheapnud deleted branch f/private-update-feeds

2026-08-22 15:45:46 +00:00