Implement invites v2#140
Conversation
|
Warning Rate limit exceeded@lourou has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds protobuf codegen to CI and Docker builds, introduces invite protobufs and generated output path changes, adds protobuf/crypto dependencies, implements a v2 /invites/:slug endpoint that decodes and verifies signed invites (with tests), updates generated-code imports/ignores, and tweaks Claude workflow model selection. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as Express /api
participant V2 as /v2 Router
participant R as Invites Router
participant H as decodeInviteSlugHandler
participant PB as Protobuf Parser
participant EC as secp256k1
C->>API: GET /api/v2/invites/:slug
API->>V2: route /v2
V2->>R: route /invites/:slug
R->>H: invoke handler
H->>H: validate slug (presence, max length)
H->>PB: base64URL decode + parse SignedInvite
PB-->>H: payload bytes + signature
H->>H: sha256(payload)
H->>EC: ecdsaRecover(signature, recoveryId, hash)
EC-->>H: signerPublicKey
alt valid parse & signature
H-->>C: 200 OK (decoded invite fields)
else invalid parse/signature
H-->>C: 400 INVALID_INVITE
end
opt validation error (missing/too-large)
H-->>C: 400 INVALID_REQUEST or 413 Payload Too Large
end
opt unexpected
H-->>C: 500 INTERNAL_ERROR
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/api/v2/invites/handlers/secp256k1.d.ts (1)
1-8: Consider relocating ambient declaration and checking for @types package.The ambient declaration provides the necessary types for
ecdsaRecover. However:
- Location: Ambient declarations are typically placed in a dedicated types directory (e.g.,
src/types/or root-leveltypes/) rather than within ahandlers/directory, per coding guidelines that recommend keeping types close to usage but in appropriate locations.- Completeness: This declares only
ecdsaRecover. If additional secp256k1 functions are needed later, the declaration will need extension.- @types package: Check whether
@types/secp256k1exists to provide comprehensive types.Run the following script to check for the @types package:
#!/bin/bash # Description: Check if @types/secp256k1 exists on npm echo "=== Checking for @types/secp256k1 on npm ===" curl -s https://registry.npmjs.org/@types/secp256k1 | jq -r 'if .error then "Package not found" else "Package exists - version: " + .["dist-tags"].latest end'If
@types/secp256k1doesn't exist, consider moving this file tosrc/types/secp256k1.d.ts:// src/types/secp256k1.d.ts declare module "secp256k1" { export function ecdsaRecover( signature: Uint8Array, recoveryId: number, messageHash: Uint8Array, compressed: boolean, ): Uint8Array; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc/notifications/gen/notifications/v1/service_pb.tsis excluded by!**/gen/**
📒 Files selected for processing (13)
.github/workflows/validate-code-quality.yml(1 hunks).gitignore(1 hunks)Dockerfile(1 hunks)buf.gen.yaml(1 hunks)eslint.config.js(1 hunks)package.json(1 hunks)proto/invite/v2/invite.proto(1 hunks)src/api/index.ts(1 hunks)src/api/v2/index.ts(1 hunks)src/api/v2/invites/handlers/decode-invite-slug.ts(1 hunks)src/api/v2/invites/handlers/secp256k1.d.ts(1 hunks)src/api/v2/invites/invites.router.ts(1 hunks)src/notifications/client.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
package.jsonsrc/api/v2/invites/invites.router.tsproto/invite/v2/invite.protosrc/notifications/client.tssrc/api/v2/index.tsDockerfileeslint.config.jssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.tsbuf.gen.yaml
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v2/invites/invites.router.tssrc/notifications/client.tssrc/api/v2/index.tssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v2/invites/invites.router.tssrc/api/v2/index.tssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v2/invites/invites.router.tssrc/api/v2/index.tssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v2/invites/invites.router.tssrc/api/v2/index.tssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v2/invites/invites.router.tssrc/api/v2/index.tssrc/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.tssrc/api/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursorrules)
Use index.ts files for exports
Files:
src/api/v2/index.tssrc/api/index.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v2/invites/handlers/decode-invite-slug.tssrc/api/v2/invites/handlers/secp256k1.d.ts
🧬 Code graph analysis (1)
src/api/v2/invites/invites.router.ts (1)
src/api/v2/invites/handlers/decode-invite-slug.ts (1)
decodeInviteSlugHandler(127-189)
🪛 GitHub Actions: Validate code quality
src/api/v2/invites/handlers/decode-invite-slug.ts
[error] 9-9: ESLint: 'InvitePayload' is defined but never used. (no-unused-vars)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: claude-review
- GitHub Check: Review for correctness
🔇 Additional comments (12)
src/notifications/client.ts (1)
10-10: LGTM! Import path correctly updated to match new protobuf generation location.The import path change from
@/notifications/gen/notifications/v1/service_pbto@/gen/notifications/v1/service_pbcorrectly reflects the centralization of all protobuf-generated code undersrc/gen, as configured inbuf.gen.yaml..github/workflows/validate-code-quality.yml (1)
21-21: LGTM! Protobuf code generation correctly integrated into CI workflow.Adding
buf:generateafter dependency installation and before starting services ensures that all generated TypeScript code is available for subsequent typecheck, lint, and test steps. The placement is optimal in the build sequence.buf.gen.yaml (1)
4-4: LGTM! Centralized output directory improves project organization.Moving the protobuf generation output from
src/notifications/gentosrc/gencentralizes all generated code in one location, making it easier to manage ignore patterns and keeping the codebase organized. This aligns with the updated.gitignoreand ESLint configuration.src/api/index.ts (2)
3-3: LGTM! V2 API router import follows established pattern.The import statement for the v2 router is consistent with the existing v1 router import pattern, using relative paths as expected.
10-12: LGTM! V2 API mounting follows RESTful versioning best practices.The v2 router is mounted at
/v2following the same pattern as the v1 API, which is a clean approach to API versioning. The implementation maintains consistency across versions..gitignore (1)
67-69: LGTM! Protobuf generated files correctly ignored.The ignore pattern
/src/gen/correctly excludes protobuf-generated TypeScript code from version control, consistent with the new output directory configured inbuf.gen.yaml. The comment and formatting match the existing style used for other generated code sections.eslint.config.js (1)
17-17: LGTM! ESLint ignore pattern correctly updated.The ignore pattern change from
"src/notifications/gen/**"to"src/gen/**"appropriately aligns with the protobuf codegen output directory relocation tosrc/gen, as configured in buf.gen.yaml.Dockerfile (3)
14-14: LGTM! Buf configuration files correctly added.Adding
buf.yamlandbuf.gen.yamlto the Docker build context is necessary for the protobuf code generation step introduced later in the build.
19-20: LGTM! Proto directory correctly staged.Creating and copying the
proto/directory is necessary for thebuf generatecommand to access the.protodefinitions.
25-26: LGTM! Protobuf codegen correctly sequenced before Prisma.Running
bun run buf:generatebefore Prisma client generation ensures that generated protobuf types are available for any code that Prisma generation might process. The sequencing is correct.src/api/v2/index.ts (1)
1-8: Approve code changes invitesV2Router exists and is properly exported.package.json (1)
55-56: Dependencies up-to-date and secure
Both protobufjs@^7.5.4 and secp256k1@^5.0.1 are at the latest published versions and include fixes for all known CVEs; no further updates required.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Add invites v2 by exposing GET /api/v2/invites/:slug to decode base64url SignedInvite payloads up to 2048 chars and recover secp256k1 public keysThis change introduces the v2 invites API and supporting build updates. It adds a new router under
📍Where to StartStart with the request flow at Macroscope summarized 772ce86. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/v2/invites/handlers/decode-invite-slug.ts(1 hunks)tests/invites-v2.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v2/invites/handlers/decode-invite-slug.tstests/invites-v2.test.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v2/invites/handlers/decode-invite-slug.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v2/invites/handlers/decode-invite-slug.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v2/invites/handlers/decode-invite-slug.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v2/invites/handlers/decode-invite-slug.tstests/invites-v2.test.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v2/invites/handlers/decode-invite-slug.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v2/invites/handlers/decode-invite-slug.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Write tests for API endpoints
Use helper functions for test setup
Mock external services
Test error cases
Follow AAA pattern (Arrange, Act, Assert)
Files:
tests/invites-v2.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-05T14:39:39.983Z
Learnt from: CR
PR: ephemeraHQ/convos-backend#0
File: .cursorrules:0-0
Timestamp: 2025-08-05T14:39:39.983Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Write tests for API endpoints
Applied to files:
tests/invites-v2.test.ts
🧬 Code graph analysis (1)
tests/invites-v2.test.ts (3)
src/middleware/pino.ts (1)
pinoMiddleware(5-24)src/middleware/json.ts (1)
jsonMiddleware(3-3)src/utils/prisma.ts (1)
prisma(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Review for correctness
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/invites-v2.test.ts (3)
14-33: Add error handling to lifecycle hooks and consider dynamic port allocation.The lifecycle management is mostly correct, but there are two areas for improvement:
- The
beforeAllhook lacks error handling—ifserver.listenfails (e.g., port already in use), the promise will never reject and tests will hang.- The hardcoded port
4001could conflict with other services or parallel test runs.Consider this improvement:
beforeAll(async () => { - await new Promise<void>((resolve) => { + await new Promise<void>((resolve, reject) => { - server = app.listen(4001, () => { + server = app.listen(4001, (err) => { + if (err) return reject(err); resolve(); }); }); });For dynamic port allocation (optional enhancement):
beforeAll(async () => { await new Promise<void>((resolve, reject) => { server = app.listen(0, (err) => { // 0 = random available port if (err) return reject(err); const addr = server.address(); baseURL = `http://localhost:${typeof addr === 'object' ? addr?.port : 4001}`; resolve(); }); }); });
117-135: Extract response validation into helper functions.This test duplicates error response validation logic from lines 50-55 and 70-75. Per coding guidelines, use helper functions to reduce duplication and improve maintainability.
Add helper functions at the top of the test file:
// Test helpers const expectErrorResponse = (data: unknown) => { const error = data as { success: boolean; error: string; message: string }; expect(error).toHaveProperty("success"); expect(error).toHaveProperty("error"); expect(error).toHaveProperty("message"); expect(error.success).toBe(false); expect(typeof error.error).toBe("string"); expect(typeof error.message).toBe("string"); return error; }; const expectSuccessResponse = (data: unknown) => { const response = data as { success: boolean; data: unknown }; expect(response).toHaveProperty("success"); expect(response).toHaveProperty("data"); expect(response.success).toBe(true); return response; };Then refactor tests to use them:
const response = await fetch(`${baseURL}/api/v2/invites/invalid`, { method: "GET", }); - const data = (await response.json()) as { - success: boolean; - error: string; - message: string; - }; - expect(data).toHaveProperty("success"); - expect(data).toHaveProperty("error"); - expect(data).toHaveProperty("message"); - expect(data.success).toBe(false); - expect(typeof data.error).toBe("string"); - expect(typeof data.message).toBe("string"); + const data = expectErrorResponse(await response.json()); + // Additional specific assertions can follow
137-147: Make internal-details check case-insensitive for "prisma".The check on line 146 only catches lowercase "prisma", but error messages might contain "Prisma" (capital P). Consider case-insensitive matching or checking both variants.
const data = (await response.json()) as { message: string }; expect(data.message).not.toContain("stack"); expect(data.message).not.toContain("Error:"); - expect(JSON.stringify(data)).not.toContain("prisma"); + expect(JSON.stringify(data).toLowerCase()).not.toContain("prisma");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/invites-v2.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
tests/invites-v2.test.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
tests/invites-v2.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Write tests for API endpoints
Use helper functions for test setup
Mock external services
Test error cases
Follow AAA pattern (Arrange, Act, Assert)
Files:
tests/invites-v2.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-05T14:39:39.983Z
Learnt from: CR
PR: ephemeraHQ/convos-backend#0
File: .cursorrules:0-0
Timestamp: 2025-08-05T14:39:39.983Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Write tests for API endpoints
Applied to files:
tests/invites-v2.test.ts
🧬 Code graph analysis (1)
tests/invites-v2.test.ts (3)
src/middleware/pino.ts (1)
pinoMiddleware(5-24)src/middleware/json.ts (1)
jsonMiddleware(3-3)src/utils/prisma.ts (1)
prisma(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Typecheck, format check, lint
🔇 Additional comments (3)
tests/invites-v2.test.ts (3)
1-13: LGTM: Clean test setup with proper middleware ordering.The imports follow path alias conventions, and the middleware stack (pino → json → router) is correctly ordered.
78-86: Good fix: oversized slug test now validates handler logic.The test now uses a 2050-character slug (just over the 2 KB limit) and correctly asserts 413 status, ensuring the request reaches the handler rather than being rejected by Node's HTTP parser.
149-161: LGTM: Path traversal test correctly validates input sanitization.The test properly encodes a path traversal attempt and validates the handler rejects it with 400, demonstrating good security posture.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
The buf:generate script was failing in the Docker build because @bufbuild/buf is in devDependencies, which are excluded when running `bun install --production`. Instead of installing all devDependencies (which would bloat the production image), install the buf CLI v1.50.0 as a system binary in the install stage.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
1 participant
Add v2 invites API with GET /v2/invites/:slug to decode protobuf SignedInvite and validate 65-byte secp256k1 signatures
This change introduces a versioned v2 invites API and wires protobuf-based invite decoding with signature validation, alongside build and CI updates to generate protobuf artifacts. It also consolidates generated code output paths.
GET /v2/invites/:slugthat base64url-decodes a slug, parses aSignedInviteprotobuf, hashes theInvitePayloadwith SHA-256, and recovers a secp256k1 public key from a 65-byte signature in src/api/v2/invites/handlers/decode-invite-slug.ts/v2and expose/v2/invitesin src/api/index.ts and src/api/v2/index.ts, with routing in src/api/v2/invites/invites.router.tssrc/genvia buf.gen.yamlprotobufjsandsecp256k1in package.json and associated lockfile updates; remove old generated file src/notifications/gen/notifications/v1/service_pb.ts and update imports in src/notifications/client.ts📍Where to Start
Start with the request handler
decodeInviteSlugHandlerin src/api/v2/invites/handlers/decode-invite-slug.ts, then follow routing from src/api/v2/invites/invites.router.ts and src/api/v2/index.ts.Macroscope summarized 90e5337.
Summary by CodeRabbit
New Features
Chores
Tests