Conversation
|
|
@greptile plz |
Greptile OverviewGreptile SummaryThis PR represents a major refactor that modernizes the SDK architecture and improves type safety significantly. Key Changes:
Issues Found:
Overall Assessment: Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| packages/server/src/nequi.ts | Core SDK class with Zod validation, error handling improvements, and tuple-based return type; contains a typo in error name |
| packages/server/src/types.ts | Added Zod schema for options validation; env field defined but not utilized in codebase |
| packages/server/src/constants/index.ts | Fixed endpoint paths to include proper base paths; environment detection relies on NODE_ENV instead of SDK config |
| packages/server/src/auth/index.ts | Improved auth with Zod validation and better error handling; properly validates API responses |
| package.json | Converted to monorepo workspace with Bun, removed Husky and commitlint, updated to Node 24+ |
| process.env.NODE_ENV === "development" | ||
| ? "https://api.sandbox.nequi.com" | ||
| : "https://api.nequi.com", | ||
| AUTH_URI: | ||
| process.env.NODE_ENV === "development" | ||
| ? "https://oauth.sandbox.nequi.com/oauth2/token" | ||
| : "https://oauth.nequi.com/oauth2/token", |
There was a problem hiding this comment.
process.env.NODE_ENV is used but the NequiOptionsSchema accepts an env field that is never utilized. The URLs should use the env option passed to the Nequi constructor instead of relying on process.env.NODE_ENV.
Currently, users cannot control which environment to use through the SDK configuration - it only respects the NODE_ENV environment variable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/server/src/constants/index.ts
Line: 3:9
Comment:
`process.env.NODE_ENV` is used but the `NequiOptionsSchema` accepts an `env` field that is never utilized. The URLs should use the `env` option passed to the `Nequi` constructor instead of relying on `process.env.NODE_ENV`.
Currently, users cannot control which environment to use through the SDK configuration - it only respects the `NODE_ENV` environment variable.
How can I resolve this? If you propose a fix, please make it concise.| apiKey: z.string().min(1, "API key is required"), | ||
| clientId: z.string().min(1, "Client ID is required"), | ||
| clientSecret: z.string().min(1, "Client secret is required"), | ||
| env: z.enum(["development", "production"]).default("development"), |
There was a problem hiding this comment.
The env field is defined in the schema but never used in the codebase. Either remove it or implement environment switching based on this configuration option.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/server/src/types.ts
Line: 10:10
Comment:
The `env` field is defined in the schema but never used in the codebase. Either remove it or implement environment switching based on this configuration option.
How can I resolve this? If you propose a fix, please make it concise.| name: | ||
| req.status === 403 ? "invalid_api_Key" : "application_error", |
There was a problem hiding this comment.
Typo in error name: invalid_api_Key should be invalid_api_key (lowercase 'k') to match the error types defined in error/types.ts:8
| name: | |
| req.status === 403 ? "invalid_api_Key" : "application_error", | |
| name: | |
| req.status === 403 ? "invalid_api_key" : "application_error", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/server/src/nequi.ts
Line: 86:87
Comment:
Typo in error name: `invalid_api_Key` should be `invalid_api_key` (lowercase 'k') to match the error types defined in `error/types.ts:8`
```suggestion
name:
req.status === 403 ? "invalid_api_key" : "application_error",
```
How can I resolve this? If you propose a fix, please make it concise.
No description provided.