Sync with InsForge-sdk-js v1.5.0

[Fermionic-Lyu]

Ports the storage changes from InsForge-sdk-js v1.5.0 (standard PUT create-or-replace semantics, paired with InsForge/InsForge#1760).

Ported

• upload(path, data) / upload(path, file) — standard PUT semantics: uploading to an existing key now replaces the object in place (previously the server silently auto-renamed the key). This is a server-side behavior change; the method signatures are unchanged, KDoc updated to document the new contract.
• uploadWithAutoKey(filename, data) / uploadWithAutoKey(file) — the backend no longer mints keys server-side, so the unique, collision-free key is now generated client-side (<sanitized-base>-<timestamp>-<random><ext>, mirroring the JS generateObjectKey logic exactly: [^a-zA-Z0-9-_] → -, 32-char base cap, file fallback) and uploaded through the standard upload() path. The dead POST /objects server-minting request path (uploadDirectAutoKey) is removed. Public API signatures unchanged.
• Tests — new StorageUploadUnitTest (MockEngine-based, runs in the plain test task) mirroring the new JS unit tests: exact-key PUT routing, client-minted key format/uniqueness, no fallback to the removed POST /objects endpoint, empty-data rejection, plus direct coverage of generateObjectKey (extension preservation, sanitization, truncation, fallback, collision-freedom).

Skipped

• JS test-harness/CI plumbing (vitest mocks, workflow YAML tweaks, package.json/lockfile version bump) — JS-only concerns.
• SDK-REFERENCE.md edit — the Kotlin repo has no equivalent reference doc; the contract note went into the KDoc instead.
• Version bump / changelog — this repo derives its version from git tags via axion-release and maintains no CHANGELOG file, so there is nothing to update.
• The Kotlin-specific upsert option is left as-is (it predates this change and remains API-compatible; with standard PUT semantics the header is now effectively a no-op server-side).

Notes

• Like the JS release, this requires an InsForge backend that includes the standard-PUT storage change (feat(storage): use standard PUT replacement semantics InsForge#1760); running against a pre-change backend will restore the old auto-rename behavior for upload and is unsupported.

Test results

• ./gradlew test (unit suite, integration-tagged tests excluded as configured): BUILD SUCCESSFUL — all tests pass, including the 9 new storage tests. (No JDK was on the machine; ran with a locally downloaded Temurin 17.)
• integrationTest requires a live InsForge backend and was not run; the existing test upload file with auto-generated key integration test remains valid under the new client-side key minting.

🤖 Generated with Claude Code

---

Summary by cubic

Syncs storage behavior with InsForge SDK JS v1.5.0 by adopting standard PUT create-or-replace semantics and moving auto-key generation to the client. Adds unit tests to cover the new behavior.

• New Features

  • upload(path, ...) now replaces an existing object at the same key (standard PUT).
  • uploadWithAutoKey(...) mints a unique key client-side (<sanitized-base>-<timestamp>-<random><ext>) and uploads via upload(). The server-side auto-key POST /objects path is removed.
  • KDoc updated; internal generateObjectKey added to mirror JS logic. New MockEngine unit tests verify exact-key PUT routing, key format/uniqueness, and empty-data rejection.

• Migration

  • Requires a backend with the standard-PUT storage change (feat(storage): use standard PUT replacement semantics InsForge#1760). Old backends will revert to auto-rename and are unsupported.
  • Public API unchanged. The Kotlin upsert option remains but is effectively a no-op server-side under standard PUT.

^{Written for commit e265813. Summary will update on new commits.}

[jwfing]

Review: Sync with InsForge-sdk-js v1.5.0

Summary: A clean, well-scoped port of the JS v1.5.0 storage change — standard PUT create-or-replace semantics plus client-side auto-key generation — with matching MockEngine unit tests; no blocking issues found.

Requirements context

No /docs/superpowers/ (or docs/specs/) directory exists in this repo, and there is no .claude/skills/ folder — no matching spec/plan found; assessed against the PR description, the linked JS release (InsForge-sdk-js v1.4.5…v1.5.0), and the companion backend change (InsForge/InsForge#1760). I fetched the JS v1.5.0 src/modules/storage.ts and compared its generateObjectKey line-by-line against the Kotlin port to validate the "mirrors JS exactly" claim.

Findings

Critical

(none)

Suggestion

• Software engineering / test coverage — BucketApi.kt:342-346: uploadWithAutoKey now forwards contentType/upsert/metadata through to upload(). This is new wiring, but no unit test asserts those options actually reach the PUT request (the x-upsert / x-metadata headers), nor is there coverage of the uploadWithAutoKey(file: java.io.File) overload (BucketApi.kt:372-392). A silent regression in option-forwarding wouldn't be caught. The added tests are otherwise thorough; a small assertion on forwarded headers would close the gap.

Information

• Functionality — BucketApi.kt:716-718: the extension (ext) is taken verbatim from the filename and is not sanitized before being concatenated into the object key (and thus into the request URL path in uploadDirect, BucketApi.kt:400). A filename like a.b c or one with a separator in the suffix would flow through unescaped. This is an exact parity match with the JS reference (ext = filename.slice(dotIndex), also unsanitized), so it is intentional and low blast radius — noting only for awareness.
• Functionality — BucketApi.kt:722-724: the random suffix is always exactly 6 chars drawn from Random.Default over [0-9a-z], whereas the JS source is Math.random().toString(36).slice(2, 8), which yields up to 6 chars (can be shorter when the value has trailing zeros). Functionally equivalent for collision-avoidance (both non-cryptographic, both paired with a millisecond timestamp), so "mirrors exactly" is accurate in spirit — just a minor, harmless divergence in the suffix distribution.

Dimension coverage

• Software engineering — New StorageUploadUnitTest runs in the plain test task (not @Tag("integration")), matching the CI split in build.gradle.kts:114-138; ktor-client-mock:2.3.7 is already a testImplementation dep. Coverage of the new behavior is solid (key format, sanitization, truncation, fallback, collision-freedom, exact-key PUT routing, no-POST /objects, empty-data rejection). The removed require(data.isNotEmpty()) in uploadWithAutoKey is correctly preserved via the delegated upload() (BucketApi.kt:310) and is regression-tested. Scope is tight — two files, no drive-by changes.
• Functionality — The implementation matches the JS v1.5.0 contract and backend #1760: server-side minting (POST /objects / uploadDirectAutoKey) is fully removed, auto-key routes through upload() which now correctly handles both DIRECT (PUT) and PRESIGNED (S3) backends — a slight improvement over the old auto-key path. Content-type detection still uses the original filename (BucketApi.kt:343) before the sanitized key. Public API signatures unchanged; sample apps and the existing integration test remain valid. The upsert header no-op is documented.
• Security — No security-relevant changes. No secrets/tokens/PII logged or newly returned. Object keys are not a security boundary here (access is auth-gated), so the non-crypto RNG is fine. No new dependencies.
• Performance — No new N+1 queries, hot-path loops, or blocking I/O. The auto-key path issues the same single getUploadStrategy call as before; generateObjectKey is trivial.

Verdict

approved — zero Critical findings. The two Suggestion/Information items are non-blocking. (Informational verdict for the bot report; explicit GitHub approval remains a human action.)

[jwfing]

LGTM - approved.

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/main/kotlin/dev/insforge/storage/BucketApi.kt">

<violation number="1" location="src/main/kotlin/dev/insforge/storage/BucketApi.kt:342">
P3: uploadWithAutoKey no longer validates that data is non-empty before generating the object key, so a rejected empty-data call still pays the cost of timestamp/random key generation before failing inside upload(). Low impact, but an early require(data.isNotEmpty()) would avoid the wasted work and make the failure path clearer.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: uploadWithAutoKey no longer validates that data is non-empty before generating the object key, so a rejected empty-data call still pays the cost of timestamp/random key generation before failing inside upload(). Low impact, but an early require(data.isNotEmpty()) would avoid the wasted work and make the failure path clearer.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/main/kotlin/dev/insforge/storage/BucketApi.kt, line 342:

<comment>uploadWithAutoKey no longer validates that data is non-empty before generating the object key, so a rejected empty-data call still pays the cost of timestamp/random key generation before failing inside upload(). Low impact, but an early require(data.isNotEmpty()) would avoid the wasted work and make the failure path clearer.</comment>

<file context>
@@ -326,25 +334,15 @@ internal class BucketApiImpl(
+        // Auto-key generation is a client-side convenience — the storage API
+        // has no server-side key minting — so mint a unique, collision-free
+        // key here and upload through the standard upload() path.
+        return upload(generateObjectKey(filename), data) {
+            contentType = uploadOptions.contentType ?: detectContentType(filename)
+            upsert = uploadOptions.upsert
</file context>