Skip to content

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

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#11
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 API changes from InsForge-sdk-js v1.5.0..v1.5.1 (release: email OTP sign-in; the diff also includes the storage batch-delete feature merged in the same window).

Ported

Auth — passwordless email OTP sign-in (mirrors auth.signInWithOtp / auth.verifyOtp):

  • client.auth.sign_in_with_otp(email=...) — POST /api/auth/email/send-otp; returns the enumeration-safe generic {success, message} payload (AuthEmailActionResponse).
  • client.auth.verify_otp(email=..., otp=..., name=None) — POST /api/auth/sessions?client_type=server with method: "otp" set last so it cannot be clobbered; returns AuthSessionResponse (user + tokens). name is only applied on first-time passwordless user creation.
  • New request models AuthSendOtpRequest / AuthVerifyOtpRequest (pydantic, non-empty field validation), matching SendOTPRequest / VerifyOtpRequest from @insforge/shared-schemas@1.2.1.

Storage — batch object deletion (mirrors the bucket.remove(paths[]) overload):

  • client.storage.delete_objects(bucket_name, object_keys) — DELETE /api/storage/buckets/{bucket}/objects with {"keys": [...]} (server limit 1000 keys, not split client-side); returns per-key results.
  • New models StorageDeleteObjectResult (status: "deleted" | "notFound" | "failed") and StorageDeleteObjectsResponse, exported from insforge.storage. Implemented as a separate method rather than a JS-style overload — idiomatic Python, keeps delete_object unchanged.

Also: README examples for both features; version bumped 0.1.0 → 0.2.0 (additive feature release; repo maintains version in pyproject.toml, no changelog file exists).

Skipped (JS-only)

  • @insforge/shared-schemas dependency bump, package.json/lockfile, tsconfig type-test config, CI action bumps — build plumbing with no Python equivalent (schema shapes were ported as pydantic models instead).
  • SSR createAuthActions() OTP mirroring — this SDK has no SSR/cookie module.
  • TS overload/type gymnastics for remove() and the PasswordSessionRequest narrowing — the Python sign_in_with_password signature already only accepts email/password.
  • Browser-mode session persistence / client_type=mobile behavior in verifyOtp — this SDK is stateless server-side and uses client_type=server, consistent with sign_in_with_password / verify_email.

Tests

Added 6 tests following existing patterns (request capture via fake http_client.request): OTP send payload/endpoint, verify payload with method: "otp" + session response, optional-name omission + 401 → InsforgeAuthError, empty-field validation, batch delete endpoint/payload/per-key results, and oversized-batch error passthrough (single request, no client-side splitting).

pytest tests -q (Python 3.12): 91 passed, 15 skipped (skips are pre-existing integration tests requiring a live backend).

🤖 Generated with Claude Code


Summary by cubic

Sync with InsForge-sdk-js v1.5.1 to add passwordless email OTP sign-in and batch storage object deletion. Docs updated and version bumped to 0.2.0.

  • New Features
    • Auth: auth.sign_in_with_otp(email) and auth.verify_otp(email, otp, name=None) for OTP-based sign-in; enumeration-safe send response; sessions created with client_type=server.
    • Storage: storage.delete_objects(bucket_name, object_keys) deletes up to 1000 objects in one call; returns per-key status (deleted | notFound | failed); exports StorageDeleteObjectResult and StorageDeleteObjectsResponse.

Written for commit 639f4c7. Summary will update on new commits.

Review in cubic

…deletion

Port the API additions from InsForge-sdk-js v1.5.0..v1.5.1:

- auth: add sign_in_with_otp(email=...) to request a passwordless 6-digit
  sign-in code via POST /api/auth/email/send-otp (enumeration-safe generic
  response), and verify_otp(email=..., otp=..., name=None) to verify the
  code and create a session via POST /api/auth/sessions with method "otp".
- storage: add delete_objects(bucket_name, object_keys) to delete up to
  1000 objects in one DELETE /api/storage/buckets/{bucket}/objects request,
  returning per-key results (deleted | notFound | failed).
- Export StorageDeleteObjectResult / StorageDeleteObjectsResponse, document
  the new methods in the README, and bump the package version to 0.2.0.

Additive and backward-compatible; existing password/verify-email flows and
single-object deletion are unchanged.

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, well-scoped port of email OTP sign-in and storage batch-delete that faithfully mirrors the existing SDK conventions with thorough test coverage; no blocking issues found.

Requirements context

The repo does have docs/superpowers/{specs,plans}/, but the only documents there (2026-03-28-python-sdk-design.md, 2026-03-28-python-sdk.md) predate this sync and contain no mention of OTP sign-in or batch delete — so no matching spec/plan was found. I assessed against the PR description, the claimed parity with InsForge-sdk-js v1.5.1, and the established conventions in the surrounding code.

Findings

Critical

(none)

Suggestion

  • Software engineering / functionality — insforge/storage/client.py:223-243: delete_objects deliberately does not split or pre-validate the 1000-key server limit (correctly documented and covered by test_delete_objects_does_not_split_oversized_batches_and_raises_http_error). This matches the JS remove() decision, so it's fine as-is. Optional: a client-side guard raising early on len(object_keys) > 1000 (or an empty list) would give a clearer error than a round-trip 400. Non-blocking — the current passthrough behavior is intentional and tested.

Information

  • Functionality — insforge/auth/client.py:90-95: Setting payload["method"] = "otp" after model_dump() is a sound defensive choice; since AuthVerifyOtpRequest uses extra="ignore" and only defines email/otp/name, the key can't be clobbered by caller input regardless — the comment accurately describes the guarantee.
  • Software engineering: New request models (AuthSendOtpRequest, AuthVerifyOtpRequest) reuse the existing ConfigDict(extra="ignore", populate_by_name=True) + Field(min_length=1) non-empty validation pattern, and the new endpoints thread exception_cls=InsforgeAuthError (auth) / default InsforgeHTTPError (storage) exactly like their neighbors. Import ordering, by_alias=True/exclude_none=True usage, and SERVER_CLIENT_TYPE_PARAMS reuse are all consistent.

Dimension coverage

  • Software engineering: ✅ 6 new tests follow the existing fake http_client.request capture pattern and cover payload/endpoint/headers, optional-name omission, 401 → InsforgeAuthError, empty-field ValidationError, per-key result parsing, and oversized-batch passthrough. New public models are exported from insforge.storage. Version bumped 0.1.0 → 0.2.0 for an additive release. Conventions upheld.
  • Functionality:sign_in_with_otpPOST /api/auth/email/send-otp (enumeration-safe generic response); verify_otpPOST /api/auth/sessions?client_type=server with method: "otp", consistent with sign_in_with_password/verify_email/create_user. name is correctly omitted via exclude_none=True when not supplied. Batch delete posts {"keys": [...]} to /api/storage/buckets/{bucket}/objects. No gaps versus the claimed JS parity.
  • Security: ✅ No new secrets/PII logged or returned. OTP send uses the generic enumeration-safe payload. quote_path_segment is applied to the bucket name in the batch path; object keys travel in the JSON body (not the URL), so no injection surface. No auth/authorization checks weakened. No new dependencies.
  • Performance: ✅ Single request per call; batch delete is one round-trip with no client-side fan-out or N+1. list(object_keys) materializes once. No blocking/sync hot-path concerns.

Verdict

approved — zero Critical findings. One optional Suggestion and two Information notes; author may address or ignore freely. (Posted as a COMMENT; explicit GitHub approval remains a separate human action.)

@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