From 37720785d388e6e0180c87966572c2a970ae3cc6 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 22:57:27 +0530 Subject: [PATCH 01/27] docs: add Java SDK nomenclature cleanup design spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the design for public interface renames per the server-side SDK nomenclature changes spec: credential field fallbacks (clientId/keyId/tokenUri), skyflow_id→skyflowId in Get/Query responses, and QueryResponse errors/tokenizedData field additions. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ...-05-13-java-nomenclature-cleanup-design.md | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md new file mode 100644 index 00000000..2d194871 --- /dev/null +++ b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md @@ -0,0 +1,116 @@ +# Java SDK Nomenclature Cleanup — Design Spec + +**Date:** 2026-05-13 +**Reference:** [Skyflow Server-Side SDK: Nomenclature changes](https://skyflow.atlassian.net/wiki/spaces/SDK1/pages/2933162001/Skyflow+Server-Side+SDK+Nomenclature+changes) +**Scope:** Public interface only (`src/main/java/com/skyflow/`) +**Target release:** v3.0.0 (HIGH), v3.1.0 (MEDIUM), v3.1.x (LOW audit) + +--- + +## Summary of changes + +| Priority | Change | Files | +|---|---|---| +| HIGH | `clientID` → `clientId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | +| HIGH | `keyID` → `keyId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | +| HIGH | `tokenURI` → `tokenUri` in credentials JSON parsing (with fallback) | `BearerToken.java` | +| MEDIUM | `skyflow_id` → `skyflowId` key in Get and Query response maps | `VaultController.java` | +| MEDIUM | `tokenizedData` as a real field on `QueryResponse` (always present) | `QueryResponse.java` | +| MEDIUM | `getErrors()` added to `QueryResponse` (field was missing entirely) | `QueryResponse.java` | +| LOW | Audit all builder setter/getter names — confirm no `setFooID()` pattern | `VaultConfig.java`, request builders | + +--- + +## Detailed design + +### HIGH: Credentials JSON field renames + +**Affected:** `BearerToken.java` (method `getBearerTokenFromCredentials`), +`SignedDataTokens.java` (method `generateSignedTokensFromCredentials`) + +The credentials JSON file (provided by users) currently uses `clientID`, `keyID`, `tokenURI`. +The new canonical form is `clientId`, `keyId`, `tokenUri` (acronyms treated as words, per Java camelCase convention). + +**Strategy:** Try the new key first; fall back to the old key if null. This allows existing credentials files to keep working during migration. + +```java +// clientID → clientId +JsonElement clientId = credentials.get("clientId"); +if (clientId == null) clientId = credentials.get("clientID"); +if (clientId == null) { + throw new SkyflowException(...MissingClientId...); +} + +// keyID → keyId +JsonElement keyId = credentials.get("keyId"); +if (keyId == null) keyId = credentials.get("keyID"); +if (keyId == null) { + throw new SkyflowException(...MissingKeyId...); +} + +// tokenURI → tokenUri (BearerToken only) +JsonElement tokenUri = credentials.get("tokenUri"); +if (tokenUri == null) tokenUri = credentials.get("tokenURI"); +if (tokenUri == null) { + throw new SkyflowException(...MissingTokenUri...); +} +``` + +Local variable names and private method parameter names updated to match (`clientId`, `keyId`, `tokenUri`). + +--- + +### MEDIUM: skyflow_id → skyflowId in Get and Query response maps + +**Affected:** `VaultController.java` — `getFormattedGetRecord()` and `getFormattedQueryRecord()` + +Insert and Update responses already use `skyflowId`. Get and Query currently call `putAll(fieldsOpt.get())` which passes through the raw API field name `skyflow_id`. After the `putAll`, rename the key: + +```java +if (record.containsKey("skyflow_id")) { + record.put("skyflowId", record.remove("skyflow_id")); +} +``` + +Applied in both `getFormattedGetRecord` and `getFormattedQueryRecord`. + +--- + +### MEDIUM: tokenizedData always present in QueryResponse + +**Affected:** `QueryResponse.java` + +Currently `tokenizedData` is only injected inside `toString()` as a hack — it is not a real field on the object. A caller accessing the query result programmatically cannot retrieve tokenized data. + +**Fix:** Add `tokenizedData` as a proper field, populated during construction from the API response data. Default to an empty map when absent. Expose via `getTokenizedData()`. Remove the manual injection from `toString()`. + +The `toString()` override is simplified to use `serializeNulls` Gson directly on the object. + +--- + +### MEDIUM: errors always present in QueryResponse + +**Affected:** `QueryResponse.java` + +`QueryResponse` has no `errors` field or `getErrors()` method today — errors are only referenced in `toString()` as a hardcoded `null`. A caller cannot access errors programmatically. + +**Fix:** Add `private final ArrayList> errors` (always `null` — not converted to empty list) and a `getErrors()` accessor. Consistent with `GetResponse`, `InsertResponse`, `UpdateResponse` which all have `getErrors()` returning null when no errors. + +--- + +### LOW: Audit builder setter/getter names + +**Affected:** `VaultConfig.java`, `InsertRequest`, `UpdateRequest`, `GetRequest`, `DeleteRequest`, `FileUploadRequest`, `QueryRequest` + +Confirm all methods follow `setFooId()` / `getFooId()` (title-case `Id`), not `setFooID()` (all-caps `ID`). + +From initial review: `setVaultId()`, `setClusterId()` in `VaultConfig` are already correct. Full grep audit required to confirm no remaining violations. + +--- + +## What is NOT in scope + +- `UpdateRequest.getData()` map key convention (user passes `skyflow_id` to identify the record to update — this is an input key, not a response key, and is not addressed in the spec) +- Any changes to generated REST client code under `com.skyflow.generated.*` +- `SKYFLOW_CREDENTIALS` environment variable name (stays `ALL_CAPS` per OS convention) +- Validation logic changes (null insert value handling is Python-only per spec) From 9afb24a95b8d7f4bfcdf8bb5eb998f1223546a9a Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 23:22:04 +0530 Subject: [PATCH 02/27] docs: clarify tokenizedData reasoning in nomenclature cleanup spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds explanation for why tokenizedData change is valid despite the Query API currently not returning tokens — based on V1FieldRecords schema support and cross-SDK consistency requirement. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ...-05-13-java-nomenclature-cleanup-design.md | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md index 2d194871..06840515 100644 --- a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md +++ b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md @@ -78,13 +78,28 @@ Applied in both `getFormattedGetRecord` and `getFormattedQueryRecord`. ### MEDIUM: tokenizedData always present in QueryResponse -**Affected:** `QueryResponse.java` +**Affected:** `VaultController.java` (`getFormattedQueryRecord`), `QueryResponse.java` + +**Why this change is valid:** + +The Skyflow API docs state that the Query endpoint "can't return tokens" today. However: + +1. The Fern-generated `V1FieldRecords` type explicitly defines a `tokens` field alongside `fields` — meaning the API contract already supports it and it may be populated in future. +2. The spec's cross-SDK requirement is that `tokenizedData` is always present per-record (even as an empty object), so callers don't need to null-check regardless of API version. +3. `getFormattedQueryRecord` currently ignores `record.getTokens()` entirely. The current `toString()` hack papers over this by injecting `tokenizedData: {}` into the serialized JSON string — but a caller doing `queryResponse.getFields().get(0).get("tokenizedData")` still gets `null`. -Currently `tokenizedData` is only injected inside `toString()` as a hack — it is not a real field on the object. A caller accessing the query result programmatically cannot retrieve tokenized data. +**Fix:** In `getFormattedQueryRecord`, populate `tokenizedData` from `record.getTokens()` (empty map when absent): -**Fix:** Add `tokenizedData` as a proper field, populated during construction from the API response data. Default to an empty map when absent. Expose via `getTokenizedData()`. Remove the manual injection from `toString()`. +```java +private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { + HashMap queryRecord = new HashMap<>(); + record.getFields().ifPresent(queryRecord::putAll); + queryRecord.put("tokenizedData", record.getTokens().orElse(new HashMap<>())); + return queryRecord; +} +``` -The `toString()` override is simplified to use `serializeNulls` Gson directly on the object. +Remove the manual `tokenizedData` injection hack from `QueryResponse.toString()`. The `toString()` override simplifies to standard Gson serialization with `serializeNulls`. --- From 850eedbb3454f88bfcda7d901d0a968e96182a73 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 23:24:39 +0530 Subject: [PATCH 03/27] docs: expand reasoning in Java nomenclature cleanup spec Adds detailed rationale to each design section: why the naming convention matters, why the fallback strategy was chosen over a hard cut, why skyflow_id normalization is inconsistent today, the tokenizedData API schema vs docs discrepancy, and why getErrors() is missing only from QueryResponse. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ...-05-13-java-nomenclature-cleanup-design.md | 98 +++++++++++++------ 1 file changed, 70 insertions(+), 28 deletions(-) diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md index 06840515..2e5e05a5 100644 --- a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md +++ b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md @@ -15,7 +15,7 @@ | HIGH | `keyID` → `keyId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | | HIGH | `tokenURI` → `tokenUri` in credentials JSON parsing (with fallback) | `BearerToken.java` | | MEDIUM | `skyflow_id` → `skyflowId` key in Get and Query response maps | `VaultController.java` | -| MEDIUM | `tokenizedData` as a real field on `QueryResponse` (always present) | `QueryResponse.java` | +| MEDIUM | `tokenizedData` per-record in QueryResponse (always present, even if empty) | `VaultController.java`, `QueryResponse.java` | | MEDIUM | `getErrors()` added to `QueryResponse` (field was missing entirely) | `QueryResponse.java` | | LOW | Audit all builder setter/getter names — confirm no `setFooID()` pattern | `VaultConfig.java`, request builders | @@ -23,15 +23,21 @@ ## Detailed design -### HIGH: Credentials JSON field renames +### HIGH: Credentials JSON field renames (`clientID` → `clientId`, `keyID` → `keyId`, `tokenURI` → `tokenUri`) -**Affected:** `BearerToken.java` (method `getBearerTokenFromCredentials`), -`SignedDataTokens.java` (method `generateSignedTokensFromCredentials`) +**Affected:** `BearerToken.java` (`getBearerTokenFromCredentials`), `SignedDataTokens.java` (`generateSignedTokensFromCredentials`) -The credentials JSON file (provided by users) currently uses `clientID`, `keyID`, `tokenURI`. -The new canonical form is `clientId`, `keyId`, `tokenUri` (acronyms treated as words, per Java camelCase convention). +**Why this change is needed:** -**Strategy:** Try the new key first; fall back to the old key if null. This allows existing credentials files to keep working during migration. +Java's naming convention treats acronyms as ordinary word components in camelCase identifiers — `Id` not `ID`, `Uri` not `URI`. The current field names `clientID`, `keyID`, `tokenURI` violate this by capitalising the acronym in full. This is inconsistent with the rest of the SDK (e.g. `setVaultId()`, `setClusterId()`) and breaks the "principle of least surprise" for Java developers who expect `clientId`. + +These field names are defined in the credentials JSON file that users create and pass to the SDK (either as a file path or as a credentials string). They are therefore part of the SDK's public contract — a change forces users to update their credentials files. This is a breaking change, which is why it is gated to the v3.0.0 major release. + +**Why a fallback is used instead of a hard cut:** + +A hard cut would silently break all existing integrations the moment users upgrade to v3. The try-new-first fallback gives users a transition window: credentials files with the old keys continue to work, and users can migrate at their own pace. The fallback can be removed in a future major version once the old form is fully deprecated. + +**Implementation strategy:** Try the new key first; fall back to the old key if null; throw if both are absent. ```java // clientID → clientId @@ -48,7 +54,7 @@ if (keyId == null) { throw new SkyflowException(...MissingKeyId...); } -// tokenURI → tokenUri (BearerToken only) +// tokenURI → tokenUri (BearerToken only — SignedDataTokens does not use tokenURI) JsonElement tokenUri = credentials.get("tokenUri"); if (tokenUri == null) tokenUri = credentials.get("tokenURI"); if (tokenUri == null) { @@ -56,15 +62,25 @@ if (tokenUri == null) { } ``` -Local variable names and private method parameter names updated to match (`clientId`, `keyId`, `tokenUri`). +Local variable names and private method parameter names are also updated to the new form (`clientId`, `keyId`, `tokenUri`) for internal consistency, though this has no effect on the public interface. --- -### MEDIUM: skyflow_id → skyflowId in Get and Query response maps +### MEDIUM: `skyflow_id` → `skyflowId` in Get and Query response maps **Affected:** `VaultController.java` — `getFormattedGetRecord()` and `getFormattedQueryRecord()` -Insert and Update responses already use `skyflowId`. Get and Query currently call `putAll(fieldsOpt.get())` which passes through the raw API field name `skyflow_id`. After the `putAll`, rename the key: +**Why this change is needed:** + +The Skyflow REST API returns records with a `skyflow_id` field in snake_case — this is the wire format. The Java SDK is responsible for translating the wire format into language-idiomatic representations before handing data to callers. Java is a camelCase language, and the SDK already normalises `skyflow_id` to `skyflowId` in Insert and Update responses: + +- `getFormattedBatchInsertRecord`: `insertRecord.put("skyflowId", recordObject.get("skyflow_id").getAsString())` +- `getFormattedBulkInsertRecord`: `insertRecord.put("skyflowId", record.getSkyflowId().get())` +- `getFormattedUpdateRecord`: `updateTokens.put("skyflowId", skyflowId)` + +However, `getFormattedGetRecord` and `getFormattedQueryRecord` call `putAll(fieldsOpt.get())` which passes the raw API map directly through — including `skyflow_id` in snake_case. This inconsistency means that developers who write `record.get("skyflowId")` after a Get or Query call get `null`, while the same code works after an Insert or Update. It forces callers to know which operation produced the response just to read a single field. + +**Implementation:** After `putAll`, check for the raw API key and rename it: ```java if (record.containsKey("skyflow_id")) { @@ -76,19 +92,33 @@ Applied in both `getFormattedGetRecord` and `getFormattedQueryRecord`. --- -### MEDIUM: tokenizedData always present in QueryResponse +### MEDIUM: `tokenizedData` always present per-record in QueryResponse **Affected:** `VaultController.java` (`getFormattedQueryRecord`), `QueryResponse.java` -**Why this change is valid:** +**Why this change is needed:** + +The cross-SDK spec requires that `tokenizedData` is always present on each record in a Query response, even when empty, to avoid nil-check boilerplate in caller code. -The Skyflow API docs state that the Query endpoint "can't return tokens" today. However: +**Current state (broken):** `getFormattedQueryRecord` only reads `record.getFields()` and completely ignores `record.getTokens()`. The `QueryResponse.toString()` method works around this with a hack — it manually injects `"tokenizedData": {}` into each record's JSON during serialisation: -1. The Fern-generated `V1FieldRecords` type explicitly defines a `tokens` field alongside `fields` — meaning the API contract already supports it and it may be populated in future. -2. The spec's cross-SDK requirement is that `tokenizedData` is always present per-record (even as an empty object), so callers don't need to null-check regardless of API version. -3. `getFormattedQueryRecord` currently ignores `record.getTokens()` entirely. The current `toString()` hack papers over this by injecting `tokenizedData: {}` into the serialized JSON string — but a caller doing `queryResponse.getFields().get(0).get("tokenizedData")` still gets `null`. +```java +for (JsonElement fieldElement : fieldsArray) { + fieldElement.getAsJsonObject().add("tokenizedData", new JsonObject()); +} +``` -**Fix:** In `getFormattedQueryRecord`, populate `tokenizedData` from `record.getTokens()` (empty map when absent): +This means `response.toString()` includes `tokenizedData` but `response.getFields().get(0).get("tokenizedData")` returns `null`. Any caller working with the Java object (rather than deserialising the string) cannot access tokenized data at all. + +**Why tokens are relevant despite the API docs:** + +The Skyflow API documentation states that the Query endpoint "can't return tokens" currently. However: + +1. The Fern-generated `V1FieldRecords` type (auto-generated from the API spec) explicitly declares a `tokens` field alongside `fields`, proving the API contract already accommodates token data in query records. The docs may lag behind the schema, or this may be intentional forward compatibility. +2. `getFormattedGetRecord` uses the same `V1FieldRecords` type and also ignores `record.getTokens()` — a parallel gap that should be fixed consistently. +3. The spec's requirement is about SDK response-shape consistency across all operations, not about what the API returns today. Callers should be able to write uniform record-access code regardless of which operation produced the response. + +**Fix:** Read `record.getTokens()` in `getFormattedQueryRecord` and always add it to the record map under the `tokenizedData` key, defaulting to an empty map when absent: ```java private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { @@ -99,17 +129,25 @@ private static synchronized HashMap getFormattedQueryRecord(V1Fi } ``` -Remove the manual `tokenizedData` injection hack from `QueryResponse.toString()`. The `toString()` override simplifies to standard Gson serialization with `serializeNulls`. +The `toString()` hack in `QueryResponse.java` is removed. The `toString()` override simplifies to standard Gson serialisation with `serializeNulls`, since the data is now correctly in the map. --- -### MEDIUM: errors always present in QueryResponse +### MEDIUM: `getErrors()` added to `QueryResponse` **Affected:** `QueryResponse.java` -`QueryResponse` has no `errors` field or `getErrors()` method today — errors are only referenced in `toString()` as a hardcoded `null`. A caller cannot access errors programmatically. +**Why this change is needed:** -**Fix:** Add `private final ArrayList> errors` (always `null` — not converted to empty list) and a `getErrors()` accessor. Consistent with `GetResponse`, `InsertResponse`, `UpdateResponse` which all have `getErrors()` returning null when no errors. +All other response types in the SDK (`GetResponse`, `InsertResponse`, `UpdateResponse`, `FileUploadResponse`) expose a `getErrors()` method. `QueryResponse` is the only one that does not — the `errors` field is referenced only inside `toString()` as a hardcoded literal `null`: + +```java +responseObject.add("errors", null); +``` + +A caller who writes `queryResponse.getErrors()` gets a compile error because the method does not exist. This breaks the consistency contract that callers rely on when writing generic response-handling code across different vault operations. + +**Fix:** Add `private final ArrayList> errors` as a constructor field (always `null` — consistent with other response types that pass `null` when there are no errors) and expose it via `getErrors()`. The field will always be `null` for QueryResponse since the Query API does not currently model partial-error responses the same way batch insert does. This is kept as `null` rather than an empty list to stay consistent with the existing pattern across other response classes. --- @@ -117,15 +155,19 @@ Remove the manual `tokenizedData` injection hack from `QueryResponse.toString()` **Affected:** `VaultConfig.java`, `InsertRequest`, `UpdateRequest`, `GetRequest`, `DeleteRequest`, `FileUploadRequest`, `QueryRequest` -Confirm all methods follow `setFooId()` / `getFooId()` (title-case `Id`), not `setFooID()` (all-caps `ID`). +**Why this change is needed:** + +The same acronym-casing rule that applies to credentials fields applies to all Java method names. Any setter or getter using `ID` (all-caps) as a suffix — e.g. `setVaultID()`, `getSkyflowID()` — is non-idiomatic and inconsistent with Java convention. The spec item 15 calls out this as a verification task. + +From initial review, `setVaultId()` and `setClusterId()` in `VaultConfig` are already correct. A full grep audit across all request builder classes is required to confirm there are no remaining `setFooID()` / `getFooID()` methods that were missed. -From initial review: `setVaultId()`, `setClusterId()` in `VaultConfig` are already correct. Full grep audit required to confirm no remaining violations. +**Outcome:** If any violations are found, rename them to `setFooId()` / `getFooId()`. If none are found, this item is closed as verified-clean. --- ## What is NOT in scope -- `UpdateRequest.getData()` map key convention (user passes `skyflow_id` to identify the record to update — this is an input key, not a response key, and is not addressed in the spec) -- Any changes to generated REST client code under `com.skyflow.generated.*` -- `SKYFLOW_CREDENTIALS` environment variable name (stays `ALL_CAPS` per OS convention) -- Validation logic changes (null insert value handling is Python-only per spec) +- **`UpdateRequest.getData()` map key**: Users currently pass `skyflow_id` (snake_case) in the data map to identify the record to update. This is an *input* key consumed by the SDK internally (`updateRequest.getData().remove("skyflow_id")`), not a response field surfaced to callers. The spec does not address this and changing it would require a separate design decision. +- **Generated REST client code** under `com.skyflow.generated.*`: These files are auto-generated by Fern from the API definition. Manual edits would be overwritten on the next regeneration. +- **`SKYFLOW_CREDENTIALS` environment variable name**: Stays `ALL_CAPS` per OS and shell convention. Only the parsed field names within the JSON value change. +- **Validation logic for null/None insert values**: The spec marks this as Python-only (item 12). Java already throws on invalid input at the API boundary. From ea9efb2c551739a974ac0e164c3f024baaab3958 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 23:29:46 +0530 Subject: [PATCH 04/27] docs: correct tokenizedData implementation rationale in spec Clarifies that ignoring record.getTokens() in getFormattedQueryRecord is intentional (Query API cannot return tokens), and that the fix is to promote the toString() hack into a real always-empty field rather than reading from the API response. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../2026-05-13-java-nomenclature-cleanup-design.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md index 2e5e05a5..c56f43f6 100644 --- a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md +++ b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md @@ -110,26 +110,24 @@ for (JsonElement fieldElement : fieldsArray) { This means `response.toString()` includes `tokenizedData` but `response.getFields().get(0).get("tokenizedData")` returns `null`. Any caller working with the Java object (rather than deserialising the string) cannot access tokenized data at all. -**Why tokens are relevant despite the API docs:** +**Why `record.getTokens()` is intentionally ignored:** -The Skyflow API documentation states that the Query endpoint "can't return tokens" currently. However: +The Skyflow API documentation explicitly states the Query endpoint cannot return tokens. Ignoring `record.getTokens()` in `getFormattedQueryRecord` is therefore a deliberate design decision — not a bug. The `toString()` hack that injects `"tokenizedData": {}` confirms this intent: the author knew tokens would never be present, so they hardcoded an always-empty field for output consistency rather than trying to surface real data from the API. -1. The Fern-generated `V1FieldRecords` type (auto-generated from the API spec) explicitly declares a `tokens` field alongside `fields`, proving the API contract already accommodates token data in query records. The docs may lag behind the schema, or this may be intentional forward compatibility. -2. `getFormattedGetRecord` uses the same `V1FieldRecords` type and also ignores `record.getTokens()` — a parallel gap that should be fixed consistently. -3. The spec's requirement is about SDK response-shape consistency across all operations, not about what the API returns today. Callers should be able to write uniform record-access code regardless of which operation produced the response. +Populating `tokenizedData` from `record.getTokens()` would be wrong — it would imply the Query operation supports token retrieval, which it does not, and would mislead future maintainers. -**Fix:** Read `record.getTokens()` in `getFormattedQueryRecord` and always add it to the record map under the `tokenizedData` key, defaulting to an empty map when absent: +**Fix:** Promote the `toString()` hack into a real field. Always write an empty map under `tokenizedData` in the record — no dependency on `record.getTokens()`: ```java private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { HashMap queryRecord = new HashMap<>(); record.getFields().ifPresent(queryRecord::putAll); - queryRecord.put("tokenizedData", record.getTokens().orElse(new HashMap<>())); + queryRecord.put("tokenizedData", new HashMap<>()); // Query API cannot return tokens; always empty return queryRecord; } ``` -The `toString()` hack in `QueryResponse.java` is removed. The `toString()` override simplifies to standard Gson serialisation with `serializeNulls`, since the data is now correctly in the map. +The `toString()` hack in `QueryResponse.java` is removed. The `toString()` override simplifies to standard Gson serialisation with `serializeNulls`, since `tokenizedData` is now a real key in each record map. --- From 54cb54bd758119d8c03aad1a62ff902349dc8993 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 23:42:21 +0530 Subject: [PATCH 05/27] docs: remove tokenizedData from scope in nomenclature cleanup spec Query API cannot return tokens; the toString() inconsistency is not worth fixing since callers have no reason to access tokenizedData programmatically on query results. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- ...-05-13-java-nomenclature-cleanup-design.md | 42 +------------------ 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md index c56f43f6..9873fdcf 100644 --- a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md +++ b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md @@ -15,7 +15,6 @@ | HIGH | `keyID` → `keyId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | | HIGH | `tokenURI` → `tokenUri` in credentials JSON parsing (with fallback) | `BearerToken.java` | | MEDIUM | `skyflow_id` → `skyflowId` key in Get and Query response maps | `VaultController.java` | -| MEDIUM | `tokenizedData` per-record in QueryResponse (always present, even if empty) | `VaultController.java`, `QueryResponse.java` | | MEDIUM | `getErrors()` added to `QueryResponse` (field was missing entirely) | `QueryResponse.java` | | LOW | Audit all builder setter/getter names — confirm no `setFooID()` pattern | `VaultConfig.java`, request builders | @@ -92,45 +91,6 @@ Applied in both `getFormattedGetRecord` and `getFormattedQueryRecord`. --- -### MEDIUM: `tokenizedData` always present per-record in QueryResponse - -**Affected:** `VaultController.java` (`getFormattedQueryRecord`), `QueryResponse.java` - -**Why this change is needed:** - -The cross-SDK spec requires that `tokenizedData` is always present on each record in a Query response, even when empty, to avoid nil-check boilerplate in caller code. - -**Current state (broken):** `getFormattedQueryRecord` only reads `record.getFields()` and completely ignores `record.getTokens()`. The `QueryResponse.toString()` method works around this with a hack — it manually injects `"tokenizedData": {}` into each record's JSON during serialisation: - -```java -for (JsonElement fieldElement : fieldsArray) { - fieldElement.getAsJsonObject().add("tokenizedData", new JsonObject()); -} -``` - -This means `response.toString()` includes `tokenizedData` but `response.getFields().get(0).get("tokenizedData")` returns `null`. Any caller working with the Java object (rather than deserialising the string) cannot access tokenized data at all. - -**Why `record.getTokens()` is intentionally ignored:** - -The Skyflow API documentation explicitly states the Query endpoint cannot return tokens. Ignoring `record.getTokens()` in `getFormattedQueryRecord` is therefore a deliberate design decision — not a bug. The `toString()` hack that injects `"tokenizedData": {}` confirms this intent: the author knew tokens would never be present, so they hardcoded an always-empty field for output consistency rather than trying to surface real data from the API. - -Populating `tokenizedData` from `record.getTokens()` would be wrong — it would imply the Query operation supports token retrieval, which it does not, and would mislead future maintainers. - -**Fix:** Promote the `toString()` hack into a real field. Always write an empty map under `tokenizedData` in the record — no dependency on `record.getTokens()`: - -```java -private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { - HashMap queryRecord = new HashMap<>(); - record.getFields().ifPresent(queryRecord::putAll); - queryRecord.put("tokenizedData", new HashMap<>()); // Query API cannot return tokens; always empty - return queryRecord; -} -``` - -The `toString()` hack in `QueryResponse.java` is removed. The `toString()` override simplifies to standard Gson serialisation with `serializeNulls`, since `tokenizedData` is now a real key in each record map. - ---- - ### MEDIUM: `getErrors()` added to `QueryResponse` **Affected:** `QueryResponse.java` @@ -165,6 +125,8 @@ From initial review, `setVaultId()` and `setClusterId()` in `VaultConfig` are al ## What is NOT in scope +- **`tokenizedData` in QueryResponse:** The Skyflow Query API explicitly cannot return tokens. The existing `toString()` hack that injects `tokenizedData: {}` is a minor inconsistency between string output and programmatic access, but since callers have no reason to access tokenized data from a query result, this is not worth fixing now. + - **`UpdateRequest.getData()` map key**: Users currently pass `skyflow_id` (snake_case) in the data map to identify the record to update. This is an *input* key consumed by the SDK internally (`updateRequest.getData().remove("skyflow_id")`), not a response field surfaced to callers. The spec does not address this and changing it would require a separate design decision. - **Generated REST client code** under `com.skyflow.generated.*`: These files are auto-generated by Fern from the API definition. Manual edits would be overwritten on the next regeneration. - **`SKYFLOW_CREDENTIALS` environment variable name**: Stays `ALL_CAPS` per OS and shell convention. Only the parsed field names within the JSON value change. From 5e484d975d3dd715c38314d83db02bd4a2316c1d Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 13 May 2026 23:47:28 +0530 Subject: [PATCH 06/27] docs: add implementation plan for Java SDK nomenclature cleanup 5-task TDD plan covering credential field renames with fallback, skyflow_id normalisation in Get/Query responses, and QueryResponse getErrors() accessor. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../2026-05-13-java-nomenclature-cleanup.md | 589 ++++++++++++++++++ 1 file changed, 589 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md diff --git a/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md b/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md new file mode 100644 index 00000000..a386accd --- /dev/null +++ b/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md @@ -0,0 +1,589 @@ +# Java SDK Nomenclature Cleanup Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Rename credential JSON field keys (`clientID`→`clientId`, `keyID`→`keyId`, `tokenURI`→`tokenUri`) with fallback support, normalise `skyflow_id`→`skyflowId` in Get and Query responses, and add `getErrors()` to `QueryResponse`. + +**Architecture:** Three independent, targeted changes to existing files — no new files, no new abstractions. Each change is a surgical edit to one method or class, verified by unit tests that already exist or that we add inline. + +**Tech Stack:** Java 11+, JUnit 4, Mockito/PowerMock, Maven (`mvn test`) + +**Design spec:** `docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md` + +--- + +## File Map + +| File | Change | +|---|---| +| `src/main/java/com/skyflow/serviceaccount/util/BearerToken.java` | Fallback lookup for `clientId`/`keyId`/`tokenUri` in `getBearerTokenFromCredentials` | +| `src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java` | Fallback lookup for `clientId`/`keyId` in `generateSignedTokensFromCredentials` | +| `src/main/java/com/skyflow/vault/controller/VaultController.java` | Rename `skyflow_id`→`skyflowId` in `getFormattedGetRecord` and `getFormattedQueryRecord` | +| `src/main/java/com/skyflow/vault/data/QueryResponse.java` | Add `errors` field and `getErrors()` accessor | +| `src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java` | Add tests for new-form keys, old-form fallback, and missing-key errors | +| `src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java` | Add tests for new-form keys, old-form fallback, and missing-key errors | +| `src/test/java/com/skyflow/vault/data/QueryResponseTest.java` | New file — tests for `getErrors()` always returning null | + +--- + +## Task 1: Credential field renames in BearerToken — new key form + +**Files:** +- Modify: `src/main/java/com/skyflow/serviceaccount/util/BearerToken.java:92-145` +- Modify: `src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java` + +### Background +`getBearerTokenFromCredentials` parses a `JsonObject` representing the credentials file. It currently looks up `clientID`, `keyID`, and `tokenURI`. We need it to accept `clientId`, `keyId`, `tokenUri` (new canonical form) while still accepting the old keys as a fallback. + +The test at line 228 of `BearerTokenTests.java` currently uses the old keys — we need a parallel test using the new keys. + +- [ ] **Step 1: Write a failing test for new-form credential keys** + +Add this test to `BearerTokenTests.java`. It uses a credentials string with `clientId`, `keyId`, `tokenUri` (new form) and expects a `SkyflowException` with the `InvalidTokenUri` message (because the URI value is invalid — not because the keys are unrecognised). This confirms the new keys are read successfully. + +```java +@Test +public void testBearerTokenWithNewFormCredentialKeys() { + try { + String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\ncHJpdmF0ZV9rZXlfdmFsdWU=\\n-----END PRIVATE KEY-----\", " + + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\", \"tokenUri\": \"invalid_token_uri\"}"; + BearerToken bearerToken = BearerToken.builder().setCredentials(credentialsString).build(); + bearerToken.getBearerToken(); + Assert.fail(EXCEPTION_NOT_THROWN); + } catch (SkyflowException e) { + Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); + Assert.assertEquals(ErrorMessage.InvalidTokenUri.getMessage(), e.getMessage()); + } +} +``` + +- [ ] **Step 2: Run the test to confirm it fails** + +```bash +mvn test -pl . -Dtest=BearerTokenTests#testBearerTokenWithNewFormCredentialKeys -q +``` + +Expected: FAIL — the test throws `MissingClientId` (because `clientId` is not found, only `clientID` is looked up). + +- [ ] **Step 3: Update `getBearerTokenFromCredentials` in `BearerToken.java`** + +Replace the three field lookups (lines 102–118) with fallback logic: + +```java +JsonElement clientId = credentials.get("clientId"); +if (clientId == null) clientId = credentials.get("clientID"); +if (clientId == null) { + LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); +} + +JsonElement keyId = credentials.get("keyId"); +if (keyId == null) keyId = credentials.get("keyID"); +if (keyId == null) { + LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); +} + +JsonElement tokenUri = credentials.get("tokenUri"); +if (tokenUri == null) tokenUri = credentials.get("tokenURI"); +if (tokenUri == null) { + LogUtil.printErrorLog(ErrorLogs.TOKEN_URI_IS_REQUIRED.getLog()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingTokenUri.getMessage()); +} +``` + +Also update the `getSignedToken` call on line 121 to use the renamed variables: + +```java +String signedUserJWT = getSignedToken( + clientId.getAsString(), keyId.getAsString(), tokenUri.getAsString(), pvtKey, context +); +String basePath = Utils.getBaseURL(tokenUri.getAsString()); +``` + +Also update the private method signature at line 147–148 to use idiomatic parameter names (internal only, no public impact): + +```java +private static String getSignedToken( + String clientId, String keyId, String tokenUri, PrivateKey pvtKey, Object context +) { + final Date createdDate = new Date(); + final Date expirationDate = new Date(createdDate.getTime() + (3600 * 1000)); + io.jsonwebtoken.JwtBuilder builder = Jwts.builder() + .claim("iss", clientId) + .claim("key", keyId) + .claim("aud", tokenUri) + .claim("sub", clientId) + .expiration(expirationDate); + if (context != null) { + builder.claim("ctx", context); + } + return builder.signWith(pvtKey, Jwts.SIG.RS256).compact(); +} +``` + +- [ ] **Step 4: Run the new test to confirm it passes** + +```bash +mvn test -pl . -Dtest=BearerTokenTests#testBearerTokenWithNewFormCredentialKeys -q +``` + +Expected: PASS — `clientId` is found, execution reaches `InvalidTokenUri`. + +- [ ] **Step 5: Run the full BearerToken test suite to confirm no regressions** + +```bash +mvn test -pl . -Dtest=BearerTokenTests -q +``` + +Expected: All existing tests pass (old-form keys `clientID`/`keyID`/`tokenURI` still work via fallback). + +- [ ] **Step 6: Commit** + +```bash +git add src/main/java/com/skyflow/serviceaccount/util/BearerToken.java \ + src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java +git commit -m "feat: accept clientId/keyId/tokenUri in BearerToken with fallback to old form" +``` + +--- + +## Task 2: Credential field renames in SignedDataTokens — new key form + +**Files:** +- Modify: `src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java:92-122` +- Modify: `src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java` + +### Background +`generateSignedTokensFromCredentials` parses a credentials `JsonObject` and looks up `clientID` and `keyID`. Same rename as Task 1, but no `tokenURI` (SignedDataTokens does not need it). + +- [ ] **Step 1: Check what the existing SignedDataTokens test uses for credential keys** + +```bash +grep -n "clientID\|keyID\|clientId\|keyId" src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java +``` + +Note the line number of any credentials string that uses `clientID`/`keyID` — you will add a parallel test using the new keys. + +- [ ] **Step 2: Write a failing test for new-form keys** + +Add this test to `SignedDataTokensTests.java`. It expects the token generation to fail at the private key parsing stage (not at the field-lookup stage), confirming `clientId` and `keyId` are successfully read: + +```java +@Test +public void testSignedDataTokensWithNewFormCredentialKeys() { + try { + String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\ncHJpdmF0ZV9rZXlfdmFsdWU=\\n-----END PRIVATE KEY-----\", " + + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\"}"; + ArrayList dataTokens = new ArrayList<>(); + dataTokens.add("test-token"); + SignedDataTokens signedDataTokens = SignedDataTokens.builder() + .setCredentials(credentialsString) + .setDataTokens(dataTokens) + .build(); + signedDataTokens.getSignedDataTokens(); + Assert.fail(EXCEPTION_NOT_THROWN); + } catch (SkyflowException e) { + // Should fail past field lookup — at private key parsing, not at MissingClientId + Assert.assertNotEquals(ErrorMessage.MissingClientId.getMessage(), e.getMessage()); + Assert.assertNotEquals(ErrorMessage.MissingKeyId.getMessage(), e.getMessage()); + } +} +``` + +- [ ] **Step 3: Run the test to confirm it fails** + +```bash +mvn test -pl . -Dtest=SignedDataTokensTests#testSignedDataTokensWithNewFormCredentialKeys -q +``` + +Expected: FAIL — throws `MissingClientId` because `clientId` is not yet recognised. + +- [ ] **Step 4: Update `generateSignedTokensFromCredentials` in `SignedDataTokens.java`** + +Replace the `clientID` and `keyID` lookups (lines 103–113) with fallback logic: + +```java +JsonElement clientId = credentials.get("clientId"); +if (clientId == null) clientId = credentials.get("clientID"); +if (clientId == null) { + LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); +} + +JsonElement keyId = credentials.get("keyId"); +if (keyId == null) keyId = credentials.get("keyID"); +if (keyId == null) { + LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); +} +``` + +Update the `getSignedToken` call on line 115 to use the renamed variables: + +```java +signedDataTokens = getSignedToken( + clientId.getAsString(), keyId.getAsString(), pvtKey, dataTokens, timeToLive, context); +``` + +Update the private method signature at line 124–125 to use idiomatic parameter names: + +```java +private static List getSignedToken( + String clientId, String keyId, PrivateKey pvtKey, + ArrayList dataTokens, Integer timeToLive, Object context +) { +``` + +And update the JWT claims inside `getSignedToken` (lines 142–143): + +```java +.claim("key", keyId) +.claim("sub", clientId) +``` + +- [ ] **Step 5: Run the new test to confirm it passes** + +```bash +mvn test -pl . -Dtest=SignedDataTokensTests#testSignedDataTokensWithNewFormCredentialKeys -q +``` + +Expected: PASS — `clientId` and `keyId` are found; exception is from private key parsing, not from missing fields. + +- [ ] **Step 6: Run the full SignedDataTokens test suite** + +```bash +mvn test -pl . -Dtest=SignedDataTokensTests -q +``` + +Expected: All existing tests pass. + +- [ ] **Step 7: Commit** + +```bash +git add src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java \ + src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java +git commit -m "feat: accept clientId/keyId in SignedDataTokens with fallback to old form" +``` + +--- + +## Task 3: Normalise `skyflow_id` → `skyflowId` in Get and Query responses + +**Files:** +- Modify: `src/main/java/com/skyflow/vault/controller/VaultController.java:121-152` +- Modify: `src/test/java/com/skyflow/vault/controller/VaultControllerTests.java` + +### Background +`getFormattedGetRecord` and `getFormattedQueryRecord` call `putAll(fieldsOpt.get())` which passes through the raw API map — including the `skyflow_id` snake_case key from the wire format. Insert and Update responses already use `skyflowId`. This inconsistency means callers must know which operation produced the response in order to read the record ID. + +The test suite does not currently test the contents of Get or Query responses (no existing tests for `skyflowId` in these paths), so we add new unit tests. + +Because the actual vault API is not called in unit tests (no mock infrastructure for it in `VaultControllerTests`), we test the formatter methods indirectly by verifying the behaviour of the public `get()` and `query()` methods throw the right validation errors — and we test the formatters directly via reflection, or we add a thin package-private helper. + +The simplest approach: add package-private unit tests for the two static formatter methods directly. + +- [ ] **Step 1: Write failing tests for the formatter methods** + +Add these tests to `VaultControllerTests.java`: + +```java +import com.skyflow.generated.rest.types.V1FieldRecords; +import java.util.HashMap; +import java.util.Map; +import java.lang.reflect.Method; + +@Test +public void testGetFormattedGetRecordNormalisesSkyflowId() throws Exception { + Map fields = new HashMap<>(); + fields.put("skyflow_id", "abc-123"); + fields.put("name", "John"); + V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); + + Method method = VaultController.class.getDeclaredMethod("getFormattedGetRecord", V1FieldRecords.class); + method.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap result = (HashMap) method.invoke(null, record); + + Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); + Assert.assertEquals("skyflowId should be present", "abc-123", result.get("skyflowId")); + Assert.assertEquals("other fields should be preserved", "John", result.get("name")); +} + +@Test +public void testGetFormattedQueryRecordNormalisesSkyflowId() throws Exception { + Map fields = new HashMap<>(); + fields.put("skyflow_id", "xyz-456"); + fields.put("email", "test@example.com"); + V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); + + Method method = VaultController.class.getDeclaredMethod("getFormattedQueryRecord", V1FieldRecords.class); + method.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap result = (HashMap) method.invoke(null, record); + + Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); + Assert.assertEquals("skyflowId should be present", "xyz-456", result.get("skyflowId")); + Assert.assertEquals("other fields should be preserved", "test@example.com", result.get("email")); +} +``` + +- [ ] **Step 2: Run the tests to confirm they fail** + +```bash +mvn test -pl . -Dtest=VaultControllerTests#testGetFormattedGetRecordNormalisesSkyflowId+testGetFormattedQueryRecordNormalisesSkyflowId -q +``` + +Expected: FAIL — `skyflow_id` is present in the result, `skyflowId` is absent. + +- [ ] **Step 3: Update `getFormattedGetRecord` in `VaultController.java`** + +After the `putAll` block (after line 131), add the key rename: + +```java +private static synchronized HashMap getFormattedGetRecord(V1FieldRecords record) { + HashMap getRecord = new HashMap<>(); + + Optional> fieldsOpt = record.getFields(); + Optional> tokensOpt = record.getTokens(); + + if (fieldsOpt.isPresent()) { + getRecord.putAll(fieldsOpt.get()); + } else if (tokensOpt.isPresent()) { + getRecord.putAll(tokensOpt.get()); + } + + if (getRecord.containsKey("skyflow_id")) { + getRecord.put("skyflowId", getRecord.remove("skyflow_id")); + } + + return getRecord; +} +``` + +- [ ] **Step 4: Update `getFormattedQueryRecord` in `VaultController.java`** + +After the `putAll` block (after line 150), add the key rename: + +```java +private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { + HashMap queryRecord = new HashMap<>(); + Optional> fieldsOpt = record.getFields(); + if (fieldsOpt.isPresent()) { + queryRecord.putAll(fieldsOpt.get()); + } + + if (queryRecord.containsKey("skyflow_id")) { + queryRecord.put("skyflowId", queryRecord.remove("skyflow_id")); + } + + return queryRecord; +} +``` + +- [ ] **Step 5: Run the new tests to confirm they pass** + +```bash +mvn test -pl . -Dtest=VaultControllerTests#testGetFormattedGetRecordNormalisesSkyflowId+testGetFormattedQueryRecordNormalisesSkyflowId -q +``` + +Expected: PASS. + +- [ ] **Step 6: Run the full VaultController test suite** + +```bash +mvn test -pl . -Dtest=VaultControllerTests -q +``` + +Expected: All existing tests pass. + +- [ ] **Step 7: Commit** + +```bash +git add src/main/java/com/skyflow/vault/controller/VaultController.java \ + src/test/java/com/skyflow/vault/controller/VaultControllerTests.java +git commit -m "feat: normalise skyflow_id to skyflowId in Get and Query response maps" +``` + +--- + +## Task 4: Add `getErrors()` to `QueryResponse` + +**Files:** +- Modify: `src/main/java/com/skyflow/vault/data/QueryResponse.java` +- Create: `src/test/java/com/skyflow/vault/data/QueryResponseTest.java` + +### Background +`QueryResponse` is the only response class without a `getErrors()` method. The field is referenced in `toString()` as a hardcoded `null` literal but is not accessible programmatically. We add the field and accessor to match the pattern in `GetResponse`, `InsertResponse`, and `UpdateResponse` (all return `null` when no errors). + +- [ ] **Step 1: Write a failing test** + +Create `src/test/java/com/skyflow/vault/data/QueryResponseTest.java`: + +```java +package com.skyflow.vault.data; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.HashMap; + +public class QueryResponseTest { + + @Test + public void testGetErrorsReturnsNull() { + ArrayList> fields = new ArrayList<>(); + HashMap record = new HashMap<>(); + record.put("skyflowId", "abc-123"); + fields.add(record); + + QueryResponse response = new QueryResponse(fields); + + Assert.assertNull("getErrors() should return null when no errors", response.getErrors()); + } + + @Test + public void testGetErrorsIsPresentInToString() { + QueryResponse response = new QueryResponse(new ArrayList<>()); + String json = response.toString(); + Assert.assertTrue("toString() should include errors field", json.contains("\"errors\"")); + } +} +``` + +- [ ] **Step 2: Run the tests to confirm they fail** + +```bash +mvn test -pl . -Dtest=QueryResponseTest -q +``` + +Expected: FAIL — compile error: `getErrors()` method does not exist on `QueryResponse`. + +- [ ] **Step 3: Update `QueryResponse.java`** + +Add the `errors` field and accessor. The `toString()` no longer needs to manually inject `errors` since `serializeNulls` will include it automatically: + +```java +package com.skyflow.vault.data; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; + +import java.util.ArrayList; +import java.util.HashMap; + +public class QueryResponse { + private final ArrayList> fields; + private final ArrayList> errors; + + public QueryResponse(ArrayList> fields) { + this.fields = fields; + this.errors = null; + } + + public ArrayList> getFields() { + return fields; + } + + public ArrayList> getErrors() { + return errors; + } + + @Override + public String toString() { + Gson gson = new GsonBuilder().serializeNulls().create(); + JsonObject responseObject = gson.toJsonTree(this).getAsJsonObject(); + // tokenizedData is intentionally not surfaced — Query API cannot return tokens + JsonArray fieldsArray = responseObject.get("fields").getAsJsonArray(); + for (JsonElement fieldElement : fieldsArray) { + fieldElement.getAsJsonObject().add("tokenizedData", new JsonObject()); + } + return responseObject.toString(); + } +} +``` + +- [ ] **Step 4: Run the new tests to confirm they pass** + +```bash +mvn test -pl . -Dtest=QueryResponseTest -q +``` + +Expected: PASS. + +- [ ] **Step 5: Run the full test suite to confirm no regressions** + +```bash +mvn test -q +``` + +Expected: All tests pass. + +- [ ] **Step 6: Commit** + +```bash +git add src/main/java/com/skyflow/vault/data/QueryResponse.java \ + src/test/java/com/skyflow/vault/data/QueryResponseTest.java +git commit -m "feat: add getErrors() accessor to QueryResponse" +``` + +--- + +## Task 5: LOW audit — verify no `setFooID` / `getFooID` violations + +**Files:** +- Read-only audit (no changes expected) + +### Background +The spec requires confirming that all builder setter/getter methods use `setFooId()` / `getFooId()` (title-case `Id`), not `setFooID()`. Initial review of `VaultConfig` already shows `setVaultId()` and `setClusterId()` are correct. This task confirms nothing was missed. + +- [ ] **Step 1: Run the grep audit** + +```bash +grep -rn "set[A-Za-z]*ID\b\|get[A-Za-z]*ID\b" \ + src/main/java/com/skyflow/config/ \ + src/main/java/com/skyflow/vault/data/ \ + src/main/java/com/skyflow/serviceaccount/ \ + --include="*.java" +``` + +Expected output: **no results** — all methods already use title-case `Id`. + +- [ ] **Step 2: If violations are found, rename them** + +For each violation (e.g. `setVaultID` → `setVaultId`), use your editor's rename refactor across all callers, then run: + +```bash +mvn test -q +``` + +Expected: All tests pass. + +- [ ] **Step 3: Commit (only if changes were made)** + +```bash +git add -p +git commit -m "fix: rename setFooID/getFooID to setFooId/getFooId per Java convention" +``` + +If no violations were found, record the result: + +```bash +git commit --allow-empty -m "chore: audit confirms no setFooID/getFooID violations in public API" +``` + +--- + +## Final verification + +- [ ] **Run the complete test suite one last time** + +```bash +mvn test -q +``` + +Expected: All tests pass with no failures or errors. From 96e7e39d6b44d5e47895d444a0d0d3ca82dba4ba Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 00:03:26 +0530 Subject: [PATCH 07/27] feat: accept clientId/keyId/tokenUri in BearerToken with fallback to old form Add fallback lookup logic so getBearerTokenFromCredentials tries new camelCase keys (clientId, keyId, tokenUri) first and falls back to the legacy all-caps forms (clientID, keyID, tokenURI) for backward compatibility during migration. Add testBearerTokenWithNewFormCredentialKeys to verify the new key form is recognized end-to-end. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../serviceaccount/util/BearerToken.java | 36 ++++++++++++------- .../serviceaccount/util/BearerTokenTests.java | 15 ++++++++ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/skyflow/serviceaccount/util/BearerToken.java b/src/main/java/com/skyflow/serviceaccount/util/BearerToken.java index 9e3a6d63..4190f1de 100644 --- a/src/main/java/com/skyflow/serviceaccount/util/BearerToken.java +++ b/src/main/java/com/skyflow/serviceaccount/util/BearerToken.java @@ -99,30 +99,40 @@ private static V1GetAuthTokenResponse getBearerTokenFromCredentials( throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingPrivateKey.getMessage()); } - JsonElement clientID = credentials.get("clientID"); - if (clientID == null) { + // Accept both new-form keys (clientId/keyId/tokenUri) and legacy all-caps form for migration + JsonElement clientId = credentials.get("clientId"); + if (clientId == null) { + clientId = credentials.get("clientID"); + } + if (clientId == null) { LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); } - JsonElement keyID = credentials.get("keyID"); - if (keyID == null) { + JsonElement keyId = credentials.get("keyId"); + if (keyId == null) { + keyId = credentials.get("keyID"); + } + if (keyId == null) { LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); } - JsonElement tokenURI = credentials.get("tokenURI"); - if (tokenURI == null) { + JsonElement tokenUri = credentials.get("tokenUri"); + if (tokenUri == null) { + tokenUri = credentials.get("tokenURI"); + } + if (tokenUri == null) { LogUtil.printErrorLog(ErrorLogs.TOKEN_URI_IS_REQUIRED.getLog()); throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingTokenUri.getMessage()); } PrivateKey pvtKey = Utils.getPrivateKeyFromPem(privateKey.getAsString()); String signedUserJWT = getSignedToken( - clientID.getAsString(), keyID.getAsString(), tokenURI.getAsString(), pvtKey, context + clientId.getAsString(), keyId.getAsString(), tokenUri.getAsString(), pvtKey, context ); - String basePath = Utils.getBaseURL(tokenURI.getAsString()); + String basePath = Utils.getBaseURL(tokenUri.getAsString()); API_CLIENT_BUILDER.url(basePath); ApiClient apiClient = API_CLIENT_BUILDER.token("token").build(); AuthenticationClient authenticationApi = apiClient.authentication(); @@ -145,15 +155,15 @@ private static V1GetAuthTokenResponse getBearerTokenFromCredentials( } private static String getSignedToken( - String clientID, String keyID, String tokenURI, PrivateKey pvtKey, Object context + String clientId, String keyId, String tokenUri, PrivateKey pvtKey, Object context ) { final Date createdDate = new Date(); final Date expirationDate = new Date(createdDate.getTime() + (3600 * 1000)); io.jsonwebtoken.JwtBuilder builder = Jwts.builder() - .claim("iss", clientID) - .claim("key", keyID) - .claim("aud", tokenURI) - .claim("sub", clientID) + .claim("iss", clientId) + .claim("key", keyId) + .claim("aud", tokenUri) + .claim("sub", clientId) .expiration(expirationDate); if (context != null) { diff --git a/src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java b/src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java index ecd38e84..77a1810a 100644 --- a/src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java +++ b/src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java @@ -249,4 +249,19 @@ public void testInvalidTokenURIInCredentialsForCredentials() throws SkyflowExcep Assert.assertEquals(ErrorMessage.InvalidTokenUri.getMessage(), e.getMessage()); } } + + @Test + public void testBearerTokenWithNewFormCredentialKeys() { + try { + String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCzLp0TVwidRMtZ\\n4tGLHPDEF6ihmE4OHSR/r5rZGqE+PNtw/uwXzBrfz1Mktb0hddMZNwC2IKhHE0Yw\\nvtBT0jsfy4OUQR13Mohn9znz+5TES/yXjkvZjhZKzs5rxNw/cO8lpKYUYdwbFzwl\\n9e3joCsWBXBDCbXdLQGPyggJV+KBI0LBal+LngNLU/U680LRlybCKCTyyrF0SERD\\npytcpnq41CS2Q0ZDfkK/zLrvsCkEBU8xYeAf/TphXMKeqvMGTqxxg6IPOKfYya7Q\\nnH9eZ1pn1SCe6N5XBUpQpB4K+1IZKvadOYpYWzRgM+tT5k4UVsg6s7kUm8k9n85/\\nNQMjMY2XAgMBAAECggEASlg05ClgcaBxn0H1H3tKipImbaX7/O8qjbAW162s6V3m\\nzuN2ogkVvXcQUFL3vkJc7EFeEjNKnvLoVKFXXvADiBWw6np591MINdrmOM1R1ICS\\ntW9dGU9TAIb+LsjneYsqLrw6DIruAG+LjVSU97UlK2XmRmppAvQBid+Rpg7I9Dsy\\naJyGjDHeC3RyYYNfpei2dBPUYlUjOkBqgYGOOyjYxHzzgYtdVZku0JPtsAey3WKL\\nSbu8ryugu7r23fxP50H3FtYz91TPlVu1zVEk9Viizp2c9642ZKEoA0bB/bSNMUnt\\nZ/kemZENAzC7tnoYgwN09rI3h0+U5jaU1BhXbrLpAQKBgQDt8eaywv6j+Hdv8i7S\\nyMnZE4CaM70Z319ctJPlt2QdCZp8dtac858qnnrrZSCWV3n3yMv//bf1WZB4Lssw\\nuxBzSCFI/imG6eY9uQA6yXLl1TY9DA5IJ8s2LGzwmtA1q+vC+jzWs+0+S/evUewo\\nTZGQuNjHMHoM22jeLErqQZkHUQKBgQDAxz1WY56ZHdC3Y4aXkDeb5Ag+ZJV8Uqwn\\nootA2zHCaEx8gM9CzChCl4pQcghHFXv4eEKqezdWSK+SIRA1CtR+q8g5dP8YtAkR\\n9Uav6/fEkM8iCUvhZg+1DPRShu15nQF0ZAleSJ9OiSW5pIfAbY79RHru8H31azhE\\nDOWezXbcZwKBgB9LAAckg+62n6aWWDgadglZekFNaqI7cUQ073p3mvACslGKI4Fy\\nvM0TGKFapGWBTaYbv1CEYqwewlQ7+zcGcwxmQRJjcryuiDw312Lj2XuGheKTclFl\\nAmG2iAFAqv9UA+aZmGS4NwxJW2KwSHmocetxk/jmVDbaqDkH5DZYuDJxAoGBAJqn\\n/PRujVEnk0dc6CB1ybcd9OMhTK/ln0lY5MDOWRgvFpWXvS9InE/4RTWOlkd42/EV\\ngd5FZbqqK3hfYCI9owZQiBxYWUMXRGOM0/3Un/ypdBNJQ//7IkTMtMH0j1XOeNlI\\nXB+wwWV/L63EakgdXOag5sMEWvjl4MjvU9PX4DCnAoGAR0c567DWbkTXvcNIjvNF\\nNK8suq/fGt4dpbkkFOEHjgqFd5RsjFHKc98JVrudPweUR7YjpeKQaeNKXfVFd4+N\\nDPOs0zWSsaHckh1g9djkZlidha9SD/V6cOpxi3g2okcn/LI7h8NyNlAwDSn2mPEi\\nMd3mrgMCZwJsXLndGQSDVUw=\\n-----END PRIVATE KEY-----\\n\", " + + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\", \"tokenUri\": \"invalid_token_uri\"}"; + BearerToken bearerToken = BearerToken.builder().setCredentials(credentialsString).build(); + bearerToken.getBearerToken(); + Assert.fail(EXCEPTION_NOT_THROWN); + } catch (SkyflowException e) { + Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); + // InvalidTokenUri means new-form keys were resolved successfully — failure is at URL parsing, not field lookup + Assert.assertEquals(ErrorMessage.InvalidTokenUri.getMessage(), e.getMessage()); + } + } } From c46ced78ed33e89d7a029fc961a12840e6b48256 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 00:37:43 +0530 Subject: [PATCH 08/27] feat: accept clientId/keyId in SignedDataTokens with fallback to old form Add fallback logic to GenerateSignedTokensFromCredentials so both new-form keys (clientId/keyId) and legacy all-caps keys (clientID/keyID) are accepted during migration. Mirrors the pattern already applied to BearerToken.java. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../serviceaccount/util/SignedDataTokens.java | 23 ++++++++++++------- .../util/SignedDataTokensTests.java | 20 ++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java b/src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java index 0ce14007..b2c28ea1 100644 --- a/src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java +++ b/src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java @@ -100,20 +100,27 @@ private static List generateSignedTokensFromCredentials throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingPrivateKey.getMessage()); } - JsonElement clientID = credentials.get("clientID"); - if (clientID == null) { + // Accept both new-form keys (clientId/keyId) and legacy all-caps form for migration + JsonElement clientId = credentials.get("clientId"); + if (clientId == null) { + clientId = credentials.get("clientID"); + } + if (clientId == null) { LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); } - JsonElement keyID = credentials.get("keyID"); - if (keyID == null) { + JsonElement keyId = credentials.get("keyId"); + if (keyId == null) { + keyId = credentials.get("keyID"); + } + if (keyId == null) { LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); } PrivateKey pvtKey = Utils.getPrivateKeyFromPem(privateKey.getAsString()); signedDataTokens = getSignedToken( - clientID.getAsString(), keyID.getAsString(), pvtKey, dataTokens, timeToLive, context); + clientId.getAsString(), keyId.getAsString(), pvtKey, dataTokens, timeToLive, context); } catch (RuntimeException e) { LogUtil.printErrorLog(ErrorLogs.SIGNED_DATA_TOKENS_REJECTED.getLog()); throw new SkyflowException(e); @@ -122,7 +129,7 @@ private static List generateSignedTokensFromCredentials } private static List getSignedToken( - String clientID, String keyID, PrivateKey pvtKey, + String clientId, String keyId, PrivateKey pvtKey, ArrayList dataTokens, Integer timeToLive, Object context ) { final Date createdDate = new Date(); @@ -139,8 +146,8 @@ private static List getSignedToken( io.jsonwebtoken.JwtBuilder builder = Jwts.builder() .claim("iss", "sdk") .claim("iat", (createdDate.getTime() / 1000)) - .claim("key", keyID) - .claim("sub", clientID) + .claim("key", keyId) + .claim("sub", clientId) .claim("tok", dataToken) .expiration(expirationDate); diff --git a/src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java b/src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java index 5e2cbe60..93d69b0e 100644 --- a/src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java +++ b/src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java @@ -221,6 +221,26 @@ public void testInvalidKeySpecInCredentials() { } } + @Test + public void testSignedDataTokensWithNewFormCredentialKeys() { + try { + String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\ncHJpdmF0ZV9rZXlfdmFsdWU=\\n-----END PRIVATE KEY-----\", " + + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\"}"; + ArrayList dataTokens = new ArrayList<>(); + dataTokens.add("test-token"); + SignedDataTokens signedDataTokens = SignedDataTokens.builder() + .setCredentials(credentialsString) + .setDataTokens(dataTokens) + .build(); + signedDataTokens.getSignedDataTokens(); + Assert.fail(EXCEPTION_NOT_THROWN); + } catch (SkyflowException e) { + // Failure at RSA key parsing (not field lookup) confirms new-form keys clientId/keyId were found + Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); + Assert.assertEquals(ErrorMessage.InvalidKeySpec.getMessage(), e.getMessage()); + } + } + @Test public void testSignedDataTokenResponse() { try { From 16e6ef5b5d0529ed121d2a6a089d687bd70698d5 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 00:45:26 +0530 Subject: [PATCH 09/27] feat: normalise skyflow_id to skyflowId in Get and Query response maps Insert and Update responses already used camelCase skyflowId; Get and Query were passing through the raw wire-format snake_case key. Add the rename in getFormattedGetRecord and getFormattedQueryRecord, and add reflection-based unit tests to cover both formatters. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../vault/controller/VaultController.java | 10 ++++ .../controller/VaultControllerTests.java | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/main/java/com/skyflow/vault/controller/VaultController.java b/src/main/java/com/skyflow/vault/controller/VaultController.java index acac9608..30b6ec49 100644 --- a/src/main/java/com/skyflow/vault/controller/VaultController.java +++ b/src/main/java/com/skyflow/vault/controller/VaultController.java @@ -129,6 +129,11 @@ private static synchronized HashMap getFormattedGetRecord(V1Fiel } else if (tokensOpt.isPresent()) { getRecord.putAll(tokensOpt.get()); } + + if (getRecord.containsKey("skyflow_id")) { + getRecord.put("skyflowId", getRecord.remove("skyflow_id")); + } + return getRecord; } @@ -148,6 +153,11 @@ private static synchronized HashMap getFormattedQueryRecord(V1Fi if (fieldsOpt.isPresent()) { queryRecord.putAll(fieldsOpt.get()); } + + if (queryRecord.containsKey("skyflow_id")) { + queryRecord.put("skyflowId", queryRecord.remove("skyflow_id")); + } + return queryRecord; } diff --git a/src/test/java/com/skyflow/vault/controller/VaultControllerTests.java b/src/test/java/com/skyflow/vault/controller/VaultControllerTests.java index 4115bc2c..2c2e8995 100644 --- a/src/test/java/com/skyflow/vault/controller/VaultControllerTests.java +++ b/src/test/java/com/skyflow/vault/controller/VaultControllerTests.java @@ -10,6 +10,7 @@ import com.skyflow.errors.HttpStatus; import com.skyflow.errors.SkyflowException; import com.skyflow.generated.rest.ApiClient; +import com.skyflow.generated.rest.types.V1FieldRecords; import com.skyflow.utils.Constants; import com.skyflow.utils.Utils; import com.skyflow.vault.data.*; @@ -19,6 +20,10 @@ import org.junit.BeforeClass; import org.junit.Test; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; + public class VaultControllerTests { private static final String INVALID_EXCEPTION_THROWN = "Should not have thrown any exception"; private static final String EXCEPTION_NOT_THROWN = "Should have thrown an exception"; @@ -185,4 +190,56 @@ public void testInvalidRequestInFileUploadMethod() { } } + @Test + public void testGetFormattedGetRecordNormalisesSkyflowId() throws Exception { + Map fields = new HashMap<>(); + fields.put("skyflow_id", "abc-123"); + fields.put("name", "John"); + V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); + + Method method = VaultController.class.getDeclaredMethod("getFormattedGetRecord", V1FieldRecords.class); + method.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap result = (HashMap) method.invoke(null, record); + + Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); + Assert.assertEquals("skyflowId should be present", "abc-123", result.get("skyflowId")); + Assert.assertEquals("other fields should be preserved", "John", result.get("name")); + } + + @Test + public void testGetFormattedQueryRecordNormalisesSkyflowId() throws Exception { + Map fields = new HashMap<>(); + fields.put("skyflow_id", "xyz-456"); + fields.put("email", "test@example.com"); + V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); + + Method method = VaultController.class.getDeclaredMethod("getFormattedQueryRecord", V1FieldRecords.class); + method.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap result = (HashMap) method.invoke(null, record); + + Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); + Assert.assertEquals("skyflowId should be present", "xyz-456", result.get("skyflowId")); + Assert.assertEquals("other fields should be preserved", "test@example.com", result.get("email")); + } + + @Test + public void testGetFormattedGetRecordNormalisesSkyflowIdInTokensBranch() throws Exception { + // tokens branch: fields absent, tokens present + Map tokens = new HashMap<>(); + tokens.put("skyflow_id", "tok-789"); + tokens.put("card_number", "tok-card-abc"); + V1FieldRecords record = V1FieldRecords.builder().tokens(tokens).build(); + + Method method = VaultController.class.getDeclaredMethod("getFormattedGetRecord", V1FieldRecords.class); + method.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap result = (HashMap) method.invoke(null, record); + + Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); + Assert.assertEquals("skyflowId should be present", "tok-789", result.get("skyflowId")); + Assert.assertEquals("other token fields should be preserved", "tok-card-abc", result.get("card_number")); + } + } From c61f6b1fc58387ca0993cf0dd1ab880a19b2ff16 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 09:26:32 +0530 Subject: [PATCH 10/27] chore: add Claude Code setup (CLAUDE.md + .claude/) Adapted from skyflow-node PR #305. Includes: - CLAUDE.md with project overview, structure, naming conventions, build commands - .claude/settings.json with PostToolUse compile+checkstyle hooks, PreToolUse generated-code guard, Stop notification; paths are relative (no hardcoded user dirs) - .claude/commands/: code-review, code-security, sdk-sample, test slash commands Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 65 ++++++++++++++++++++++ .claude/commands/code-security.md | 58 ++++++++++++++++++++ .claude/commands/sdk-sample.md | 35 ++++++++++++ .claude/commands/test.md | 62 +++++++++++++++++++++ .claude/settings.json | 56 +++++++++++++++++++ CLAUDE.md | 91 +++++++++++++++++++++++++++++++ 6 files changed, 367 insertions(+) create mode 100644 .claude/commands/code-review.md create mode 100644 .claude/commands/code-security.md create mode 100644 .claude/commands/sdk-sample.md create mode 100644 .claude/commands/test.md create mode 100644 .claude/settings.json create mode 100644 CLAUDE.md diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md new file mode 100644 index 00000000..a6671b18 --- /dev/null +++ b/.claude/commands/code-review.md @@ -0,0 +1,65 @@ +You are a senior engineer performing a thorough code review on the Skyflow Java SDK. + +## Review Mode + +Use `$ARGUMENTS` to determine scope: +- `full review` — scan all files under `src/main/java/com/skyflow/` recursively (exclude `generated/`) +- A file or directory path — review only that path +- Empty / default — review files changed on current branch vs `main`: + ```bash + git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' + ``` + +## What to Review + +**Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. + +### 1. Request / Response / Options patterns +- Request builders must validate required fields in `build()` and throw `SkyflowException` with `ErrorCode.INVALID_INPUT` +- Response objects must expose typed getters — no raw `HashMap` returned without a getter +- All response classes must have `getErrors()` returning `null` (not absent) when no errors + +### 2. Error handling +- All public methods must declare `throws SkyflowException` +- `SkyflowException` must be thrown (not swallowed) on invalid input +- No `System.out.println` or bare `e.printStackTrace()` — use `LogUtil` +- Catch blocks must not silently drop exceptions + +### 3. Naming conventions +- Classes: `PascalCase` +- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI` +- Constants: `UPPER_SNAKE_CASE` +- Builder methods: `setFooId()` not `setFooID()` + +### 4. Response field normalisation +- All response maps must use `skyflowId` (camelCase), never `skyflow_id` (snake_case) +- `getErrors()` must be present on every response class + +### 5. Test coverage +- Every public method must have at least one positive and one negative test +- Tests must use `Assert.assertEquals` / `Assert.assertNull` — not just `Assert.fail` guards +- No mocking of the production class under test + +### 6. Code quality +- No magic strings — use `Constants` or `ErrorMessage` enums +- No duplicate validation logic across request classes +- Methods over 40 lines are a smell — flag for decomposition +- No `@SuppressWarnings` without a comment explaining why + +## Output Format + +Group findings by file. For each file: + +``` +### path/to/File.java + +| Severity | Line | Finding | +|---|---|---| +| Critical | 42 | SkyflowException swallowed in catch block | +| Bug | 87 | skyflow_id not normalised to skyflowId | +| Quality | 103 | Magic string "records" — use Constants | +``` + +Severities: **Critical** (data loss / silent failure) | **Bug** (wrong behaviour) | **Edge Case** (unhandled input) | **Quality** (maintainability) | **Smell** (minor style) + +End with a tech-debt summary table and a verdict: `APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES`. diff --git a/.claude/commands/code-security.md b/.claude/commands/code-security.md new file mode 100644 index 00000000..7a2ffcf6 --- /dev/null +++ b/.claude/commands/code-security.md @@ -0,0 +1,58 @@ +You are a security engineer auditing the Skyflow Java SDK for vulnerabilities. + +## Audit Scope + +Use `$ARGUMENTS` to determine target files. If none provided, run: +```bash +git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' +``` + +**Skip:** `src/main/java/com/skyflow/generated/` — observations only, no edits. + +## Security Checks + +### 1. Credential and token exposure (Critical) +- Bearer tokens, API keys, and private keys must never appear in logs, error messages, exception messages, or `toString()` output +- `Credentials` fields (`path`, `token`, `apiKey`, `credentialsString`) must not be serialised to logs +- JWT claims must not be logged + +### 2. Input validation (High) +- All string inputs from callers must be null/empty checked before use +- File paths passed to `new File(path)` must not allow path traversal (`../`) +- JSON strings parsed with `JsonParser` must be wrapped in try/catch for `JsonSyntaxException` + +### 3. Credentials file handling (High) +- Credentials files must only be read from paths provided by the caller — no environment variable path injection without sanitisation +- `FileReader` must be in a try-with-resources or explicitly closed + +### 4. HTTP security (Medium) +- All API calls must go over HTTPS — verify `Utils.getBaseURL` enforces this +- Authorization headers must not be logged at any log level +- HTTP timeouts must be configured + +### 5. Error information leakage (Medium) +- `SkyflowException` messages must not include raw server response bodies that could contain PII +- Stack traces must not be surfaced to callers — wrap in `SkyflowException` + +### 6. Dependency vulnerabilities (Low) +- Note any dependencies that are known to have CVEs (check pom.xml versions) + +### 7. Authentication lifecycle (Medium) +- Bearer token caching must check expiry before reuse +- Token refresh must be thread-safe (`synchronized` or equivalent) + +## Output Format + +For each finding: + +``` +### path/to/File.java : line N + +**Severity:** Critical / High / Medium / Low / Info +**Risk:** What an attacker could do +**Trigger:** Input or code path that triggers the vulnerability +**Fix:** Concrete remediation with code example +**CWE:** CWE-NNN +``` + +End with a summary table and overall risk rating. diff --git a/.claude/commands/sdk-sample.md b/.claude/commands/sdk-sample.md new file mode 100644 index 00000000..f984878e --- /dev/null +++ b/.claude/commands/sdk-sample.md @@ -0,0 +1,35 @@ +Create a Skyflow Java SDK sample file demonstrating: $ARGUMENTS + +## Requirements + +### File placement +- Create under `samples/src/main/java/com/example//` +- Name: `Example.java` +- Package: `com.example.` + +### Structure (follow this order) +1. Package declaration +2. Imports — only from `com.skyflow.*`, `java.*`; never from `com.skyflow.generated.*` +3. Public class with `main(String[] args) throws Exception` +4. Credentials setup using `Credentials` with `setPath()` pointing to `"credentials.json"` +5. `VaultConfig` with `setVaultId`, `setClusterId`, `setEnv(Env.PROD)` +6. `Skyflow` client via `Skyflow.builder().addVaultConfig(vaultConfig).build()` +7. Request object built via the appropriate `*Request.builder()` pattern +8. Options object if applicable (e.g. `InsertOptions`) +9. Call the vault method inside a try/catch for `SkyflowException` +10. Print the response using `System.out.println(response)` + +### Rules +- All vault IDs / cluster IDs use placeholder strings: `""`, `""` +- Credentials file path: `"credentials.json"` (relative — do not hardcode absolute paths) +- Always catch `SkyflowException` and print `e.getMessage()` +- Keep under 80 lines +- No business logic — just the minimal SDK usage pattern + +### After creating the file +Run a compile check: +```bash +cd samples && mvn compile -q 2>&1 | tail -20 +``` + +Report the file path and any compile errors. diff --git a/.claude/commands/test.md b/.claude/commands/test.md new file mode 100644 index 00000000..7e7bdcb2 --- /dev/null +++ b/.claude/commands/test.md @@ -0,0 +1,62 @@ +Run the Skyflow Java SDK quality pipeline. + +Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite. + +## Pipeline + +### Step 1 — Compile +```bash +mvn compile -q 2>&1 | tail -20 +``` +Expected: no output (clean compile). Report any errors. + +### Step 2 — Checkstyle +```bash +mvn checkstyle:check -q 2>&1 | tail -20 +``` +Expected: BUILD SUCCESS. Report any violations (excludes `generated/` per pom.xml config). + +### Step 3 — Build +```bash +mvn package -DskipTests -q 2>&1 | tail -20 +``` +Expected: BUILD SUCCESS. + +### Step 4 — Tests +If `$ARGUMENTS` is set: +```bash +mvn test -Dtest=$ARGUMENTS -q 2>&1 | tail -40 +``` +Otherwise: +```bash +mvn test -q 2>&1 | tail -40 +``` +Report: tests run, failures, errors. Flag any pre-existing failures separately from new ones. + +### Step 5 — Coverage analysis +Flag any public interface class (`src/main/java/com/skyflow/vault/`, `src/main/java/com/skyflow/config/`, `src/main/java/com/skyflow/serviceaccount/`) that has no corresponding test file under `src/test/`. + +For classes that do have tests, check whether each public method has at least one positive and one negative test case. List any gaps. + +### Step 6 — Edge case identification +For any test class below complete coverage, identify missing scenarios: +- Null / empty inputs +- Invalid types / wrong enum values +- Concurrent / reuse scenarios +- Error paths (API rejection, network failure) + +Write concrete JUnit 4 test method stubs (not full implementations) for each gap. + +### Step 7 — Report + +``` +| Step | Status | Notes | +|---|---|---| +| Compile | ✅ / ❌ | ... | +| Checkstyle | ✅ / ❌ | ... | +| Build | ✅ / ❌ | ... | +| Tests | ✅ / ❌ | N passed, M failed | +| Coverage gaps | ... | list classes | +``` + +Conclude with **READY TO MERGE** or **NEEDS FIXES** and a prioritised fix list. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..be1f09bc --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,56 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n subprocess.run(['mvn', 'checkstyle:check', '-q'], capture_output=True)\n\"" + } + ] + }, + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n r = subprocess.run(['mvn', 'compile', '-q'], capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "python3 -c \"import sys,json; d=json.load(sys.stdin); p=d.get('tool_input',{}).get('file_path',d.get('file_path','')); banned='generated'; (sys.stderr.write('BLOCKED: Fern-generated code — do not edit manually\\n'), sys.exit(2)) if banned in p and 'src/main/java/com/skyflow/generated' in p else sys.exit(0)\"" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "osascript -e 'display notification \"Claude finished\" with title \"Claude Code\"' 2>/dev/null || true" + } + ] + } + ] + }, + "permissions": { + "allow": [ + "Bash(mvn *)", + "Bash(java *)", + "Bash(python3 *)" + ], + "deny": [ + "Edit(src/main/java/com/skyflow/generated/**)", + "Write(src/main/java/com/skyflow/generated/**)" + ] + } +} diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..1c7fa068 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,91 @@ +# Skyflow Java SDK — Claude Code Instructions + +## Project Overview + +This is the Skyflow Java SDK (`skyflow-java`). It provides a Java interface to the Skyflow Data Privacy Vault API — vault operations (insert, get, update, delete, query, tokenize, detokenize), service account authentication (bearer tokens, signed data tokens), connections, detect, and audit. + +**Current version:** 2.x (v3.0.0 in preparation — see `docs/superpowers/specs/`) + +## Critical Boundary — Generated Code + +**Never edit files under `src/main/java/com/skyflow/generated/`.** + +These are auto-generated by [Fern](https://buildwithfern.com) from the Skyflow API definition. Manual edits are overwritten on the next generation run. If you find a bug in generated code, report it — do not patch it directly. + +The `pom.xml` checkstyle and test configs already exclude `generated/` from all checks. + +## Project Structure + +``` +src/ + main/java/com/skyflow/ + config/ # VaultConfig, Credentials, ConnectionConfig + vault/ + controller/ # VaultController — core SDK logic, API call orchestration + data/ # Request/Response objects: InsertRequest, GetResponse, etc. + tokens/ # DetokenizeRequest/Response, TokenizeRequest/Response + connection/ # InvokeConnectionRequest/Response + audit/ # ListEventRequest/Response + detect/ # Deidentify/Reidentify requests/responses + serviceaccount/ + util/ # BearerToken, SignedDataTokens — credential parsing + JWT + enums/ # LogLevel, RedactionType, TokenMode, Env, etc. + errors/ # SkyflowException, ErrorCode, ErrorMessage + utils/ # Utils, Constants, HttpUtility, LogUtil + generated/ # ← FERN-GENERATED, DO NOT EDIT + test/java/com/skyflow/ + ... # JUnit 4 tests mirroring the main structure +samples/ # Standalone Maven project with usage examples +docs/ + superpowers/ + specs/ # Design specs for in-progress features + plans/ # Implementation plans +``` + +## Naming Conventions + +- **Acronyms as words:** `skyflowId` (not `skyflowID`), `clientId` (not `clientID`), `tokenUri` (not `tokenURI`), `keyId` (not `keyID`) +- **Builder setters:** `setVaultId()`, `setClusterId()`, `setSkyflowId()` — never `setVaultID()` +- **Response maps:** always use `skyflowId` (camelCase) — the raw API returns `skyflow_id` (snake_case) which VaultController normalises before returning to callers +- **Constants class:** use `com.skyflow.utils.Constants` for string literals; `ErrorMessage` enum for error message strings + +## Build and Test + +```bash +mvn compile -q # compile +mvn checkstyle:check -q # lint (config: checkstyle.xml) +mvn test -q # full test suite (JUnit 4) +mvn test -Dtest=ClassName # single test class +mvn package -DskipTests -q # build jar +``` + +Samples (separate Maven project): +```bash +cd samples && mvn compile -q +``` + +## Credentials JSON Format + +The SDK reads a `credentials.json` file for service account authentication. The canonical field names (v3+) are: + +```json +{ + "clientId": "...", + "keyId": "...", + "tokenUri": "...", + "privateKey": "..." +} +``` + +The legacy all-caps forms (`clientID`, `keyID`, `tokenURI`) are accepted as fallbacks for migration. + +## Active Work + +See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers/plans/` for implementation plans. + +## Slash Commands + +- `/code-review` — code review against SDK patterns (see `.claude/commands/code-review.md`) +- `/code-security` — security audit (see `.claude/commands/code-security.md`) +- `/sdk-sample ` — generate a sample file for a feature +- `/test [ClassName]` — run quality pipeline (compile → checkstyle → build → test → coverage) From dea0384b69a50a97fc73f6f47a567371a3d58875 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 09:54:12 +0530 Subject: [PATCH 11/27] chore: fix gaps and inaccuracies in Claude setup files - CLAUDE.md: add vault/bin/ package, all 5 controllers, pre-existing test failure baseline - settings.json: fix checkstyle hook to print violations (was silently swallowing output with capture_output=True) - sdk-sample.md: fix InsertOptions (doesn't exist), correct sample package structure, correct credential type per feature - code-review.md: fix validation location (controller not build()), fix HashMap rule (SDK pattern is raw HashMaps) - test.md: document pre-existing failures, note checkstyle failsOnError Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 7 ++-- .claude/commands/sdk-sample.md | 67 ++++++++++++++++++++++----------- .claude/commands/test.md | 12 +++++- .claude/settings.json | 2 +- CLAUDE.md | 19 +++++++++- 5 files changed, 79 insertions(+), 28 deletions(-) diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md index a6671b18..6bd6a69c 100644 --- a/.claude/commands/code-review.md +++ b/.claude/commands/code-review.md @@ -15,9 +15,10 @@ Use `$ARGUMENTS` to determine scope: **Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. ### 1. Request / Response / Options patterns -- Request builders must validate required fields in `build()` and throw `SkyflowException` with `ErrorCode.INVALID_INPUT` -- Response objects must expose typed getters — no raw `HashMap` returned without a getter -- All response classes must have `getErrors()` returning `null` (not absent) when no errors +- Request builders are plain data holders — validation happens in `Validations.validateXxxRequest()` inside the controller, not in `build()`. Flag if validation logic is duplicated outside `Validations`. +- Response getters returning `ArrayList>` is the established SDK pattern — do not flag these as violations. +- All response classes must have `getErrors()` returning `null` (not absent) when no errors. `QueryResponse` is the historical exception — it now has `getErrors()` too. +- No separate `*Options` classes exist — options are fields on the request builder itself. ### 2. Error handling - All public methods must declare `throws SkyflowException` diff --git a/.claude/commands/sdk-sample.md b/.claude/commands/sdk-sample.md index f984878e..79e702aa 100644 --- a/.claude/commands/sdk-sample.md +++ b/.claude/commands/sdk-sample.md @@ -1,33 +1,58 @@ Create a Skyflow Java SDK sample file demonstrating: $ARGUMENTS -## Requirements +## File placement -### File placement -- Create under `samples/src/main/java/com/example//` -- Name: `Example.java` -- Package: `com.example.` +| Feature type | Package | Directory | +|---|---|---| +| Vault ops (insert/get/update/delete/query/tokenize) | `com.example.vault` | `samples/src/main/java/com/example/vault/` | +| Service account auth | `com.example.serviceaccount` | `samples/src/main/java/com/example/serviceaccount/` | +| Connection | `com.example.connection` | `samples/src/main/java/com/example/connection/` | +| Detect | `com.example.detect` | `samples/src/main/java/com/example/detect/` | + +File name: `Example.java` + +## Structure (follow this order) -### Structure (follow this order) 1. Package declaration 2. Imports — only from `com.skyflow.*`, `java.*`; never from `com.skyflow.generated.*` -3. Public class with `main(String[] args) throws Exception` -4. Credentials setup using `Credentials` with `setPath()` pointing to `"credentials.json"` -5. `VaultConfig` with `setVaultId`, `setClusterId`, `setEnv(Env.PROD)` -6. `Skyflow` client via `Skyflow.builder().addVaultConfig(vaultConfig).build()` -7. Request object built via the appropriate `*Request.builder()` pattern -8. Options object if applicable (e.g. `InsertOptions`) -9. Call the vault method inside a try/catch for `SkyflowException` -10. Print the response using `System.out.println(response)` - -### Rules -- All vault IDs / cluster IDs use placeholder strings: `""`, `""` -- Credentials file path: `"credentials.json"` (relative — do not hardcode absolute paths) +3. Public class with `main(String[] args) throws SkyflowException` +4. Credentials setup — choose based on feature: + - **Vault ops:** `credentials.setApiKey("")` or `credentials.setCredentialsString("")` + - **Service account:** `credentials.setPath("credentials.json")` (path to the service account JSON file) +5. `VaultConfig` with `setVaultId`, `setClusterId`, `setEnv(Env.PROD)`, `setCredentials(credentials)` +6. Build the Skyflow client: + ```java + Skyflow skyflowClient = Skyflow.builder() + .setLogLevel(LogLevel.DEBUG) + .addVaultConfig(vaultConfig) + .build(); + ``` +7. Request object via `*Request.builder()` — options go directly on the builder (no separate Options class): + ```java + // Example: InsertRequest with tokenMode + InsertRequest request = InsertRequest.builder() + .table("...") + .values(records) + .tokenMode(TokenMode.ENABLE) + .build(); + ``` +8. Call the vault method inside a try/catch for `SkyflowException`: + ```java + InsertResponse response = skyflowClient.vault().insert(request); + System.out.println(response); + ``` + +## Rules + +- Vault IDs / cluster IDs use placeholders: `""`, `""` +- Credential values use placeholders: `""`, `""` +- Credentials file path: `"credentials.json"` (relative — no absolute paths) - Always catch `SkyflowException` and print `e.getMessage()` +- No separate `*Options` classes — they don't exist in this SDK; use request builder methods - Keep under 80 lines -- No business logic — just the minimal SDK usage pattern -### After creating the file -Run a compile check: +## After creating the file + ```bash cd samples && mvn compile -q 2>&1 | tail -20 ``` diff --git a/.claude/commands/test.md b/.claude/commands/test.md index 7e7bdcb2..98397f8f 100644 --- a/.claude/commands/test.md +++ b/.claude/commands/test.md @@ -2,6 +2,16 @@ Run the Skyflow Java SDK quality pipeline. Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite. +## Known Pre-existing Failures (not regressions) + +Before reporting failures, check against this baseline: +- `HttpUtilityTests` — ALL tests fail (JDK 21 + PowerMock `InaccessibleObject` incompatibility) +- `TokenTests#testExpiredTokenForIsExpiredToken` — needs live credentials +- `VaultClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var +- `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var + +Baseline: 374 tests, ~5 failures, ~4 errors. Only report failures **beyond** this baseline. + ## Pipeline ### Step 1 — Compile @@ -14,7 +24,7 @@ Expected: no output (clean compile). Report any errors. ```bash mvn checkstyle:check -q 2>&1 | tail -20 ``` -Expected: BUILD SUCCESS. Report any violations (excludes `generated/` per pom.xml config). +Note: `failsOnError=false` in pom.xml means the build will not fail even if violations exist — check the output for `[WARN]` checkstyle lines. Violations are excluded from `generated/` by pom config. ### Step 3 — Build ```bash diff --git a/.claude/settings.json b/.claude/settings.json index be1f09bc..3c084d46 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n subprocess.run(['mvn', 'checkstyle:check', '-q'], capture_output=True)\n\"" + "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n r = subprocess.run(['mvn', 'checkstyle:check', '-q'], capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" } ] }, diff --git a/CLAUDE.md b/CLAUDE.md index 1c7fa068..30753abc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,11 +21,13 @@ src/ main/java/com/skyflow/ config/ # VaultConfig, Credentials, ConnectionConfig vault/ - controller/ # VaultController — core SDK logic, API call orchestration + controller/ # VaultController, AuditController, BinLookupController, + # ConnectionController, DetectController data/ # Request/Response objects: InsertRequest, GetResponse, etc. tokens/ # DetokenizeRequest/Response, TokenizeRequest/Response connection/ # InvokeConnectionRequest/Response audit/ # ListEventRequest/Response + bin/ # GetBinRequest/Response (BIN lookup) detect/ # Deidentify/Reidentify requests/responses serviceaccount/ util/ # BearerToken, SignedDataTokens — credential parsing + JWT @@ -35,7 +37,7 @@ src/ generated/ # ← FERN-GENERATED, DO NOT EDIT test/java/com/skyflow/ ... # JUnit 4 tests mirroring the main structure -samples/ # Standalone Maven project with usage examples +samples/ # Standalone Maven project — com.example.vault / .serviceaccount / .detect / .connection docs/ superpowers/ specs/ # Design specs for in-progress features @@ -79,6 +81,19 @@ The SDK reads a `credentials.json` file for service account authentication. The The legacy all-caps forms (`clientID`, `keyID`, `tokenURI`) are accepted as fallbacks for migration. +## Known Pre-existing Test Failures + +These failures exist on `main` and are **not regressions** — do not investigate them unless specifically asked: + +| Test class | Failure | Cause | +|---|---|---| +| `HttpUtilityTests` | `InaccessibleObject` (all tests) | JDK 21 + PowerMock incompatibility — PowerMock cannot reflect into `java.net` | +| `TokenTests#testExpiredTokenForIsExpiredToken` | Environment error | Requires live credentials | +| `VaultClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | +| `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | + +Run `mvn test -q 2>&1 | grep -E "Tests run|FAIL|ERROR"` to see current baseline (374 tests, ~5 failures, ~4 errors). + ## Active Work See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers/plans/` for implementation plans. From 33ee7cd792fccfbf3a578d175343182ed68a7a0c Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Thu, 14 May 2026 15:04:00 +0530 Subject: [PATCH 12/27] chore: segregate code smells into dedicated section in code-review command Splits old Section 6 into: - Section 6: Code quality (actionable correctness checks) - Section 7: Code smells (structural signals, flagged at Smell severity) Code smell catalogue covers: long methods/classes, business logic in data classes, toString() with logic, deep nesting, magic numbers, raw HashMap chains, dead code, stale comments, temporary fields. Severity table clarified: Critical/Bug/Edge Case/Quality = fix before merge; Smell = flag and track. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 113 +++++++++++++++++++++++++------- 1 file changed, 91 insertions(+), 22 deletions(-) diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md index 6bd6a69c..23653ac2 100644 --- a/.claude/commands/code-review.md +++ b/.claude/commands/code-review.md @@ -10,42 +10,100 @@ Use `$ARGUMENTS` to determine scope: git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' ``` -## What to Review - **Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. -### 1. Request / Response / Options patterns +--- + +## 1. Request / Response / Options patterns + - Request builders are plain data holders — validation happens in `Validations.validateXxxRequest()` inside the controller, not in `build()`. Flag if validation logic is duplicated outside `Validations`. - Response getters returning `ArrayList>` is the established SDK pattern — do not flag these as violations. -- All response classes must have `getErrors()` returning `null` (not absent) when no errors. `QueryResponse` is the historical exception — it now has `getErrors()` too. +- All response classes must have `getErrors()` returning `null` (not absent) when no errors. - No separate `*Options` classes exist — options are fields on the request builder itself. +- SDK must not add field-level null/empty validation on top of what the backend enforces. Only structural checks (`table == null`, `values == null`) are permitted. + +--- + +## 2. Error handling -### 2. Error handling - All public methods must declare `throws SkyflowException` - `SkyflowException` must be thrown (not swallowed) on invalid input - No `System.out.println` or bare `e.printStackTrace()` — use `LogUtil` - Catch blocks must not silently drop exceptions +- `catch (Exception e)` without re-throw or explicit handling is a critical issue + +--- + +## 3. Naming conventions -### 3. Naming conventions - Classes: `PascalCase` -- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI` +- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI`, `downloadUrl` not `downloadURL` - Constants: `UPPER_SNAKE_CASE` -- Builder methods: `setFooId()` not `setFooID()` +- Builder setter methods: `setFooId()` not `setFooID()` +- Deprecated methods must use `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to the replacement + +--- + +## 4. Response field normalisation -### 4. Response field normalisation - All response maps must use `skyflowId` (camelCase), never `skyflow_id` (snake_case) - `getErrors()` must be present on every response class -### 5. Test coverage +--- + +## 5. Test coverage + - Every public method must have at least one positive and one negative test - Tests must use `Assert.assertEquals` / `Assert.assertNull` — not just `Assert.fail` guards - No mocking of the production class under test +- Reflection-based tests on private methods are acceptable only when no public API exercises the method + +--- -### 6. Code quality -- No magic strings — use `Constants` or `ErrorMessage` enums -- No duplicate validation logic across request classes -- Methods over 40 lines are a smell — flag for decomposition +## 6. Code quality + +- No magic strings for API field names — use `Constants` or `ErrorMessage` enums +- No duplicate validation logic across request classes — belongs in `Validations` - No `@SuppressWarnings` without a comment explaining why +- `LogUtil.printWarningLog` must be used for deprecation warnings, not `System.err` + +--- + +## 7. Code smells + +Code smells are structural signals — they may not need immediate fixes but must be flagged. Report them at **Smell** severity. + +### Method & class size +- **Long method** — any method over 40 lines. Candidate for decomposition into private helpers. +- **Long class** — any class over 300 lines. May be taking on too many responsibilities. +- **Large parameter list** — more than 4 parameters on a method. Consider a config/options object. + +### Responsibility violations +- **Business logic in Request/Response classes** — these are data holders. If a Request/Response class contains conditional logic beyond null-safe getters, flag it. +- **toString() with business logic** — `toString()` should only serialise state. Logic like field renaming, manual JSON construction, or conditional field injection belongs in the controller or formatter methods. +- **Validation outside Validations.java** — any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced. + +### Control flow +- **Deep nesting** — more than 3 levels of `if`/`for`/`try` nesting. Extract inner blocks to named methods. +- **Long if-else chains** — more than 4 branches. Consider a map, switch, or polymorphism. +- **Null checks scattered** — multiple consecutive null guards that could be replaced with `Optional` or early return. + +### Data +- **Magic numbers** — literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`. +- **Raw HashMap chains** — `HashMap` passed through more than 2 method boundaries without a typed wrapper or comment explaining why. Flag for awareness; don't require a fix. +- **Temporary field** — a class field that is only set in certain code paths and `null` the rest of the time. Should be a local variable or method parameter instead. + +### Dead code +- **Unused private methods** — private methods with no callers. +- **Unused imports** — any `import` not referenced in the file. +- **Unreachable code** — code after `return`/`throw` in the same branch. +- **Commented-out code** — blocks of commented code without explanation. Remove or add a TODO with a ticket reference. + +### Comments +- **Explains what, not why** — a comment that restates what the code does (`// get the vault ID`) is noise. Only flag comments that explain the *what* without adding *why*. +- **Stale comment** — a comment that contradicts the current code (e.g. references a removed parameter or old method name). + +--- ## Output Format @@ -54,13 +112,24 @@ Group findings by file. For each file: ``` ### path/to/File.java -| Severity | Line | Finding | -|---|---|---| -| Critical | 42 | SkyflowException swallowed in catch block | -| Bug | 87 | skyflow_id not normalised to skyflowId | -| Quality | 103 | Magic string "records" — use Constants | +| Severity | Line | Finding | +|------------|------|------------------------------------------------------------| +| Critical | 42 | SkyflowException swallowed in catch block | +| Bug | 87 | skyflow_id not normalised to skyflowId | +| Quality | 103 | Magic string "records" — use Constants | +| Smell | 210 | toString() renames map keys — move to formatter method | +| Smell | 315 | Method is 58 lines — candidate for decomposition | ``` -Severities: **Critical** (data loss / silent failure) | **Bug** (wrong behaviour) | **Edge Case** (unhandled input) | **Quality** (maintainability) | **Smell** (minor style) - -End with a tech-debt summary table and a verdict: `APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES`. +**Severities:** +| Level | Meaning | +|---|---| +| **Critical** | Data loss, silent failure, security risk — must fix before merge | +| **Bug** | Wrong behaviour, incorrect output — must fix before merge | +| **Edge Case** | Unhandled input that will cause runtime failure — fix before merge | +| **Quality** | Maintainability issue, naming violation, missing pattern — fix before merge | +| **Smell** | Structural signal, technical debt — flag and track, fix when in the area | + +End with: +1. A tech-debt summary table grouped by category (Error handling / Naming / Smells / Tests) +2. A verdict: `APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES` From 92e7b30bae76f8f7d12884a378758fca3a665fe5 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 15:30:04 +0530 Subject: [PATCH 13/27] =?UTF-8?q?chore:=20update=20CLAUDE.md=20=E2=80=94?= =?UTF-8?q?=20add=20code-smell=20command,=20update=20slash=20commands?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CLAUDE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 30753abc..10288ad7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,7 +100,8 @@ See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers ## Slash Commands -- `/code-review` — code review against SDK patterns (see `.claude/commands/code-review.md`) -- `/code-security` — security audit (see `.claude/commands/code-security.md`) +- `/code-review` — full review: SDK patterns + code smells + security checks (reads `.claude/commands/code-smell.md` and `.claude/commands/code-security.md` inline) +- `/code-smell` — standalone structural smell analysis only (long methods, dead code, misplaced logic) +- `/code-security` — standalone security audit only (credentials, input validation, HTTP security) - `/sdk-sample ` — generate a sample file for a feature - `/test [ClassName]` — run quality pipeline (compile → checkstyle → build → test → coverage) From b3b8c8110e79468a2a754595a2406827b2f4ca91 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 15:41:47 +0530 Subject: [PATCH 14/27] fix: changes to claude --- CLAUDE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 10288ad7..1e1d966b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,3 +1,13 @@ +--- +name: skyflow-java-sdk +description: Skyflow Java SDK project context — naming conventions, build commands, known failures, and slash commands. Loaded for all Java source, test, and sample files. +paths: + - src/**/*.java + - samples/**/*.java + - pom.xml + - checkstyle.xml +--- + # Skyflow Java SDK — Claude Code Instructions ## Project Overview From 392481ae1fd4038b4f25707463f081e4141a2eca Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 16:20:31 +0530 Subject: [PATCH 15/27] chore: apply Claude Code setup improvements - settings.json: single PostToolUse hook with per-file checkstyle, removed redundant compile hook, PreToolUse, and macOS Stop notification; permissions.deny handles generated/ protection - YAML front matter added to CLAUDE.md and all .claude/commands/*.md - code-smell.md: new standalone structural smell analysis command - .claude/skills/: project-level requesting-code-review override with Java-specific guidance (warnOnce, security review lane, branch SHA pattern) Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 8 ++ .claude/commands/code-security.md | 11 ++ .claude/commands/code-smell.md | 133 ++++++++++++++++++ .claude/commands/sdk-sample.md | 8 ++ .claude/commands/test.md | 8 ++ .../skills/requesting-code-review/SKILL.md | 127 +++++++++++++++++ 6 files changed, 295 insertions(+) create mode 100644 .claude/commands/code-smell.md create mode 100644 .claude/skills/requesting-code-review/SKILL.md diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md index 23653ac2..e842f60a 100644 --- a/.claude/commands/code-review.md +++ b/.claude/commands/code-review.md @@ -1,3 +1,11 @@ +--- +name: code-review +description: Full code review — SDK patterns, naming, test coverage, code smells, and security. Reads code-smell.md and code-security.md inline. +paths: + - src/main/java/**/*.java + - src/test/java/**/*.java +--- + You are a senior engineer performing a thorough code review on the Skyflow Java SDK. ## Review Mode diff --git a/.claude/commands/code-security.md b/.claude/commands/code-security.md index 7a2ffcf6..0fa69923 100644 --- a/.claude/commands/code-security.md +++ b/.claude/commands/code-security.md @@ -1,3 +1,14 @@ +--- +name: code-security +description: Security audit — credential exposure, input validation, path traversal, HTTP security, token lifecycle, dependency CVEs. +paths: + - src/main/java/com/skyflow/serviceaccount/**/*.java + - src/main/java/com/skyflow/config/**/*.java + - src/main/java/com/skyflow/utils/**/*.java + - src/main/java/com/skyflow/vault/controller/**/*.java + - pom.xml +--- + You are a security engineer auditing the Skyflow Java SDK for vulnerabilities. ## Audit Scope diff --git a/.claude/commands/code-smell.md b/.claude/commands/code-smell.md new file mode 100644 index 00000000..0b992265 --- /dev/null +++ b/.claude/commands/code-smell.md @@ -0,0 +1,133 @@ +--- +name: code-smell +description: Standalone structural smell analysis — long methods, dead code, misplaced validation, deep nesting, magic numbers. Does not check patterns or security. +paths: + - src/main/java/**/*.java +--- + +You are a senior engineer performing a code smell analysis on the Skyflow Java SDK. + +## Scope + +Use `$ARGUMENTS` to determine scope: +- A file or directory path — analyse only that path +- Empty / default — analyse files changed on current branch vs `main`: + ```bash + git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' + ``` + +**Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. + +--- + +## What Are Code Smells + +Code smells are structural signals — they do not necessarily mean the code is broken, but they indicate areas of technical debt, reduced readability, or future maintenance risk. All findings are reported at **Smell** severity and do not block merge unless they indicate a design violation. + +--- + +## Smell Catalogue + +### Method & Class Size + +**Long method** — any method over 40 lines. +Signal: the method is doing too much. Candidate for decomposition into named private helpers. + +**Long class** — any class over 300 lines. +Signal: the class may be taking on too many responsibilities. Check if it can be split by concern. + +**Large parameter list** — more than 4 parameters on a method. +Signal: consider a config/options object or a builder to group related parameters. + +--- + +### Responsibility Violations + +**Business logic in Request/Response classes** +Request and Response classes are data holders — they carry data, nothing more. Flag any conditional logic, field transformation, or computation beyond null-safe getters. +Example of a violation: a Response class that renames map keys in `toString()` instead of letting the controller do it. + +**toString() with business logic** +`toString()` should only serialise state for debugging. Logic like field renaming, manual JSON construction, conditional field injection, or iteration belongs in the controller or formatter methods. + +**Validation outside `Validations.java`** +Any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced validation. All request validation must live in `Validations.validateXxxRequest()`. + +--- + +### Control Flow + +**Deep nesting** — more than 3 levels of `if` / `for` / `try` nesting. +Signal: extract inner blocks to named private methods. Deep nesting hides the happy path. + +**Long if-else chains** — more than 4 branches on the same condition. +Signal: consider a `Map`, `switch`, or polymorphism. + +**Null checks scattered** +Multiple consecutive null guards that could be replaced with `Optional` or an early return guard clause. + +--- + +### Data + +**Magic numbers** +Literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`. + +**Raw HashMap chains** +`HashMap` passed through more than 2 method boundaries without a typed wrapper or explanatory comment. Flag for awareness — do not require an immediate fix. + +**Temporary field** +A class field that is only set in certain code paths and is `null` the rest of the time. Should be a local variable or method parameter instead. + +--- + +### Dead Code + +**Unused private methods** — private methods with no callers. + +**Unused imports** — any `import` not referenced in the file. + +**Unreachable code** — code after `return` / `throw` in the same branch. + +**Commented-out code** — blocks of commented code without explanation. Remove entirely or add a `// TODO: [ticket]` with context. + +--- + +### Comments + +**Explains what, not why** +A comment that restates what the code does (`// get the vault ID`) adds no value. Only flag comments that explain the *what* without explaining *why*. + +**Stale comment** +A comment that contradicts the current code — e.g. references a removed parameter, an old method name, or a behaviour that has changed. + +--- + +## Output Format + +Group findings by file: + +``` +### path/to/File.java + +| Smell | Line | Detail | +|---------------------------|------|-----------------------------------------------------------| +| Long method | 42 | processInsertResponse() is 67 lines — decompose | +| Business logic in Response| 88 | toString() renames skyflow_id — move to formatter | +| Magic number | 103 | Literal 25 — extract to Constants.MAX_QUERY_RECORDS | +| Stale comment | 210 | References removed tokenizedData field | +| Dead code | 315 | Private method buildHeaders() has no callers | +``` + +End with a **Smell Summary** table: + +``` +| Category | Count | Files affected | +|-----------------------|-------|------------------------| +| Long methods | 2 | VaultController.java | +| Business logic in DTO | 1 | QueryResponse.java | +| Magic numbers | 3 | Validations.java | +| Dead code | 2 | Utils.java | +``` + +Close with a recommendation: **CLEAN** / **MINOR DEBT** / **SIGNIFICANT DEBT** and a one-sentence summary. diff --git a/.claude/commands/sdk-sample.md b/.claude/commands/sdk-sample.md index 79e702aa..b067c71a 100644 --- a/.claude/commands/sdk-sample.md +++ b/.claude/commands/sdk-sample.md @@ -1,3 +1,11 @@ +--- +name: sdk-sample +description: Generate a Skyflow Java SDK sample file for a vault feature or service account operation. Compile-verified after creation. +paths: + - samples/**/*.java + - samples/pom.xml +--- + Create a Skyflow Java SDK sample file demonstrating: $ARGUMENTS ## File placement diff --git a/.claude/commands/test.md b/.claude/commands/test.md index 98397f8f..d4174495 100644 --- a/.claude/commands/test.md +++ b/.claude/commands/test.md @@ -1,3 +1,11 @@ +--- +name: test +description: Quality pipeline — compile, checkstyle, build, tests, coverage analysis. Pass a class name to target a single test class. +paths: + - src/**/*.java + - pom.xml +--- + Run the Skyflow Java SDK quality pipeline. Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite. diff --git a/.claude/skills/requesting-code-review/SKILL.md b/.claude/skills/requesting-code-review/SKILL.md new file mode 100644 index 00000000..8ed60464 --- /dev/null +++ b/.claude/skills/requesting-code-review/SKILL.md @@ -0,0 +1,127 @@ +--- +name: requesting-code-review +description: Use when completing tasks, implementing major features, or before merging to verify work meets requirements +paths: + - src/main/java/**/*.java + - src/test/java/**/*.java +--- + +# Requesting Code Review + +Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work. + +**Core principle:** Review early, review often. + +## When to Request Review + +**Mandatory:** +- After each task in subagent-driven development +- After completing major feature +- Before merge to main + +**Optional but valuable:** +- When stuck (fresh perspective) +- Before refactoring (baseline check) +- After fixing complex bug + +## How to Request + +**1. Get git SHAs:** +```bash +BASE_SHA=$(git rev-parse HEAD~1) # or origin/main +HEAD_SHA=$(git rev-parse HEAD) +``` + +**2. Choose the right review type for this project:** + +| Change type | Use | +|---|---| +| SDK logic, patterns, naming, tests | `/code-review` — runs SDK checks + smell + security | +| Structural debt only | `/code-smell` — standalone smell analysis | +| Auth, credentials, tokens, HTTP | `/code-security` — standalone security audit | +| Full review via subagent | Dispatch with template below | + +For a full feature branch vs main: +```bash +BASE_SHA=$(git merge-base main HEAD) +HEAD_SHA=$(git rev-parse HEAD) +``` + +For security-sensitive changes (auth, credentials, bearer tokens, HTTP headers) — dispatch both quality and security: +```bash +/code-review src/serviceaccount/ +/code-security src/serviceaccount/ +``` + +**3. Dispatch code reviewer subagent:** + +Use Task tool with `general-purpose` type, fill template at `code-reviewer.md` + +**Placeholders:** +- `{DESCRIPTION}` - Brief summary of what you built +- `{PLAN_OR_REQUIREMENTS}` - What it should do +- `{BASE_SHA}` - Starting commit +- `{HEAD_SHA}` - Ending commit + +**4. Act on feedback:** +- Fix Critical issues immediately +- Fix Important issues before proceeding +- Note Minor issues for later +- Push back if reviewer is wrong (with reasoning) + +## Example + +``` +[Just completed Task 2: Add verification function] + +You: Let me request code review before proceeding. + +BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}') +HEAD_SHA=$(git rev-parse HEAD) + +[Dispatch code reviewer subagent] + DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types + PLAN_OR_REQUIREMENTS: Task 2 from docs/superpowers/plans/deployment-plan.md + BASE_SHA: a7981ec + HEAD_SHA: 3df7661 + +[Subagent returns]: + Strengths: Clean architecture, real tests + Issues: + Important: Missing progress indicators + Minor: Magic number (100) for reporting interval + Assessment: Ready to proceed + +You: [Fix progress indicators] +[Continue to Task 3] +``` + +## Integration with Workflows + +**Subagent-Driven Development:** +- Review after EACH task +- Catch issues before they compound +- Fix before moving to next task + +**Executing Plans:** +- Review after each task or at natural checkpoints +- Get feedback, apply, continue + +**Ad-Hoc Development:** +- Review before merge +- Review when stuck + +## Red Flags + +**Never:** +- Skip review because "it's simple" +- Ignore Critical issues +- Proceed with unfixed Important issues +- Argue with valid technical feedback + +**If reviewer wrong:** +- Push back with technical reasoning +- Show code/tests that prove it works +- Request clarification + +See template at: requesting-code-review/code-reviewer.md From 12d8375080f5911162e674e5ad64c6d10c08d833 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 16:23:49 +0530 Subject: [PATCH 16/27] chore: add British English spellings and Maven -D flag pattern to cspell Claude command files use British English (serialise, normalise, behaviour, sanitisation, prioritised). Maven -D flags (-DskipTests, -Dtest) are now ignored by cspell regex pattern. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .cspell.json | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/.cspell.json b/.cspell.json index a0ec0be6..c7dd22b9 100644 --- a/.cspell.json +++ b/.cspell.json @@ -75,7 +75,20 @@ "Prioritise", "Timeto", "Wdex", - "jacoco" + "jacoco", + "serialise", + "serialised", + "serialises", + "serialising", + "normalise", + "normalised", + "normalises", + "normalising", + "behaviour", + "behaviours", + "sanitisation", + "prioritised", + "prioritise" ], "languageSettings": [ { @@ -106,6 +119,7 @@ "/(eyJ[A-Za-z0-9+/=_-]+\\.)+[A-Za-z0-9+/=_-]+/g", "/[A-Za-z0-9_.~-]*%[0-9A-Fa-f]{2}[A-Za-z0-9_.~%-]*/g", "/\\b[A-Za-z0-9_]{7,}\\b(?=])/g", - "/\"[A-Za-z0-9+/=]{15,}\"/g" + "/\"[A-Za-z0-9+/=]{15,}\"/g", + "/-D[A-Za-z][A-Za-z0-9.]*/g" ] } From 5f17686c84d2c855e0ffebf0e117295030f11fa8 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 16:25:15 +0530 Subject: [PATCH 17/27] chore: add spell check step to code-smell command Runs cspell before structural analysis. Reports unknown words at Smell severity. Points to .cspell.json for adding project-specific terms. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-smell.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.claude/commands/code-smell.md b/.claude/commands/code-smell.md index 0b992265..f456a8d8 100644 --- a/.claude/commands/code-smell.md +++ b/.claude/commands/code-smell.md @@ -1,6 +1,6 @@ --- name: code-smell -description: Standalone structural smell analysis — long methods, dead code, misplaced validation, deep nesting, magic numbers. Does not check patterns or security. +description: Structural smell analysis + spell check — long methods, dead code, misplaced validation, deep nesting, magic numbers. Does not check patterns or security. paths: - src/main/java/**/*.java --- @@ -20,6 +20,18 @@ Use `$ARGUMENTS` to determine scope: --- +## Spell check + +Before analysing smells, run cspell on the files in scope: + +```bash +npx cspell --no-progress "src/**/*.java" ".claude/**/*.md" "CLAUDE.md" "docs/**/*.md" 2>&1 | grep "Unknown word" +``` + +Report any spelling violations at **Smell** severity in the per-file table. The word list is in `.cspell.json` — add legitimate project-specific terms there rather than fixing them as typos. + +--- + ## What Are Code Smells Code smells are structural signals — they do not necessarily mean the code is broken, but they indicate areas of technical debt, reduced readability, or future maintenance risk. All findings are reported at **Smell** severity and do not block merge unless they indicate a design violation. From 3de3b915d53daf083eab4fb4ddae1b81964f8d13 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 16:26:20 +0530 Subject: [PATCH 18/27] chore: simplify settings.json hooks - Single PostToolUse: per-file checkstyle only (removed redundant compile hook, removed PreToolUse generated-code hook, removed macOS Stop notification) - Per-file checkstyle via -Dcheckstyle.includes= for speed - permissions.deny handles generated/ protection natively Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/settings.json | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 3c084d46..126ebf6e 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,37 +6,7 @@ "hooks": [ { "type": "command", - "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n r = subprocess.run(['mvn', 'checkstyle:check', '-q'], capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" - } - ] - }, - { - "matcher": "Edit|Write", - "hooks": [ - { - "type": "command", - "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java') and 'generated' not in f:\n r = subprocess.run(['mvn', 'compile', '-q'], capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" - } - ] - } - ], - "PreToolUse": [ - { - "matcher": "Edit|Write", - "hooks": [ - { - "type": "command", - "command": "python3 -c \"import sys,json; d=json.load(sys.stdin); p=d.get('tool_input',{}).get('file_path',d.get('file_path','')); banned='generated'; (sys.stderr.write('BLOCKED: Fern-generated code — do not edit manually\\n'), sys.exit(2)) if banned in p and 'src/main/java/com/skyflow/generated' in p else sys.exit(0)\"" - } - ] - } - ], - "Stop": [ - { - "hooks": [ - { - "type": "command", - "command": "osascript -e 'display notification \"Claude finished\" with title \"Claude Code\"' 2>/dev/null || true" + "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java'):\n marker = 'src/main/java/'\n if marker in f:\n rel = f.split(marker, 1)[1]\n args = ['mvn', 'checkstyle:check', '-q', '-Dcheckstyle.includes=' + rel]\n else:\n args = ['mvn', 'checkstyle:check', '-q']\n r = subprocess.run(args, capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" } ] } From e48ef5918506e77172d4d4f3a6c27f06613b40a4 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 18 May 2026 17:17:49 +0530 Subject: [PATCH 19/27] chore: add Claude Code setup (CLAUDE.md, .claude/, .cspell.json) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md: project context, naming conventions, build commands, known test failures, slash command reference - .claude/settings.json: PostToolUse per-file checkstyle hook; permissions.deny blocks edits to generated/ code - .claude/commands/: code-review, code-security, code-smell, sdk-sample, test — all with YAML front matter (name, description, paths) - .claude/skills/requesting-code-review/SKILL.md: Java-specific review guidance (review type routing, branch SHA pattern, security lane) - .cspell.json: add British English spellings, Maven -D flag regex, ignorePaths for RUNNING_SAMPLES.md and docs/superpowers/ Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 143 +++++++++++++++++ .claude/commands/code-security.md | 69 +++++++++ .claude/commands/code-smell.md | 145 ++++++++++++++++++ .claude/commands/sdk-sample.md | 68 ++++++++ .claude/commands/test.md | 80 ++++++++++ .claude/settings.json | 26 ++++ .../skills/requesting-code-review/SKILL.md | 127 +++++++++++++++ .cspell.json | 26 +++- CLAUDE.md | 117 ++++++++++++++ 9 files changed, 798 insertions(+), 3 deletions(-) create mode 100644 .claude/commands/code-review.md create mode 100644 .claude/commands/code-security.md create mode 100644 .claude/commands/code-smell.md create mode 100644 .claude/commands/sdk-sample.md create mode 100644 .claude/commands/test.md create mode 100644 .claude/settings.json create mode 100644 .claude/skills/requesting-code-review/SKILL.md create mode 100644 CLAUDE.md diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md new file mode 100644 index 00000000..e842f60a --- /dev/null +++ b/.claude/commands/code-review.md @@ -0,0 +1,143 @@ +--- +name: code-review +description: Full code review — SDK patterns, naming, test coverage, code smells, and security. Reads code-smell.md and code-security.md inline. +paths: + - src/main/java/**/*.java + - src/test/java/**/*.java +--- + +You are a senior engineer performing a thorough code review on the Skyflow Java SDK. + +## Review Mode + +Use `$ARGUMENTS` to determine scope: +- `full review` — scan all files under `src/main/java/com/skyflow/` recursively (exclude `generated/`) +- A file or directory path — review only that path +- Empty / default — review files changed on current branch vs `main`: + ```bash + git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' + ``` + +**Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. + +--- + +## 1. Request / Response / Options patterns + +- Request builders are plain data holders — validation happens in `Validations.validateXxxRequest()` inside the controller, not in `build()`. Flag if validation logic is duplicated outside `Validations`. +- Response getters returning `ArrayList>` is the established SDK pattern — do not flag these as violations. +- All response classes must have `getErrors()` returning `null` (not absent) when no errors. +- No separate `*Options` classes exist — options are fields on the request builder itself. +- SDK must not add field-level null/empty validation on top of what the backend enforces. Only structural checks (`table == null`, `values == null`) are permitted. + +--- + +## 2. Error handling + +- All public methods must declare `throws SkyflowException` +- `SkyflowException` must be thrown (not swallowed) on invalid input +- No `System.out.println` or bare `e.printStackTrace()` — use `LogUtil` +- Catch blocks must not silently drop exceptions +- `catch (Exception e)` without re-throw or explicit handling is a critical issue + +--- + +## 3. Naming conventions + +- Classes: `PascalCase` +- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI`, `downloadUrl` not `downloadURL` +- Constants: `UPPER_SNAKE_CASE` +- Builder setter methods: `setFooId()` not `setFooID()` +- Deprecated methods must use `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to the replacement + +--- + +## 4. Response field normalisation + +- All response maps must use `skyflowId` (camelCase), never `skyflow_id` (snake_case) +- `getErrors()` must be present on every response class + +--- + +## 5. Test coverage + +- Every public method must have at least one positive and one negative test +- Tests must use `Assert.assertEquals` / `Assert.assertNull` — not just `Assert.fail` guards +- No mocking of the production class under test +- Reflection-based tests on private methods are acceptable only when no public API exercises the method + +--- + +## 6. Code quality + +- No magic strings for API field names — use `Constants` or `ErrorMessage` enums +- No duplicate validation logic across request classes — belongs in `Validations` +- No `@SuppressWarnings` without a comment explaining why +- `LogUtil.printWarningLog` must be used for deprecation warnings, not `System.err` + +--- + +## 7. Code smells + +Code smells are structural signals — they may not need immediate fixes but must be flagged. Report them at **Smell** severity. + +### Method & class size +- **Long method** — any method over 40 lines. Candidate for decomposition into private helpers. +- **Long class** — any class over 300 lines. May be taking on too many responsibilities. +- **Large parameter list** — more than 4 parameters on a method. Consider a config/options object. + +### Responsibility violations +- **Business logic in Request/Response classes** — these are data holders. If a Request/Response class contains conditional logic beyond null-safe getters, flag it. +- **toString() with business logic** — `toString()` should only serialise state. Logic like field renaming, manual JSON construction, or conditional field injection belongs in the controller or formatter methods. +- **Validation outside Validations.java** — any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced. + +### Control flow +- **Deep nesting** — more than 3 levels of `if`/`for`/`try` nesting. Extract inner blocks to named methods. +- **Long if-else chains** — more than 4 branches. Consider a map, switch, or polymorphism. +- **Null checks scattered** — multiple consecutive null guards that could be replaced with `Optional` or early return. + +### Data +- **Magic numbers** — literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`. +- **Raw HashMap chains** — `HashMap` passed through more than 2 method boundaries without a typed wrapper or comment explaining why. Flag for awareness; don't require a fix. +- **Temporary field** — a class field that is only set in certain code paths and `null` the rest of the time. Should be a local variable or method parameter instead. + +### Dead code +- **Unused private methods** — private methods with no callers. +- **Unused imports** — any `import` not referenced in the file. +- **Unreachable code** — code after `return`/`throw` in the same branch. +- **Commented-out code** — blocks of commented code without explanation. Remove or add a TODO with a ticket reference. + +### Comments +- **Explains what, not why** — a comment that restates what the code does (`// get the vault ID`) is noise. Only flag comments that explain the *what* without adding *why*. +- **Stale comment** — a comment that contradicts the current code (e.g. references a removed parameter or old method name). + +--- + +## Output Format + +Group findings by file. For each file: + +``` +### path/to/File.java + +| Severity | Line | Finding | +|------------|------|------------------------------------------------------------| +| Critical | 42 | SkyflowException swallowed in catch block | +| Bug | 87 | skyflow_id not normalised to skyflowId | +| Quality | 103 | Magic string "records" — use Constants | +| Smell | 210 | toString() renames map keys — move to formatter method | +| Smell | 315 | Method is 58 lines — candidate for decomposition | +``` + +**Severities:** +| Level | Meaning | +|---|---| +| **Critical** | Data loss, silent failure, security risk — must fix before merge | +| **Bug** | Wrong behaviour, incorrect output — must fix before merge | +| **Edge Case** | Unhandled input that will cause runtime failure — fix before merge | +| **Quality** | Maintainability issue, naming violation, missing pattern — fix before merge | +| **Smell** | Structural signal, technical debt — flag and track, fix when in the area | + +End with: +1. A tech-debt summary table grouped by category (Error handling / Naming / Smells / Tests) +2. A verdict: `APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES` diff --git a/.claude/commands/code-security.md b/.claude/commands/code-security.md new file mode 100644 index 00000000..0fa69923 --- /dev/null +++ b/.claude/commands/code-security.md @@ -0,0 +1,69 @@ +--- +name: code-security +description: Security audit — credential exposure, input validation, path traversal, HTTP security, token lifecycle, dependency CVEs. +paths: + - src/main/java/com/skyflow/serviceaccount/**/*.java + - src/main/java/com/skyflow/config/**/*.java + - src/main/java/com/skyflow/utils/**/*.java + - src/main/java/com/skyflow/vault/controller/**/*.java + - pom.xml +--- + +You are a security engineer auditing the Skyflow Java SDK for vulnerabilities. + +## Audit Scope + +Use `$ARGUMENTS` to determine target files. If none provided, run: +```bash +git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' +``` + +**Skip:** `src/main/java/com/skyflow/generated/` — observations only, no edits. + +## Security Checks + +### 1. Credential and token exposure (Critical) +- Bearer tokens, API keys, and private keys must never appear in logs, error messages, exception messages, or `toString()` output +- `Credentials` fields (`path`, `token`, `apiKey`, `credentialsString`) must not be serialised to logs +- JWT claims must not be logged + +### 2. Input validation (High) +- All string inputs from callers must be null/empty checked before use +- File paths passed to `new File(path)` must not allow path traversal (`../`) +- JSON strings parsed with `JsonParser` must be wrapped in try/catch for `JsonSyntaxException` + +### 3. Credentials file handling (High) +- Credentials files must only be read from paths provided by the caller — no environment variable path injection without sanitisation +- `FileReader` must be in a try-with-resources or explicitly closed + +### 4. HTTP security (Medium) +- All API calls must go over HTTPS — verify `Utils.getBaseURL` enforces this +- Authorization headers must not be logged at any log level +- HTTP timeouts must be configured + +### 5. Error information leakage (Medium) +- `SkyflowException` messages must not include raw server response bodies that could contain PII +- Stack traces must not be surfaced to callers — wrap in `SkyflowException` + +### 6. Dependency vulnerabilities (Low) +- Note any dependencies that are known to have CVEs (check pom.xml versions) + +### 7. Authentication lifecycle (Medium) +- Bearer token caching must check expiry before reuse +- Token refresh must be thread-safe (`synchronized` or equivalent) + +## Output Format + +For each finding: + +``` +### path/to/File.java : line N + +**Severity:** Critical / High / Medium / Low / Info +**Risk:** What an attacker could do +**Trigger:** Input or code path that triggers the vulnerability +**Fix:** Concrete remediation with code example +**CWE:** CWE-NNN +``` + +End with a summary table and overall risk rating. diff --git a/.claude/commands/code-smell.md b/.claude/commands/code-smell.md new file mode 100644 index 00000000..f456a8d8 --- /dev/null +++ b/.claude/commands/code-smell.md @@ -0,0 +1,145 @@ +--- +name: code-smell +description: Structural smell analysis + spell check — long methods, dead code, misplaced validation, deep nesting, magic numbers. Does not check patterns or security. +paths: + - src/main/java/**/*.java +--- + +You are a senior engineer performing a code smell analysis on the Skyflow Java SDK. + +## Scope + +Use `$ARGUMENTS` to determine scope: +- A file or directory path — analyse only that path +- Empty / default — analyse files changed on current branch vs `main`: + ```bash + git diff main...HEAD --name-only | grep '\.java$' | grep -v 'generated' + ``` + +**Skip entirely:** `src/main/java/com/skyflow/generated/` — Fern-generated REST client, read-only. + +--- + +## Spell check + +Before analysing smells, run cspell on the files in scope: + +```bash +npx cspell --no-progress "src/**/*.java" ".claude/**/*.md" "CLAUDE.md" "docs/**/*.md" 2>&1 | grep "Unknown word" +``` + +Report any spelling violations at **Smell** severity in the per-file table. The word list is in `.cspell.json` — add legitimate project-specific terms there rather than fixing them as typos. + +--- + +## What Are Code Smells + +Code smells are structural signals — they do not necessarily mean the code is broken, but they indicate areas of technical debt, reduced readability, or future maintenance risk. All findings are reported at **Smell** severity and do not block merge unless they indicate a design violation. + +--- + +## Smell Catalogue + +### Method & Class Size + +**Long method** — any method over 40 lines. +Signal: the method is doing too much. Candidate for decomposition into named private helpers. + +**Long class** — any class over 300 lines. +Signal: the class may be taking on too many responsibilities. Check if it can be split by concern. + +**Large parameter list** — more than 4 parameters on a method. +Signal: consider a config/options object or a builder to group related parameters. + +--- + +### Responsibility Violations + +**Business logic in Request/Response classes** +Request and Response classes are data holders — they carry data, nothing more. Flag any conditional logic, field transformation, or computation beyond null-safe getters. +Example of a violation: a Response class that renames map keys in `toString()` instead of letting the controller do it. + +**toString() with business logic** +`toString()` should only serialise state for debugging. Logic like field renaming, manual JSON construction, conditional field injection, or iteration belongs in the controller or formatter methods. + +**Validation outside `Validations.java`** +Any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced validation. All request validation must live in `Validations.validateXxxRequest()`. + +--- + +### Control Flow + +**Deep nesting** — more than 3 levels of `if` / `for` / `try` nesting. +Signal: extract inner blocks to named private methods. Deep nesting hides the happy path. + +**Long if-else chains** — more than 4 branches on the same condition. +Signal: consider a `Map`, `switch`, or polymorphism. + +**Null checks scattered** +Multiple consecutive null guards that could be replaced with `Optional` or an early return guard clause. + +--- + +### Data + +**Magic numbers** +Literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`. + +**Raw HashMap chains** +`HashMap` passed through more than 2 method boundaries without a typed wrapper or explanatory comment. Flag for awareness — do not require an immediate fix. + +**Temporary field** +A class field that is only set in certain code paths and is `null` the rest of the time. Should be a local variable or method parameter instead. + +--- + +### Dead Code + +**Unused private methods** — private methods with no callers. + +**Unused imports** — any `import` not referenced in the file. + +**Unreachable code** — code after `return` / `throw` in the same branch. + +**Commented-out code** — blocks of commented code without explanation. Remove entirely or add a `// TODO: [ticket]` with context. + +--- + +### Comments + +**Explains what, not why** +A comment that restates what the code does (`// get the vault ID`) adds no value. Only flag comments that explain the *what* without explaining *why*. + +**Stale comment** +A comment that contradicts the current code — e.g. references a removed parameter, an old method name, or a behaviour that has changed. + +--- + +## Output Format + +Group findings by file: + +``` +### path/to/File.java + +| Smell | Line | Detail | +|---------------------------|------|-----------------------------------------------------------| +| Long method | 42 | processInsertResponse() is 67 lines — decompose | +| Business logic in Response| 88 | toString() renames skyflow_id — move to formatter | +| Magic number | 103 | Literal 25 — extract to Constants.MAX_QUERY_RECORDS | +| Stale comment | 210 | References removed tokenizedData field | +| Dead code | 315 | Private method buildHeaders() has no callers | +``` + +End with a **Smell Summary** table: + +``` +| Category | Count | Files affected | +|-----------------------|-------|------------------------| +| Long methods | 2 | VaultController.java | +| Business logic in DTO | 1 | QueryResponse.java | +| Magic numbers | 3 | Validations.java | +| Dead code | 2 | Utils.java | +``` + +Close with a recommendation: **CLEAN** / **MINOR DEBT** / **SIGNIFICANT DEBT** and a one-sentence summary. diff --git a/.claude/commands/sdk-sample.md b/.claude/commands/sdk-sample.md new file mode 100644 index 00000000..b067c71a --- /dev/null +++ b/.claude/commands/sdk-sample.md @@ -0,0 +1,68 @@ +--- +name: sdk-sample +description: Generate a Skyflow Java SDK sample file for a vault feature or service account operation. Compile-verified after creation. +paths: + - samples/**/*.java + - samples/pom.xml +--- + +Create a Skyflow Java SDK sample file demonstrating: $ARGUMENTS + +## File placement + +| Feature type | Package | Directory | +|---|---|---| +| Vault ops (insert/get/update/delete/query/tokenize) | `com.example.vault` | `samples/src/main/java/com/example/vault/` | +| Service account auth | `com.example.serviceaccount` | `samples/src/main/java/com/example/serviceaccount/` | +| Connection | `com.example.connection` | `samples/src/main/java/com/example/connection/` | +| Detect | `com.example.detect` | `samples/src/main/java/com/example/detect/` | + +File name: `Example.java` + +## Structure (follow this order) + +1. Package declaration +2. Imports — only from `com.skyflow.*`, `java.*`; never from `com.skyflow.generated.*` +3. Public class with `main(String[] args) throws SkyflowException` +4. Credentials setup — choose based on feature: + - **Vault ops:** `credentials.setApiKey("")` or `credentials.setCredentialsString("")` + - **Service account:** `credentials.setPath("credentials.json")` (path to the service account JSON file) +5. `VaultConfig` with `setVaultId`, `setClusterId`, `setEnv(Env.PROD)`, `setCredentials(credentials)` +6. Build the Skyflow client: + ```java + Skyflow skyflowClient = Skyflow.builder() + .setLogLevel(LogLevel.DEBUG) + .addVaultConfig(vaultConfig) + .build(); + ``` +7. Request object via `*Request.builder()` — options go directly on the builder (no separate Options class): + ```java + // Example: InsertRequest with tokenMode + InsertRequest request = InsertRequest.builder() + .table("...") + .values(records) + .tokenMode(TokenMode.ENABLE) + .build(); + ``` +8. Call the vault method inside a try/catch for `SkyflowException`: + ```java + InsertResponse response = skyflowClient.vault().insert(request); + System.out.println(response); + ``` + +## Rules + +- Vault IDs / cluster IDs use placeholders: `""`, `""` +- Credential values use placeholders: `""`, `""` +- Credentials file path: `"credentials.json"` (relative — no absolute paths) +- Always catch `SkyflowException` and print `e.getMessage()` +- No separate `*Options` classes — they don't exist in this SDK; use request builder methods +- Keep under 80 lines + +## After creating the file + +```bash +cd samples && mvn compile -q 2>&1 | tail -20 +``` + +Report the file path and any compile errors. diff --git a/.claude/commands/test.md b/.claude/commands/test.md new file mode 100644 index 00000000..d4174495 --- /dev/null +++ b/.claude/commands/test.md @@ -0,0 +1,80 @@ +--- +name: test +description: Quality pipeline — compile, checkstyle, build, tests, coverage analysis. Pass a class name to target a single test class. +paths: + - src/**/*.java + - pom.xml +--- + +Run the Skyflow Java SDK quality pipeline. + +Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite. + +## Known Pre-existing Failures (not regressions) + +Before reporting failures, check against this baseline: +- `HttpUtilityTests` — ALL tests fail (JDK 21 + PowerMock `InaccessibleObject` incompatibility) +- `TokenTests#testExpiredTokenForIsExpiredToken` — needs live credentials +- `VaultClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var +- `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var + +Baseline: 374 tests, ~5 failures, ~4 errors. Only report failures **beyond** this baseline. + +## Pipeline + +### Step 1 — Compile +```bash +mvn compile -q 2>&1 | tail -20 +``` +Expected: no output (clean compile). Report any errors. + +### Step 2 — Checkstyle +```bash +mvn checkstyle:check -q 2>&1 | tail -20 +``` +Note: `failsOnError=false` in pom.xml means the build will not fail even if violations exist — check the output for `[WARN]` checkstyle lines. Violations are excluded from `generated/` by pom config. + +### Step 3 — Build +```bash +mvn package -DskipTests -q 2>&1 | tail -20 +``` +Expected: BUILD SUCCESS. + +### Step 4 — Tests +If `$ARGUMENTS` is set: +```bash +mvn test -Dtest=$ARGUMENTS -q 2>&1 | tail -40 +``` +Otherwise: +```bash +mvn test -q 2>&1 | tail -40 +``` +Report: tests run, failures, errors. Flag any pre-existing failures separately from new ones. + +### Step 5 — Coverage analysis +Flag any public interface class (`src/main/java/com/skyflow/vault/`, `src/main/java/com/skyflow/config/`, `src/main/java/com/skyflow/serviceaccount/`) that has no corresponding test file under `src/test/`. + +For classes that do have tests, check whether each public method has at least one positive and one negative test case. List any gaps. + +### Step 6 — Edge case identification +For any test class below complete coverage, identify missing scenarios: +- Null / empty inputs +- Invalid types / wrong enum values +- Concurrent / reuse scenarios +- Error paths (API rejection, network failure) + +Write concrete JUnit 4 test method stubs (not full implementations) for each gap. + +### Step 7 — Report + +``` +| Step | Status | Notes | +|---|---|---| +| Compile | ✅ / ❌ | ... | +| Checkstyle | ✅ / ❌ | ... | +| Build | ✅ / ❌ | ... | +| Tests | ✅ / ❌ | N passed, M failed | +| Coverage gaps | ... | list classes | +``` + +Conclude with **READY TO MERGE** or **NEEDS FIXES** and a prioritised fix list. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..126ebf6e --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,26 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java'):\n marker = 'src/main/java/'\n if marker in f:\n rel = f.split(marker, 1)[1]\n args = ['mvn', 'checkstyle:check', '-q', '-Dcheckstyle.includes=' + rel]\n else:\n args = ['mvn', 'checkstyle:check', '-q']\n r = subprocess.run(args, capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" + } + ] + } + ] + }, + "permissions": { + "allow": [ + "Bash(mvn *)", + "Bash(java *)", + "Bash(python3 *)" + ], + "deny": [ + "Edit(src/main/java/com/skyflow/generated/**)", + "Write(src/main/java/com/skyflow/generated/**)" + ] + } +} diff --git a/.claude/skills/requesting-code-review/SKILL.md b/.claude/skills/requesting-code-review/SKILL.md new file mode 100644 index 00000000..8ed60464 --- /dev/null +++ b/.claude/skills/requesting-code-review/SKILL.md @@ -0,0 +1,127 @@ +--- +name: requesting-code-review +description: Use when completing tasks, implementing major features, or before merging to verify work meets requirements +paths: + - src/main/java/**/*.java + - src/test/java/**/*.java +--- + +# Requesting Code Review + +Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work. + +**Core principle:** Review early, review often. + +## When to Request Review + +**Mandatory:** +- After each task in subagent-driven development +- After completing major feature +- Before merge to main + +**Optional but valuable:** +- When stuck (fresh perspective) +- Before refactoring (baseline check) +- After fixing complex bug + +## How to Request + +**1. Get git SHAs:** +```bash +BASE_SHA=$(git rev-parse HEAD~1) # or origin/main +HEAD_SHA=$(git rev-parse HEAD) +``` + +**2. Choose the right review type for this project:** + +| Change type | Use | +|---|---| +| SDK logic, patterns, naming, tests | `/code-review` — runs SDK checks + smell + security | +| Structural debt only | `/code-smell` — standalone smell analysis | +| Auth, credentials, tokens, HTTP | `/code-security` — standalone security audit | +| Full review via subagent | Dispatch with template below | + +For a full feature branch vs main: +```bash +BASE_SHA=$(git merge-base main HEAD) +HEAD_SHA=$(git rev-parse HEAD) +``` + +For security-sensitive changes (auth, credentials, bearer tokens, HTTP headers) — dispatch both quality and security: +```bash +/code-review src/serviceaccount/ +/code-security src/serviceaccount/ +``` + +**3. Dispatch code reviewer subagent:** + +Use Task tool with `general-purpose` type, fill template at `code-reviewer.md` + +**Placeholders:** +- `{DESCRIPTION}` - Brief summary of what you built +- `{PLAN_OR_REQUIREMENTS}` - What it should do +- `{BASE_SHA}` - Starting commit +- `{HEAD_SHA}` - Ending commit + +**4. Act on feedback:** +- Fix Critical issues immediately +- Fix Important issues before proceeding +- Note Minor issues for later +- Push back if reviewer is wrong (with reasoning) + +## Example + +``` +[Just completed Task 2: Add verification function] + +You: Let me request code review before proceeding. + +BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}') +HEAD_SHA=$(git rev-parse HEAD) + +[Dispatch code reviewer subagent] + DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types + PLAN_OR_REQUIREMENTS: Task 2 from docs/superpowers/plans/deployment-plan.md + BASE_SHA: a7981ec + HEAD_SHA: 3df7661 + +[Subagent returns]: + Strengths: Clean architecture, real tests + Issues: + Important: Missing progress indicators + Minor: Magic number (100) for reporting interval + Assessment: Ready to proceed + +You: [Fix progress indicators] +[Continue to Task 3] +``` + +## Integration with Workflows + +**Subagent-Driven Development:** +- Review after EACH task +- Catch issues before they compound +- Fix before moving to next task + +**Executing Plans:** +- Review after each task or at natural checkpoints +- Get feedback, apply, continue + +**Ad-Hoc Development:** +- Review before merge +- Review when stuck + +## Red Flags + +**Never:** +- Skip review because "it's simple" +- Ignore Critical issues +- Proceed with unfixed Important issues +- Argue with valid technical feedback + +**If reviewer wrong:** +- Push back with technical reasoning +- Show code/tests that prove it works +- Request clarification + +See template at: requesting-code-review/code-reviewer.md diff --git a/.cspell.json b/.cspell.json index a0ec0be6..abe743cb 100644 --- a/.cspell.json +++ b/.cspell.json @@ -73,9 +73,26 @@ "pkcs", "prioritise", "Prioritise", + "prioritised", "Timeto", "Wdex", - "jacoco" + "jacoco", + "serialise", + "serialised", + "serialises", + "serialising", + "normalise", + "Normalise", + "normalised", + "normalises", + "Normalises", + "normalising", + "behaviour", + "Behaviour", + "behaviours", + "sanitisation", + "recognised", + "unrecognised" ], "languageSettings": [ { @@ -98,7 +115,9 @@ "src/main/java/com/skyflow/generated/**", "**/*.ts", "**/processed-*", - "samples/src/main/java/com/example/credentials.json" + "samples/src/main/java/com/example/credentials.json", + "RUNNING_SAMPLES.md", + "docs/superpowers/**" ], "ignoreRegExpList": [ "/\\b[A-Z][A-Z0-9_]{2,}\\b/g", @@ -106,6 +125,7 @@ "/(eyJ[A-Za-z0-9+/=_-]+\\.)+[A-Za-z0-9+/=_-]+/g", "/[A-Za-z0-9_.~-]*%[0-9A-Fa-f]{2}[A-Za-z0-9_.~%-]*/g", "/\\b[A-Za-z0-9_]{7,}\\b(?=])/g", - "/\"[A-Za-z0-9+/=]{15,}\"/g" + "/\"[A-Za-z0-9+/=]{15,}\"/g", + "/-D[A-Za-z][A-Za-z0-9.]*/g" ] } diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..1e1d966b --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,117 @@ +--- +name: skyflow-java-sdk +description: Skyflow Java SDK project context — naming conventions, build commands, known failures, and slash commands. Loaded for all Java source, test, and sample files. +paths: + - src/**/*.java + - samples/**/*.java + - pom.xml + - checkstyle.xml +--- + +# Skyflow Java SDK — Claude Code Instructions + +## Project Overview + +This is the Skyflow Java SDK (`skyflow-java`). It provides a Java interface to the Skyflow Data Privacy Vault API — vault operations (insert, get, update, delete, query, tokenize, detokenize), service account authentication (bearer tokens, signed data tokens), connections, detect, and audit. + +**Current version:** 2.x (v3.0.0 in preparation — see `docs/superpowers/specs/`) + +## Critical Boundary — Generated Code + +**Never edit files under `src/main/java/com/skyflow/generated/`.** + +These are auto-generated by [Fern](https://buildwithfern.com) from the Skyflow API definition. Manual edits are overwritten on the next generation run. If you find a bug in generated code, report it — do not patch it directly. + +The `pom.xml` checkstyle and test configs already exclude `generated/` from all checks. + +## Project Structure + +``` +src/ + main/java/com/skyflow/ + config/ # VaultConfig, Credentials, ConnectionConfig + vault/ + controller/ # VaultController, AuditController, BinLookupController, + # ConnectionController, DetectController + data/ # Request/Response objects: InsertRequest, GetResponse, etc. + tokens/ # DetokenizeRequest/Response, TokenizeRequest/Response + connection/ # InvokeConnectionRequest/Response + audit/ # ListEventRequest/Response + bin/ # GetBinRequest/Response (BIN lookup) + detect/ # Deidentify/Reidentify requests/responses + serviceaccount/ + util/ # BearerToken, SignedDataTokens — credential parsing + JWT + enums/ # LogLevel, RedactionType, TokenMode, Env, etc. + errors/ # SkyflowException, ErrorCode, ErrorMessage + utils/ # Utils, Constants, HttpUtility, LogUtil + generated/ # ← FERN-GENERATED, DO NOT EDIT + test/java/com/skyflow/ + ... # JUnit 4 tests mirroring the main structure +samples/ # Standalone Maven project — com.example.vault / .serviceaccount / .detect / .connection +docs/ + superpowers/ + specs/ # Design specs for in-progress features + plans/ # Implementation plans +``` + +## Naming Conventions + +- **Acronyms as words:** `skyflowId` (not `skyflowID`), `clientId` (not `clientID`), `tokenUri` (not `tokenURI`), `keyId` (not `keyID`) +- **Builder setters:** `setVaultId()`, `setClusterId()`, `setSkyflowId()` — never `setVaultID()` +- **Response maps:** always use `skyflowId` (camelCase) — the raw API returns `skyflow_id` (snake_case) which VaultController normalises before returning to callers +- **Constants class:** use `com.skyflow.utils.Constants` for string literals; `ErrorMessage` enum for error message strings + +## Build and Test + +```bash +mvn compile -q # compile +mvn checkstyle:check -q # lint (config: checkstyle.xml) +mvn test -q # full test suite (JUnit 4) +mvn test -Dtest=ClassName # single test class +mvn package -DskipTests -q # build jar +``` + +Samples (separate Maven project): +```bash +cd samples && mvn compile -q +``` + +## Credentials JSON Format + +The SDK reads a `credentials.json` file for service account authentication. The canonical field names (v3+) are: + +```json +{ + "clientId": "...", + "keyId": "...", + "tokenUri": "...", + "privateKey": "..." +} +``` + +The legacy all-caps forms (`clientID`, `keyID`, `tokenURI`) are accepted as fallbacks for migration. + +## Known Pre-existing Test Failures + +These failures exist on `main` and are **not regressions** — do not investigate them unless specifically asked: + +| Test class | Failure | Cause | +|---|---|---| +| `HttpUtilityTests` | `InaccessibleObject` (all tests) | JDK 21 + PowerMock incompatibility — PowerMock cannot reflect into `java.net` | +| `TokenTests#testExpiredTokenForIsExpiredToken` | Environment error | Requires live credentials | +| `VaultClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | +| `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | + +Run `mvn test -q 2>&1 | grep -E "Tests run|FAIL|ERROR"` to see current baseline (374 tests, ~5 failures, ~4 errors). + +## Active Work + +See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers/plans/` for implementation plans. + +## Slash Commands + +- `/code-review` — full review: SDK patterns + code smells + security checks (reads `.claude/commands/code-smell.md` and `.claude/commands/code-security.md` inline) +- `/code-smell` — standalone structural smell analysis only (long methods, dead code, misplaced logic) +- `/code-security` — standalone security audit only (credentials, input validation, HTTP security) +- `/sdk-sample ` — generate a sample file for a feature +- `/test [ClassName]` — run quality pipeline (compile → checkstyle → build → test → coverage) From ed8718dc07f9357c6e411f8ecfb09dedf140b5b7 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Wed, 20 May 2026 11:54:44 +0530 Subject: [PATCH 20/27] Update CLAUDE.md --- CLAUDE.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1e1d966b..a60fab8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,11 +1,6 @@ --- name: skyflow-java-sdk description: Skyflow Java SDK project context — naming conventions, build commands, known failures, and slash commands. Loaded for all Java source, test, and sample files. -paths: - - src/**/*.java - - samples/**/*.java - - pom.xml - - checkstyle.xml --- # Skyflow Java SDK — Claude Code Instructions From 2e8d6bf736f67b637c665164753e38bf7a296695 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 17:58:11 +0530 Subject: [PATCH 21/27] chore: improve Claude Code setup - Extract checkstyle hook from inline JSON to .claude/hooks/checkstyle-on-edit.py - Remove redundant mvn exec:java entries from settings.local.json - Add audit/BIN packages to sdk-sample.md file placement table - Add Commit & PR Guidelines section to CLAUDE.md (Jira ID requirement, branch naming, PR template sections) Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/sdk-sample.md | 2 ++ .claude/hooks/checkstyle-on-edit.py | 19 +++++++++++++++++++ .claude/settings.json | 2 +- CLAUDE.md | 28 +++++++++++++++++++++++++++- 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 .claude/hooks/checkstyle-on-edit.py diff --git a/.claude/commands/sdk-sample.md b/.claude/commands/sdk-sample.md index b067c71a..a7b84a65 100644 --- a/.claude/commands/sdk-sample.md +++ b/.claude/commands/sdk-sample.md @@ -16,6 +16,8 @@ Create a Skyflow Java SDK sample file demonstrating: $ARGUMENTS | Service account auth | `com.example.serviceaccount` | `samples/src/main/java/com/example/serviceaccount/` | | Connection | `com.example.connection` | `samples/src/main/java/com/example/connection/` | | Detect | `com.example.detect` | `samples/src/main/java/com/example/detect/` | +| Audit event operations | `com.example.audit` | `samples/src/main/java/com/example/audit/` | +| BIN lookup | `com.example.bin` | `samples/src/main/java/com/example/bin/` | File name: `Example.java` diff --git a/.claude/hooks/checkstyle-on-edit.py b/.claude/hooks/checkstyle-on-edit.py new file mode 100644 index 00000000..4fd0787d --- /dev/null +++ b/.claude/hooks/checkstyle-on-edit.py @@ -0,0 +1,19 @@ +import sys, json, subprocess, os + +d = json.load(sys.stdin) +f = d.get('tool_input', {}).get('file_path', d.get('file_path', '')) +if not f or not f.endswith('.java'): + sys.exit(0) + +root = '/home/devb/SDK/skyflow-java' +marker = 'src/main/java/' +if marker in f: + rel = f.split(marker, 1)[1] + args = ['mvn', 'checkstyle:check', '-q', '-Dcheckstyle.includes=' + rel] +else: + args = ['mvn', 'checkstyle:check', '-q'] + +r = subprocess.run(args, capture_output=True, text=True, cwd=root) +out = (r.stdout + r.stderr).strip() +if out: + print('\n'.join(out.splitlines()[-20:])) diff --git a/.claude/settings.json b/.claude/settings.json index 126ebf6e..6908179b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "python3 -c \"\nimport sys, json, subprocess\nd = json.load(sys.stdin)\nf = d.get('tool_input', {}).get('file_path', d.get('file_path', ''))\nif f and f.endswith('.java'):\n marker = 'src/main/java/'\n if marker in f:\n rel = f.split(marker, 1)[1]\n args = ['mvn', 'checkstyle:check', '-q', '-Dcheckstyle.includes=' + rel]\n else:\n args = ['mvn', 'checkstyle:check', '-q']\n r = subprocess.run(args, capture_output=True, text=True)\n out = (r.stdout + r.stderr).strip()\n if out:\n lines = out.splitlines()\n print('\\n'.join(lines[-20:]))\n\"" + "command": "python3 .claude/hooks/checkstyle-on-edit.py" } ] } diff --git a/CLAUDE.md b/CLAUDE.md index 1e1d966b..353912ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,8 +110,34 @@ See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers ## Slash Commands -- `/code-review` — full review: SDK patterns + code smells + security checks (reads `.claude/commands/code-smell.md` and `.claude/commands/code-security.md` inline) +- `/code-review` — full review: SDK patterns + code smells + security (Steps 2 and 3 read `.claude/commands/code-smell.md` and `.claude/commands/code-security.md` at runtime) - `/code-smell` — standalone structural smell analysis only (long methods, dead code, misplaced logic) - `/code-security` — standalone security audit only (credentials, input validation, HTTP security) - `/sdk-sample ` — generate a sample file for a feature - `/test [ClassName]` — run quality pipeline (compile → checkstyle → build → test → coverage) + +## Commit & PR Guidelines + +### Commit messages +Every commit on a PR branch **must** include a Jira ticket ID — enforced by the `check-commit-message` step in `.github/workflows/pr.yml`. + +Accepted formats: +``` +SK-1234 short description +SK-1234: short description +feat: SK-1234 short description +fix(SK-1234): short description +``` + +Exempt patterns (no ticket needed): +- `[AUTOMATED]` — release version bumps only +- `Merge ...` — merge commits +- `Release ...` — release commits + +Conventional Commits prefixes (`feat:`, `fix:`, `chore:`, `docs:`) are encouraged but only valid alongside a Jira ID. + +### Branch naming +`/-` — e.g. `devesh/SK-1234-add-detokenize-support` + +### PR template +The `.github/pull_request_template.md` requires: **Why**, **Goal**, **Testing** sections. Tech debt section is optional. From 099df93467b41793b26466371e1cbd535da20ed8 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 17:58:47 +0530 Subject: [PATCH 22/27] chore: update branch naming convention in CLAUDE.md Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CLAUDE.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 353912ff..2451f9a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -137,7 +137,13 @@ Exempt patterns (no ticket needed): Conventional Commits prefixes (`feat:`, `fix:`, `chore:`, `docs:`) are encouraged but only valid alongside a Jira ID. ### Branch naming -`/-` — e.g. `devesh/SK-1234-add-detokenize-support` +Branch name must include your GitHub username: + +``` +/- +``` + +Example: `karthik/GV-770-ext-auth-json-error` ### PR template The `.github/pull_request_template.md` requires: **Why**, **Goal**, **Testing** sections. Tech debt section is optional. From bbd4eac9454378cb8c9bb1c35093089b63414d11 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 18:06:08 +0530 Subject: [PATCH 23/27] chore: clarify v2.1/v3 version scope in CLAUDE.md v2.1 is current stable, v3 is pre-release for Flow DB only (bulkInsert, batchProcessing) used by Spark wrapper. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CLAUDE.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2451f9a4..23340dc6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,13 @@ paths: This is the Skyflow Java SDK (`skyflow-java`). It provides a Java interface to the Skyflow Data Privacy Vault API — vault operations (insert, get, update, delete, query, tokenize, detokenize), service account authentication (bearer tokens, signed data tokens), connections, detect, and audit. -**Current version:** 2.x (v3.0.0 in preparation — see `docs/superpowers/specs/`) +**Current stable version: v2.1** — supports PDB vaults. This is what customers use. + +**v3 (pre-release, Flow DB only):** v3 is *not* a full replacement for v2. It adds Flow DB-specific operations used by the [Spark wrapper](https://github.com/skyflowapi/vault-workflows): +- `bulkInsert` +- `batchProcessing` (`batchSize` + `concurrencyLimit`) + +v3 does not yet have full parity with v2. Do not treat v3 as the general SDK — scope v3 work strictly to Flow DB features unless explicitly told otherwise. ## Critical Boundary — Generated Code From 4fb5855e793f53f2a7c5e7f5ac697039633ba156 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 18:07:14 +0530 Subject: [PATCH 24/27] chore: add v1 maintenance mode and EOL info to CLAUDE.md Co-Authored-By: Claude Sonnet 4.6 (1M context) --- CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 23340dc6..f33d7f0c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,6 +14,8 @@ paths: This is the Skyflow Java SDK (`skyflow-java`). It provides a Java interface to the Skyflow Data Privacy Vault API — vault operations (insert, get, update, delete, query, tokenize, detokenize), service account authentication (bearer tokens, signed data tokens), connections, detect, and audit. +**v1 (maintenance mode, `v1` branch):** Security and bug fixes only — no new features. EOL announced: **October 31, 2026**. Do not add features or refactor v1 code unless it is a security fix or bug fix. + **Current stable version: v2.1** — supports PDB vaults. This is what customers use. **v3 (pre-release, Flow DB only):** v3 is *not* a full replacement for v2. It adds Flow DB-specific operations used by the [Spark wrapper](https://github.com/skyflowapi/vault-workflows): From 98834218c56ce43a0edec2d32fa2cdf01684d994 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 18:28:39 +0530 Subject: [PATCH 25/27] =?UTF-8?q?chore:=20improve=20Claude=20setup=20?= =?UTF-8?q?=E2=80=94=20token=20reduction,=20/commit=20command,=20CI/CD=20i?= =?UTF-8?q?ntegration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Token reduction: - Condense CLAUDE.md project structure tree (27 → 14 lines) - Remove stale test count from CLAUDE.md - Remove duplicated failures table from test.md (reference CLAUDE.md instead) - Collapse duplicate naming/normalisation sections in code-review.md Productivity: - Add /commit command with Jira ticket ID extraction from branch name - Add git/find/grep/npx cspell to settings.json allow list CI/CD: - Add claude-pr-review.yml: automated SDK patterns + security review on PRs - Add claude-changelog.yml: auto-generated release notes on tag push Requires ANTHROPIC_API_KEY secret in GitHub Actions. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .claude/commands/code-review.md | 108 ++++++----------- .claude/commands/commit.md | 55 +++++++++ .claude/commands/test.md | 11 +- .claude/settings.json | 6 +- .github/workflows/claude-changelog.yml | 86 ++++++++++++++ .github/workflows/claude-pr-review.yml | 157 +++++++++++++++++++++++++ CLAUDE.md | 31 ++--- 7 files changed, 352 insertions(+), 102 deletions(-) create mode 100644 .claude/commands/commit.md create mode 100644 .github/workflows/claude-changelog.yml create mode 100644 .github/workflows/claude-pr-review.yml diff --git a/.claude/commands/code-review.md b/.claude/commands/code-review.md index e842f60a..498fcfac 100644 --- a/.claude/commands/code-review.md +++ b/.claude/commands/code-review.md @@ -1,6 +1,6 @@ --- name: code-review -description: Full code review — SDK patterns, naming, test coverage, code smells, and security. Reads code-smell.md and code-security.md inline. +description: Full code review — SDK patterns, naming, test coverage, then runs /code-smell and /code-security. paths: - src/main/java/**/*.java - src/test/java/**/*.java @@ -8,7 +8,7 @@ paths: You are a senior engineer performing a thorough code review on the Skyflow Java SDK. -## Review Mode +## Scope Use `$ARGUMENTS` to determine scope: - `full review` — scan all files under `src/main/java/com/skyflow/` recursively (exclude `generated/`) @@ -22,7 +22,11 @@ Use `$ARGUMENTS` to determine scope: --- -## 1. Request / Response / Options patterns +## Step 1 — SDK Pattern Review + +Check the files in scope against the rules below. + +### 1. Request / Response / Options patterns - Request builders are plain data holders — validation happens in `Validations.validateXxxRequest()` inside the controller, not in `build()`. Flag if validation logic is duplicated outside `Validations`. - Response getters returning `ArrayList>` is the established SDK pattern — do not flag these as violations. @@ -30,9 +34,7 @@ Use `$ARGUMENTS` to determine scope: - No separate `*Options` classes exist — options are fields on the request builder itself. - SDK must not add field-level null/empty validation on top of what the backend enforces. Only structural checks (`table == null`, `values == null`) are permitted. ---- - -## 2. Error handling +### 2. Error handling - All public methods must declare `throws SkyflowException` - `SkyflowException` must be thrown (not swallowed) on invalid input @@ -40,82 +42,31 @@ Use `$ARGUMENTS` to determine scope: - Catch blocks must not silently drop exceptions - `catch (Exception e)` without re-throw or explicit handling is a critical issue ---- - -## 3. Naming conventions - -- Classes: `PascalCase` -- Methods / fields: `camelCase` — acronyms as words: `skyflowId` not `skyflowID`, `tokenUri` not `tokenURI`, `downloadUrl` not `downloadURL` -- Constants: `UPPER_SNAKE_CASE` -- Builder setter methods: `setFooId()` not `setFooID()` -- Deprecated methods must use `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to the replacement +### 3. Naming conventions and response field normalisation ---- +Follow the conventions in CLAUDE.md under "Naming Conventions". Key enforcement points: +- Acronyms as words: `skyflowId`, `tokenUri`, `clientId` — never uppercase abbreviations +- Builder setters: `setFooId()` not `setFooID()`; constants: `UPPER_SNAKE_CASE`; classes: `PascalCase` +- Response maps: `skyflowId` (camelCase) only — never `skyflow_id`; `getErrors()` must be present on every response class +- Deprecated methods: `@Deprecated(since = "x.x", forRemoval = true)` + `@deprecated` Javadoc with `{@link}` to replacement -## 4. Response field normalisation - -- All response maps must use `skyflowId` (camelCase), never `skyflow_id` (snake_case) -- `getErrors()` must be present on every response class - ---- - -## 5. Test coverage +### 5. Test coverage - Every public method must have at least one positive and one negative test - Tests must use `Assert.assertEquals` / `Assert.assertNull` — not just `Assert.fail` guards - No mocking of the production class under test - Reflection-based tests on private methods are acceptable only when no public API exercises the method ---- - -## 6. Code quality +### 6. Code quality - No magic strings for API field names — use `Constants` or `ErrorMessage` enums - No duplicate validation logic across request classes — belongs in `Validations` - No `@SuppressWarnings` without a comment explaining why - `LogUtil.printWarningLog` must be used for deprecation warnings, not `System.err` ---- - -## 7. Code smells - -Code smells are structural signals — they may not need immediate fixes but must be flagged. Report them at **Smell** severity. - -### Method & class size -- **Long method** — any method over 40 lines. Candidate for decomposition into private helpers. -- **Long class** — any class over 300 lines. May be taking on too many responsibilities. -- **Large parameter list** — more than 4 parameters on a method. Consider a config/options object. - -### Responsibility violations -- **Business logic in Request/Response classes** — these are data holders. If a Request/Response class contains conditional logic beyond null-safe getters, flag it. -- **toString() with business logic** — `toString()` should only serialise state. Logic like field renaming, manual JSON construction, or conditional field injection belongs in the controller or formatter methods. -- **Validation outside Validations.java** — any `if (x == null) throw new SkyflowException(...)` outside `src/main/java/com/skyflow/utils/validations/` is misplaced. +### Output for Step 1 -### Control flow -- **Deep nesting** — more than 3 levels of `if`/`for`/`try` nesting. Extract inner blocks to named methods. -- **Long if-else chains** — more than 4 branches. Consider a map, switch, or polymorphism. -- **Null checks scattered** — multiple consecutive null guards that could be replaced with `Optional` or early return. - -### Data -- **Magic numbers** — literal integers or sizes (e.g. `25`, `3600`, `100`) without a named constant. Use `Constants`. -- **Raw HashMap chains** — `HashMap` passed through more than 2 method boundaries without a typed wrapper or comment explaining why. Flag for awareness; don't require a fix. -- **Temporary field** — a class field that is only set in certain code paths and `null` the rest of the time. Should be a local variable or method parameter instead. - -### Dead code -- **Unused private methods** — private methods with no callers. -- **Unused imports** — any `import` not referenced in the file. -- **Unreachable code** — code after `return`/`throw` in the same branch. -- **Commented-out code** — blocks of commented code without explanation. Remove or add a TODO with a ticket reference. - -### Comments -- **Explains what, not why** — a comment that restates what the code does (`// get the vault ID`) is noise. Only flag comments that explain the *what* without adding *why*. -- **Stale comment** — a comment that contradicts the current code (e.g. references a removed parameter or old method name). - ---- - -## Output Format - -Group findings by file. For each file: +Group findings by file: ``` ### path/to/File.java @@ -125,8 +76,6 @@ Group findings by file. For each file: | Critical | 42 | SkyflowException swallowed in catch block | | Bug | 87 | skyflow_id not normalised to skyflowId | | Quality | 103 | Magic string "records" — use Constants | -| Smell | 210 | toString() renames map keys — move to formatter method | -| Smell | 315 | Method is 58 lines — candidate for decomposition | ``` **Severities:** @@ -136,8 +85,23 @@ Group findings by file. For each file: | **Bug** | Wrong behaviour, incorrect output — must fix before merge | | **Edge Case** | Unhandled input that will cause runtime failure — fix before merge | | **Quality** | Maintainability issue, naming violation, missing pattern — fix before merge | -| **Smell** | Structural signal, technical debt — flag and track, fix when in the area | -End with: -1. A tech-debt summary table grouped by category (Error handling / Naming / Smells / Tests) +--- + +## Step 2 — Code Smell Analysis + +Read the file `.claude/commands/code-smell.md` and follow all of its instructions for the same files in scope. Produce its full output (per-file smell table + smell summary + recommendation). + +--- + +## Step 3 — Security Audit + +Read the file `.claude/commands/code-security.md` and follow all of its instructions for the same files in scope. Produce its full output (per-finding blocks + summary table + overall risk rating). + +--- + +## Final Verdict + +After all three steps, close with: +1. A tech-debt summary table grouped by category (SDK Patterns / Error Handling / Naming / Tests / Smells / Security) 2. A verdict: `APPROVE` / `APPROVE WITH FIXES` / `REQUEST CHANGES` diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md new file mode 100644 index 00000000..6aae4614 --- /dev/null +++ b/.claude/commands/commit.md @@ -0,0 +1,55 @@ +--- +name: commit +description: Stage check + Jira-aware commit — extracts ticket ID from branch name and validates against pr.yml commit-message check. +--- + +Create a git commit for staged changes on the current branch. + +Use `$ARGUMENTS` as the commit message description. If empty, ask the user for a description before proceeding. + +## Step 1 — Extract ticket ID from branch name + +```bash +git rev-parse --abbrev-ref HEAD +``` + +Extract the Jira ticket ID using the pattern `[A-Z]{1,10}-[0-9]+`: +- `devesh/SK-1234-fix-foo` → `SK-1234` +- `karthik/GV-770-ext-auth-json-error` → `GV-770` +- `username/SDK-2814-some-fix` → `SDK-2814` + +If no ticket ID is found, **stop** and ask the user to provide one before continuing. + +## Step 2 — Check what is staged + +```bash +git status --short +git diff --cached --stat +``` + +If nothing is staged, list the unstaged files and ask the user which files to stage. Do not run `git add .` — ask for explicit paths (`.env`, `credentials.json`, and `generated/` must never be staged). + +## Step 3 — Assemble and validate the commit message + +Build the message as: +``` + +``` + +If the user provided a Conventional Commits prefix (`feat`, `fix`, `chore`, `docs`, `refactor`, `test`), prepend it: +``` +feat: SK-1234 add bulk insert support +fix: GV-770 handle null bearer token on refresh +``` + +Validate against the `pr.yml` enforced pattern: `(\[?[A-Z]{1,10}-[1-9][0-9]*)|(\[AUTOMATED\])|(Merge)|(Release)` +- Must contain a Jira ID — a bare description without a ticket ID will fail CI. +- If validation fails, report the exact requirement and stop. + +## Step 4 — Commit + +```bash +git commit -m "" +``` + +Report the resulting commit SHA and the commit message first line. diff --git a/.claude/commands/test.md b/.claude/commands/test.md index d4174495..e690a355 100644 --- a/.claude/commands/test.md +++ b/.claude/commands/test.md @@ -10,15 +10,8 @@ Run the Skyflow Java SDK quality pipeline. Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite. -## Known Pre-existing Failures (not regressions) - -Before reporting failures, check against this baseline: -- `HttpUtilityTests` — ALL tests fail (JDK 21 + PowerMock `InaccessibleObject` incompatibility) -- `TokenTests#testExpiredTokenForIsExpiredToken` — needs live credentials -- `VaultClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var -- `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` — needs `SKYFLOW_CREDENTIALS` env var - -Baseline: 374 tests, ~5 failures, ~4 errors. Only report failures **beyond** this baseline. +> Baseline failures are listed in CLAUDE.md under "Known Pre-existing Test Failures". +> Do not investigate them unless specifically asked. Only report failures **beyond** that baseline. ## Pipeline diff --git a/.claude/settings.json b/.claude/settings.json index 6908179b..1d00baed 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -16,7 +16,11 @@ "allow": [ "Bash(mvn *)", "Bash(java *)", - "Bash(python3 *)" + "Bash(python3 *)", + "Bash(git *)", + "Bash(find *)", + "Bash(grep *)", + "Bash(npx cspell *)" ], "deny": [ "Edit(src/main/java/com/skyflow/generated/**)", diff --git a/.github/workflows/claude-changelog.yml b/.github/workflows/claude-changelog.yml new file mode 100644 index 00000000..aae70fa2 --- /dev/null +++ b/.github/workflows/claude-changelog.yml @@ -0,0 +1,86 @@ +name: Claude Changelog + +on: + push: + tags: + - '[0-9]+.[0-9]+.[0-9]+' + - '*.*.*-beta.*' + +permissions: + contents: write + +jobs: + generate-changelog: + name: Generate Release Notes + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Get previous tag + id: previoustag + uses: WyriHaximus/github-action-get-previous-tag@v1 + with: + fallback: '0.0.0' + + - name: Get commits since previous tag + id: commits + run: | + PREV="${{ steps.previoustag.outputs.tag }}" + CURR="${{ github.ref_name }}" + COMMITS=$(git log "${PREV}..${CURR}" --oneline \ + | grep -v '^\S* \[AUTOMATED\]' \ + | grep -v '^\S* Merge ' \ + | grep -v '^\S* \[AUTOMATED\]') + echo "log<> $GITHUB_OUTPUT + echo "$COMMITS" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + - name: Install Claude CLI + run: npm install -g @anthropic-ai/claude-code + + - name: Generate release notes + id: notes + continue-on-error: true + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + PREV="${{ steps.previoustag.outputs.tag }}" + CURR="${{ github.ref_name }}" + COMMITS="${{ steps.commits.outputs.log }}" + NOTES=$(claude --print --model claude-sonnet-4-5 -p " + Generate GitHub Release notes for the Skyflow Java SDK. + + Release: $CURR (previous: $PREV) + + Commits: + $COMMITS + + Rules: + - Group into sections: ## Features, ## Bug Fixes, ## Security, ## Breaking Changes + - Omit any section with no entries + - Each entry: bullet point with a concise one-line description; include the Jira ticket ID if present (e.g. SK-1234) + - Strip PR merge numbers like (#323) — keep the substance + - Skip [AUTOMATED] commits, version bump commits, and bare merge commits + - Breaking Changes section must come first if present + - End with: _Full changelog: https://github.com/skyflowapi/skyflow-java/compare/${PREV}...${CURR}_ + + Output only the markdown. No preamble or explanation. + ") + echo "notes<> $GITHUB_OUTPUT + echo "$NOTES" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + - name: Create or update GitHub Release + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + TAG="${{ github.ref_name }}" + NOTES="${{ steps.notes.outputs.notes }}" + if gh release view "$TAG" > /dev/null 2>&1; then + gh release edit "$TAG" --notes "$NOTES" + else + gh release create "$TAG" --notes "$NOTES" --title "Release $TAG" + fi diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml new file mode 100644 index 00000000..821b2d50 --- /dev/null +++ b/.github/workflows/claude-pr-review.yml @@ -0,0 +1,157 @@ +name: Claude PR Review + +on: + pull_request: + branches: [main] + paths: + - 'src/**/*.java' + +permissions: + pull-requests: write + contents: read + +jobs: + sdk-patterns-review: + name: SDK Patterns & Naming Review + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install Claude CLI + run: npm install -g @anthropic-ai/claude-code + + - name: Get changed Java files + id: changed-files + run: | + FILES=$(git diff --name-only origin/${{ github.base_ref }}...${{ github.sha }} \ + | grep '\.java$' \ + | grep -v 'generated' \ + | tr '\n' ' ') + echo "files=$FILES" >> $GITHUB_OUTPUT + + - name: Run SDK patterns review + if: steps.changed-files.outputs.files != '' + id: review + continue-on-error: true + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + FILES="${{ steps.changed-files.outputs.files }}" + REVIEW=$(claude --print --model claude-sonnet-4-5 -p " + You are a senior engineer reviewing the Skyflow Java SDK. + + Review the following changed Java files for SDK pattern violations: + 1. Request/Response/Options patterns — builders are data holders, validation in Validations.java only + 2. Error handling — all public methods throw SkyflowException, no swallowed exceptions, no bare println/printStackTrace + 3. Naming — acronyms as words (skyflowId not skyflowID, tokenUri not tokenURI); UPPER_SNAKE constants; PascalCase classes + 4. Response normalisation — skyflowId not skyflow_id in response maps; getErrors() present on every response class + 5. Code quality — no magic strings (use Constants), no @SuppressWarnings without comment, deprecation via LogUtil.printWarningLog + + Skip src/main/java/com/skyflow/generated/ entirely. + + Files to review: $FILES + + For each file with findings, produce a markdown table: + | Severity | Line | Finding | + Severities: Critical (data loss/security), Bug (wrong behaviour), Quality (naming/patterns). + Skip Info-level observations. If no findings for a file, omit it. + If no findings at all, write: 'No issues found.' + + End with one of: APPROVE / APPROVE WITH FIXES / REQUEST CHANGES + ") + echo "result<> $GITHUB_OUTPUT + echo "$REVIEW" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + - name: Post review comment + if: steps.changed-files.outputs.files != '' + uses: actions/github-script@v7 + with: + script: | + const review = `${{ steps.review.outputs.result }}`; + const files = `${{ steps.changed-files.outputs.files }}`; + await github.rest.issues.createComment({ + ...context.repo, + issue_number: context.payload.pull_request.number, + body: `## Claude SDK Patterns Review\n\n${review}\n\n---\n_Files reviewed: \`${files}\`_` + }); + + security-review: + name: Security Review (serviceaccount changes) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for serviceaccount changes + id: filter + uses: dorny/paths-filter@v3 + with: + filters: | + serviceaccount: + - 'src/**/serviceaccount/**/*.java' + + - name: Install Claude CLI + if: steps.filter.outputs.serviceaccount == 'true' + run: npm install -g @anthropic-ai/claude-code + + - name: Get changed serviceaccount files + if: steps.filter.outputs.serviceaccount == 'true' + id: sa-files + run: | + FILES=$(git diff --name-only origin/${{ github.base_ref }}...${{ github.sha }} \ + | grep 'serviceaccount' \ + | grep '\.java$' \ + | tr '\n' ' ') + echo "files=$FILES" >> $GITHUB_OUTPUT + + - name: Run security audit + if: steps.filter.outputs.serviceaccount == 'true' && steps.sa-files.outputs.files != '' + id: security + continue-on-error: true + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + FILES="${{ steps.sa-files.outputs.files }}" + AUDIT=$(claude --print --model claude-sonnet-4-5 -p " + You are a security engineer auditing the Skyflow Java SDK serviceaccount module. + + Audit the following files for: + 1. Credential and token exposure — bearer tokens, API keys, private keys must never appear in logs, error messages, or toString() output + 2. Path traversal — file paths passed to new File(path) must not allow ../ + 3. JSON parsing — JsonParser calls must be wrapped in try/catch for JsonSyntaxException + 4. HTTP security — all API calls must be HTTPS; Authorization headers must not be logged at any level + 5. Token lifecycle — bearer token caching must check expiry before reuse; token refresh must be thread-safe + + Files: $FILES + + For each finding: + **Severity:** Critical / High / Medium / Low + **File:Line:** path:N + **Risk:** one sentence + **Fix:** one concrete sentence + + If no findings, write: 'No security issues found.' + End with overall risk rating: LOW / MEDIUM / HIGH / CRITICAL + ") + echo "result<> $GITHUB_OUTPUT + echo "$AUDIT" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + + - name: Post security comment + if: steps.filter.outputs.serviceaccount == 'true' && steps.sa-files.outputs.files != '' + uses: actions/github-script@v7 + with: + script: | + const audit = `${{ steps.security.outputs.result }}`; + const files = `${{ steps.sa-files.outputs.files }}`; + await github.rest.issues.createComment({ + ...context.repo, + issue_number: context.payload.pull_request.number, + body: `## Claude Security Audit (serviceaccount)\n\n${audit}\n\n---\n_Files audited: \`${files}\`_` + }); diff --git a/CLAUDE.md b/CLAUDE.md index f33d7f0c..00904528 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ paths: This is the Skyflow Java SDK (`skyflow-java`). It provides a Java interface to the Skyflow Data Privacy Vault API — vault operations (insert, get, update, delete, query, tokenize, detokenize), service account authentication (bearer tokens, signed data tokens), connections, detect, and audit. -**v1 (maintenance mode, `v1` branch):** Security and bug fixes only — no new features. EOL announced: **October 31, 2026**. Do not add features or refactor v1 code unless it is a security fix or bug fix. +**v1 (maintenance mode, `v1` branch):** Security and bug fixes only — no new features. EOL announced: **October 31, 2026**. **Current stable version: v2.1** — supports PDB vaults. This is what customers use. @@ -38,28 +38,18 @@ The `pom.xml` checkstyle and test configs already exclude `generated/` from all src/ main/java/com/skyflow/ config/ # VaultConfig, Credentials, ConnectionConfig - vault/ - controller/ # VaultController, AuditController, BinLookupController, - # ConnectionController, DetectController - data/ # Request/Response objects: InsertRequest, GetResponse, etc. - tokens/ # DetokenizeRequest/Response, TokenizeRequest/Response - connection/ # InvokeConnectionRequest/Response - audit/ # ListEventRequest/Response - bin/ # GetBinRequest/Response (BIN lookup) - detect/ # Deidentify/Reidentify requests/responses - serviceaccount/ - util/ # BearerToken, SignedDataTokens — credential parsing + JWT - enums/ # LogLevel, RedactionType, TokenMode, Env, etc. + vault/ # controller/, data/, tokens/, connection/, audit/, bin/, detect/ + serviceaccount/ # BearerToken, SignedDataTokens (JWT + credential parsing) + enums/ # LogLevel, RedactionType, TokenMode, Env errors/ # SkyflowException, ErrorCode, ErrorMessage - utils/ # Utils, Constants, HttpUtility, LogUtil + utils/ # Utils, Constants, HttpUtility, LogUtil, Validations generated/ # ← FERN-GENERATED, DO NOT EDIT test/java/com/skyflow/ - ... # JUnit 4 tests mirroring the main structure -samples/ # Standalone Maven project — com.example.vault / .serviceaccount / .detect / .connection + ... # JUnit 4 tests mirroring main structure +samples/ # Standalone Maven project — vault / serviceaccount / detect / connection docs/ - superpowers/ - specs/ # Design specs for in-progress features - plans/ # Implementation plans + superpowers/specs/ # Design specs + superpowers/plans/ # Implementation plans ``` ## Naming Conventions @@ -110,7 +100,7 @@ These failures exist on `main` and are **not regressions** — do not investigat | `VaultClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | | `ConnectionClientTests#testSetBearerTokenWithEnvCredentials` | Environment error | Requires `SKYFLOW_CREDENTIALS` env var | -Run `mvn test -q 2>&1 | grep -E "Tests run|FAIL|ERROR"` to see current baseline (374 tests, ~5 failures, ~4 errors). +Run `mvn test -q 2>&1 | grep -E "Tests run|FAIL|ERROR"` to see the current baseline. ## Active Work @@ -123,6 +113,7 @@ See `docs/superpowers/specs/` for in-progress design specs and `docs/superpowers - `/code-security` — standalone security audit only (credentials, input validation, HTTP security) - `/sdk-sample ` — generate a sample file for a feature - `/test [ClassName]` — run quality pipeline (compile → checkstyle → build → test → coverage) +- `/commit ` — stage check + Jira-aware commit (extracts ticket ID from branch name) ## Commit & PR Guidelines From 75a51632e226e478d815b2436210d7b89b8b3f7e Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 18:33:06 +0530 Subject: [PATCH 26/27] chore: update requesting-code-review skill with context fork Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../skills/requesting-code-review/SKILL.md | 119 +++++------------- 1 file changed, 34 insertions(+), 85 deletions(-) diff --git a/.claude/skills/requesting-code-review/SKILL.md b/.claude/skills/requesting-code-review/SKILL.md index 8ed60464..9f662842 100644 --- a/.claude/skills/requesting-code-review/SKILL.md +++ b/.claude/skills/requesting-code-review/SKILL.md @@ -8,120 +8,69 @@ paths: # Requesting Code Review -Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work. - -**Core principle:** Review early, review often. +**Core principle:** Review early, review often. Review after each task — catch issues before they compound. ## When to Request Review **Mandatory:** - After each task in subagent-driven development -- After completing major feature +- After completing a major feature - Before merge to main **Optional but valuable:** - When stuck (fresh perspective) - Before refactoring (baseline check) -- After fixing complex bug +- After fixing a complex bug ## How to Request -**1. Get git SHAs:** -```bash -BASE_SHA=$(git rev-parse HEAD~1) # or origin/main -HEAD_SHA=$(git rev-parse HEAD) -``` - -**2. Choose the right review type for this project:** +**1. Pick the right command:** -| Change type | Use | +| Change type | Command | |---|---| -| SDK logic, patterns, naming, tests | `/code-review` — runs SDK checks + smell + security | +| SDK logic, patterns, naming, tests | `/code-review` — SDK checks + smell + security | | Structural debt only | `/code-smell` — standalone smell analysis | | Auth, credentials, tokens, HTTP | `/code-security` — standalone security audit | -| Full review via subagent | Dispatch with template below | -For a full feature branch vs main: +For security-sensitive changes, run both: ```bash -BASE_SHA=$(git merge-base main HEAD) -HEAD_SHA=$(git rev-parse HEAD) +/code-review src/main/java/com/skyflow/serviceaccount/ +/code-security src/main/java/com/skyflow/serviceaccount/ ``` -For security-sensitive changes (auth, credentials, bearer tokens, HTTP headers) — dispatch both quality and security: -```bash -/code-review src/serviceaccount/ -/code-security src/serviceaccount/ -``` +**2. Fork context — dispatch a subagent reviewer:** -**3. Dispatch code reviewer subagent:** +The commands above run in the current session and share your context. For an independent second opinion (no confirmation bias, preserved main context window), dispatch a fresh subagent: -Use Task tool with `general-purpose` type, fill template at `code-reviewer.md` +``` +Agent tool (general-purpose): + description: "SDK code review" + prompt: | + You are a senior engineer reviewing the Skyflow Java SDK. -**Placeholders:** -- `{DESCRIPTION}` - Brief summary of what you built -- `{PLAN_OR_REQUIREMENTS}` - What it should do -- `{BASE_SHA}` - Starting commit -- `{HEAD_SHA}` - Ending commit + Read CLAUDE.md for project conventions, then read and follow + .claude/commands/code-review.md for the full review process. -**4. Act on feedback:** -- Fix Critical issues immediately -- Fix Important issues before proceeding -- Note Minor issues for later -- Push back if reviewer is wrong (with reasoning) + Git range to review: + Base: {BASE_SHA} + Head: {HEAD_SHA} -## Example + Run: + git diff --stat {BASE_SHA}..{HEAD_SHA} + git diff {BASE_SHA}..{HEAD_SHA} + Description of what was implemented: + {DESCRIPTION} ``` -[Just completed Task 2: Add verification function] - -You: Let me request code review before proceeding. -BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}') +Get the SHAs: +```bash +BASE_SHA=$(git merge-base main HEAD) # branch vs main HEAD_SHA=$(git rev-parse HEAD) - -[Dispatch code reviewer subagent] - DESCRIPTION: Added verifyIndex() and repairIndex() with 4 issue types - PLAN_OR_REQUIREMENTS: Task 2 from docs/superpowers/plans/deployment-plan.md - BASE_SHA: a7981ec - HEAD_SHA: 3df7661 - -[Subagent returns]: - Strengths: Clean architecture, real tests - Issues: - Important: Missing progress indicators - Minor: Magic number (100) for reporting interval - Assessment: Ready to proceed - -You: [Fix progress indicators] -[Continue to Task 3] ``` -## Integration with Workflows - -**Subagent-Driven Development:** -- Review after EACH task -- Catch issues before they compound -- Fix before moving to next task - -**Executing Plans:** -- Review after each task or at natural checkpoints -- Get feedback, apply, continue - -**Ad-Hoc Development:** -- Review before merge -- Review when stuck - -## Red Flags - -**Never:** -- Skip review because "it's simple" -- Ignore Critical issues -- Proceed with unfixed Important issues -- Argue with valid technical feedback - -**If reviewer wrong:** -- Push back with technical reasoning -- Show code/tests that prove it works -- Request clarification - -See template at: requesting-code-review/code-reviewer.md +**3. Act on feedback:** +- Fix Critical issues immediately +- Fix Important issues before proceeding +- Note Minor/Smell issues for later +- Push back with reasoning if you disagree From de03b757be5bfcddc46939a1268a4db67f3cc22f Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Fri, 29 May 2026 18:35:00 +0530 Subject: [PATCH 27/27] chore: remove docs changes from claude-code-setup branch Co-Authored-By: Claude Sonnet 4.6 (1M context) --- docs/migrate_to_v2.md | 284 --------- .../2026-05-13-java-nomenclature-cleanup.md | 589 ------------------ ...-05-13-java-nomenclature-cleanup-design.md | 133 ---- 3 files changed, 1006 deletions(-) delete mode 100644 docs/migrate_to_v2.md delete mode 100644 docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md delete mode 100644 docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md diff --git a/docs/migrate_to_v2.md b/docs/migrate_to_v2.md deleted file mode 100644 index 671210fd..00000000 --- a/docs/migrate_to_v2.md +++ /dev/null @@ -1,284 +0,0 @@ -# Skyflow Java SDK — V1 to V2 Migration Guide - -This guide covers the steps to migrate the Skyflow Java SDK from v1 to v2. - ---- - -## Authentication options - -In V2, multiple authentication options are available. You can now provide credentials in the following ways: - -- Environment variable (`SKYFLOW_CREDENTIALS`) _(Recommended)_ -- API Key -- Path to credentials JSON file -- Stringified JSON of credentials -- Bearer token - -**V1 (Old)** - -```java -static class DemoTokenProvider implements TokenProvider { - @Override - public String getBearerToken() throws Exception { - ResponseToken res = null; - try { - String filePath = ""; - res = Token.generateBearerToken(filePath); - } catch (SkyflowException e) { - e.printStackTrace(); - } - return res.getAccessToken(); - } -} -``` - -**V2 (New): Choose one of the following:** - -```java -// Option 1: API Key (Recommended) -Credentials skyflowCredentials = new Credentials(); -skyflowCredentials.setApiKey(""); - -// Option 2: Environment Variable (Recommended) -// Set SKYFLOW_CREDENTIALS in your environment - -// Option 3: Credentials File -skyflowCredentials.setPath(""); - -// Option 4: Stringified JSON -skyflowCredentials.setCredentialsString(""); - -// Option 5: Bearer Token -skyflowCredentials.setToken(""); -``` - -> **Notes:** -> - Use only ONE authentication method per credentials object. -> - API Key or environment variable are recommended for production. -> - For priority order see [Quickstart — Initialize the client](../README.md#initialize-the-client). - ---- - -## Initializing the client - -V2 introduces a builder pattern for client initialization with multi-vault support. - -**Key changes:** -- `vaultUrl` replaced with `clusterId` (derived from vault URL) -- Added `env` specification (e.g. `Env.PROD`, `Env.SANDBOX`) -- Log level is now per-client-instance - -**V1 (Old)** - -```java -DemoTokenProvider demoTokenProvider = new DemoTokenProvider(); -SkyflowConfiguration skyflowConfig = new SkyflowConfiguration( - "", "", demoTokenProvider -); -Skyflow skyflowClient = Skyflow.init(skyflowConfig); -``` - -**V2 (New)** - -```java -Credentials credentials = new Credentials(); -credentials.setPath(""); - -VaultConfig config = new VaultConfig(); -config.setVaultId(""); -config.setClusterId(""); -config.setEnv(Env.PROD); -config.setCredentials(credentials); - -Skyflow skyflowClient = Skyflow.builder() - .setLogLevel(LogLevel.DEBUG) - .addVaultConfig(config) - .build(); -``` - ---- - -## Request and response structure - -V2 removes third-party JSON objects in favour of native `ArrayList` and `HashMap` with a builder pattern for requests. - -**V1 (Old) — Request** - -```java -JSONObject recordsJson = new JSONObject(); -JSONArray recordsArrayJson = new JSONArray(); -JSONObject recordJson = new JSONObject(); -recordJson.put("table", "cards"); -JSONObject fieldsJson = new JSONObject(); -fieldsJson.put("cardNumber", "41111111111"); -fieldsJson.put("cvv", "123"); -recordJson.put("fields", fieldsJson); -recordsArrayJson.add(recordJson); -recordsJson.put("records", recordsArrayJson); -try { - JSONObject insertResponse = skyflowClient.insert(records); -} catch (SkyflowException e) { - System.out.println(e); -} -``` - -**V2 (New) — Request** - -```java -HashMap value = new HashMap<>(); -value.put("", ""); -value.put("", ""); -ArrayList> values = new ArrayList<>(); -values.add(value); - -InsertRequest insertRequest = InsertRequest.builder() - .table("") - .values(values) - .returnTokens(true) - .build(); - -InsertResponse response = skyflowClient.vault().insert(insertRequest); -``` - -**V1 (Old) — Response** - -```json -{ - "records": [ - { - "table": "cards", - "fields": { - "skyflow_id": "16419435-aa63-4823-aae7-19c6a2d6a19f", - "cardNumber": "f3907186-e7e2-466f-91e5-48e12c2bcbc1", - "cvv": "1989cb56-63da-4482-a2df-1f74cd0dd1a5" - } - } - ] -} -``` - -**V2 (New) — Response** - -```json -{ - "insertedFields": [ - { - "skyflowId": "9fac9201-7b8a-4446-93f8-5244e1213bd1", - "card_number": "5484-7829-1702-9110", - "cardholder_name": "b2308e2a-c1f5-469b-97b7-1f193159399b" - } - ], - "errors": null -} -``` - ---- - -## Request options - -V2 builder pattern replaces V1 options objects. - -**V1 (Old)** - -```java -InsertOptions insertOptions = new InsertOptions(true); -``` - -**V2 (New)** - -```java -InsertRequest request = InsertRequest.builder() - .table("") - .values(values) - .continueOnError(false) - .tokenMode(TokenMode.DISABLE) - .returnTokens(false) - .upsert("") - .build(); -``` - ---- - -## Error structure - -V2 provides richer error details for easier debugging. - -**V1 (Old)** - -```json -{ - "code": "", - "description": "" -} -``` - -**V2 (New)** - -```json -{ - "httpStatus": "", - "grpcCode": "", - "httpCode": "", - "message": "", - "requestId": "", - "details": ["
"] -} -``` - ---- - -## Credential field names (v2.1+) - -The credentials JSON file field names are updated to follow Java camelCase conventions. Both old and new forms are permanently accepted. - -| Old form (still accepted) | New form (preferred) | -|---|---| -| `clientID` | `clientId` | -| `keyID` | `keyId` | -| `tokenURI` | `tokenUri` | - ---- - -## Response field names (v2.1+) - -Response maps now return `skyflowId` (camelCase). The legacy `skyflow_id` key is still present for backward compatibility but is deprecated. - -| Deprecated (still returned) | Preferred | -|---|---| -| `skyflow_id` | `skyflowId` | - ---- - -## Update request data key (v2.1+) - -When calling `update()`, use `skyflowId` (camelCase) as the key in the data map to identify the record. Using `skyflow_id` still works but emits a deprecation warning. If both keys are present, `skyflowId` takes precedence. - -```java -HashMap data = new HashMap<>(); -data.put("skyflowId", ""); // preferred -data.put("card_number", ""); - -UpdateRequest request = UpdateRequest.builder() - .table("") - .data(data) - .returnTokens(true) - .build(); - -skyflowClient.vault().update(request); -``` - ---- - -## Method renames (v2.1+) - -The following instance methods have been renamed for consistency. The old names still work but emit deprecation warnings. - -| Deprecated | Preferred | -|---|---| -| `skyflowClient.updateLogLevel(logLevel)` | `skyflowClient.setLogLevel(logLevel)` | -| `TokenMode.getBYOT()` | `TokenMode.getByot()` | -| `DetokenizeRequest.builder().downloadURL(b)` | `DetokenizeRequest.builder().downloadUrl(b)` | - ---- - -For the full list of changes see [CHANGELOG.md](../CHANGELOG.md). diff --git a/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md b/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md deleted file mode 100644 index a386accd..00000000 --- a/docs/superpowers/plans/2026-05-13-java-nomenclature-cleanup.md +++ /dev/null @@ -1,589 +0,0 @@ -# Java SDK Nomenclature Cleanup Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Rename credential JSON field keys (`clientID`→`clientId`, `keyID`→`keyId`, `tokenURI`→`tokenUri`) with fallback support, normalise `skyflow_id`→`skyflowId` in Get and Query responses, and add `getErrors()` to `QueryResponse`. - -**Architecture:** Three independent, targeted changes to existing files — no new files, no new abstractions. Each change is a surgical edit to one method or class, verified by unit tests that already exist or that we add inline. - -**Tech Stack:** Java 11+, JUnit 4, Mockito/PowerMock, Maven (`mvn test`) - -**Design spec:** `docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md` - ---- - -## File Map - -| File | Change | -|---|---| -| `src/main/java/com/skyflow/serviceaccount/util/BearerToken.java` | Fallback lookup for `clientId`/`keyId`/`tokenUri` in `getBearerTokenFromCredentials` | -| `src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java` | Fallback lookup for `clientId`/`keyId` in `generateSignedTokensFromCredentials` | -| `src/main/java/com/skyflow/vault/controller/VaultController.java` | Rename `skyflow_id`→`skyflowId` in `getFormattedGetRecord` and `getFormattedQueryRecord` | -| `src/main/java/com/skyflow/vault/data/QueryResponse.java` | Add `errors` field and `getErrors()` accessor | -| `src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java` | Add tests for new-form keys, old-form fallback, and missing-key errors | -| `src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java` | Add tests for new-form keys, old-form fallback, and missing-key errors | -| `src/test/java/com/skyflow/vault/data/QueryResponseTest.java` | New file — tests for `getErrors()` always returning null | - ---- - -## Task 1: Credential field renames in BearerToken — new key form - -**Files:** -- Modify: `src/main/java/com/skyflow/serviceaccount/util/BearerToken.java:92-145` -- Modify: `src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java` - -### Background -`getBearerTokenFromCredentials` parses a `JsonObject` representing the credentials file. It currently looks up `clientID`, `keyID`, and `tokenURI`. We need it to accept `clientId`, `keyId`, `tokenUri` (new canonical form) while still accepting the old keys as a fallback. - -The test at line 228 of `BearerTokenTests.java` currently uses the old keys — we need a parallel test using the new keys. - -- [ ] **Step 1: Write a failing test for new-form credential keys** - -Add this test to `BearerTokenTests.java`. It uses a credentials string with `clientId`, `keyId`, `tokenUri` (new form) and expects a `SkyflowException` with the `InvalidTokenUri` message (because the URI value is invalid — not because the keys are unrecognised). This confirms the new keys are read successfully. - -```java -@Test -public void testBearerTokenWithNewFormCredentialKeys() { - try { - String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\ncHJpdmF0ZV9rZXlfdmFsdWU=\\n-----END PRIVATE KEY-----\", " - + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\", \"tokenUri\": \"invalid_token_uri\"}"; - BearerToken bearerToken = BearerToken.builder().setCredentials(credentialsString).build(); - bearerToken.getBearerToken(); - Assert.fail(EXCEPTION_NOT_THROWN); - } catch (SkyflowException e) { - Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); - Assert.assertEquals(ErrorMessage.InvalidTokenUri.getMessage(), e.getMessage()); - } -} -``` - -- [ ] **Step 2: Run the test to confirm it fails** - -```bash -mvn test -pl . -Dtest=BearerTokenTests#testBearerTokenWithNewFormCredentialKeys -q -``` - -Expected: FAIL — the test throws `MissingClientId` (because `clientId` is not found, only `clientID` is looked up). - -- [ ] **Step 3: Update `getBearerTokenFromCredentials` in `BearerToken.java`** - -Replace the three field lookups (lines 102–118) with fallback logic: - -```java -JsonElement clientId = credentials.get("clientId"); -if (clientId == null) clientId = credentials.get("clientID"); -if (clientId == null) { - LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); -} - -JsonElement keyId = credentials.get("keyId"); -if (keyId == null) keyId = credentials.get("keyID"); -if (keyId == null) { - LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); -} - -JsonElement tokenUri = credentials.get("tokenUri"); -if (tokenUri == null) tokenUri = credentials.get("tokenURI"); -if (tokenUri == null) { - LogUtil.printErrorLog(ErrorLogs.TOKEN_URI_IS_REQUIRED.getLog()); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingTokenUri.getMessage()); -} -``` - -Also update the `getSignedToken` call on line 121 to use the renamed variables: - -```java -String signedUserJWT = getSignedToken( - clientId.getAsString(), keyId.getAsString(), tokenUri.getAsString(), pvtKey, context -); -String basePath = Utils.getBaseURL(tokenUri.getAsString()); -``` - -Also update the private method signature at line 147–148 to use idiomatic parameter names (internal only, no public impact): - -```java -private static String getSignedToken( - String clientId, String keyId, String tokenUri, PrivateKey pvtKey, Object context -) { - final Date createdDate = new Date(); - final Date expirationDate = new Date(createdDate.getTime() + (3600 * 1000)); - io.jsonwebtoken.JwtBuilder builder = Jwts.builder() - .claim("iss", clientId) - .claim("key", keyId) - .claim("aud", tokenUri) - .claim("sub", clientId) - .expiration(expirationDate); - if (context != null) { - builder.claim("ctx", context); - } - return builder.signWith(pvtKey, Jwts.SIG.RS256).compact(); -} -``` - -- [ ] **Step 4: Run the new test to confirm it passes** - -```bash -mvn test -pl . -Dtest=BearerTokenTests#testBearerTokenWithNewFormCredentialKeys -q -``` - -Expected: PASS — `clientId` is found, execution reaches `InvalidTokenUri`. - -- [ ] **Step 5: Run the full BearerToken test suite to confirm no regressions** - -```bash -mvn test -pl . -Dtest=BearerTokenTests -q -``` - -Expected: All existing tests pass (old-form keys `clientID`/`keyID`/`tokenURI` still work via fallback). - -- [ ] **Step 6: Commit** - -```bash -git add src/main/java/com/skyflow/serviceaccount/util/BearerToken.java \ - src/test/java/com/skyflow/serviceaccount/util/BearerTokenTests.java -git commit -m "feat: accept clientId/keyId/tokenUri in BearerToken with fallback to old form" -``` - ---- - -## Task 2: Credential field renames in SignedDataTokens — new key form - -**Files:** -- Modify: `src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java:92-122` -- Modify: `src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java` - -### Background -`generateSignedTokensFromCredentials` parses a credentials `JsonObject` and looks up `clientID` and `keyID`. Same rename as Task 1, but no `tokenURI` (SignedDataTokens does not need it). - -- [ ] **Step 1: Check what the existing SignedDataTokens test uses for credential keys** - -```bash -grep -n "clientID\|keyID\|clientId\|keyId" src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java -``` - -Note the line number of any credentials string that uses `clientID`/`keyID` — you will add a parallel test using the new keys. - -- [ ] **Step 2: Write a failing test for new-form keys** - -Add this test to `SignedDataTokensTests.java`. It expects the token generation to fail at the private key parsing stage (not at the field-lookup stage), confirming `clientId` and `keyId` are successfully read: - -```java -@Test -public void testSignedDataTokensWithNewFormCredentialKeys() { - try { - String credentialsString = "{\"privateKey\": \"-----BEGIN PRIVATE KEY-----\\ncHJpdmF0ZV9rZXlfdmFsdWU=\\n-----END PRIVATE KEY-----\", " - + "\"clientId\": \"client_id_value\", \"keyId\": \"key_id_value\"}"; - ArrayList dataTokens = new ArrayList<>(); - dataTokens.add("test-token"); - SignedDataTokens signedDataTokens = SignedDataTokens.builder() - .setCredentials(credentialsString) - .setDataTokens(dataTokens) - .build(); - signedDataTokens.getSignedDataTokens(); - Assert.fail(EXCEPTION_NOT_THROWN); - } catch (SkyflowException e) { - // Should fail past field lookup — at private key parsing, not at MissingClientId - Assert.assertNotEquals(ErrorMessage.MissingClientId.getMessage(), e.getMessage()); - Assert.assertNotEquals(ErrorMessage.MissingKeyId.getMessage(), e.getMessage()); - } -} -``` - -- [ ] **Step 3: Run the test to confirm it fails** - -```bash -mvn test -pl . -Dtest=SignedDataTokensTests#testSignedDataTokensWithNewFormCredentialKeys -q -``` - -Expected: FAIL — throws `MissingClientId` because `clientId` is not yet recognised. - -- [ ] **Step 4: Update `generateSignedTokensFromCredentials` in `SignedDataTokens.java`** - -Replace the `clientID` and `keyID` lookups (lines 103–113) with fallback logic: - -```java -JsonElement clientId = credentials.get("clientId"); -if (clientId == null) clientId = credentials.get("clientID"); -if (clientId == null) { - LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog()); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage()); -} - -JsonElement keyId = credentials.get("keyId"); -if (keyId == null) keyId = credentials.get("keyID"); -if (keyId == null) { - LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog()); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage()); -} -``` - -Update the `getSignedToken` call on line 115 to use the renamed variables: - -```java -signedDataTokens = getSignedToken( - clientId.getAsString(), keyId.getAsString(), pvtKey, dataTokens, timeToLive, context); -``` - -Update the private method signature at line 124–125 to use idiomatic parameter names: - -```java -private static List getSignedToken( - String clientId, String keyId, PrivateKey pvtKey, - ArrayList dataTokens, Integer timeToLive, Object context -) { -``` - -And update the JWT claims inside `getSignedToken` (lines 142–143): - -```java -.claim("key", keyId) -.claim("sub", clientId) -``` - -- [ ] **Step 5: Run the new test to confirm it passes** - -```bash -mvn test -pl . -Dtest=SignedDataTokensTests#testSignedDataTokensWithNewFormCredentialKeys -q -``` - -Expected: PASS — `clientId` and `keyId` are found; exception is from private key parsing, not from missing fields. - -- [ ] **Step 6: Run the full SignedDataTokens test suite** - -```bash -mvn test -pl . -Dtest=SignedDataTokensTests -q -``` - -Expected: All existing tests pass. - -- [ ] **Step 7: Commit** - -```bash -git add src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java \ - src/test/java/com/skyflow/serviceaccount/util/SignedDataTokensTests.java -git commit -m "feat: accept clientId/keyId in SignedDataTokens with fallback to old form" -``` - ---- - -## Task 3: Normalise `skyflow_id` → `skyflowId` in Get and Query responses - -**Files:** -- Modify: `src/main/java/com/skyflow/vault/controller/VaultController.java:121-152` -- Modify: `src/test/java/com/skyflow/vault/controller/VaultControllerTests.java` - -### Background -`getFormattedGetRecord` and `getFormattedQueryRecord` call `putAll(fieldsOpt.get())` which passes through the raw API map — including the `skyflow_id` snake_case key from the wire format. Insert and Update responses already use `skyflowId`. This inconsistency means callers must know which operation produced the response in order to read the record ID. - -The test suite does not currently test the contents of Get or Query responses (no existing tests for `skyflowId` in these paths), so we add new unit tests. - -Because the actual vault API is not called in unit tests (no mock infrastructure for it in `VaultControllerTests`), we test the formatter methods indirectly by verifying the behaviour of the public `get()` and `query()` methods throw the right validation errors — and we test the formatters directly via reflection, or we add a thin package-private helper. - -The simplest approach: add package-private unit tests for the two static formatter methods directly. - -- [ ] **Step 1: Write failing tests for the formatter methods** - -Add these tests to `VaultControllerTests.java`: - -```java -import com.skyflow.generated.rest.types.V1FieldRecords; -import java.util.HashMap; -import java.util.Map; -import java.lang.reflect.Method; - -@Test -public void testGetFormattedGetRecordNormalisesSkyflowId() throws Exception { - Map fields = new HashMap<>(); - fields.put("skyflow_id", "abc-123"); - fields.put("name", "John"); - V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); - - Method method = VaultController.class.getDeclaredMethod("getFormattedGetRecord", V1FieldRecords.class); - method.setAccessible(true); - @SuppressWarnings("unchecked") - HashMap result = (HashMap) method.invoke(null, record); - - Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); - Assert.assertEquals("skyflowId should be present", "abc-123", result.get("skyflowId")); - Assert.assertEquals("other fields should be preserved", "John", result.get("name")); -} - -@Test -public void testGetFormattedQueryRecordNormalisesSkyflowId() throws Exception { - Map fields = new HashMap<>(); - fields.put("skyflow_id", "xyz-456"); - fields.put("email", "test@example.com"); - V1FieldRecords record = V1FieldRecords.builder().fields(fields).build(); - - Method method = VaultController.class.getDeclaredMethod("getFormattedQueryRecord", V1FieldRecords.class); - method.setAccessible(true); - @SuppressWarnings("unchecked") - HashMap result = (HashMap) method.invoke(null, record); - - Assert.assertFalse("skyflow_id (snake_case) should not be present", result.containsKey("skyflow_id")); - Assert.assertEquals("skyflowId should be present", "xyz-456", result.get("skyflowId")); - Assert.assertEquals("other fields should be preserved", "test@example.com", result.get("email")); -} -``` - -- [ ] **Step 2: Run the tests to confirm they fail** - -```bash -mvn test -pl . -Dtest=VaultControllerTests#testGetFormattedGetRecordNormalisesSkyflowId+testGetFormattedQueryRecordNormalisesSkyflowId -q -``` - -Expected: FAIL — `skyflow_id` is present in the result, `skyflowId` is absent. - -- [ ] **Step 3: Update `getFormattedGetRecord` in `VaultController.java`** - -After the `putAll` block (after line 131), add the key rename: - -```java -private static synchronized HashMap getFormattedGetRecord(V1FieldRecords record) { - HashMap getRecord = new HashMap<>(); - - Optional> fieldsOpt = record.getFields(); - Optional> tokensOpt = record.getTokens(); - - if (fieldsOpt.isPresent()) { - getRecord.putAll(fieldsOpt.get()); - } else if (tokensOpt.isPresent()) { - getRecord.putAll(tokensOpt.get()); - } - - if (getRecord.containsKey("skyflow_id")) { - getRecord.put("skyflowId", getRecord.remove("skyflow_id")); - } - - return getRecord; -} -``` - -- [ ] **Step 4: Update `getFormattedQueryRecord` in `VaultController.java`** - -After the `putAll` block (after line 150), add the key rename: - -```java -private static synchronized HashMap getFormattedQueryRecord(V1FieldRecords record) { - HashMap queryRecord = new HashMap<>(); - Optional> fieldsOpt = record.getFields(); - if (fieldsOpt.isPresent()) { - queryRecord.putAll(fieldsOpt.get()); - } - - if (queryRecord.containsKey("skyflow_id")) { - queryRecord.put("skyflowId", queryRecord.remove("skyflow_id")); - } - - return queryRecord; -} -``` - -- [ ] **Step 5: Run the new tests to confirm they pass** - -```bash -mvn test -pl . -Dtest=VaultControllerTests#testGetFormattedGetRecordNormalisesSkyflowId+testGetFormattedQueryRecordNormalisesSkyflowId -q -``` - -Expected: PASS. - -- [ ] **Step 6: Run the full VaultController test suite** - -```bash -mvn test -pl . -Dtest=VaultControllerTests -q -``` - -Expected: All existing tests pass. - -- [ ] **Step 7: Commit** - -```bash -git add src/main/java/com/skyflow/vault/controller/VaultController.java \ - src/test/java/com/skyflow/vault/controller/VaultControllerTests.java -git commit -m "feat: normalise skyflow_id to skyflowId in Get and Query response maps" -``` - ---- - -## Task 4: Add `getErrors()` to `QueryResponse` - -**Files:** -- Modify: `src/main/java/com/skyflow/vault/data/QueryResponse.java` -- Create: `src/test/java/com/skyflow/vault/data/QueryResponseTest.java` - -### Background -`QueryResponse` is the only response class without a `getErrors()` method. The field is referenced in `toString()` as a hardcoded `null` literal but is not accessible programmatically. We add the field and accessor to match the pattern in `GetResponse`, `InsertResponse`, and `UpdateResponse` (all return `null` when no errors). - -- [ ] **Step 1: Write a failing test** - -Create `src/test/java/com/skyflow/vault/data/QueryResponseTest.java`: - -```java -package com.skyflow.vault.data; - -import org.junit.Assert; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.HashMap; - -public class QueryResponseTest { - - @Test - public void testGetErrorsReturnsNull() { - ArrayList> fields = new ArrayList<>(); - HashMap record = new HashMap<>(); - record.put("skyflowId", "abc-123"); - fields.add(record); - - QueryResponse response = new QueryResponse(fields); - - Assert.assertNull("getErrors() should return null when no errors", response.getErrors()); - } - - @Test - public void testGetErrorsIsPresentInToString() { - QueryResponse response = new QueryResponse(new ArrayList<>()); - String json = response.toString(); - Assert.assertTrue("toString() should include errors field", json.contains("\"errors\"")); - } -} -``` - -- [ ] **Step 2: Run the tests to confirm they fail** - -```bash -mvn test -pl . -Dtest=QueryResponseTest -q -``` - -Expected: FAIL — compile error: `getErrors()` method does not exist on `QueryResponse`. - -- [ ] **Step 3: Update `QueryResponse.java`** - -Add the `errors` field and accessor. The `toString()` no longer needs to manually inject `errors` since `serializeNulls` will include it automatically: - -```java -package com.skyflow.vault.data; - -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.google.gson.JsonArray; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; - -import java.util.ArrayList; -import java.util.HashMap; - -public class QueryResponse { - private final ArrayList> fields; - private final ArrayList> errors; - - public QueryResponse(ArrayList> fields) { - this.fields = fields; - this.errors = null; - } - - public ArrayList> getFields() { - return fields; - } - - public ArrayList> getErrors() { - return errors; - } - - @Override - public String toString() { - Gson gson = new GsonBuilder().serializeNulls().create(); - JsonObject responseObject = gson.toJsonTree(this).getAsJsonObject(); - // tokenizedData is intentionally not surfaced — Query API cannot return tokens - JsonArray fieldsArray = responseObject.get("fields").getAsJsonArray(); - for (JsonElement fieldElement : fieldsArray) { - fieldElement.getAsJsonObject().add("tokenizedData", new JsonObject()); - } - return responseObject.toString(); - } -} -``` - -- [ ] **Step 4: Run the new tests to confirm they pass** - -```bash -mvn test -pl . -Dtest=QueryResponseTest -q -``` - -Expected: PASS. - -- [ ] **Step 5: Run the full test suite to confirm no regressions** - -```bash -mvn test -q -``` - -Expected: All tests pass. - -- [ ] **Step 6: Commit** - -```bash -git add src/main/java/com/skyflow/vault/data/QueryResponse.java \ - src/test/java/com/skyflow/vault/data/QueryResponseTest.java -git commit -m "feat: add getErrors() accessor to QueryResponse" -``` - ---- - -## Task 5: LOW audit — verify no `setFooID` / `getFooID` violations - -**Files:** -- Read-only audit (no changes expected) - -### Background -The spec requires confirming that all builder setter/getter methods use `setFooId()` / `getFooId()` (title-case `Id`), not `setFooID()`. Initial review of `VaultConfig` already shows `setVaultId()` and `setClusterId()` are correct. This task confirms nothing was missed. - -- [ ] **Step 1: Run the grep audit** - -```bash -grep -rn "set[A-Za-z]*ID\b\|get[A-Za-z]*ID\b" \ - src/main/java/com/skyflow/config/ \ - src/main/java/com/skyflow/vault/data/ \ - src/main/java/com/skyflow/serviceaccount/ \ - --include="*.java" -``` - -Expected output: **no results** — all methods already use title-case `Id`. - -- [ ] **Step 2: If violations are found, rename them** - -For each violation (e.g. `setVaultID` → `setVaultId`), use your editor's rename refactor across all callers, then run: - -```bash -mvn test -q -``` - -Expected: All tests pass. - -- [ ] **Step 3: Commit (only if changes were made)** - -```bash -git add -p -git commit -m "fix: rename setFooID/getFooID to setFooId/getFooId per Java convention" -``` - -If no violations were found, record the result: - -```bash -git commit --allow-empty -m "chore: audit confirms no setFooID/getFooID violations in public API" -``` - ---- - -## Final verification - -- [ ] **Run the complete test suite one last time** - -```bash -mvn test -q -``` - -Expected: All tests pass with no failures or errors. diff --git a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md b/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md deleted file mode 100644 index 9873fdcf..00000000 --- a/docs/superpowers/specs/2026-05-13-java-nomenclature-cleanup-design.md +++ /dev/null @@ -1,133 +0,0 @@ -# Java SDK Nomenclature Cleanup — Design Spec - -**Date:** 2026-05-13 -**Reference:** [Skyflow Server-Side SDK: Nomenclature changes](https://skyflow.atlassian.net/wiki/spaces/SDK1/pages/2933162001/Skyflow+Server-Side+SDK+Nomenclature+changes) -**Scope:** Public interface only (`src/main/java/com/skyflow/`) -**Target release:** v3.0.0 (HIGH), v3.1.0 (MEDIUM), v3.1.x (LOW audit) - ---- - -## Summary of changes - -| Priority | Change | Files | -|---|---|---| -| HIGH | `clientID` → `clientId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | -| HIGH | `keyID` → `keyId` in credentials JSON parsing (with fallback) | `BearerToken.java`, `SignedDataTokens.java` | -| HIGH | `tokenURI` → `tokenUri` in credentials JSON parsing (with fallback) | `BearerToken.java` | -| MEDIUM | `skyflow_id` → `skyflowId` key in Get and Query response maps | `VaultController.java` | -| MEDIUM | `getErrors()` added to `QueryResponse` (field was missing entirely) | `QueryResponse.java` | -| LOW | Audit all builder setter/getter names — confirm no `setFooID()` pattern | `VaultConfig.java`, request builders | - ---- - -## Detailed design - -### HIGH: Credentials JSON field renames (`clientID` → `clientId`, `keyID` → `keyId`, `tokenURI` → `tokenUri`) - -**Affected:** `BearerToken.java` (`getBearerTokenFromCredentials`), `SignedDataTokens.java` (`generateSignedTokensFromCredentials`) - -**Why this change is needed:** - -Java's naming convention treats acronyms as ordinary word components in camelCase identifiers — `Id` not `ID`, `Uri` not `URI`. The current field names `clientID`, `keyID`, `tokenURI` violate this by capitalising the acronym in full. This is inconsistent with the rest of the SDK (e.g. `setVaultId()`, `setClusterId()`) and breaks the "principle of least surprise" for Java developers who expect `clientId`. - -These field names are defined in the credentials JSON file that users create and pass to the SDK (either as a file path or as a credentials string). They are therefore part of the SDK's public contract — a change forces users to update their credentials files. This is a breaking change, which is why it is gated to the v3.0.0 major release. - -**Why a fallback is used instead of a hard cut:** - -A hard cut would silently break all existing integrations the moment users upgrade to v3. The try-new-first fallback gives users a transition window: credentials files with the old keys continue to work, and users can migrate at their own pace. The fallback can be removed in a future major version once the old form is fully deprecated. - -**Implementation strategy:** Try the new key first; fall back to the old key if null; throw if both are absent. - -```java -// clientID → clientId -JsonElement clientId = credentials.get("clientId"); -if (clientId == null) clientId = credentials.get("clientID"); -if (clientId == null) { - throw new SkyflowException(...MissingClientId...); -} - -// keyID → keyId -JsonElement keyId = credentials.get("keyId"); -if (keyId == null) keyId = credentials.get("keyID"); -if (keyId == null) { - throw new SkyflowException(...MissingKeyId...); -} - -// tokenURI → tokenUri (BearerToken only — SignedDataTokens does not use tokenURI) -JsonElement tokenUri = credentials.get("tokenUri"); -if (tokenUri == null) tokenUri = credentials.get("tokenURI"); -if (tokenUri == null) { - throw new SkyflowException(...MissingTokenUri...); -} -``` - -Local variable names and private method parameter names are also updated to the new form (`clientId`, `keyId`, `tokenUri`) for internal consistency, though this has no effect on the public interface. - ---- - -### MEDIUM: `skyflow_id` → `skyflowId` in Get and Query response maps - -**Affected:** `VaultController.java` — `getFormattedGetRecord()` and `getFormattedQueryRecord()` - -**Why this change is needed:** - -The Skyflow REST API returns records with a `skyflow_id` field in snake_case — this is the wire format. The Java SDK is responsible for translating the wire format into language-idiomatic representations before handing data to callers. Java is a camelCase language, and the SDK already normalises `skyflow_id` to `skyflowId` in Insert and Update responses: - -- `getFormattedBatchInsertRecord`: `insertRecord.put("skyflowId", recordObject.get("skyflow_id").getAsString())` -- `getFormattedBulkInsertRecord`: `insertRecord.put("skyflowId", record.getSkyflowId().get())` -- `getFormattedUpdateRecord`: `updateTokens.put("skyflowId", skyflowId)` - -However, `getFormattedGetRecord` and `getFormattedQueryRecord` call `putAll(fieldsOpt.get())` which passes the raw API map directly through — including `skyflow_id` in snake_case. This inconsistency means that developers who write `record.get("skyflowId")` after a Get or Query call get `null`, while the same code works after an Insert or Update. It forces callers to know which operation produced the response just to read a single field. - -**Implementation:** After `putAll`, check for the raw API key and rename it: - -```java -if (record.containsKey("skyflow_id")) { - record.put("skyflowId", record.remove("skyflow_id")); -} -``` - -Applied in both `getFormattedGetRecord` and `getFormattedQueryRecord`. - ---- - -### MEDIUM: `getErrors()` added to `QueryResponse` - -**Affected:** `QueryResponse.java` - -**Why this change is needed:** - -All other response types in the SDK (`GetResponse`, `InsertResponse`, `UpdateResponse`, `FileUploadResponse`) expose a `getErrors()` method. `QueryResponse` is the only one that does not — the `errors` field is referenced only inside `toString()` as a hardcoded literal `null`: - -```java -responseObject.add("errors", null); -``` - -A caller who writes `queryResponse.getErrors()` gets a compile error because the method does not exist. This breaks the consistency contract that callers rely on when writing generic response-handling code across different vault operations. - -**Fix:** Add `private final ArrayList> errors` as a constructor field (always `null` — consistent with other response types that pass `null` when there are no errors) and expose it via `getErrors()`. The field will always be `null` for QueryResponse since the Query API does not currently model partial-error responses the same way batch insert does. This is kept as `null` rather than an empty list to stay consistent with the existing pattern across other response classes. - ---- - -### LOW: Audit builder setter/getter names - -**Affected:** `VaultConfig.java`, `InsertRequest`, `UpdateRequest`, `GetRequest`, `DeleteRequest`, `FileUploadRequest`, `QueryRequest` - -**Why this change is needed:** - -The same acronym-casing rule that applies to credentials fields applies to all Java method names. Any setter or getter using `ID` (all-caps) as a suffix — e.g. `setVaultID()`, `getSkyflowID()` — is non-idiomatic and inconsistent with Java convention. The spec item 15 calls out this as a verification task. - -From initial review, `setVaultId()` and `setClusterId()` in `VaultConfig` are already correct. A full grep audit across all request builder classes is required to confirm there are no remaining `setFooID()` / `getFooID()` methods that were missed. - -**Outcome:** If any violations are found, rename them to `setFooId()` / `getFooId()`. If none are found, this item is closed as verified-clean. - ---- - -## What is NOT in scope - -- **`tokenizedData` in QueryResponse:** The Skyflow Query API explicitly cannot return tokens. The existing `toString()` hack that injects `tokenizedData: {}` is a minor inconsistency between string output and programmatic access, but since callers have no reason to access tokenized data from a query result, this is not worth fixing now. - -- **`UpdateRequest.getData()` map key**: Users currently pass `skyflow_id` (snake_case) in the data map to identify the record to update. This is an *input* key consumed by the SDK internally (`updateRequest.getData().remove("skyflow_id")`), not a response field surfaced to callers. The spec does not address this and changing it would require a separate design decision. -- **Generated REST client code** under `com.skyflow.generated.*`: These files are auto-generated by Fern from the API definition. Manual edits would be overwritten on the next regeneration. -- **`SKYFLOW_CREDENTIALS` environment variable name**: Stays `ALL_CAPS` per OS and shell convention. Only the parsed field names within the JSON value change. -- **Validation logic for null/None insert values**: The spec marks this as Python-only (item 12). Java already throws on invalid input at the API boundary.