Feature/ai code reviewer#70
Conversation
…publish permissions
UI/UX: - GitHub-style settings layout with clean sidebar navigation - Responsive design across all pages (mobile-first) - Profile page redesigned to match GitHub's layout - Header navbar cleaned up with proper active states - Settings pages (profile, account, security) rewritten - API documentation page with 100+ endpoint categories - Trending repos section responsive on mobile - Hero section stats stack on small screens Documentation: - Complete docs-site sidebar with 70+ pages - Configuration reference with all 165 env vars - Architecture page updated with accurate system design - Installation guide with correct ports and secrets - AI review docs updated with 10+ providers - Quick start and first-repo guides rewritten - Local dev setup with all scripts - Database schema docs for 38 tables - Contributing guide with project structure Security (Critical Fixes): - Git HTTP routes now require authentication - RCE via new Function() replaced with safe evaluator - CSRF bypass via fake Bearer header fixed - PR number race condition fixed with atomic SQL - PR title/body input validation added - Password strength enforced on registration - process.env leak to workflows prevented - Docker stream parsing fixed for multi-frame - require() in ESM modules converted to import - Command injection via execSync fixed (execFileSync) - Path traversal in git-storage rejected - SSH rate limiter memory leak fixed (max 10K entries) SSH/Git: - SSH server reads GIT_SSH_PORT, GIT_REPOS_PATH, GIT_SSH_HOST_KEY - Admin permissions passed via SSH - Docker Compose SSH service added - Pkt-line length calculation fixed Deployment: - render.yaml Blueprint for Render (Singapore region) - Dockerfile.worker for background jobs - FREE-DEPLOYMENT.md guide (Oracle, Fly.io, etc.) - RENDER-DEPLOYMENT.md step-by-step guide - Upstash Redis + Render PostgreSQL free tier setup
# Conflicts: # package.json
- Fix count() usage in index.astro with proper type cast - Fix schema.activity -> schema.activities in profile page - Fix HTMLElement.disabled cast in security settings - Add missing docs to docs-site for parity check - All CI checks should now pass
- Regenerate bun.lock to sync with package.json (fixes frozen lockfile CI error) - Fix ThemeToggle.tsx type narrowing error (activeTheme: string)
📝 WalkthroughWalkthroughThe pull request adds an explicit theme value type, updates theme end-to-end tests, adds deployment documentation titles, changes documentation accent colors, adjusts test mocks and assertions, updates a full-flow password, inserts null bytes into a test file, and refreshes a cron lock record. ChangesRepository updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/ThemeToggle.tsx`:
- Around line 22-26: Resolve the conflict in ThemeToggle by removing the
conflict markers and retaining a single activeTheme declaration, preferably the
explicitly typed string version. Preserve the existing system-theme selection
behavior so the file remains valid TypeScript.
🪄 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: d68493fb-ab3b-4c3a-aa49-a30149c92071
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
src/components/ThemeToggle.tsx
| <<<<<<< HEAD | ||
| const activeTheme: string = theme === "system" ? (isSystemDark ? "dark" : "light") : theme; | ||
| ======= | ||
| const activeTheme = theme === "system" ? (isSystemDark ? "dark" : "light") : theme; | ||
| >>>>>>> origin/main |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Resolve the merge conflict before merging.
The unresolved conflict markers make ThemeToggle.tsx invalid TypeScript and cause the build to fail. Keep one activeTheme declaration—preferably the explicitly typed version—and remove the markers.
-<<<<<<< HEAD
const activeTheme: string = theme === "system" ? (isSystemDark ? "dark" : "light") : theme;
-=======
-const activeTheme = theme === "system" ? (isSystemDark ? "dark" : "light") : theme;
->>>>>> origin/main📝 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.
| <<<<<<< HEAD | |
| const activeTheme: string = theme === "system" ? (isSystemDark ? "dark" : "light") : theme; | |
| ======= | |
| const activeTheme = theme === "system" ? (isSystemDark ? "dark" : "light") : theme; | |
| >>>>>>> origin/main | |
| const activeTheme: string = theme === "system" ? (isSystemDark ? "dark" : "light") : theme; |
🧰 Tools
🪛 Biome (2.5.3)
[error] 22-22: Expected a statement but instead found '<<<<<<< HEAD'.
(parse)
[error] 24-24: Expected a statement but instead found '======='.
(parse)
[error] 26-26: Expected a statement but instead found '>>>>>>> origin/main'.
(parse)
🤖 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/components/ThemeToggle.tsx` around lines 22 - 26, Resolve the conflict in
ThemeToggle by removing the conflict markers and retaining a single activeTheme
declaration, preferably the explicitly typed string version. Preserve the
existing system-theme selection behavior so the file remains valid TypeScript.
Source: Linters/SAST tools
- Fix markdown test to account for syntax highlighting spans - Mock validatePasswordStrength in auth register test - Add pipelineRunners to admin-stats test schema mock
- Darken docs accent color (#2dd4bf -> #0d9488) for WCAG AA contrast - Fix theme toggle test to click menu item instead of just button - Fix full-flow test password to meet strength requirements - Add missing frontmatter to deployment docs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/e2e/theme-toggle.spec.ts (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hardcoded timeouts and conditional interactions in tests.
Both instances bypass Playwright's built-in auto-waiting mechanism.
click()automatically waits for the target element to become visible and actionable. UsingwaitForTimeoutfollowed by an immediate, non-waitingisVisible()check is an anti-pattern that can cause tests to flake or silently skip assertions.
tests/e2e/theme-toggle.spec.ts#L25-L34: RemovewaitForTimeoutand theisVisibleconditional, allowing Playwright to auto-wait for thelightOptionclick.tests/e2e/theme-toggle.spec.ts#L53-L62: Apply the same refactoring to rely on auto-waiting.🤖 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 `@tests/e2e/theme-toggle.spec.ts` around lines 25 - 34, Remove the hardcoded waitForTimeout calls and isVisible conditionals around the Light option interactions in tests/e2e/theme-toggle.spec.ts at lines 25-34 and 53-62; click each lightOption directly so Playwright’s built-in auto-waiting handles readiness.
🤖 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 @.mimocode/.cron-lock:
- Line 1: Remove the tracked .mimocode/.cron-lock file from the repository, and
ensure runtime cron-lock generation remains responsible for creating it; exclude
this generated lock artifact from source control so stale PID and timestamp
state cannot be committed.
In `@tests/e2e/full-flow.spec.ts`:
- Line 57: Remove the unexpected null-byte characters from the affected line in
the test file, leaving the surrounding test content unchanged so the file parses
and the suite can execute.
---
Nitpick comments:
In `@tests/e2e/theme-toggle.spec.ts`:
- Around line 25-34: Remove the hardcoded waitForTimeout calls and isVisible
conditionals around the Light option interactions in
tests/e2e/theme-toggle.spec.ts at lines 25-34 and 53-62; click each lightOption
directly so Playwright’s built-in auto-waiting handles readiness.
🪄 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: d79bb737-1e6d-4945-a7b2-d08be425523a
📒 Files selected for processing (10)
.mimocode/.cron-lockdocs-site/src/content/docs/FREE-DEPLOYMENT.mddocs-site/src/content/docs/RENDER-DEPLOYMENT.mddocs-site/src/custom.csssrc/components/ThemeToggle.tsxtests/e2e/full-flow.spec.tstests/e2e/theme-toggle.spec.tstests/integration/admin-stats-route.test.tstests/integration/auth-register-route.test.tstests/unit/markdown.test.ts
💤 Files with no reviewable changes (1)
- src/components/ThemeToggle.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/e2e/theme-toggle.spec.ts (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hardcoded timeouts and conditional interactions in tests.
Both instances bypass Playwright's built-in auto-waiting mechanism.
click()automatically waits for the target element to become visible and actionable. UsingwaitForTimeoutfollowed by an immediate, non-waitingisVisible()check is an anti-pattern that can cause tests to flake or silently skip assertions.
tests/e2e/theme-toggle.spec.ts#L25-L34: RemovewaitForTimeoutand theisVisibleconditional, allowing Playwright to auto-wait for thelightOptionclick.tests/e2e/theme-toggle.spec.ts#L53-L62: Apply the same refactoring to rely on auto-waiting.🤖 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 `@tests/e2e/theme-toggle.spec.ts` around lines 25 - 34, Remove the hardcoded waitForTimeout calls and isVisible conditionals around the Light option interactions in tests/e2e/theme-toggle.spec.ts at lines 25-34 and 53-62; click each lightOption directly so Playwright’s built-in auto-waiting handles readiness.
🤖 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 @.mimocode/.cron-lock:
- Line 1: Remove the tracked .mimocode/.cron-lock file from the repository, and
ensure runtime cron-lock generation remains responsible for creating it; exclude
this generated lock artifact from source control so stale PID and timestamp
state cannot be committed.
In `@tests/e2e/full-flow.spec.ts`:
- Line 57: Remove the unexpected null-byte characters from the affected line in
the test file, leaving the surrounding test content unchanged so the file parses
and the suite can execute.
---
Nitpick comments:
In `@tests/e2e/theme-toggle.spec.ts`:
- Around line 25-34: Remove the hardcoded waitForTimeout calls and isVisible
conditionals around the Light option interactions in
tests/e2e/theme-toggle.spec.ts at lines 25-34 and 53-62; click each lightOption
directly so Playwright’s built-in auto-waiting handles readiness.
🪄 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: d79bb737-1e6d-4945-a7b2-d08be425523a
📒 Files selected for processing (10)
.mimocode/.cron-lockdocs-site/src/content/docs/FREE-DEPLOYMENT.mddocs-site/src/content/docs/RENDER-DEPLOYMENT.mddocs-site/src/custom.csssrc/components/ThemeToggle.tsxtests/e2e/full-flow.spec.tstests/e2e/theme-toggle.spec.tstests/integration/admin-stats-route.test.tstests/integration/auth-register-route.test.tstests/unit/markdown.test.ts
💤 Files with no reviewable changes (1)
- src/components/ThemeToggle.tsx
🛑 Comments failed to post (2)
.mimocode/.cron-lock (1)
1-1: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -n -C3 'cron-lock|startedAt|"pid"|\.mimocode/.cron-lock' .Repository: swadhinbiswas/OpencodeHub
Length of output: 34898
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'Tracked .mimocode files:\n' git ls-files '.mimocode/*' || true printf '\nTree under .mimocode (if present):\n' find .mimocode -maxdepth 2 -type f 2>/dev/null | sort || true printf '\nReferences to .cron-lock or cron-lock helpers:\n' rg -n --hidden --glob '!**/.git/**' '\.cron-lock|cron-lock' . || true printf '\nIgnore rules mentioning .mimocode or cron lock:\n' rg -n --hidden --glob '!**/.git/**' 'mimocode|cron-lock' .gitignore **/.gitignore || trueRepository: swadhinbiswas/OpencodeHub
Length of output: 363
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'Files mentioning mimocode:\n' rg -n --hidden --glob '!**/.git/**' 'mimocode' . || true printf '\nTop-level and .mimocode tree details:\n' find . -maxdepth 2 \( -path './.git' -o -path './node_modules' -o -path './dist' -o -path './build' \) -prune -o -type f | sort | sed -n '1,200p' printf '\nContents of .mimocode/.cron-lock:\n' cat -n .mimocode/.cron-lockRepository: swadhinbiswas/OpencodeHub
Length of output: 3668
Remove the committed cron lock file
.mimocode/.cron-lockis tracked and contains a PID/timestamp, so it will go stale as soon as that process exits. If any cron lock check reads it as live state, this can block scheduled work or misidentify ownership after PID reuse. Generate it at runtime or keep it out of source control.🤖 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 @.mimocode/.cron-lock at line 1, Remove the tracked .mimocode/.cron-lock file from the repository, and ensure runtime cron-lock generation remains responsible for creating it; exclude this generated lock artifact from source control so stale PID and timestamp state cannot be committed.tests/e2e/full-flow.spec.ts (1)
57-57: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove unexpected null-byte characters.
This line contains unexpected null-byte characters (``) which will cause a parse error, breaking the test suite execution. As indicated by static analysis tools, these invalid characters must be removed.
🐛 Proposed fix
- +📝 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.🧰 Tools
🪛 Biome (2.5.3)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
[error] 57-57: unexpected token
�(parse)
🤖 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 `@tests/e2e/full-flow.spec.ts` at line 57, Remove the unexpected null-byte characters from the affected line in the test file, leaving the surrounding test content unchanged so the file parses and the suite can execute.Source: Linters/SAST tools
- Darken docs accent color (#2dd4bf -> #0d9488) for WCAG AA contrast - Fix theme toggle test to click menu item instead of just button - Fix full-flow test password to meet strength requirements
Summary by CodeRabbit
Greptile Summary
This PR resolves the previously-flagged merge conflict in
ThemeToggle.tsx(choosing the: stringtype annotation) and brings several test fixes into sync with the current production code: a newpipelineRunnersschema field in the admin-stats mock, avalidatePasswordStrengthmock in the register route test, and updated E2E assertions that interact with the reworked theme dropdown. New dependencies (rehype-highlight,highlight.js,astro-icon) are also locked in to support syntax highlighting and file-icon features.ThemeToggle.tsx: Conflict resolved with a one-character type annotation change; no functional difference to theme switching logic.markdown.test.tsassertion loosened to accommodaterehype-highlightHTML token wrapping.bun.lock/docs-site: Lock-file updated with new icon/highlighting packages; docs pages gain required Starlight front-mattertitlefield and a refreshed accent palette.Confidence Score: 5/5
Safe to merge — the only production-code change is a one-character type annotation in ThemeToggle.tsx; all other changes are test fixes and documentation.
The production surface touched is minimal: a single explicit
: stringtype annotation with no runtime effect. The test changes fix stale mocks and bring assertions into alignment with current behaviour; two assertions are slightly looser than ideal but do not hide real failures. Docs and lock-file changes carry no risk.tests/e2e/theme-toggle.spec.ts — the
initialStoragevariable is captured but dropped from the assertion, leaving the change-detection check weaker than before.Important Files Changed
: stringtype annotation onactiveTheme; no functional change to theme logic.initialStorageis now captured but unused — the assertion weakened from 'theme changed' to 'storage is non-null'.rehype-highlighttoken wrapping; single-character"x"substring check is very broad.pipelineRunnersfield to fake schema mock to match current route implementation.validatePasswordStrengthmock to match updated route implementation.astro-icon,highlight.js,rehype-highlight, and iconify icon-set packages for syntax highlighting and file icons.Reviews (6): Last reviewed commit: "fix: resolve E2E test failures" | Re-trigger Greptile