Add Makefile, Docker, and update GitHub API client #87
Conversation
Introduce a Makefile for Docker and development shortcuts, add VSCode settings for Makefile integration, and update the README with detailed Docker and development instructions. Update the GitHub API client for improved token handling, error management, and code style consistency. Adjust the dev script in package.json to use the correct Next.js CLI flag and host binding.
WalkthroughAdds a Makefile and VS Code setting for Makefile handling, expands README with Docker and local setup instructions, updates the Next.js dev script, and significantly refactors the GitHub API client types, token handling/validation, and related error/logging behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant App as App (frontend/backend)
participant Client as GitHubAPIClient
participant Store as TokenStore
rect rgba(180,230,255,0.4)
Dev->>App: provide token (UI / env)
App->>Client: setUserToken(token)
Client->>Client: validate token by type (regex)
alt token valid
Client->>Store: persist token (memory/env)
Client-->>App: success
else invalid
Client-->>App: return validation error
end
end
rect rgba(220,255,200,0.4)
App->>Client: getTokenInfo()
Client->>Store: read token metadata
Client-->>App: tokenInfo { tokenPrefix, source, ... }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
206-213: Clarify auth model (PAT vs OAuth) and scopesUsage suggests PAT (“Generate new token (classic)”), while “OAuth scopes” section lists minimal OAuth scopes. Please clarify which flows are supported (OAuth via NextAuth vs PAT entry) and align scope guidance.
Also applies to: 109-115
🧹 Nitpick comments (7)
Makefile (3)
1-1: Declare alias targets as PHONYAdd aliases to .PHONY to silence linters and avoid name clashes.
-.PHONY: help setup env docker-dev docker-dev-build docker-dev-logs docker-dev-down docker-prod docker-up docker-down docker-logs docker-restart clean-docker +.PHONY: help setup env docker-dev docker-dev-build docker-dev-logs docker-dev-down docker-prod docker-up docker-down docker-logs docker-restart clean-docker \ + dev prod up down logs restart
44-51: Guard for missing .env.exampleFail fast with a clear message if the template is absent.
env: - @if [ ! -f .env.local ]; then \ + @if [ ! -f .env.local ]; then \ + if [ ! -f .env.example ]; then \ + echo "❌ .env.example not found. Please add it or run the standard setup."; \ + exit 1; \ + fi; \ cp .env.example .env.local; \ echo "✅ Created .env.local - Please edit it with your credentials"; \ else \ echo "⚠️ .env.local already exists"; \ fi
93-99: Cleaning is very aggressive
docker system prune -fremoves unused resources globally. Consider a safer default and an explicit “all” variant.clean-docker: - @echo "🧹 Removing all containers and volumes..." - $(DOCKER_COMPOSE) down -v - $(DOCKER_COMPOSE_DEV) down -v - docker system prune -f - @echo "✅ Docker cleanup complete" + @echo "🧹 Removing project containers and volumes..." + $(DOCKER_COMPOSE) down -v || true + $(DOCKER_COMPOSE_DEV) down -v || true + @echo "✅ Docker cleanup complete" + +.PHONY: clean-docker-all +clean-docker-all: + @echo "🧹 Pruning unused Docker resources (GLOBAL)..." + docker system prune -fREADME.md (1)
231-246: Use headings instead of bold for “Option …”Replace bold “Option 1/2/3” with
#### Option 1/2/3to satisfy MD036 and improve structure.src/lib/api/github-api-client.ts (3)
181-182: Align hasValidToken with minimum length
setUserTokenenforces min 40;hasValidTokenuses 20. Make them consistent.- return this.githubToken.length >= 20; + return this.githubToken.length >= 40;
7-11: Don’t re‑declare shared typesUse the existing
GitHubSearchResponsefrom@/types/apito avoid drift.-import type { TrendingRepo, TopContributor } from "@/types/oss-insight"; +import type { TrendingRepo, TopContributor } from "@/types/oss-insight"; +import type { GitHubSearchResponse } from "@/types/api"; @@ -interface GitHubSearchResponse<T> { - items: T[]; - total_count: number; - incomplete_results: boolean; -} +// (Use shared GitHubSearchResponse from src/types/api.ts)
1239-1243: clearUserCache misses repo-scoped keysAlso clear
/repos/<username>/...entries to avoid stale commit/PR caches.- const keysToDelete = Array.from(this.cache.keys()).filter((key) => - key.includes(`/users/${username}`) - ); + const keysToDelete = Array.from(this.cache.keys()).filter( + (key) => + key.includes(`/users/${username}`) || key.includes(`/repos/${username}/`) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.vscode/settings.json(1 hunks)Makefile(1 hunks)README.md(4 hunks)package.json(1 hunks)src/lib/api/github-api-client.ts(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api/github-api-client.ts (4)
src/types/api.ts (1)
GitHubSearchResponse(126-130)src/lib/api/github-graphql-client.ts (1)
query(157-197)src/types/oss-insight.ts (2)
TrendingRepo(1-36)TopContributor(69-87)src/types/github.ts (2)
GitHubUserDetailed(73-106)GitHubRepositoryDetailed(108-195)
🪛 Biome (2.1.2)
src/lib/api/github-api-client.ts
[error] 110-111: Expected a statement but instead found '<<<<<<< Updated upstream'.
Expected a statement here.
(parse)
[error] 113-114: Expected a statement but instead found '======='.
Expected a statement here.
(parse)
[error] 120-122: Expected a statement but instead found '>>>>>>> Stashed changes
this.githubToken = process.env.GITHUB_TOKEN'.
Expected a statement here.
(parse)
[error] 126-126: expected , but instead found :
Remove :
(parse)
[error] 126-126: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 180-180: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 184-184: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 189-189: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 191-192: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '<<'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 192-192: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 192-192: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 192-192: expected > but instead found upstream
Remove upstream
(parse)
[error] 194-194: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 195-208: Expected a statement but instead found '=======
tokenPrefix: this.githubToken
? this.githubToken.substring(0, 10) + "..."
: "NO_TOKEN",
source:
typeof window === "undefined" &&
typeof process !== "undefined" &&
process.env?.GITHUB_TOKEN &&
this.githubToken === process.env.GITHUB_TOKEN
? "ENV_VAR"
: this.githubToken
? "USER_SET"
: "NONE",'.
Expected a statement here.
(parse)
[error] 209-210: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '>'.
Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.
(parse)
[error] 210-210: expected a semicolon to end the class property, but found none
(parse)
[error] 214-214: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 214-214: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 216-216: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 217-217: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 218-218: Expected a statement but instead found '>'.
Expected a statement here.
(parse)
[error] 220-220: Illegal return statement outside of a function
(parse)
[error] 228-228: Illegal return statement outside of a function
(parse)
[error] 230-233: Illegal return statement outside of a function
(parse)
[error] 237-237: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 237-237: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 237-237: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 238-238: expected , but instead found :
Remove :
(parse)
[error] 239-239: expected , but instead found useGithub
Remove useGithub
(parse)
[error] 241-241: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 247-247: Illegal return statement outside of a function
(parse)
[error] 289-290: Illegal return statement outside of a function
(parse)
[error] 298-302: Expected a statement but instead found '<<<<<<< Updated upstream
// Check rate limit status
// Rate limit headers available but not currently used
======='.
Expected a statement here.
(parse)
[error] 331-333: Expected a statement but instead found '>>>>>>> Stashed changes
this.cache.set(cacheKey,'.
Expected a statement here.
(parse)
[error] 333-333: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 334-334: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 334-335: Illegal return statement outside of a function
(parse)
[error] 337-339: Illegal return statement outside of a function
(parse)
[error] 345-346: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 346-347: expected , but instead found :
Remove :
(parse)
[error] 347-347: expected , but instead found sort
Remove sort
(parse)
[error] 347-347: expected , but instead found :
Remove :
(parse)
[error] 348-349: expected , but instead found limit
Remove limit
(parse)
[error] 349-349: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 361-386: Illegal return statement outside of a function
(parse)
[error] 387-389: Illegal return statement outside of a function
(parse)
[error] 391-392: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 392-393: expected , but instead found :
Remove :
(parse)
[error] 393-393: expected , but instead found type
Remove type
(parse)
[error] 393-393: expected , but instead found :
Remove :
(parse)
[error] 394-395: expected , but instead found limit
Remove limit
(parse)
[error] 395-395: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 409-424: Illegal return statement outside of a function
(parse)
[error] 181-181: The constructor should not return a value.
The constructor is here:
Returning a value from a constructor may confuse users of the class.
(lint/correctness/noConstructorReturn)
[error] 190-192: The constructor should not return a value.
The constructor is here:
Returning a value from a constructor may confuse users of the class.
(lint/correctness/noConstructorReturn)
[error] 434-436: Illegal return statement outside of a function
(parse)
[error] 444-461: Illegal return statement outside of a function
(parse)
[error] 463-465: Illegal return statement outside of a function
(parse)
[error] 467-467: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 467-467: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 467-467: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 469-472: Illegal return statement outside of a function
(parse)
[error] 522-526: Illegal return statement outside of a function
(parse)
[error] 527-529: Illegal return statement outside of a function
(parse)
[error] 531-531: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 531-531: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 531-531: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 533-535: Illegal return statement outside of a function
(parse)
[error] 543-559: Illegal return statement outside of a function
(parse)
[error] 561-563: Illegal return statement outside of a function
(parse)
[error] 565-565: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 565-565: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 565-565: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 569-570: Illegal return statement outside of a function
(parse)
[error] 579-595: Illegal return statement outside of a function
(parse)
[error] 598-601: Illegal return statement outside of a function
(parse)
[error] 602-602: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 603-603: Expected an expression, or an assignment but instead found ':'.
Expected an expression, or an assignment here.
(parse)
[error] 604-604: expected , but instead found :
Remove :
(parse)
[error] 604-604: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 607-609: Illegal return statement outside of a function
(parse)
[error] 621-648: Illegal return statement outside of a function
(parse)
[error] 669-671: Illegal return statement outside of a function
(parse)
[error] 740-741: Illegal return statement outside of a function
(parse)
[error] 743-746: Illegal return statement outside of a function
(parse)
[error] 746-746: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 746-746: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 747-750: Illegal return statement outside of a function
(parse)
[error] 750-750: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 750-750: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 751-758: Illegal return statement outside of a function
(parse)
[error] 758-758: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 758-759: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 759-759: expected , but instead found :
Remove :
(parse)
[error] 760-760: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 774-776: Illegal return statement outside of a function
(parse)
[error] 783-784: Illegal return statement outside of a function
(parse)
[error] 791-793: Illegal return statement outside of a function
(parse)
[error] 795-796: Illegal return statement outside of a function
(parse)
[error] 797-800: Illegal return statement outside of a function
(parse)
[error] 801-803: Illegal return statement outside of a function
(parse)
[error] 805-805: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 805-805: expected , but instead found :
Remove :
(parse)
[error] 805-805: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 808-809: Illegal return statement outside of a function
(parse)
[error] 811-814: Illegal return statement outside of a function
(parse)
[error] 814-815: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 816-816: expected , but instead found :
Remove :
(parse)
[error] 817-817: expected , but instead found limit
Remove limit
(parse)
[error] 817-817: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 827-827: Illegal return statement outside of a function
(parse)
[error] 829-830: Illegal return statement outside of a function
(parse)
[error] 832-835: Illegal return statement outside of a function
(parse)
[error] 835-836: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 837-837: expected , but instead found :
Remove :
(parse)
[error] 837-837: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 839-839: Expected a statement but instead found '>>'.
Expected a statement here.
(parse)
[error] 843-845: Illegal return statement outside of a function
(parse)
[error] 857-861: Illegal return statement outside of a function
(parse)
[error] 863-866: Illegal return statement outside of a function
(parse)
[error] 866-866: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 866-866: expected , but instead found :
Remove :
(parse)
[error] 867-867: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 882-883: Expected a statement but instead found '| null>'.
Expected a statement here.
(parse)
[error] 909-912: Illegal return statement outside of a function
(parse)
[error] 915-920: Illegal return statement outside of a function
(parse)
[error] 924-924: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 924-924: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 924-925: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 926-926: expected , but instead found :
Remove :
(parse)
[error] 926-926: expected , but instead found repos
Remove repos
(parse)
[error] 926-926: expected , but instead found :
Remove :
(parse)
[error] 927-927: expected , but instead found [
Remove [
(parse)
[error] 928-928: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 930-930: Expected a statement but instead found '>
'.
Expected a statement here.
(parse)
[error] 972-972: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 972-972: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 972-973: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 974-974: expected , but instead found :
Remove :
(parse)
[error] 974-975: expected , but instead found repoName
Remove repoName
(parse)
[error] 975-975: expected , but instead found :
Remove :
(parse)
[error] 975-975: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 1041-1043: Illegal return statement outside of a function
(parse)
[error] 1045-1047: Illegal return statement outside of a function
(parse)
[error] 1048-1048: Illegal return statement outside of a function
(parse)
[error] 1050-1052: Illegal return statement outside of a function
(parse)
[error] 1052-1052: Illegal use of reserved keyword private as an identifier in strict mode
(parse)
[error] 1052-1052: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 1052-1053: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 1054-1054: expected , but instead found :
Remove :
(parse)
[error] 1055-1055: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 1057-1058: Expected a statement but instead found '>
'.
Expected a statement here.
(parse)
[error] 1077-1088: Illegal return statement outside of a function
(parse)
[error] 1239-1239: expected , but instead found :
Remove :
(parse)
[error] 1239-1239: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 1246-1246: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🪛 checkmake (0.2.2)
Makefile
[warning] 8-8: Target body for "help" exceeds allowed length of 5 (28).
(maxbodylength)
[warning] 44-44: Target body for "env" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
[warning] 101-101: Target "dev" should be declared PHONY.
(phonydeclared)
[warning] 102-102: Target "prod" should be declared PHONY.
(phonydeclared)
[warning] 103-103: Target "up" should be declared PHONY.
(phonydeclared)
[warning] 104-104: Target "down" should be declared PHONY.
(phonydeclared)
[warning] 105-105: Target "logs" should be declared PHONY.
(phonydeclared)
[warning] 106-106: Target "restart" should be declared PHONY.
(phonydeclared)
🪛 LanguageTool
README.md
[style] ~216-~216: It’s more common nowadays to write this noun as one word.
Context: ...ely ## Settings - Change organization/user name and token anytime from the Settings pag...
(RECOMMENDED_COMPOUNDS)
🪛 markdownlint-cli2 (0.18.1)
README.md
237-237: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (3)
.vscode/settings.json (1)
1-3: LGTMWorkspace setting aligns with the new Makefile flow.
README.md (1)
202-203: “Quick aliases” note: ensure Makefile matchesAliases listed here require PHONY declarations and targets in Makefile (dev, prod, up, down, logs, restart). Current Makefile defines them; keep docs and Makefile in lockstep.
package.json (1)
6-6: Verification confirmed:-Hflag is supportedThe
-Halias is fully supported in your Next.js version and works correctly with the hostname parameter. The code is acceptable as written.
| # Environment Setup | ||
| setup: env | ||
| @echo "🔧 Setting up Docker environment..." | ||
| @echo "✅ Setup complete! Edit .env.local with your credentials" | ||
| @echo "Then run: make docker-dev" | ||
|
|
There was a problem hiding this comment.
“setup” help text vs behavior
Help says “env + build” but target only creates .env.local. Either call a build target or adjust the message.
-setup: env
- @echo "🔧 Setting up Docker environment..."
- @echo "✅ Setup complete! Edit .env.local with your credentials"
- @echo "Then run: make docker-dev"
+setup: env docker-dev-build
+ @echo "🔧 Setting up Docker environment..."
+ @echo "✅ Setup complete! Edit .env.local with your credentials"
+ @echo "Then run: make docker-dev"Or keep setup: env and change the echo to avoid implying a build.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Makefile around lines 38 to 43, the "setup" target's help text implies it
does "env + build" but currently only depends on the env target (creating
.env.local); either update the target to run both env and build (e.g., change
dependency to "env build" or add a build invocation) or change the echoed
messages to accurately reflect behavior (remove or reword the "env + build"
implication and keep instructions like "Edit .env.local then run: make
docker-dev"); make the change so the help text and actual target behavior match.
| "Invalid GitHub token format. Expected:\n" + | ||
| "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | ||
| "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" + | ||
| "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | ||
| "- OAuth token: 20-255 character alphanumeric string\n" + | ||
| "- Legacy token: 40 character hexadecimal string" | ||
| ); |
There was a problem hiding this comment.
Token error message and regex lengths disagree
Message says FG token is “94 chars” but regex expects {82}. Remove hardcoded counts to avoid drift.
- "Invalid GitHub token format. Expected:\n" +
- "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" +
- "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" +
- "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" +
- "- OAuth token: 20-255 character alphanumeric string\n" +
- "- Legacy token: 40 character hexadecimal string"
+ "Invalid GitHub token format. Expected one of:\n" +
+ "- Classic token: ghp_...\n" +
+ "- Fine-grained token: github_pat_...\n" +
+ "- GitHub App token: ghs_...\n" +
+ "- OAuth token: gho_... (or a 20–255 char token)\n" +
+ "- Legacy hex token (40 chars)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Invalid GitHub token format. Expected:\n" + | |
| "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | |
| "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" + | |
| "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + | |
| "- OAuth token: 20-255 character alphanumeric string\n" + | |
| "- Legacy token: 40 character hexadecimal string" | |
| ); | |
| "Invalid GitHub token format. Expected one of:\n" + | |
| "- Classic token: ghp_...\n" + | |
| "- Fine-grained token: github_pat_...\n" + | |
| "- GitHub App token: ghs_...\n" + | |
| "- OAuth token: gho_... (or a 20–255 char token)\n" + | |
| "- Legacy hex token (40 chars)" | |
| ); |
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 156 to 162, the error message
lists hardcoded character counts for token types (e.g., "94 chars") that
disagree with the regex used (which expects 82 for fine-grained tokens) and can
drift; update the message to remove exact numeric lengths and instead describe
token formats generically (e.g., mention prefixes and general length/character
constraints) or programmatically derive the length from the regex if exact
counts are required so the message always matches validation.
| const cacheKey = endpoint; | ||
| const cached = this.cache.get(cacheKey); | ||
| const timeout = isCommitData ? this.commitCacheTimeout : this.cacheTimeout; | ||
|
|
||
| if (cached && Date.now() - cached.timestamp < timeout) { |
There was a problem hiding this comment.
Cache key ignores token — can mix data across users
Include a token component for authenticated requests to avoid cross-user cache leaks.
- const cacheKey = endpoint;
+ const keyTokenPart =
+ useGithub && this.githubToken ? `#t:${this.githubToken.slice(0, 8)}` : "";
+ const cacheKey = `${endpoint}${keyTokenPart}`;
const cached = this.cache.get(cacheKey);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cacheKey = endpoint; | |
| const cached = this.cache.get(cacheKey); | |
| const timeout = isCommitData ? this.commitCacheTimeout : this.cacheTimeout; | |
| if (cached && Date.now() - cached.timestamp < timeout) { | |
| const keyTokenPart = | |
| useGithub && this.githubToken ? `#t:${this.githubToken.slice(0, 8)}` : ""; | |
| const cacheKey = `${endpoint}${keyTokenPart}`; | |
| const cached = this.cache.get(cacheKey); | |
| const timeout = isCommitData ? this.commitCacheTimeout : this.cacheTimeout; | |
| if (cached && Date.now() - cached.timestamp < timeout) { |
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 242 to 246, the cache key is
currently just the endpoint which can leak cached responses across different
authenticated users; update the cacheKey to include an identifier for the auth
token used for the request (e.g. append a token-derived component) so
authenticated requests are keyed per-user. If you must avoid storing raw tokens,
compute a short stable hash (or masked prefix) of the token and append it to the
endpoint (fall back to a public marker when no token). Ensure all cache.get/set
and eviction use the same composed key.
| if (overview.length === 0) { | ||
| console.warn('No repositories found, using demo overview'); | ||
| console.warn("No repositories found, using demo overview"); | ||
| return this.getUserAnalytics(username); | ||
| } | ||
There was a problem hiding this comment.
Infinite recursion when no repos found
return this.getUserAnalytics(username) loops forever. Return a safe fallback instead.
- if (overview.length === 0) {
- console.warn("No repositories found, using demo overview");
- return this.getUserAnalytics(username);
- }
+ if (overview.length === 0) {
+ console.warn("No repositories found, returning empty overview");
+ const behavior = await this.getWeeklyBehaviorData(username);
+ return { profile, overview: [], languages, behavior };
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 907 to 911, the code currently
does `return this.getUserAnalytics(username)` when no repositories are found,
which causes infinite recursion; replace that recursive call with a safe
fallback value instead (for example return a predefined demo overview/analytics
object, an empty analytics response, or null) and ensure any callers can handle
the fallback; remove the recursive call and construct or call a non-recursive
demo/fallback provider (e.g., `getDemoAnalytics()` or a literal empty analytics
object) so the method returns a concrete safe value rather than recursing.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/api/github-api-client.ts (1)
965-1034: Commit count fallback never runs when stats returns emptyYou only fall back to commits listing in catch; if /stats returns (often empty/202), you return 0. Run the fallback when stats is empty or user not found.
- try { - const stats = await this.fetchWithCache< - Array<{ author: { login: string }; total: number }> - >(statsEndpoint, true, true); - - if (stats && Array.isArray(stats)) { - const userStats = stats.find( - (stat) => stat.author?.login === username - ); - if (userStats && userStats.total > 0) { - return userStats.total; - } - } - } catch { - const oneYearAgo = new Date(); - oneYearAgo.setFullYear(oneYearAgo.getFullYear() - 1); + let shouldFallback = true; + try { + const stats = await this.fetchWithCache< + Array<{ author: { login: string }; total: number }> + >(statsEndpoint, true, true); + if (stats && Array.isArray(stats)) { + const userStats = stats.find((s) => s.author?.login === username); + if (userStats && userStats.total > 0) { + return userStats.total; + } + } + // stats present but empty/zero ⇒ fallback + shouldFallback = true; + } catch { + // request failed/202 ⇒ fallback + shouldFallback = true; + } + if (shouldFallback) { + const oneYearAgo = new Date(); + oneYearAgo.setFullYear(oneYearAgo.getFullYear() - 1); const endpoint = `/repos/${username}/${repoName}/commits?author=${username}&since=${oneYearAgo.toISOString()}&per_page=100`; let totalCommits = 0; let page = 1; const maxPages = 3; while (page <= maxPages) { const commits = await this.fetchWithCache<GitHubCommitResponse[]>( `${endpoint}&page=${page}`, true, true ); if (!commits || commits.length === 0) { break; } totalCommits += commits.length; if (commits.length < 100) { break; } page++; await new Promise((resolve) => setTimeout(resolve, 150)); } if (totalCommits === 0) { try { const sixMonthsAgo = new Date(); sixMonthsAgo.setMonth(sixMonthsAgo.getMonth() - 6); const broadEndpoint = `/repos/${username}/${repoName}/commits?since=${sixMonthsAgo.toISOString()}&per_page=30`; const recentCommits = await this.fetchWithCache< GitHubCommitResponse[] >(broadEndpoint, true, true); if (recentCommits && Array.isArray(recentCommits)) { const userCommits = recentCommits.filter( (commit) => commit.author?.login === username || commit.commit?.author?.name ?.toLowerCase() .includes(username.toLowerCase()) ); return userCommits.length; } } catch {} } return totalCommits; - } - return 0; + }
♻️ Duplicate comments (3)
src/lib/api/github-api-client.ts (3)
151-157: Token error message drifts from regex; remove hardcoded lengthsMessage claims different lengths (e.g., FG “94 chars”) than the regex ({82}). Describe formats generically to avoid drift.
- throw new Error( - "Invalid GitHub token format. Expected:\n" + - "- Classic token: ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + - "- Fine-grained token: github_pat_xxxxxxxxxx... (94 chars)\n" + - "- GitHub App token: ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (40 chars)\n" + - "- OAuth token: 20-255 character alphanumeric string\n" + - "- Legacy token: 40 character hexadecimal string" - ); + throw new Error( + "Invalid GitHub token format. Expected one of:\n" + + "- Classic PAT: ghp_...\n" + + "- Fine‑grained PAT: github_pat_...\n" + + "- GitHub App token: ghs_...\n" + + "- OAuth token: gho_... (or a 20–255 char token)\n" + + "- Legacy hex token" + );If you need exact lengths, derive them from the regex at runtime to keep message and validation in sync.
231-233: Cache key ignores token — cross‑user cache leak riskKeying only by endpoint mixes authenticated responses across users and between authed/anon states (e.g., /user, search visibility). Include a token component.
- const cacheKey = endpoint; + const keyTokenPart = + useGithub && this.githubToken ? `#t:${this.githubToken.slice(0, 8)}` : ""; + const cacheKey = `${endpoint}${keyTokenPart}`;Note: prefer a short hash if feasible; masked prefix is acceptable if hashing isn’t available in both runtimes.
892-895: Infinite recursion when no repos foundCalling getUserAnalytics() again loops forever. Return a safe fallback instead.
- if (overview.length === 0) { - console.warn("No repositories found, using demo overview"); - return this.getUserAnalytics(username); - } + if (overview.length === 0) { + console.warn("No repositories found, returning empty overview"); + const behavior = await this.getWeeklyBehaviorData(username); + return { profile, overview: [], languages, behavior }; + }
🧹 Nitpick comments (6)
src/lib/api/github-api-client.ts (6)
112-118: Constructor env read: add non‑null assertion for TS strictnessprocess.env.GITHUB_TOKEN is typed as string | undefined; assert non‑null after your guard.
- this.githubToken = process.env.GITHUB_TOKEN; + this.githubToken = process.env.GITHUB_TOKEN!;
175-177: Align hasValidToken with validation logicLength check (>= 20) can report “valid” for tokens failing your regexes.
- hasValidToken(): boolean { - return this.githubToken.length >= 20; - } + hasValidToken(): boolean { + return !!this.githubToken; + }Alternatively, reuse the same regex predicates used in setUserToken.
489-505: Mixed id types in mentions/review itemsUsing number ids for mentions and string ids like
review-123for reviews yields heterogeneous arrays and brittle consumers.Unify on a single id type (prefer string) and type these methods (e.g., ActionItem[]) instead of unknown[].
Verify downstream code expectations before changing; I can generate a small migration diff once confirmed.
905-907: Method signature says “| null” but you rethrowEither return null on failure or drop “| null” in the signature.
- } catch (_error) { - throw _error; - } + } catch (_error) { + console.error("getUserAnalytics failed:", _error); + return null; + }
1045-1047: Remove unused variableoneWeekAgo is computed but not used here.
- const oneWeekAgo = new Date(); - oneWeekAgo.setDate(oneWeekAgo.getDate() - 7);
1226-1230: User cache cleanup misses /repos and search keysAnalytics populates endpoints under /repos/{user}/… and search queries with author/mentions/review‑requested. Expand the eviction patterns.
- const keysToDelete = Array.from(this.cache.keys()).filter((key) => - key.includes(`/users/${username}`) - ); + const patterns = [ + `/users/${username}`, + `/repos/${username}/`, + `author:${username}`, + `assignee:${username}`, + `mentions:${username}`, + `review-requested:${username}`, + ]; + const keysToDelete = Array.from(this.cache.keys()).filter((key) => + patterns.some((p) => key.includes(p)) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/api/github-api-client.ts(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/api/github-api-client.ts (4)
src/types/api.ts (1)
GitHubSearchResponse(126-130)src/lib/api/github-graphql-client.ts (1)
query(157-199)src/types/oss-insight.ts (2)
TrendingRepo(1-36)TopContributor(69-87)src/types/github.ts (2)
GitHubUserDetailed(73-106)GitHubRepositoryDetailed(108-195)
🔇 Additional comments (2)
src/lib/api/github-api-client.ts (2)
287-316: Rate‑limit propagation to UI looks goodClient‑side extraction and updateRateLimit(headers) mirror the GraphQL client.
1233-1233: I have enough information to provide a comprehensive rewritten review. The key findings:
- setUserToken is never called in the codebase (no call sites found)
- Cache is shared across requests using endpoint-only keys, which creates a data leak risk in SSR
- Client-side imports exist but are mitigated by the window check in constructor
- Server routes only use env token, not setUserToken
Cache isolation missing in shared singleton – data leak across requests
The module-level
githubAPIClientsingleton shares aMap-based cache keyed only by endpoint (line ~192:const cacheKey = endpoint). In a server-side rendering scenario with concurrent requests, responses from one user's API calls can be served to other users via the shared cache.Current mitigations:
setUserTokenis unused in the codebase (no call sites)- Server routes use only
process.env.GITHUB_TOKENfrom constructor- Window check prevents client-side token exposure
Remaining concerns:
- Cache design (endpoint-only keys) creates data leak potential if user-scoped tokens or per-request isolation is added later
- Module-level singleton re-instantiated in SSR context
Recommendations:
- Include request or session ID in cache keys to isolate responses per user/session
- Or: Move cache outside the singleton (e.g.,
WeakMapkeyed by request context)- Document that cache is shared and only safe for public GitHub data accessed with a single token
| const headers: HeadersInit = { | ||
| 'Accept': 'application/vnd.github.v3+json', | ||
| 'User-Agent': 'GitHubMon/1.0' | ||
| } | ||
| Accept: "application/vnd.github.v3+json", | ||
| "User-Agent": "GitHubMon/1.0", | ||
| }; | ||
|
|
||
| // Use token if available, but don't fail if not | ||
| if (useGithub && this.githubToken) { | ||
| headers['Authorization'] = `Bearer ${this.githubToken}` | ||
| headers["Authorization"] = `Bearer ${this.githubToken}`; | ||
| } |
There was a problem hiding this comment.
Headers typing and User‑Agent in browser
- HeadersInit + index assignment is unsafe (union not indexable).
- User‑Agent is forbidden in browsers; set only on server to avoid errors.
- const headers: HeadersInit = {
- Accept: "application/vnd.github.v3+json",
- "User-Agent": "GitHubMon/1.0",
- };
+ const headers: Record<string, string> = {
+ Accept: "application/vnd.github.v3+json",
+ };
+ if (typeof window === "undefined") {
+ headers["User-Agent"] = "GitHubMon/1.0";
+ }| const endpoint = `/users/${username}`; | ||
| return await this.fetchWithCache(endpoint, true); | ||
| } catch (error) { |
There was a problem hiding this comment.
Missing generic on fetchWithCache — likely TS error
Inferencing won’t resolve T here; specify the expected type.
- return await this.fetchWithCache(endpoint, true);
+ return await this.fetchWithCache<GitHubUserDetailed>(endpoint, true);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const endpoint = `/users/${username}`; | |
| return await this.fetchWithCache(endpoint, true); | |
| } catch (error) { | |
| const endpoint = `/users/${username}`; | |
| return await this.fetchWithCache<GitHubUserDetailed>(endpoint, true); | |
| } catch (error) { |
🤖 Prompt for AI Agents
In src/lib/api/github-api-client.ts around lines 792 to 794, the call to
this.fetchWithCache(endpoint, true) is missing an explicit generic type
parameter causing TypeScript to not infer T; update the call to provide the
expected response type (for example this.fetchWithCache<GitHubUser>(endpoint,
true)) and ensure the GitHubUser (or whichever interface/type matches the
/users/:username response) is imported or defined; if the project uses a
different name for the user DTO use that type instead so the fetchWithCache
generic is explicitly set.
2 participants
Summary by CodeRabbit
Documentation
Chores
Refactor