Backend sprint 1.8.3 hygiene: /v1/events 204, signing-seeds TTL, 404 audit, auth-poll diagnostics, jsdelivr mirror#14
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
rainxchzed has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR updates the GitHub Store API surface by adding a new root greeting endpoint, refactoring centralized 404 handling with request logging, deprecating legacy OAuth/device-flow paths with 410 Gone responses, disabling event telemetry collection, adding mirror traffic-kind metadata, implementing conditional caching for signing seeds via ETags, enhancing device-flow poll diagnostics with structured telemetry, and making the device client testable through constructor injection of OAuth credentials. ChangesAPI Route Updates and Deprecations
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/zed/rainxch/githubstore/mirrors/Mirror.kt (1)
55-105: ⚡ Quick winConsider extracting traffic-kind string literals to constants.
The strings
"release_asset"and"raw_file"appear as literals in multiple locations (here, line 127, and test assertions). Extracting them toconst valdeclarations would prevent typos and clarify the contract.♻️ Proposed refactor
object MirrorPresets { private const val PROBE_ASSET = "https://github.com/cli/cli/releases/download/v2.40.0/gh_2.40.0_checksums.txt" + const val TRAFFIC_KIND_RELEASE_ASSET = "release_asset" + const val TRAFFIC_KIND_RAW_FILE = "raw_file" + - private val FULL_PROXY_KINDS = listOf("release_asset", "raw_file") + private val FULL_PROXY_KINDS = listOf(TRAFFIC_KIND_RELEASE_ASSET, TRAFFIC_KIND_RAW_FILE)Then update line 127 and test assertions to reference these constants.
🤖 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 `@src/main/kotlin/zed/rainxch/githubstore/mirrors/Mirror.kt` around lines 55 - 105, Extract the repeated traffic-kind string literals into constants (e.g. add const val RELEASE_ASSET = "release_asset" and const val RAW_FILE = "raw_file") and replace the literal usages in FULL_PROXY_KINDS with these constants; update any MirrorPreset initializations that currently reference the string literals and adjust test assertions to import/reference RELEASE_ASSET and RAW_FILE instead of hard-coded strings so all places (including the FULL_PROXY_KINDS list and tests) use the shared constants.
🤖 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 `@src/main/kotlin/zed/rainxch/githubstore/Plugins.kt`:
- Around line 339-340: Remove or narrow the global StatusPages 404 handler in
Plugins.kt (the status(HttpStatusCode.NotFound) block) because it overrides
route-level 404 bodies; either delete that handler and rely on route code
calling the centralized respondNotFound(call) helper, or refactor all explicit
404 responses in InternalRoutes (the handlers returning mapOf("error" to "Not
found")), BadgeRoutes (handlers returning "unknown kind: ..."), and
RepoRefreshRoutes (already using ApiError) to call respondNotFound(call) instead
so formats stay consistent; alternatively, if you want a global handler, change
it to only catch unmatched routes (if Ktor supports it) and leave route-level
not-found responses untouched.
---
Nitpick comments:
In `@src/main/kotlin/zed/rainxch/githubstore/mirrors/Mirror.kt`:
- Around line 55-105: Extract the repeated traffic-kind string literals into
constants (e.g. add const val RELEASE_ASSET = "release_asset" and const val
RAW_FILE = "raw_file") and replace the literal usages in FULL_PROXY_KINDS with
these constants; update any MirrorPreset initializations that currently
reference the string literals and adjust test assertions to import/reference
RELEASE_ASSET and RAW_FILE instead of hard-coded strings so all places
(including the FULL_PROXY_KINDS list and tests) use the shared constants.
🪄 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: aef8d1da-a66d-4ad7-ba02-810256b8af73
📒 Files selected for processing (17)
CLAUDE.mdsrc/main/kotlin/zed/rainxch/githubstore/Plugins.ktsrc/main/kotlin/zed/rainxch/githubstore/ingest/GitHubDeviceClient.ktsrc/main/kotlin/zed/rainxch/githubstore/mirrors/Mirror.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/AuthRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/DeprecationRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/EventRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/MirrorRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/RootRoutes.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/Routing.ktsrc/main/kotlin/zed/rainxch/githubstore/routes/SigningSeedsRoutes.ktsrc/test/kotlin/zed/rainxch/githubstore/match/SigningSeedsRouteTest.ktsrc/test/kotlin/zed/rainxch/githubstore/mirrors/MirrorRoutesTest.ktsrc/test/kotlin/zed/rainxch/githubstore/routes/AuthPollDiagnosticsTest.ktsrc/test/kotlin/zed/rainxch/githubstore/routes/DeprecationRoutesTest.ktsrc/test/kotlin/zed/rainxch/githubstore/routes/EventRoutesTest.ktsrc/test/kotlin/zed/rainxch/githubstore/routes/RootRoutesTest.kt
There was a problem hiding this comment.
rainxchzed has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your 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.
Summary
Tier A + B tasks from the backend sprint brief. 8 logically-atomic commits, each independently revertable.
Changes
Cloudflare analytics → expected impact (30d window)
/v1/events180k 4xx → 204 (no body, ~0 origin cost per call)/v1/signing-seeds1.15M requests + low hit rate → 86400/604800 TTL with ETag, expect Hit ratio >90% within 7d/v1/repo/login/{device,oauth}(~110/week), root greeting for/(~54/week), edge-cachednot_foundJSON for the rest[auth-poll rid=… ] dch=<sha256-prefix> ghs=<upstream-status> gh_err=<github-code> lat_ms=<ms> ua=<UA>— no token, no raw device_codeMirror task — operator note
feat/mirrors-add-fastgitwas the original branch with both mirrors. After CodeRabbit's trust concern about fastgit.cc, verification turned up no public artifact tying the .cc TLD to the FastGitORG team (their domains are .org + .xyz; .org is officially shut down). Dropped from the list. Whatsnew bullet at OpenHub-Store/GitHub-Store#599 was amended accordingly.New
traffic_kindsfield on every mirror entry (additive; pre-1.8.3 clients ignore it). 1.8.3+ clients MUST consult this list before routing — jsDelivr is["raw_file"]-only (no release assets under /gh/), whole-URL proxies stay["release_asset", "raw_file"].Test plan
/v1/signing-seedshit ratio rises within 7dhttps://api.github-store.org/and confirm greeting JSONhttps://api.github-store.org/v1/repo/login/deviceand confirm 410 withuse_insteadhintCoordination
TelemetryRepositoryImpl.postEvents) still calls /v1/events. 204 stops the spam; 1.8.3 client follow-up should add a sticky-disable-on-non-2xx flag, after which this endpoint can flip back to 410 Gone for a real signalIf-None-Matchtraffic_kindsbefore 1.8.3 release or jsdelivr will 404 release-asset URLsSummary by CodeRabbit
New Features
Deprecated
Improvements
Tests