Skip to content

Sync with InsForge-sdk-js v1.5.1 - #20

Open
agent-zhang-beihai[bot] wants to merge 1 commit into
mainfrom
sdk-sync/v1.5.1
Open

Sync with InsForge-sdk-js v1.5.1#20
agent-zhang-beihai[bot] wants to merge 1 commit into
mainfrom
sdk-sync/v1.5.1

Conversation

@agent-zhang-beihai

@agent-zhang-beihai agent-zhang-beihai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Ports the v1.5.0..v1.5.1 changes 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 via POST /api/auth/email/send-otp; returns the enumeration-safe generic SendOtpResponse(success, message).
  • auth.verifyOtp(email, otp, name?) — verifies the code via POST /api/auth/sessions?client_type=... with method: "otp", returns SignInResponse, and persists the session (in-memory StateFlow + SessionStorage) exactly like signIn. name is only sent when provided (applied server-side on first-time user creation).
  • New models: SendOtpRequest, SendOtpResponse, VerifyOtpRequest.
  • Kotlin idiom: parameters instead of the JS request-object/discriminated-union types (PasswordSessionRequest narrowing is a TS-only concern — signIn(email, password) already has the narrowed shape here).

Batch object delete

  • BucketApi.delete(paths: Collection<String>) (and the vararg overload) now issues a single DELETE /api/storage/buckets/{bucket}/objects request with a {"keys": [...]} body (server limit: 1000 keys) instead of the previous one-request-per-key loop, and returns DeleteObjectsResponse with one per-key result (deleted / notFound / failed) — mirroring the JS bucket.remove(string[]) overload. Single-path delete(path) is unchanged.
  • New models: DeleteObjectsRequest, DeleteObjectsResponse, DeleteObjectResult, DeleteObjectStatus.
  • Oversized batches are not silently split; the server's 400 is surfaced (matches baseline behavior/tests).

Docs

  • README / GETTING_STARTED feature lists and Storage KDoc updated.

Skipped (JS-only)

  • src/index.ts type re-exports and the PasswordSessionRequest/VerifyOtpRequest TS type gymnastics — Kotlin methods use plain parameters; new Kotlin models are public.
  • src/ssr/auth-actions.ts (createAuthActions OTP mirrors) — the Kotlin SDK has no SSR module.
  • @insforge/shared-schemas bump, package.json/lockfile, tsconfig.type-tests.json, and CI workflow changes — JS packaging/tooling.
  • Version bump / changelog: this repo derives its version from git tags via axion-release and keeps no changelog file, so nothing to update.

Tests

  • Unit suite (./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).
  • Integration suite (./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 new test signInWithOtp returns generic response and test delete multiple files (batch endpoint with per-key result assertions) passed against the live backend. test verifyOtp with invalid code fails and test 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-js v1.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 generic SendOtpResponse (no account enumeration).
    • auth.verifyOtp(email, otp, name?) creates and persists a session via POST /api/auth/sessions with method: "otp".
    • Storage: BucketApi.delete(paths|vararg) now calls the batch endpoint once and returns DeleteObjectsResponse with deleted / notFound / failed.
    • New models: SendOtpRequest, SendOtpResponse, VerifyOtpRequest, DeleteObjectsRequest, DeleteObjectsResponse, DeleteObjectResult, DeleteObjectStatus.
  • Migration

    • BucketApi.delete(Collection|vararg) now returns DeleteObjectsResponse (was Unit); update call sites to handle or ignore the result.
    • Batch delete accepts up to 1000 keys per call; oversized or empty batches return a 400 error (no silent splitting).

Written for commit 6381436. Summary will update on new commits.

Review in cubic

…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 jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-js src/modules/auth/auth.tssignInWithOtpPOST /api/auth/email/send-otp {email}; verifyOtpPOST /api/auth/sessions with {...request, method:'otp'}, persists session. Kotlin matches.
  • Batch delete: src/modules/storage.ts remove(string[])DELETE /api/storage/buckets/{bucket}/objects with {keys:[...]}. Kotlin matches.
  • Response/enum shapes: InsForge/InsForge packages/shared-schemas/src/storage-api.schema.tsdeleteObjectResultSchema = {key, status: 'deleted'|'notFound'|'failed', message?}, wrapped in {results:[…]}, keys .min(1).max(1000). The Kotlin DeleteObjectStatus @SerialNames (deleted/notFound/failed) and DeleteObjectResult/DeleteObjectsResponse/DeleteObjectsRequest fields 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 the vararg overload change their return type from UnitDeleteObjectsResponse, and the semantics change: the old loop threw InsforgeHttpException on the first missing/failed key, whereas the batch call now returns per-key notFound/failed without 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 in AuthTest.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.kt surfaces the 400 and confirms no split) — noted, not a defect.
  • Security: No security regressions. signInWithOtp returns the enumeration-safe generic SendOtpResponse (intent documented + tested); verifyOtp correctly 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 (the println calls 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.

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant