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)