Conversation
- Bump SDK version to v0.2.0 and monorepo to v0.9.0. - Implement new modules: Mail and Schema introspection. - Enhance AuthModule with profile updates, social auth, and session management. - Add DatabaseModule query builder (filter, sort, page, populate) and patch support. - Fix vitest type configuration and test expectations. - Update client with unified SK/PK security warnings and module integration.
📝 WalkthroughWalkthroughBumps package versions and removes an outdated roadmap from AGENTS.md; expands the TypeScript SDK with Schema and Mail modules, extends Auth (social auth, token handling, profile/password flows) and Database (query params, patch, token forwarding) modules, updates types, and adds/updates tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client Application
participant SDK as urbackend-sdk (AuthModule)
participant API as Backend API
participant OAuth as OAuth Provider
App->>SDK: socialStart(provider)
SDK->>API: GET /api/auth/social/start?provider=github&key=pk_live...
API-->>SDK: { authorizationUrl }
SDK-->>App: authorizationUrl
App->>OAuth: Redirect to authorizationUrl
OAuth-->>App: Redirect back with code
App->>SDK: socialExchange({ code, provider })
SDK->>API: POST /api/auth/social/exchange { code, provider }
API->>OAuth: Exchange code for tokens
OAuth-->>API: access token / profile
API-->>SDK: { accessToken, user, ... }
SDK-->>App: AuthResponse (accessToken, user)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Pull request overview
This PR upgrades the @urbackend/sdk to v0.2.0, expanding the TypeScript SDK to cover additional Public API capabilities (schema/mail modules, richer auth/database operations) while improving request handling and type definitions.
Changes:
- Added new SDK modules for schema introspection and server-side mail sending, plus new Vitest coverage.
- Expanded Auth and Database modules (token refresh/rotation helpers, social auth helpers, PATCH + query params, optional bearer token for RLS writes).
- Updated client internals (lazy module getters, request option support, User-Agent/version bumps) and bumped repo/sdk versions.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/urbackend-sdk/tests/schema.test.ts | Adds schema module tests |
| sdks/urbackend-sdk/tests/mail.test.ts | Adds mail module tests |
| sdks/urbackend-sdk/tests/database.test.ts | Updates DB tests for query params, patch, tokenized writes |
| sdks/urbackend-sdk/tests/auth.test.ts | Updates auth tests for accessToken, logout, refresh, social/public profile |
| sdks/urbackend-sdk/src/types/index.ts | Adds/extends request/query/auth/db/mail/schema response types |
| sdks/urbackend-sdk/src/modules/schema.ts | Introduces SchemaModule |
| sdks/urbackend-sdk/src/modules/mail.ts | Introduces MailModule |
| sdks/urbackend-sdk/src/modules/database.ts | Adds query builder, patch, tokenized write support |
| sdks/urbackend-sdk/src/modules/auth.ts | Adds expanded auth surface (refresh/logout/social/profile/etc.) |
| sdks/urbackend-sdk/src/index.ts | Re-exports modules/types for consumers |
| sdks/urbackend-sdk/src/client.ts | Lazy module init; request handling updates; UA bump |
| sdks/urbackend-sdk/package.json | SDK version bump to 0.2.0 |
| package.json | Monorepo version bump to 0.9.0 |
| package-lock.json | Lockfile version updates for bumps |
| AGENTS.md | Removes outdated version/planning notes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public async socialExchange(payload: SocialExchangePayload): Promise<ApiResponse<SocialExchangeResponse>> { | ||
| const response = await this.client.request<ApiResponse<SocialExchangeResponse>>('POST', '/api/userAuth/social/exchange', { | ||
| body: payload, | ||
| }); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
socialExchange() is typed to return ApiResponse<SocialExchangeResponse>, but UrBackendClient.request() unwraps { success, data, message } responses by returning json.data when present. Since the public API actually returns { success: true, data: { refreshToken }, ... } for this endpoint, this method will resolve to { refreshToken: ... } (data only), not an ApiResponse. Update the return type/implementation to return SocialExchangeResponse (or add a request option to disable unwrapping and return the full envelope) so consumers don't break when they try to access .data.refreshToken.
| /** | ||
| * Returns the start URL for social authentication. | ||
| * Redirect the user's browser to this URL to begin the flow. | ||
| */ | ||
| public socialStart(provider: 'github' | 'google'): string { | ||
| return `${this.client['baseUrl']}/api/userAuth/social/${provider}/start?key=${this.client['apiKey']}`; |
There was a problem hiding this comment.
socialStart() is reaching into UrBackendClient private fields via bracket access (this.client['baseUrl'], this.client['apiKey']). This bypasses encapsulation and can break with minification/refactors. Expose public getters (e.g. getBaseUrl(), getApiKey()), or have the client provide a helper to build absolute URLs for navigation endpoints.
| /** | |
| * Returns the start URL for social authentication. | |
| * Redirect the user's browser to this URL to begin the flow. | |
| */ | |
| public socialStart(provider: 'github' | 'google'): string { | |
| return `${this.client['baseUrl']}/api/userAuth/social/${provider}/start?key=${this.client['apiKey']}`; | |
| private getClientBaseUrl(): string { | |
| const clientWithGetters = this.client as UrBackendClient & { | |
| getBaseUrl?: () => string; | |
| }; | |
| if (typeof clientWithGetters.getBaseUrl !== 'function') { | |
| throw new Error('UrBackendClient must expose a public getBaseUrl() method for social auth navigation.'); | |
| } | |
| return clientWithGetters.getBaseUrl(); | |
| } | |
| private getClientApiKey(): string { | |
| const clientWithGetters = this.client as UrBackendClient & { | |
| getApiKey?: () => string; | |
| }; | |
| if (typeof clientWithGetters.getApiKey !== 'function') { | |
| throw new Error('UrBackendClient must expose a public getApiKey() method for social auth navigation.'); | |
| } | |
| return clientWithGetters.getApiKey(); | |
| } | |
| /** | |
| * Returns the start URL for social authentication. | |
| * Redirect the user's browser to this URL to begin the flow. | |
| */ | |
| public socialStart(provider: 'github' | 'google'): string { | |
| return `${this.getClientBaseUrl()}/api/userAuth/social/${provider}/start?key=${this.getClientApiKey()}`; |
| public async refreshToken(refreshToken?: string): Promise<AuthResponse> { | ||
| const options: any = {}; | ||
| if (refreshToken) { | ||
| options.headers = { 'x-refresh-token': refreshToken, 'x-refresh-token-mode': 'header' }; | ||
| } else { | ||
| options.credentials = 'include'; | ||
| } | ||
|
|
||
| const response = await this.client.request<AuthResponse>('POST', '/api/userAuth/refresh-token', options); |
There was a problem hiding this comment.
refreshToken() uses const options: any = {} and relies on a non-typed headers property. Since the client already has a RequestOptions type, this should be strongly typed (and RequestOptions should include an optional headers field) to avoid type holes and accidental misuse.
| body?: unknown; | ||
| token?: string; | ||
| isMultipart?: boolean; | ||
| credentials?: RequestCredentials; |
There was a problem hiding this comment.
RequestOptions is missing a headers?: Record<string,string> field, but the client merges (options as any).headers into the request headers. Add headers to RequestOptions so module code (e.g. refresh-token header mode) can be type-safe and you can remove the any casts in client.request().
| credentials?: RequestCredentials; | |
| credentials?: RequestCredentials; | |
| headers?: Record<string, string>; |
| export interface AuthResponse { | ||
| accessToken: string; | ||
| /** @deprecated use accessToken instead */ | ||
| token: string; | ||
| user: AuthUser; | ||
| expiresIn?: string; | ||
| userId?: string; | ||
| user?: AuthUser; | ||
| } |
There was a problem hiding this comment.
AuthResponse currently requires both accessToken and the deprecated token. However, the SDK logic explicitly supports servers that return only one of them (response.accessToken || response.token). Make token optional (and likely accessToken optional as well) so the public types match actual/forward-compatible API behavior.
sdks/urbackend-sdk/src/client.ts
Outdated
| if ((options as any).headers) { | ||
| Object.assign(headers, (options as any).headers); |
There was a problem hiding this comment.
client.request() currently uses (options as any).headers to merge custom headers. Once RequestOptions.headers exists, remove the any cast and use the typed property; this prevents silently accepting non-string header values and improves type safety across modules.
| if ((options as any).headers) { | |
| Object.assign(headers, (options as any).headers); | |
| if (options.headers) { | |
| Object.assign(headers, options.headers); |
| * Fetch the schema definition for a collection | ||
| */ | ||
| public async getSchema(collection: string): Promise<CollectionSchema> { | ||
| return this.client.request<CollectionSchema>('GET', `/api/schemas/${collection}`); |
There was a problem hiding this comment.
The Public API /api/schemas/:collectionName route returns { message, collection } (see apps/public-api/src/controllers/schema.controller.js). getSchema() currently returns the entire response as CollectionSchema, which means callers will actually receive an object with message/collection keys and not the schema itself. Adjust this method to return the collection field (or update the request generic accordingly) so the runtime value matches Promise<CollectionSchema>.
| return this.client.request<CollectionSchema>('GET', `/api/schemas/${collection}`); | |
| const response = await this.client.request<{ | |
| message: string; | |
| collection: CollectionSchema; | |
| }>('GET', `/api/schemas/${collection}`); | |
| return response.collection; |
| vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| headers: new Headers({ 'content-type': 'application/json' }), | ||
| json: () => Promise.resolve({ success: true, data: mockSchema }), |
There was a problem hiding this comment.
This test stubs the schema endpoint to return { success: true, data: mockSchema }, but the actual Public API schema endpoint responds with { message: 'Schema exists', collection: <schema> }. Update the mock (and/or the SDK implementation) so the test reflects the real response shape and prevents false positives.
| json: () => Promise.resolve({ success: true, data: mockSchema }), | |
| json: () => Promise.resolve({ message: 'Schema exists', collection: mockSchema }), |
| const result = await this.client.request<any>('DELETE', `/api/data/${collection}/${id}`, { | ||
| token | ||
| }); | ||
| // The API might return { message: "Document deleted", id: "..." } | ||
| return { deleted: !!result }; |
There was a problem hiding this comment.
delete() maps any truthy API response to { deleted: true } via !!result, which will report success even if the server returns an unexpected non-empty payload. Since the Public API returns { message: 'Document deleted', id }, consider validating expected keys (e.g. result.id or a known message) or at least checking result is an object to avoid incorrectly returning deleted: true for malformed responses.
| const result = await this.client.request<any>('DELETE', `/api/data/${collection}/${id}`, { | |
| token | |
| }); | |
| // The API might return { message: "Document deleted", id: "..." } | |
| return { deleted: !!result }; | |
| const result = await this.client.request<{ message?: string; id?: string } | null>( | |
| 'DELETE', | |
| `/api/data/${collection}/${id}`, | |
| { | |
| token | |
| } | |
| ); | |
| const deleted = | |
| typeof result === 'object' && | |
| result !== null && | |
| (result.id === id || result.message === 'Document deleted'); | |
| return { deleted }; |
| beforeEach(() => { | ||
| vi.resetAllMocks(); | ||
| client.auth.logout(); |
There was a problem hiding this comment.
AuthModule.logout() is now async and may attempt a network call when a token is present. In beforeEach, it's invoked without await, which can cause cross-test leakage (token not cleared before next test) and can accidentally hit fetch when no mock is installed. Make the hook async and await client.auth.logout() (or clear the token synchronously) to keep tests deterministic.
| beforeEach(() => { | |
| vi.resetAllMocks(); | |
| client.auth.logout(); | |
| beforeEach(async () => { | |
| vi.resetAllMocks(); | |
| await client.auth.logout(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/urbackend-sdk/src/client.ts (1)
85-97:⚠️ Potential issue | 🟡 Minor
Content-Typeoverride negates merged custom headers, and falsy JSON bodies are skipped.Lines 85–88 merge custom headers, but Line 96 overwrites
Content-Typeunconditionally. Also, Line 95 uses a truthy check, so payloads likefalse/0won’t be sent.Proposed fix
- } else if (options.body) { - headers['Content-Type'] = 'application/json'; + } else if (options.body !== undefined) { + const hasContentType = Object.keys(headers).some( + (key) => key.toLowerCase() === 'content-type', + ); + if (!hasContentType) { + headers['Content-Type'] = 'application/json'; + } requestBody = JSON.stringify(options.body); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/client.ts` around lines 85 - 97, The merged custom headers are being overwritten and some valid falsy bodies are skipped: update the request construction in client.ts so that when options.isMultipart is true you use options.body as FormData, otherwise treat presence of a body by checking "body" in options (not truthiness) and always JSON.stringify(options.body) even if it's false/0; set headers['Content-Type']='application/json' only if no Content-Type was provided by merged headers and options.isMultipart is false. Modify the code around headers, requestBody, options.isMultipart and options.body to respect existing headers and allow falsy JSON payloads.
🧹 Nitpick comments (4)
sdks/urbackend-sdk/src/client.ts (1)
6-16: Please gate this merge with broader regression suites beyond SDK unit tests.Given the expanded auth/schema-capable client surface, run the public-api and dashboard-api test suites before release to catch integration drift early.
Based on learnings: Before shipping auth, RLS, or schema changes: run
apps/public-apitests, runapps/dashboard-apitests, runapps/web-dashboardlint, runapps/web-dashboardbuild when frontend changed.Also applies to: 52-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/client.ts` around lines 6 - 16, This change expands the UrBackendClient surface (symbols: UrBackendClient, _auth, _schema, AuthModule, SchemaModule) and must be gated by broader integration/regression suites: run the apps/public-api and apps/dashboard-api test suites and, if frontend assets changed, run apps/web-dashboard lint and build before merging; include the test results in the PR and/or add CI jobs to automatically run these suites so integration drift is caught prior to releasing changes to Auth/RLS/Schema (note same requirement applies to the other related block around the _db/_storage fields).sdks/urbackend-sdk/src/modules/auth.ts (2)
142-153: Avoid usinganytype for options object.The
optionsvariable is typed asany, which loses type safety. Consider using a proper type that extendsRequestOptions.Proposed fix
public async refreshToken(refreshToken?: string): Promise<AuthResponse> { - const options: any = {}; + const options: { headers?: Record<string, string>; credentials?: RequestCredentials } = {}; if (refreshToken) { options.headers = { 'x-refresh-token': refreshToken, 'x-refresh-token-mode': 'header' }; } else { options.credentials = 'include'; } const response = await this.client.request<AuthResponse>('POST', '/api/userAuth/refresh-token', options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/modules/auth.ts` around lines 142 - 153, The options object in refreshToken is typed as any; change it to the proper request options type (e.g., RequestOptions or Partial<RequestOptions> as used by this.client.request) so you keep type safety, update any necessary import of RequestOptions, and then assign options.headers and options.credentials with the correct types; ensure the call this.client.request<AuthResponse>('POST','/api/userAuth/refresh-token', options) matches the new options type and adjust the types for sessionToken assignment if needed.
159-161: Accessing private client properties via bracket notation.Using
this.client['baseUrl']andthis.client['apiKey']bypasses TypeScript's access control. Consider adding public getters toUrBackendClientor passing these values to theAuthModuleconstructor for cleaner encapsulation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/modules/auth.ts` around lines 159 - 161, The socialStart method uses bracket notation to read private fields (this.client['baseUrl'], this.client['apiKey']); update the design to avoid bypassing TypeScript access control by either adding public getters on UrBackendClient (e.g., getBaseUrl(), getApiKey() or readonly properties baseUrl/apiKey) and change AuthModule.socialStart to call these getters, or modify AuthModule's constructor to accept baseUrl and apiKey and store them on the module instance; adjust the socialStart implementation to use the new public getters or the stored constructor values and remove any bracket-access to this.client.sdks/urbackend-sdk/src/modules/database.ts (1)
84-90: Thedeletedcheck may incorrectly returnfalsefor valid deletions.The expression
{ deleted: !!result }relies on the truthiness ofresult. If the API returns{ message: "Deleted" }(as per the comment),!!resultwill betrue. However, if the API returns an empty object{}or ifclient.requestreturnsundefinedfor some edge case, the result could be misleading. Consider checking for a successful response more explicitly.Proposed improvement for clarity
public async delete(collection: string, id: string, token?: string): Promise<{ deleted: boolean }> { - const result = await this.client.request<any>('DELETE', `/api/data/${collection}/${id}`, { + await this.client.request<unknown>('DELETE', `/api/data/${collection}/${id}`, { token }); - // The API might return { message: "Document deleted", id: "..." } - return { deleted: !!result }; + // If request succeeds without throwing, the delete was successful + return { deleted: true }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/modules/database.ts` around lines 84 - 90, The delete method's returned { deleted: !!result } is too coarse; update the logic in delete(collection, id, token?) to explicitly determine success from the API response returned by this.client.request (e.g., check for known success indicators such as result.id, result.deleted === true, or a result.message like "Deleted"), and fall back to false only when result is null/undefined and no success fields exist; ensure the returned object is { deleted: boolean } derived from those explicit checks rather than just truthiness of result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdks/urbackend-sdk/src/modules/schema.ts`:
- Around line 10-11: The getSchema method currently interpolates the collection
param directly into the URL; update getSchema to guard against empty or
whitespace-only collection names and to URL-encode the collection before
inserting into the path (e.g., trim and check collection !== '' then use
encodeURIComponent(collection)) so reserved characters can't break the route;
keep the return type Promise<CollectionSchema> and the call to
this.client.request('GET', ...) but pass the encoded collection value.
In `@sdks/urbackend-sdk/src/types/index.ts`:
- Around line 84-91: The AuthResponse interface declares accessToken as required
but legacy backends may return only the deprecated token field; update the
AuthResponse type (interface AuthResponse) to make accessToken optional
(accessToken?: string) so code paths in login() and refreshToken() that use
response.accessToken || response.token compile correctly, keep the existing
deprecated comment on token to indicate legacy usage.
In `@sdks/urbackend-sdk/tests/auth.test.ts`:
- Around line 7-10: The beforeEach hook calls the async client.auth.logout()
without awaiting it, which can cause races; make the beforeEach async and await
client.auth.logout() (i.e., change the beforeEach to an async function and use
await client.auth.logout()) so the logout completes before each test runs,
ensuring deterministic teardown for tests that depend on auth state.
---
Outside diff comments:
In `@sdks/urbackend-sdk/src/client.ts`:
- Around line 85-97: The merged custom headers are being overwritten and some
valid falsy bodies are skipped: update the request construction in client.ts so
that when options.isMultipart is true you use options.body as FormData,
otherwise treat presence of a body by checking "body" in options (not
truthiness) and always JSON.stringify(options.body) even if it's false/0; set
headers['Content-Type']='application/json' only if no Content-Type was provided
by merged headers and options.isMultipart is false. Modify the code around
headers, requestBody, options.isMultipart and options.body to respect existing
headers and allow falsy JSON payloads.
---
Nitpick comments:
In `@sdks/urbackend-sdk/src/client.ts`:
- Around line 6-16: This change expands the UrBackendClient surface (symbols:
UrBackendClient, _auth, _schema, AuthModule, SchemaModule) and must be gated by
broader integration/regression suites: run the apps/public-api and
apps/dashboard-api test suites and, if frontend assets changed, run
apps/web-dashboard lint and build before merging; include the test results in
the PR and/or add CI jobs to automatically run these suites so integration drift
is caught prior to releasing changes to Auth/RLS/Schema (note same requirement
applies to the other related block around the _db/_storage fields).
In `@sdks/urbackend-sdk/src/modules/auth.ts`:
- Around line 142-153: The options object in refreshToken is typed as any;
change it to the proper request options type (e.g., RequestOptions or
Partial<RequestOptions> as used by this.client.request) so you keep type safety,
update any necessary import of RequestOptions, and then assign options.headers
and options.credentials with the correct types; ensure the call
this.client.request<AuthResponse>('POST','/api/userAuth/refresh-token', options)
matches the new options type and adjust the types for sessionToken assignment if
needed.
- Around line 159-161: The socialStart method uses bracket notation to read
private fields (this.client['baseUrl'], this.client['apiKey']); update the
design to avoid bypassing TypeScript access control by either adding public
getters on UrBackendClient (e.g., getBaseUrl(), getApiKey() or readonly
properties baseUrl/apiKey) and change AuthModule.socialStart to call these
getters, or modify AuthModule's constructor to accept baseUrl and apiKey and
store them on the module instance; adjust the socialStart implementation to use
the new public getters or the stored constructor values and remove any
bracket-access to this.client.
In `@sdks/urbackend-sdk/src/modules/database.ts`:
- Around line 84-90: The delete method's returned { deleted: !!result } is too
coarse; update the logic in delete(collection, id, token?) to explicitly
determine success from the API response returned by this.client.request (e.g.,
check for known success indicators such as result.id, result.deleted === true,
or a result.message like "Deleted"), and fall back to false only when result is
null/undefined and no success fields exist; ensure the returned object is {
deleted: boolean } derived from those explicit checks rather than just
truthiness of result.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75441482-3c9d-4453-b60e-bca6259a2cb4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
AGENTS.mdpackage.jsonsdks/urbackend-sdk/package.jsonsdks/urbackend-sdk/src/client.tssdks/urbackend-sdk/src/index.tssdks/urbackend-sdk/src/modules/auth.tssdks/urbackend-sdk/src/modules/database.tssdks/urbackend-sdk/src/modules/mail.tssdks/urbackend-sdk/src/modules/schema.tssdks/urbackend-sdk/src/types/index.tssdks/urbackend-sdk/tests/auth.test.tssdks/urbackend-sdk/tests/database.test.tssdks/urbackend-sdk/tests/mail.test.tssdks/urbackend-sdk/tests/schema.test.ts
💤 Files with no reviewable changes (1)
- AGENTS.md
|
@copilot apply changes based on the comments in this thread |
- Add headers to RequestOptions for type-safe header passing - Make accessToken/token optional in AuthResponse - Remove (options as any).headers cast in client.ts - Add public getBaseUrl()/getApiKey() getters to UrBackendClient - Fix refreshToken() to use RequestOptions instead of any - Fix socialStart() to use public getters instead of bracket access - Fix socialExchange() return type to SocialExchangeResponse - Fix getSchema() to extract response.collection - Fix delete() to validate response keys properly - Update schema/database test mocks to match real API shapes - Make beforeEach in auth test async and await logout() Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/2c6f46de-4645-4d3e-8ae1-10e938a9e258 Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
Applied all 10 review comments in commit
All 19 tests pass and TypeScript type check is clean. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
sdks/urbackend-sdk/src/modules/database.ts (1)
84-97: Response format dependency is brittle.The
deleteddetermination relies on specific server response shapes (result.id === idorresult.message === 'Document deleted'). If the API response format changes, this could returndeleted: falseeven on successful deletion.Consider documenting the expected API response contract, or alternatively returning
deleted: truefor any 2xx response without an error (relying on HTTP semantics rather than body inspection).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/src/modules/database.ts` around lines 84 - 97, The delete method's deleted determination is brittle because it inspects response body fields; update the delete function to treat any successful HTTP response (i.e., the client.request call completed without throwing and/or returns a 2xx status) as deleted instead of relying on result.id or result.message. Locate the delete method and the this.client.request call in database.ts and change the logic to set deleted = true when the request succeeds (or when a 2xx status is present if client.request exposes it), and only use body-inspection as a fallback; also add a short comment documenting the expected API contract for deletions in the delete method.sdks/urbackend-sdk/tests/auth.test.ts (1)
27-45: Tighten token-rotation coverage, not just request shape.These cases only protect the
accessTokenhappy path and the transport mode.sdks/urbackend-sdk/src/modules/auth.tsstill hydrates auth state fromaccessToken || token, so a regression in rotated-token persistence or legacytokenfallback would still pass here. Please add an assertion onclient.auth.getToken()forrefreshToken()and keep one deprecated-response test.Also applies to: 109-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/tests/auth.test.ts` around lines 27 - 45, The test only asserts accessToken from login; extend coverage to assert persisted token behavior for token rotation and legacy fallback by adding assertions that client.auth.getToken() returns the right value after calling refreshToken() (verify refresh returns and persists a new refreshToken/accessToken pair) and add one deprecated-response test that supplies a legacy "token" field in the mocked JSON and asserts client.auth.getToken() still hydrates from that legacy token; modify tests around client.auth.login and the existing refreshToken() call to validate both returned values and persisted state rather than only the request shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdks/urbackend-sdk/tests/auth.test.ts`:
- Around line 4-5: The tests create a shared client instance (const client =
urBackend({ apiKey: mockApiKey })) which can carry auth state across tests;
change this to recreate the client in a beforeEach hook: declare a let client
variable at the top, keep mockApiKey, and instantiate client = urBackend({
apiKey: mockApiKey }) inside beforeEach so every test gets a fresh urBackend
client (remove reliance on logout cleanup and any top-level client
initialization).
---
Nitpick comments:
In `@sdks/urbackend-sdk/src/modules/database.ts`:
- Around line 84-97: The delete method's deleted determination is brittle
because it inspects response body fields; update the delete function to treat
any successful HTTP response (i.e., the client.request call completed without
throwing and/or returns a 2xx status) as deleted instead of relying on result.id
or result.message. Locate the delete method and the this.client.request call in
database.ts and change the logic to set deleted = true when the request succeeds
(or when a 2xx status is present if client.request exposes it), and only use
body-inspection as a fallback; also add a short comment documenting the expected
API contract for deletions in the delete method.
In `@sdks/urbackend-sdk/tests/auth.test.ts`:
- Around line 27-45: The test only asserts accessToken from login; extend
coverage to assert persisted token behavior for token rotation and legacy
fallback by adding assertions that client.auth.getToken() returns the right
value after calling refreshToken() (verify refresh returns and persists a new
refreshToken/accessToken pair) and add one deprecated-response test that
supplies a legacy "token" field in the mocked JSON and asserts
client.auth.getToken() still hydrates from that legacy token; modify tests
around client.auth.login and the existing refreshToken() call to validate both
returned values and persisted state rather than only the request shape.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8f1daa5-696c-409b-b943-f9b5a8e8b796
📒 Files selected for processing (8)
sdks/urbackend-sdk/src/client.tssdks/urbackend-sdk/src/modules/auth.tssdks/urbackend-sdk/src/modules/database.tssdks/urbackend-sdk/src/modules/schema.tssdks/urbackend-sdk/src/types/index.tssdks/urbackend-sdk/tests/auth.test.tssdks/urbackend-sdk/tests/database.test.tssdks/urbackend-sdk/tests/schema.test.ts
✅ Files skipped from review due to trivial changes (1)
- sdks/urbackend-sdk/tests/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- sdks/urbackend-sdk/src/modules/schema.ts
- sdks/urbackend-sdk/tests/database.test.ts
- sdks/urbackend-sdk/src/types/index.ts
- sdks/urbackend-sdk/src/modules/auth.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
yash-pouranik
temporarily deployed
to
feat/sdk-v0.2.0-upgrade - urBackend-frankfrut PR #90
Render
Destroyed
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdks/urbackend-sdk/tests/auth.test.ts (2)
165-168:publicProfileassertion is too permissive on request options.
expect.any(Object)can pass even with wrong HTTP verb/options. Tighten this to assert at least the method.Proposed test hardening
expect(fetchMock).toHaveBeenCalledWith( expect.stringContaining('/api/userAuth/public/yash'), - expect.any(Object), + expect.objectContaining({ + method: 'GET', + }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/tests/auth.test.ts` around lines 165 - 168, The test currently uses expect.any(Object) for the fetch options which allows wrong HTTP verbs; update the assertion that calls fetchMock (the expect(fetchMock)... line) to assert the request options include the correct HTTP method (e.g., use expect.objectContaining({ method: 'GET' }) or the right verb used by publicProfile) and any required headers instead of expect.any(Object) so the test fails if the request uses the wrong HTTP verb or missing critical options.
97-104: Strengthen logout request contract assertion.
logout()currently also depends on cookie mode; assertingcredentials: 'include'here would better protect against regressions.Proposed test hardening
expect(logoutFetchMock).toHaveBeenCalledWith( expect.stringContaining('/api/userAuth/logout'), expect.objectContaining({ method: 'POST', + credentials: 'include', headers: expect.objectContaining({ Authorization: 'Bearer test-token', }), }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/urbackend-sdk/tests/auth.test.ts` around lines 97 - 104, The test for logout() currently asserts the logout fetch URL and Authorization header but omits cookie mode; update the assertion on logoutFetchMock to also expect objectContaining({ credentials: 'include' }) so the fetch call includes cookies. Locate the call checked in auth.test.ts (logoutFetchMock) and add the credentials: 'include' expectation alongside method and headers to harden the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdks/urbackend-sdk/tests/auth.test.ts`:
- Around line 165-168: The test currently uses expect.any(Object) for the fetch
options which allows wrong HTTP verbs; update the assertion that calls fetchMock
(the expect(fetchMock)... line) to assert the request options include the
correct HTTP method (e.g., use expect.objectContaining({ method: 'GET' }) or the
right verb used by publicProfile) and any required headers instead of
expect.any(Object) so the test fails if the request uses the wrong HTTP verb or
missing critical options.
- Around line 97-104: The test for logout() currently asserts the logout fetch
URL and Authorization header but omits cookie mode; update the assertion on
logoutFetchMock to also expect objectContaining({ credentials: 'include' }) so
the fetch call includes cookies. Locate the call checked in auth.test.ts
(logoutFetchMock) and add the credentials: 'include' expectation alongside
method and headers to harden the contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f32ea2cb-7b54-4de2-832c-c3270acde920
📒 Files selected for processing (1)
sdks/urbackend-sdk/tests/auth.test.ts
This PR upgrades the @urbackend/sdk from v0.1.1 to v0.2.0. This is a significant update that brings the TypeScript SDK to full parity with the
urBackend Public API, addressing several feature gaps, fixing bugs in token management, and adding comprehensive support for Row-Level Security
(RLS) and query building.
Key Changes
🔐 Authentication Module
responses.
🗄️ Database Module
backend's suffix pattern (e.g., { price_gt: 100 }).
Publishable Keys.
📦 New Modules
🛠️ Client & Types
Publishable Keys.
Technical Notes
pass tokens into database operations: client.db.insert('posts', data, client.auth.getToken()).
Verification Results
1 Test Files 5 passed (5)
2 Tests 19 passed (19)
3 Duration 1.20s
Built with ❤️ for urBackend.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores