Devesh/sk 2832 claude setup v2

.claude/commands/code-quality.md

@@ -0,0 +1,90 @@
+---
+name: code-quality
+description: Quality pipeline — compile, checkstyle, build, tests, coverage check. Pass a class name to target a single test class.
+paths:
+  - src/**/*.java
+  - pom.xml
+exclude:
+  - src/main/java/com/skyflow/generated/**
+context: fork
+---
+
+Run the Skyflow Java SDK quality pipeline.
+
+Use `$ARGUMENTS` to target a specific test class (e.g. `BearerTokenTests`). If empty, run the full suite.
+
+> Baseline failures are documented in the Known Pre-existing Test Failures table.
+> Do not investigate them unless specifically asked. Only report failures **beyond** that baseline.
+
+## Coverage Requirements
+
+Follow the Tests coding rules (100% instruction + branch coverage). Public interface packages:
+- `src/main/java/com/skyflow/vault/` (controllers, data, tokens, connection, audit, bin, detect)
+- `src/main/java/com/skyflow/config/`
+- `src/main/java/com/skyflow/serviceaccount/`
+
+Flag any gap as a blocker — **NEEDS FIXES** if coverage is below 100% on Claude-written or public interface code.
+
+---
+
+## 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
+For every public interface class and every class touched by Claude in this session:
+- Check for a corresponding test file under `src/test/`
+- Check that every public method has at least one positive and one negative test case
+- Check that every branch (if/else, switch, try/catch) is covered
+
+List all gaps. Any gap on Claude-written or public interface code is a **blocker**.
+
+### Step 6 — Edge case identification
+For any class below 100% 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 (100%)  | ✅ / ❌   | list classes with gaps       |
+```
+
+Conclude with **READY TO MERGE** or **NEEDS FIXES** and a prioritised fix list.

.claude/commands/code-review.md

@@ -0,0 +1,107 @@
+---
+name: code-review
+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
+exclude:
+  - src/main/java/com/skyflow/generated/**
+context: fork
+---
+
+You are a senior engineer performing a thorough code review on the Skyflow Java SDK.
+
+## Pre-requisite
+
+If `GITHUB_ACTIONS` environment variable is set, skip this step (CI runs compile/test in a separate job).
+
+Otherwise, confirm `/code-quality` has been run and passed (compile, tests, 100% coverage). If it has not been run, run it now before proceeding with the review.
+
+## Scope
+
+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 PR/branch vs base:
+  ```bash
+  # REVIEW_BASE_SHA, when set (CI incremental review), is the last commit already
+  # reviewed by the bot — diff only lines added since then. Otherwise fall back to
+  # the PR base branch.
+  BASE="${REVIEW_BASE_SHA:-${GITHUB_BASE_REF:+origin/$GITHUB_BASE_REF}}"
+  BASE="${BASE:-main}"
+  git diff "$BASE"...HEAD --name-only | grep '\.java$' | grep -v 'generated'
+  ```
+  **If `GITHUB_ACTIONS` is set:** work from the diff output directly (changed lines only) instead of reading full files:
+  ```bash
+  git diff "$BASE"...HEAD -- '*.java' | grep -v 'src/main/java/com/skyflow/generated/'
+  ```
+  Review only added lines (`+` prefix) from the diff. Do not comment on unchanged context lines or pre-existing code.
+
+---
+
+## Step 1 — SDK Pattern Review
+
+Review all files in scope against the rules defined in `CLAUDE.md` (loaded automatically from the project root). Check every rule category: naming conventions, error handling, request/response patterns, string literals, tests, and code quality.
+
+Group findings by file and produce a table:
+
+```
+### path/to/File.java
+
+| Severity | Category | Line | Finding |
+|----------|----------|------|---------|
+| Critical | Security    | 42  | SkyflowException swallowed in catch block |
+| High     | Correctness | 87  | skyflow_id not normalised to skyflowId |
+| Low      | Pattern     | 103 | Magic string "records" — use Constants |
+```
+
+Every finding has **two independent axes** — don't conflate them:
+
+**Severity** — *how serious* (one scale shared by all three steps):
+
+| Severity | Meaning | Blocks merge? |
+|---|---|---|
+| **Critical** | Data loss, security breach, silent failure | Yes |
+| **High** | Wrong behaviour / bug / guaranteed runtime failure | Yes |
+| **Medium** | Likely problem, risky or unhandled input, missing safeguard | Yes |
+| **Low** | Minor maintainability, naming, style, code smell | No — advisory |
+| **Info** | Note / FYI | No — advisory |
+
+**Category** — *what kind*: `Correctness` (a bug), `Edge case`, `Security`, `Pattern`, `Naming`, `Tests`, `Smell`.
+
+A logic bug is **Severity `High`/`Critical` + Category `Correctness`** — never severity "Bug". A magic string is **Severity `Low` + Category `Pattern`** — never severity "Quality". Keep level in the Severity column and kind in the Category column.
+
+---
+
+## 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).
+
+**If `GITHUB_ACTIONS` is set:** apply that command's **PR / CI mode** — report only smells introduced by added (`+`) lines; do not report whole-file metrics (Long class/method, large parameter list) or any pre-existing debt. Do **not** print code-smell's standalone tables, summary, or recommendation — collect its findings into the single consolidated report defined in **Output (PR / CI mode)** below.
+
+---
+
+## 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).
+
+**If `GITHUB_ACTIONS` is set:** apply that command's **PR / CI mode** — report only issues introduced by added (`+`) lines; do not raise pre-existing vulnerabilities or whole-project checks the diff does not touch. **The security audit's scope additionally includes a changed `pom.xml`** (its own diff command covers it): when this PR modifies `pom.xml`, audit the added/changed dependency lines for known CVEs even though Steps 1–2 stay `.java`-only. Do **not** print code-security's standalone per-finding blocks, summary, or risk rating — collect its findings into the single consolidated report defined in **Output (PR / CI mode)** below.
+
+---
+
+## Final Verdict (local mode only)
+
+> Skip this section when `GITHUB_ACTIONS` is set — use **Output (PR / CI mode)** instead.
+
+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`
+3. Remind: run `/code-quality` again after any fixes before merging.
+
+---
+
+## Output (PR / CI mode)
+
+**If `GITHUB_ACTIONS` is set:** read `.claude/includes/code-review-ci.md` and produce **exactly** the consolidated code-review-comment report it specifies — nothing else. Do **not** emit the local-mode **Final Verdict** above. (This file is fetched only in CI; local runs never read it.)
+
+Otherwise (local mode), this section does not apply — use the **Final Verdict (local mode)** section above.

.claude/commands/code-security.md

@@ -0,0 +1,84 @@
+---
+name: code-security
+description: Security audit — credential exposure, input validation, path traversal, HTTP security, token lifecycle, dependency CVEs.
+paths:
+  - src/main/java/com/skyflow/**/*.java
+  - pom.xml
+exclude:
+  - src/main/java/com/skyflow/generated/**
+context: fork
+---
+
+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
+# CI: GITHUB_BASE_REF is set (e.g. "main") — use origin/ prefix
+# Local: unset — use main directly
+BASE="${GITHUB_BASE_REF:+origin/$GITHUB_BASE_REF}"
+BASE="${BASE:-main}"
+git diff "$BASE"...HEAD --name-only | grep -E '\.java$|(^|/)pom\.xml$' | grep -v 'generated'
+```
+
+**If `GITHUB_ACTIONS` is set (PR review mode):** audit only the code this PR changed. Work from the diff — **note the pathspec includes `pom.xml` so dependency changes are never invisible to the audit:**
+```bash
+git diff "$BASE"...HEAD -- '*.java' 'pom.xml' | grep -v 'src/main/java/com/skyflow/generated/'
+```
+Report a finding **only if an added line (`+` prefix) introduces or directly exposes it.** Do not raise pre-existing vulnerabilities in unchanged code, and skip whole-project checks the diff does not touch. **The diff above includes `pom.xml`; whenever a changed `<dependency>` appears in it, you MUST run §6 against those lines — do not treat the audit as `.java`-only.** If the added lines introduce no security issues, state that explicitly rather than listing pre-existing risks. (Local / non-CI runs and explicit file arguments keep full-file auditing.)
+
+## Security Checks
+
+Where a finding maps to an **OWASP Top 10** category (e.g. `A01 — Broken Access Control`, `A06 — Vulnerable and Outdated Components`), tag it with that category in the output — only where it genuinely applies; don't force a mapping.
+
+### 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 (Critical)
+- Flag any dependency with a known CVE (check `pom.xml` versions). Report at **Critical** severity so it surfaces in the serious-findings table and gets an inline comment on the changed `pom.xml` line.
+
+### 7. Authentication lifecycle (Medium)
+- Bearer token caching must check expiry before reuse
+- Token refresh must be thread-safe (`synchronized` or equivalent)
+
+## Account for every check
+
+Before writing the report, walk checks **1–7 in order** against the changed lines and account for each one — do not report only the issues that first stand out. The Medium-severity categories (§4 HTTP, §5 error leakage, §7 auth lifecycle) and the dependency check (§6) are missed far more often than credential exposure (§1); give them equal scrutiny.
+
+## 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
+**OWASP:** Relevant OWASP Top 10 category, e.g. `A06 — Vulnerable and Outdated Components` — include only when the finding clearly maps to one; omit otherwise.
+```
+
+End with a summary table and overall risk rating.