fix: exclude sessionToken from sessions API response#1602
Conversation
|
Someone is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR addresses two independent concerns: restricting the sessions API to return only safe metadata fields (preventing session token exposure), and adding comprehensive test coverage for the PatchValidatorService validation logic. ChangesSession API Field Restriction
PatchValidator Test Suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🎉 Thanks for your contribution, @Akshita-2307!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
1 similar comment
🎉 Thanks for your contribution, @Akshita-2307!Your PR has passed our automated GSSoC quality checks. Here's a quick summary:
A maintainer will review your PR soon. Please be patient and available for feedback. 💪 GSSoC'26 automation · Maintainer: @nisshchayarathi |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/services/__tests__/patch-validator.test.ts (2)
6-16: ⚡ Quick winConsider using a type-safe signature for the makePatch factory.
The factory currently uses
Record<string, unknown>which accepts any property with any value, bypassing TypeScript's type checking. This can allow test data that doesn't match the actual patch interface to slip through at compile time.♻️ Proposed refactor for type safety
-function makePatch(overrides: Record<string, unknown> = {}) { +function makePatch(overrides: Partial<Parameters<typeof service.validatePatch>[0]> = {}) { return { file: "test.ts", suggestionBody: "const x = 1;", startLine: 1, endLine: 1, confidenceScore: 90, status: "pending" as const, ...overrides, - }; + } as Parameters<typeof service.validatePatch>[0]; }Alternatively, if the Patch type is exported from
types/self-healing.ts, you could import and use it directly:import type { Patch } from "../../types/self-healing"; function makePatch(overrides: Partial<Patch> = {}): Patch { // ... }🤖 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 `@lib/services/__tests__/patch-validator.test.ts` around lines 6 - 16, Update the makePatch factory to be type-safe by importing the Patch type (e.g. from "../../types/self-healing") and changing the signature from makePatch(overrides: Record<string, unknown> = {}) to makePatch(overrides: Partial<Patch> = {}): Patch; inside the function keep the same defaults (file, suggestionBody, startLine, endLine, confidenceScore, status) and spread overrides to produce and return a value that conforms to Patch so TypeScript enforces correct test data.
25-31: Update confidence threshold expectation (it matches the implementation).
SELF_HEAL_CONFIDENCE_THRESHOLDis set to85intypes/self-healing.ts, andvalidatePatchassignslow_confidenceonly whenconfidenceScore < SELF_HEAL_CONFIDENCE_THRESHOLD—soconfidenceScore: 50correctly hitslow_confidence. Consider adding a boundary test forconfidenceScore: 85to ensure it does not becomelow_confidence.🤖 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 `@lib/services/__tests__/patch-validator.test.ts` around lines 25 - 31, Add a boundary unit test to ensure validatePatch (the service.validatePatch function) treats a confidenceScore equal to SELF_HEAL_CONFIDENCE_THRESHOLD as not "low_confidence": create a new test similar to the existing one that calls service.validatePatch(makePatch({ confidenceScore: 85 }), "const a = 0;") and assert result.status is not "low_confidence" (or assert the expected status for >= threshold); reference SELF_HEAL_CONFIDENCE_THRESHOLD (from types/self-healing.ts), validatePatch, and makePatch to locate the code under test.
🤖 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 `@lib/services/__tests__/patch-validator.test.ts`:
- Around line 62-68: The test for empty suggestionBody uses a weak negative
assertion; update the assertion in the "should handle empty suggestionBody
gracefully" test to a specific positive expectation for
service.validatePatch(makePatch({ suggestionBody: "" }), "const a = 0;") — e.g.,
assert the exact expected status string ("valid", "invalid", or whatever the
intended outcome is) and/or specific result fields (like result.status ===
"valid" or result.error === null) instead of `.not.toBe("invalid_syntax")`;
locate the test block and replace the negative assertion with the precise
expected status/value for validatePatch to make the test deterministic.
---
Nitpick comments:
In `@lib/services/__tests__/patch-validator.test.ts`:
- Around line 6-16: Update the makePatch factory to be type-safe by importing
the Patch type (e.g. from "../../types/self-healing") and changing the signature
from makePatch(overrides: Record<string, unknown> = {}) to makePatch(overrides:
Partial<Patch> = {}): Patch; inside the function keep the same defaults (file,
suggestionBody, startLine, endLine, confidenceScore, status) and spread
overrides to produce and return a value that conforms to Patch so TypeScript
enforces correct test data.
- Around line 25-31: Add a boundary unit test to ensure validatePatch (the
service.validatePatch function) treats a confidenceScore equal to
SELF_HEAL_CONFIDENCE_THRESHOLD as not "low_confidence": create a new test
similar to the existing one that calls service.validatePatch(makePatch({
confidenceScore: 85 }), "const a = 0;") and assert result.status is not
"low_confidence" (or assert the expected status for >= threshold); reference
SELF_HEAL_CONFIDENCE_THRESHOLD (from types/self-healing.ts), validatePatch, and
makePatch to locate the code under test.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4f9aae43-3800-4d2f-81b0-fecd0eb7a940
📒 Files selected for processing (2)
app/api/auth/sessions/route.tslib/services/__tests__/patch-validator.test.ts
| it("should handle empty suggestionBody gracefully", () => { | ||
| const result = service.validatePatch( | ||
| makePatch({ suggestionBody: "" }), | ||
| "const a = 0;" | ||
| ); | ||
| expect(result.status).not.toBe("invalid_syntax"); | ||
| }); |
There was a problem hiding this comment.
Strengthen the assertion for empty suggestionBody test.
The test uses .not.toBe("invalid_syntax") which is a weak negative assertion. If the service returns an unexpected status like "error" or "unknown", the test would still pass, potentially hiding bugs.
✅ Proposed fix to use a positive assertion
it("should handle empty suggestionBody gracefully", () => {
const result = service.validatePatch(
makePatch({ suggestionBody: "" }),
"const a = 0;"
);
- expect(result.status).not.toBe("invalid_syntax");
+ expect(result.status).toBe("valid");
});If the expected behavior for empty suggestionBody is different (e.g., it should return "invalid" or another specific status), update the assertion to match that expected behavior.
🤖 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 `@lib/services/__tests__/patch-validator.test.ts` around lines 62 - 68, The
test for empty suggestionBody uses a weak negative assertion; update the
assertion in the "should handle empty suggestionBody gracefully" test to a
specific positive expectation for service.validatePatch(makePatch({
suggestionBody: "" }), "const a = 0;") — e.g., assert the exact expected status
string ("valid", "invalid", or whatever the intended outcome is) and/or specific
result fields (like result.status === "valid" or result.error === null) instead
of `.not.toBe("invalid_syntax")`; locate the test block and replace the negative
assertion with the precise expected status/value for validatePatch to make the
test deterministic.
1 participant
The authenticated sessions endpoint returned full Prisma session records including sessionToken. Updated query to use select and return only id, expires, createdAt, and updatedAt.
Closes #1543
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests