Conversation
github-actions
Bot
added
the
maintainer:e2e:ok
Contributor
|
Warning Review limit reached
More reviews will be available in 3 minutes and 51 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughRemoves all inline ESLint disable directives from ChangesEliminate inline ESLint directives in packages/storage (
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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 `@packages/storage/src/conversation/ConversationFileWriter.test.ts`:
- Around line 261-294: The ConversationFileWriter tests only override HOME, but
the writer resolves its default location via os.homedir(), so Windows CI can
still use USERPROFILE and miss the temp directory. Update the two tests in
ConversationFileWriter.test.ts to save, override, and restore USERPROFILE
alongside HOME before creating ConversationFileWriter, ensuring the default-path
assertions always point to the temp home.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 09758990-29ee-4a62-a034-a1ff9097dfa9
📒 Files selected for processing (6)
eslint.config.jspackages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.tspackages/storage/src/utils/gitIgnoreParser.tsscripts/tests/eslint-guard.test.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run LLxprt review
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-06-10T18:18:09.253Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 1983
File: packages/policy/src/policy-engine.ts:263-263
Timestamp: 2026-06-10T18:18:09.253Z
Learning: In this repository, the ESLint rule `sonarjs/too-many-break-or-continue-in-loop` is configured to allow at most 1 `break`/`continue` per loop (it is stricter than the SonarJS default). During code review, treat `// eslint-disable-next-line sonarjs/too-many-break-or-continue-in-loop` on loops with 2+ `break`/`continue` as intentional and do not suggest removing or changing those directives. Only consider a change if the rule is violated without an appropriate intentional disable.
Applied to files:
scripts/tests/eslint-guard.test.jseslint.config.jspackages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/utils/gitIgnoreParser.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.ts
📚 Learning: 2026-03-16T20:36:45.254Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 1733
File: eslint.config.js:0-0
Timestamp: 2026-03-16T20:36:45.254Z
Learning: In vybestack/llxprt-code, the global baseline for no-console is intentionally set to 'warn' across core/cli packages. Do not flag this in reviews or suggest removing it. Entry points or CLI-specific configurations may override this to 'off' in a later pass when the rule is tightened to 'error'. Treat this as a deliberate, repository-wide baseline, and only flag console usage if there is an explicit deviation from the stated intent or an automation gate indicates improper override.
Applied to files:
eslint.config.js
📚 Learning: 2026-02-06T15:52:42.315Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 1305
File: scripts/generate-keybindings-doc.ts:1-5
Timestamp: 2026-02-06T15:52:42.315Z
Learning: In reviews of vybestack/llxprt-code, do not suggest changing existing copyright headers from 'Google LLC' to 'Vybestack LLC' for files that originated from upstream. Preserve upstream copyrights in files that came from upstream, and only apply 'Vybestack LLC' copyright on newly created, original LLxprt files. If a file is clearly LLxprt-original, it may carry the Vybestack header; if it is upstream-originated, keep the original sponsor header.
Applied to files:
packages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/utils/gitIgnoreParser.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.ts
📚 Learning: 2026-03-31T02:12:43.093Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 1854
File: packages/core/src/core/subagentRuntimeSetup.test.ts:77-84
Timestamp: 2026-03-31T02:12:43.093Z
Learning: In this codebase, tool declarations should follow the single required contract `parametersJsonSchema`; do not ask to preserve or reintroduce the legacy `parameters` fallback field. Reviewers should not flag assertions/checks for missing `parameters` or suggest backward-compatibility behavior for `parameters`. Schema converters/providers are expected to error if `parametersJsonSchema` is absent instead of falling back to `parameters`.
Applied to files:
packages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/utils/gitIgnoreParser.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.ts
📚 Learning: 2026-06-10T18:18:08.545Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 1983
File: packages/policy/src/policy-engine.ts:156-156
Timestamp: 2026-06-10T18:18:08.545Z
Learning: In this repo, ESLint rule `sonarjs/too-many-break-or-continue-in-loop` is set to fail loops that contain more than 1 `break`/`continue` total per loop (or both present). When a loop violates this (e.g., it contains a `break` and a `continue`, or has multiple `break`s/`continue`s), the code will not lint unless the violating line includes `// eslint-disable-next-line sonarjs/too-many-break-or-continue-in-loop`. In code reviews, do not suggest removing these `eslint-disable-next-line` directives (use refactoring only if it eliminates the underlying >1 break/continue pattern).
Applied to files:
packages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/utils/gitIgnoreParser.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.ts
📚 Learning: 2026-06-19T17:16:56.523Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 2108
File: packages/agents/src/api/agentImpl.ts:1047-1079
Timestamp: 2026-06-19T17:16:56.523Z
Learning: When the fake-provider test seam is active in vybestack/llxprt-code, `process.env.LLXPRT_FAKE_RESPONSES` is set to a fixture file path ending in a `.jsonl` (not to the string `'1'` or any other boolean-like value). In code, detect the seam by checking `process.env.LLXPRT_FAKE_RESPONSES !== undefined` (and/or that it is a non-empty string), rather than using `process.env.LLXPRT_FAKE_RESPONSES === '1'`. Update any callers of the env var accordingly (see `packages/providers/src/composition/providerManagerInstance.ts` and harness usages).
Applied to files:
packages/storage/src/conversation/ConversationFileWriter.test.tspackages/storage/src/utils/gitIgnoreParser.tspackages/storage/src/conversation/ConversationFileWriter.tspackages/storage/src/secure-store/secure-store.ts
🔇 Additional comments (7)
eslint.config.js (1)
173-173: LGTM!Also applies to: 612-612, 703-706
scripts/tests/eslint-guard.test.js (1)
233-265: LGTM!packages/storage/src/conversation/ConversationFileWriter.ts (1)
13-30: LGTM!packages/storage/src/secure-store/secure-store.ts (3)
94-138: LGTM!
256-260: LGTM!
682-739: LGTM!packages/storage/src/utils/gitIgnoreParser.ts (1)
186-227: LGTM!
Contributor
LLxprt PR Review – PR #2148Issue AlignmentEvidence: Issue #2119 requires eliminating all inline ESLint disable/enable directives in packages/storage/src. This PR:
Verdict: Fully aligned. The implementation directly addresses the issue requirements. Side Effects
Code QualityStrengths:
Notes:
Tests and Coverage
VerdictReady The PR eliminates inline ESLint directives through code refactoring rather than suppression migration, which is the correct approach per issue #2119. The automated guard tests ensure future regressions fail immediately. Code quality improvements are sound and maintain existing behavior. |
# Conflicts: # eslint.config.js
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
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.
TLDR
Eliminates the remaining inline ESLint disable and enable directives from packages/storage and locks the module with automated guard coverage. The storage cleanup scope is now marked complete so future inline directive regressions fail immediately.
Dive Deeper
Changes included:
Reviewer Test Plan
Recommended validation:
Verification run locally on macOS:
Testing Matrix
Linked issues / bugs
Fixes #2119