Sync with InsForge-sdk-js v1.5.1 - #8
Conversation
Port of the v1.5.0..v1.5.1 baseline changes:
- Auth: passwordless email OTP sign-in — AuthClient.signInWithOtp()
requests a 6-digit code via POST /api/auth/email/send-otp
(enumeration-safe generic response), and AuthClient.verifyOtp()
verifies it via POST /api/auth/sessions with method 'otp',
establishing/persisting/emitting a session like other flows.
- Storage: StorageFileApi.deleteAll() now uses the batch-delete
endpoint (DELETE .../objects with {keys}, max 1000 keys) in a single
request and returns List<DeleteObjectResult> (per-key
deleted | notFound | failed status) instead of looping DELETEs.
- Version bumped to 0.2.0 across both packages; changelogs and README
updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwfing
left a comment
There was a problem hiding this comment.
Review: Sync with InsForge-sdk-js v1.5.1
Summary: A faithful, well-tested port of the JS SDK's passwordless email-OTP sign-in and one-request batch object deletion into the Dart/Flutter SDK — no blocking issues found.
Requirements context
The repo's spec/plan docs under docs/superpowers/{specs,plans}/ predate this change — they describe the original 0.1.0 build and explicitly state "the API has no batch delete," which this PR supersedes. Since this is a sync PR, the authoritative contract is the upstream InsForge-sdk-js v1.5.0..v1.5.1 diff. I verified the Dart implementation directly against the JS source and its own tests at tag v1.5.1 (src/modules/auth/auth.ts, src/modules/storage.ts and their __tests__). Endpoints, payloads, and response shapes all match:
signInWithOtp→POST /api/auth/email/send-otpbody{email}✓verifyOtp→POST /api/auth/sessions?client_type=mobilebody{method:'otp', email, otp, name?}✓deleteAll→DELETE /api/storage/buckets/{bucket}/objectsbody{keys}, response{results:[{key,status,message?}]}✓
Findings
Critical
(none)
Suggestion
- Software engineering / test coverage —
packages/insforge/lib/src/storage/storage_file_api.dart:152-163: the defensive fallbacks indeleteAll(server returns a bare list instead of{results:[...]}, or a non-list/empty body →<DeleteObjectResult>[]) are not exercised by any test. The happy path ({results:[...]}) is well covered infile_ops_test.dart, but a case asserting the bare-list branch and the empty-fallback branch would lock in the parser's resilience. Low blast radius.
Information
- Functionality / behavioral change —
storage_file_api.dart:146:deleteAllchanges from throwing on the first failed DELETE (the old per-path loop) to issuing one batch request where per-key failures come back asstatus:'failed'entries and do not throw. This matches the JS SDK and is documented in the README/CHANGELOG, but callers upgrading from 0.1.0 who relied on an exception to signal a failed delete must now inspectresult.deleted/result.status. The return-type change (Future<void>→Future<List<DeleteObjectResult>>) is a breaking change, appropriately gated behind the 0.1.0→0.2.0 bump (pre-1.0 semver). - Security —
packages/insforge/lib/src/auth/auth_client.dart:219-251: The JS SDK passesskipAuthRefresh:trueon both OTP calls. In Dart,verifyOtpposts to/api/auth/sessions, which is already exempted from the reactive 401-refresh interceptor (http_client.dart:119-123) — so the equivalent protection is in place.signInWithOtphits the non-exempt/api/auth/email/send-otp, but this is consistent with the existingsendVerificationEmail/sendPasswordResetsiblings and is harmless (no active session → the refresh callback no-ops). The enumeration-safe generic server message is preserved. No secrets, tokens, OTPs, or PII are logged or newly exposed; no new dependencies. - Software engineering / style —
packages/insforge/test/auth/auth_client_otp_test.dart:1: the header comment references the old package layout (packages/insforge_auth/test/...) rather than the actualpackages/insforge/test/auth/...path. This is consistent with the repo-wide pattern (e.g.auth_client.dart:1,storage_file_api.dart:1carry the same staleinsforge_auth/insforge_storageheaders), so it's noted only for awareness, not as a defect introduced here.
Positives worth calling out: new DeleteObjectResult parsing mirrors the existing list() idiom (whereType<Map<dynamic,dynamic>>() + Map<String,dynamic>.from); verifyOtp reuses the shared _applySession/_clientTypeQuery plumbing exactly like signIn/verifyEmail; the version bump is in lockstep across both pubspecs, version.dart, integration_tests, and samples; and the OTP suite covers the invalid-code (401) error path plus the name-omitted variant. Performance: batch delete replaces N sequential DELETEs with a single request — a strict improvement.
Verdict
approved (informational — no Critical findings; explicit GitHub approval remains a human action). The two non-blocking items above are optional follow-ups.
Ports the InsForge-sdk-js
v1.5.0..v1.5.1changes into the Dart/Flutter SDK.Ported
Auth — passwordless email OTP sign-in
AuthClient.signInWithOtp(email: ...)— requests a 6-digit sign-in code viaPOST /api/auth/email/send-otp. ReturnsFuture<void>(matching the existingsendVerificationEmail/sendPasswordResetidiom); the server response is intentionally generic whether or not an account exists (enumeration-safe).AuthClient.verifyOtp(email: ..., otp: ..., name: ...)— verifies the code viaPOST /api/auth/sessionswithmethod: 'otp'(plus the existingclient_typequery), and establishes/persists/emits the session exactly likesignIn/verifyEmail.namesets the display name only on first-time user creation.Storage — batch object deletion
StorageFileApi.deleteAll(paths)now issues a single batch request (DELETE /api/storage/buckets/{bucket}/objectswith{keys}, max 1000 keys — not split client-side, matching the JS SDK) instead of one DELETE per path, and returnsList<DeleteObjectResult>(one result per key) instead ofvoid.DeleteObjectResultmodel:key,status(deleted | notFound | failed), optionalmessage, plus adeletedconvenience getter. (Mirrors the JSremove(string[])overload; the Dart SDK keeps its existingdelete/deleteAllnaming, and single-pathdeleteis unchanged.)Housekeeping
dart run tool/set_version.dart 0.2.0(pubspecs +version.dartin lockstep);integration_testsandsamples/twitter_appconstraints updated to^0.2.0.Skipped (JS-only)
PasswordSessionRequest/VerifyOtpRequest/SendOTPRequesttype exports and thesignInWithPasswordrequest-type narrowing — TypeScript union gymnastics; Dart already uses named parameters, so no equivalent is needed.createAuthActions()mirror — the Dart SDK has no SSR layer.@insforge/shared-schemasbump,tsconfig.type-tests.json, CI workflow tweaks, package.json plumbing — JS toolchain concerns.Tests
Toolchain: Dart 3.12.2 / Flutter (local toolchain).
dart run tool/set_version.dart --check— in sync ✅dart analyze .— no issues ✅packages/insforge:dart test— 188/188 passed ✅ (includes newauth_client_otp_test.dart, updatedfile_ops_test.dartbatch-delete tests, andDeleteObjectResultmodel tests)packages/insforge_flutter:flutter test— 5/5 passed ✅🤖 Generated with Claude Code
Summary by cubic
Sync with InsForge SDK JS v1.5.1 to add passwordless email OTP sign-in and one-request batch storage deletion. Bumps
insforgeandinsforge_flutterto 0.2.0.New Features
AuthClient.signInWithOtp(email)sends a 6-digit code (enumeration-safe).AuthClient.verifyOtp(email, otp, name?)verifies viamethod: 'otp'and creates a session.StorageFileApi.deleteAll(paths)now calls the batch endpoint once and returnsList<DeleteObjectResult>withstatus: deleted | notFound | failed.Migration
insforge ^0.2.0(andinsforge_flutter ^0.2.0).deleteAllnow returns a list; handle or ignore the result as needed.Written for commit 5a2c7bc. Summary will update on new commits.