fix(reviewers): add uptime context and review root settings#7
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR SummaryMedium Risk Overview Settings → Code Review (and Reviewer Agents UI now accepts remediation targets with optional Agent system prompts and Local-review tools and reviewer-agent flows use the new validation and persist resolved repo paths in artifacts. Reviewed by Cursor Bugbot for commit 72baec8. Configure here. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7c2a1939-3e76-4e29-984e-b78cd773bb37) |
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
📝 WalkthroughWalkthroughAdds repository containment enforcement for local code review: repo paths are resolved to real paths and validated against the workspace or a configurable ChangesRepository Containment for Local Review
Server Runtime Context
ReviewerAgentsPage: optional role field
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
Code Review
This pull request introduces repository containment security for local code review tools, restricting reviews to the workspace or explicitly configured allowed repository roots. It adds a new settings page, backend routes, and validation logic to manage these roots, alongside a new server runtime context tracking process uptime. The review feedback highlights three key issues: a bug where the arguments to isPathInsideOrEqual are reversed when validating the Git top-level directory, a potential crash if any configured repository root does not exist on the filesystem, and a potential TypeError in the settings API route if a literal null request body is received.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/client/pages/reviewer-agents/ReviewerAgentsPage.test.tsx (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the tsconfig alias for this import.
This relative import breaks the repo’s TS/TSX import rule. Switch it to the explicit alias path for
ReviewerAgentsPage.As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use absolute imports with tsconfig aliases and avoid index barrels in deep folders; prefer explicit imports.🤖 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/client/pages/reviewer-agents/ReviewerAgentsPage.test.tsx` at line 2, The import in ReviewerAgentsPage.test.tsx uses a relative path instead of the required tsconfig alias, which violates the repo’s TS/TSX import rule. Update the import of hasAgentsResponse to use the explicit alias path for ReviewerAgentsPage rather than a local relative reference, keeping the rest of the test file unchanged.Source: Coding guidelines
🤖 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/client/pages/reviewer-agents/ReviewerAgentsPage.tsx`:
- Line 665: The list item key in ReviewerAgentsPage is not guaranteed unique
because target.agentSlug ?? target.role ?? target.label can collide for
role-based targets. Update the key logic in the target rendering block to use a
composite of stable fields from the target object instead of a single fallback,
so each item in the list is uniquely identified and avoids reconciliation
issues.
In `@src/client/pages/settings/CodeReviewSettings.tsx`:
- Around line 1-10: The CodeReviewSettings page is hardcoding user-facing copy
instead of using locale keys, so non-English settings UIs will stay partially
English. Update the component to use the useTranslation() hook and replace all
visible text in CodeReviewSettings with translation lookups, then add the
missing keys to en.json and keep the same key parity across the supported locale
files. Use the existing settings.codeReview.title key as the pattern and extend
it for the rest of the section’s labels, buttons, alerts, placeholders, and
status text.
- Around line 82-116: `handleSave` and `handleReset` can run concurrently,
causing a race where the later response may overwrite the admin’s intended
state. Add a guard in `CodeReviewSettings` so either action is ignored while the
other is in flight, or consolidate `saving`/`resetting` into a single shared
busy state that both handlers check before calling `api.put` or `api.delete`.
Also disable the Save and Reset controls using that shared in-flight state so
users cannot trigger overlapping requests.
In `@src/server/config.test.ts`:
- Around line 278-280: The test for HIVEKEEP_CODE_REVIEW_ALLOWED_ROOTS is using
a hard-coded colon delimiter, but the parsing logic in loadConfigWithEnv follows
path.delimiter. Update the fixture in config.test.ts to build the path-delimited
portion with delimiter from node:path instead of assuming ":" so the assertion
stays portable across platforms.
In `@src/server/services/local-review.ts`:
- Line 646: The repo path validation currently resolves input.repoPath against
the server process cwd, which causes relative paths to point to the wrong
location when workspaceRoot is provided. Update the path resolution in the
local-review flow so validateReviewRepoPathEffective receives repoPath resolved
relative to input.workspaceRoot when present (and only falls back to
process.cwd() when workspaceRoot is absent), keeping the change localized around
the repoPath assignment in local-review.ts.
In `@src/server/services/reviewer-agents.ts`:
- Around line 210-211: listReviewerAgents is resolving a relative repoPath
against the server cwd instead of the provided workspaceRoot, so adjust the
initial path normalization to use workspaceRoot as the base when repoPath is not
absolute. Update the logic in listReviewerAgents to resolve repoPath relative to
workspaceRoot before calling validateReviewRepoPathEffective, while preserving
the existing defaults and behavior for absolute paths.
In `@src/server/services/server-runtime.ts`:
- Around line 1-25: The runtime context is using wall-clock time for uptime,
which can drift if the system clock changes. Update getServerRuntimeContext in
server-runtime.ts to keep startedAt/startedAtIso based on SERVER_STARTED_AT, but
compute uptimeMs from a monotonic source like process.uptime() or
performance.now(), and make the same monotonic change in server/app.ts wherever
uptime is reported. Reuse the existing
getServerRuntimeContext/ServerRuntimeContext symbols so the health/info/prompt
context stays consistent.
---
Nitpick comments:
In `@src/client/pages/reviewer-agents/ReviewerAgentsPage.test.tsx`:
- Line 2: The import in ReviewerAgentsPage.test.tsx uses a relative path instead
of the required tsconfig alias, which violates the repo’s TS/TSX import rule.
Update the import of hasAgentsResponse to use the explicit alias path for
ReviewerAgentsPage rather than a local relative reference, keeping the rest of
the test file unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27377218-ac49-4872-811e-5eddc325525a
📒 Files selected for processing (30)
docs/local-review-agents.mdsrc/client/locales/de.jsonsrc/client/locales/en.jsonsrc/client/locales/es.jsonsrc/client/locales/fr.jsonsrc/client/locales/it.jsonsrc/client/locales/ja.jsonsrc/client/locales/pl.jsonsrc/client/locales/pt-BR.jsonsrc/client/locales/ru.jsonsrc/client/locales/zh-CN.jsonsrc/client/pages/reviewer-agents/ReviewerAgentsPage.test.tsxsrc/client/pages/reviewer-agents/ReviewerAgentsPage.tsxsrc/client/pages/settings/CodeReviewSettings.tsxsrc/client/pages/settings/SettingsPage.tsxsrc/server/app.tssrc/server/config.test.tssrc/server/config.tssrc/server/routes/settings.test.tssrc/server/routes/settings.tssrc/server/services/app-settings.tssrc/server/services/local-review.test.tssrc/server/services/local-review.tssrc/server/services/prompt-builder.test.tssrc/server/services/prompt-builder.tssrc/server/services/reviewer-agents.test.tssrc/server/services/reviewer-agents.tssrc/server/services/server-runtime.tssrc/server/tools/code-review-tools.tssrc/test-helpers.ts
| const envFileLine = env?.envFilePath | ||
| ? `\nConfig file: ${env.envFilePath}` | ||
| : '' | ||
| const runtime = getServerRuntimeContext(now.getTime()) |
There was a problem hiding this comment.
CRITICAL: Call-site not updated after the server-runtime signature change — uptime parameter now expects uptimeMs (pre-computed), not wall-clock time.
server-runtime.ts:17 was changed in this PR from getServerRuntimeContext(now = Date.now()) (the parameter was wall-clock time and the function computed now - SERVER_STARTED_AT) to getServerRuntimeContext(uptimeMs = process.uptime() * 1000) (the parameter is interpreted as already-computed uptime in ms). The companion fix in app.ts and the runtime helper itself uses the parameterless default, but this call site still passes now.getTime() — a Date.now()-style ms-since-epoch value (≈1.78e12 in 2026). That value passes Math.max(0, Math.floor(uptimeMs)), then is divided by 1000 to produce uptimeSeconds ≈1.78e9 (≈56 years), and formatDuration(runtime.uptimeMs) renders e.g. "20600d 0h". The "Server process uptime" line in every Agent's system prompt will therefore report a multi-decade value that grows by decades every second the host is up, instead of resetting when Hivekeep restarts.
The PR's own test (prompt-builder.test.ts:149-156) only asserts the substrings Server process uptime: and resets when Hivekeep restarts are present, not a plausible uptime value, so this regression is not caught by CI.
| const runtime = getServerRuntimeContext(now.getTime()) | |
| const runtime = getServerRuntimeContext() |
Reply with @kilo-code-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (29 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 104.4K · Output: 17.5K · Cached: 1.3M |
The getServerRuntimeContext() signature changed to accept pre-computed uptime in ms (defaulting to process.uptime() * 1000), but the call site in buildContextBlock() was still passing now.getTime() — a Date.now() epoch value (~1.78e12 ms). This caused the system prompt to report a multi-decade server uptime (e.g. "20600d 0h") instead of the actual process uptime. Fix: call getServerRuntimeContext() with no arguments to use the default. Also add a regression test that rejects epoch-scale uptime values. Addresses kilo-code-bot review comment on PR #7.
Summary
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Documentation