Sync with InsForge-sdk-js v1.5.1 - #20
Open
agent-zhang-beihai[bot] wants to merge 1 commit into
Open
Conversation
…ct delete)
Ports the v1.5.0..v1.5.1 changes from the baseline JS SDK:
- auth.signInWithOtp(email): request a 6-digit passwordless sign-in code
via POST /api/auth/email/send-otp (enumeration-safe generic response)
- auth.verifyOtp(email, otp, name?): verify the code via
POST /api/auth/sessions with method "otp", create and persist a session
- New auth models: SendOtpRequest, SendOtpResponse, VerifyOtpRequest
- BucketApi.delete(paths) now uses the batch endpoint
(DELETE /api/storage/buckets/{bucket}/objects with a keys body, max
1000 keys) in a single request instead of deleting one-by-one, and
returns per-key results (deleted / notFound / failed)
- New storage models: DeleteObjectsRequest, DeleteObjectsResponse,
DeleteObjectResult, DeleteObjectStatus
- MockEngine unit tests for both flows; integration tests extended
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwfing
reviewed
Jul 27, 2026
jwfing
left a comment
Member
There was a problem hiding this comment.
Review — Sync with InsForge-sdk-js v1.5.1
Summary: A clean, faithful Kotlin port of the JS SDK v1.5.1 changes (email OTP sign-in + single-request batch object delete) with strong MockEngine test coverage; no blocking issues found.
Requirements context
No /docs/superpowers/ (or any spec dir) exists in this repo — assessing against the PR description, the upstream InsForge-sdk-js v1.5.1 source, and the backend contract. I verified the port line-by-line against the live sources rather than the PR narrative:
- OTP:
InsForge/InsForge-sdk-jssrc/modules/auth/auth.ts—signInWithOtp→POST /api/auth/email/send-otp{email};verifyOtp→POST /api/auth/sessionswith{...request, method:'otp'}, persists session. Kotlin matches. - Batch delete:
src/modules/storage.tsremove(string[])→DELETE /api/storage/buckets/{bucket}/objectswith{keys:[...]}. Kotlin matches. - Response/enum shapes:
InsForge/InsForgepackages/shared-schemas/src/storage-api.schema.ts—deleteObjectResultSchema={key, status: 'deleted'|'notFound'|'failed', message?}, wrapped in{results:[…]}, keys.min(1).max(1000). The KotlinDeleteObjectStatus@SerialNames (deleted/notFound/failed) andDeleteObjectResult/DeleteObjectsResponse/DeleteObjectsRequestfields match exactly. No stale/hallucinated API.
Findings
Critical
(none)
Suggestion
- Functionality / API compatibility —
src/main/kotlin/dev/insforge/storage/BucketApi.kt:172-188, 603-610:delete(Collection<String>)and thevarargoverload change their return type fromUnit→DeleteObjectsResponse, and the semantics change: the old loop threwInsforgeHttpExceptionon the first missing/failed key, whereas the batch call now returns per-keynotFound/failedwithout throwing. This is the intended port and is source-compatible for callers that ignore the result (no in-repo/samples/caller passes a collection — all existing.delete(...)sites are single-path), but it is binary-incompatible for pre-compiled consumers and silently swallows what previously surfaced as an exception. Worth an explicit release note so downstream users update their error handling. Low blast radius → non-blocking.
Information
- Software engineering — test coverage: Excellent and matches existing conventions. 8 new MockEngine unit tests (
AuthOtpTest.kt,BucketBatchDeleteTest.kt) cover the endpoint/payload,client_type,method:"otp", name-omitted-when-absent, session persistence on success vs. non-persistence on error, single-vs-batch routing, per-key result parsing, vararg delegation, and the "no silent split of >1000 keys" case (BucketBatchDeleteTest.kt:118-151). Integration tests added inAuthTest.kt/StorageTest.kt. Note: CI (./gradlew test) is the authoritative signal — I could not run gradle locally (no JDK in the review sandbox), so I relied on the PR's reported 21/21 pass plus static verification. - Functionality — input validation: Empty collection and >1000-key batches are not validated client-side; the SDK relies on the server's 400 (
min(1)/max(1000)). This mirrors the JS overload and is explicitly asserted (BucketBatchDeleteTest.ktsurfaces the 400 and confirms no split) — noted, not a defect. - Security: No security regressions.
signInWithOtpreturns the enumeration-safe genericSendOtpResponse(intent documented + tested);verifyOtpcorrectly does not persist a session on error (Auth.kt:272-274+AuthOtpTest.kt:141-163). No new dependencies; ktor pinned at 2.3.7. No secrets/PII newly logged in production paths (theprintlncalls are test-only). - Performance: Net improvement — batch delete collapses the previous one-request-per-key loop into a single
DELETE, eliminating an N+1 request pattern. No new hot-path allocations or blocking I/O.
Verdict
approved (informational — human still approves via the approve flow). Zero Critical findings; the port is accurate, well-scoped (JS-only bits correctly skipped per the PR body), and well-tested. Posting as a COMMENT per policy.
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
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.
Ports the
v1.5.0..v1.5.1changes from InsForge-sdk-js (release v1.5.1) into the Kotlin SDK.Ported
Email OTP sign-in (passwordless)
auth.signInWithOtp(email)— requests a 6-digit sign-in code viaPOST /api/auth/email/send-otp; returns the enumeration-safe genericSendOtpResponse(success, message).auth.verifyOtp(email, otp, name?)— verifies the code viaPOST /api/auth/sessions?client_type=...withmethod: "otp", returnsSignInResponse, and persists the session (in-memory StateFlow +SessionStorage) exactly likesignIn.nameis only sent when provided (applied server-side on first-time user creation).SendOtpRequest,SendOtpResponse,VerifyOtpRequest.PasswordSessionRequestnarrowing is a TS-only concern —signIn(email, password)already has the narrowed shape here).Batch object delete
BucketApi.delete(paths: Collection<String>)(and thevarargoverload) now issues a singleDELETE /api/storage/buckets/{bucket}/objectsrequest with a{"keys": [...]}body (server limit: 1000 keys) instead of the previous one-request-per-key loop, and returnsDeleteObjectsResponsewith one per-key result (deleted/notFound/failed) — mirroring the JSbucket.remove(string[])overload. Single-pathdelete(path)is unchanged.DeleteObjectsRequest,DeleteObjectsResponse,DeleteObjectResult,DeleteObjectStatus.Docs
StorageKDoc updated.Skipped (JS-only)
src/index.tstype re-exports and thePasswordSessionRequest/VerifyOtpRequestTS type gymnastics — Kotlin methods use plain parameters; new Kotlin models are public.src/ssr/auth-actions.ts(createAuthActionsOTP mirrors) — the Kotlin SDK has no SSR module.@insforge/shared-schemasbump,package.json/lockfile,tsconfig.type-tests.json, and CI workflow changes — JS packaging/tooling.Tests
./gradlew test, what CI runs): all 21 tests pass, including 8 new MockEngine tests covering: send-otp endpoint/payload, verify-otp endpoint/client_type/body (method:"otp", name omitted when absent), session persistence on success and non-persistence on error, single vs. batch delete routing, per-key result parsing, vararg delegation, and no silent splitting of >1000-key batches (single request, error surfaced)../gradlew integrationTest, live backend): the shared test instance (pg6afqz9.us-east.insforge.app) was intermittently returning 503 "No backend services available" during the run. While it was up, the newtest signInWithOtp returns generic responseandtest delete multiple files(batch endpoint with per-key result assertions) passed against the live backend.test verifyOtp with invalid code failsandtest delete with vararg— along with many pre-existing tests (signIn with valid credentials,create bucket,sendPasswordReset, …) — failed only with socket timeouts/503s from the outage (verified independently with curl: the backend returned 503 on every endpoint for 10+ minutes). No failure was an assertion caused by this change; these should be re-run once the test instance is back.Backend feature: InsForge/InsForge#1798.
🤖 Generated with Claude Code
Summary by cubic
Syncs with
InsForge-sdk-jsv1.5.1 to add passwordless email OTP sign-in and single-request batch object delete with per-key results. Also updates docs.New Features
auth.signInWithOtp(email)sends a 6-digit code; returns genericSendOtpResponse(no account enumeration).auth.verifyOtp(email, otp, name?)creates and persists a session viaPOST /api/auth/sessionswithmethod: "otp".BucketApi.delete(paths|vararg)now calls the batch endpoint once and returnsDeleteObjectsResponsewithdeleted/notFound/failed.SendOtpRequest,SendOtpResponse,VerifyOtpRequest,DeleteObjectsRequest,DeleteObjectsResponse,DeleteObjectResult,DeleteObjectStatus.Migration
BucketApi.delete(Collection|vararg)now returnsDeleteObjectsResponse(wasUnit); update call sites to handle or ignore the result.Written for commit 6381436. Summary will update on new commits.