feat(sdk-review): batch 6/6 — wire-up + docs + CONTRIBUTING amendment

.claude/commands/review-new-module.md

@@ -0,0 +1,19 @@
+# /review-new-module — orchestrator slash command
+Run the SDK Module Review skill against a PR.
+
+## Usage
+- `/review-new-module <PR_NUMBER>` — full review, posts findings
+- `/review-new-module <PR_NUMBER> --dry-run` — analysis only, no posting
+
+## What it does
+1. Detects language (Python vs Java) via `pyproject.toml` / `pom.xml`
+2. Fetches PR diff + body
+3. Runs 20 deterministic checks in parallel (secrets, license, disclosure, hardcode, telemetry, docs, bdd, patterns, versioning, commits, errors-logging, testing-depth, http-hygiene, concurrency, deps-supply, deletion-hygiene, constants, binding-shape, quality-gate-parity, pr-size)
+4. Applies baseline exemptions + scope predicates + tier gating
+5. Detects breaking changes via AST diff
+6. Posts 4 signals: inline comments, summary comment, check-run, label
+
+## Implementation
+Run: `bash .claude/scripts/orchestrate.sh <PR_NUMBER>`
+
+The orchestrator is 100% deterministic — no LLM calls in CI. When run inside Claude Code, the LLM can enrich the summary comment before posting (this happens naturally in the session).

.claude/config/rules.yaml

@@ -71,8 +71,8 @@ rules:
   BDD-02:       { tier: BLOCK }
 
   # -------- PATTERNS --------
-  PY-PT-01:     { tier: BLOCK, predicates: { module_shape: client } }
-  JV-PT-01:     { tier: BLOCK, predicates: { module_shape: client } }
+  PY-PT-01:     { tier: FLAG, predicates: { module_shape: client } }
+  JV-PT-01:     { tier: FLAG, predicates: { module_shape: client } }
   PY-PT-03:     { tier: FLAG }
   PY-PT-04:     { tier: FLAG }
   PY-PT-08:     { tier: FLAG }

.claude/scripts/aggregate.sh

@@ -0,0 +1,114 @@
+#!/usr/bin/env bash
+# aggregate.sh — merge N check reports into a single summary, applying rule tiers.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+TMPDIR_RUN="$1"
+RULES_YAML="${RULES_YAML:-$SCRIPT_DIR/../config/rules.yaml}"
+
+# get_tier <rule> — reads rules.yaml, returns tier or empty
+get_tier() {
+  local rule="$1"
+  # rules.yaml format: "  RULE-ID: { tier: X, ... }"
+  awk -v rule="$rule" '
+    match($0, "^  " rule ":") {
+      if (match($0, /tier:[[:space:]]*[A-Z_]+/)) {
+        t = substr($0, RSTART, RLENGTH)
+        sub(/^tier:[[:space:]]*/, "", t)
+        print t
+        exit
+      }
+    }
+  ' "$RULES_YAML" 2>/dev/null
+}
+
+# Collect all report-*.json files. Skip files that are not valid JSON
+# (e.g. a check that timed out or crashed producing partial stdout) so a
+# single bad report can't take the whole aggregation down.
+raw_reports=$(ls "$TMPDIR_RUN"/report-*.json 2>/dev/null | sort)
+reports=""
+for r in $raw_reports; do
+  if jq -e '.' "$r" >/dev/null 2>&1; then
+    reports="$reports $r"
+  else
+    echo "WARN: skipping invalid JSON report: $r" >&2
+  fi
+done
+if [ -z "$reports" ]; then
+  echo '{"findings":[],"shadow_findings":[],"summary":{"block_count":0,"flag_count":0,"shadow_count":0,"locked_count":0},"per_check_summary":{}}'
+  exit 0
+fi
+
+# Merge raw findings, then re-classify each finding by tier from rules.yaml
+merged=$(mktemp)
+# shellcheck disable=SC2086
+jq -s '.' $reports > "$merged"
+
+# For each finding, look up its rule tier and adjust
+retagged_findings="[]"
+retagged_shadow="[]"
+locked_count=0
+
+n=$(jq '[.[] | .findings // [] | length] | add // 0' "$merged")
+if [ "$n" -gt 0 ]; then
+  all_findings=$(jq '[.[] | .findings // []] | flatten' "$merged")
+  rule_ids=$(echo "$all_findings" | jq -r '.[] | .rule' | sort -u)
+
+  # Build lookup: rule -> tier
+  tier_map='{}'
+  while IFS= read -r rule; do
+    [ -z "$rule" ] && continue
+    tier=$(get_tier "$rule")
+    [ -z "$tier" ] && tier="FLAG"   # default
+    tier_map=$(echo "$tier_map" | jq --arg r "$rule" --arg t "$tier" '. + {($r): $t}')
+  done <<< "$rule_ids"
+
+  # Split findings into posted vs shadow based on tier
+  retagged_findings=$(echo "$all_findings" | jq --argjson tiers "$tier_map" '
+    map(
+      . as $f
+      | ($tiers[$f.rule] // "FLAG") as $tier
+      | if $tier == "SHADOW" then empty
+        elif $tier == "BLOCK_LOCKED" then . + {severity: "BLOCK", tier: "BLOCK_LOCKED", locked: true}
+        elif $tier == "BLOCK" then . + {severity: "BLOCK", tier: "BLOCK"}
+        elif $tier == "FLAG" then . + {severity: "FLAG", tier: "FLAG"}
+        else . + {tier: $tier} end
+    )
+  ')
+  retagged_shadow=$(echo "$all_findings" | jq --argjson tiers "$tier_map" '
+    map(
+      . as $f
+      | ($tiers[$f.rule] // "FLAG") as $tier
+      | if $tier == "SHADOW" then . + {tier: "SHADOW"} else empty end
+    )
+  ')
+  locked_count=$(echo "$retagged_findings" | jq '[.[] | select(.locked == true)] | length')
+fi
+
+block_count=$(echo "$retagged_findings" | jq '[.[] | select(.severity == "BLOCK")] | length')
+flag_count=$(echo "$retagged_findings" | jq '[.[] | select(.severity == "FLAG")] | length')
+shadow_count=$(echo "$retagged_shadow" | jq 'length')
+per_check=$(jq '[.[] | {(.check): {status: .status, count: ((.findings // []) | length)}}] | add' "$merged")
+
+jq -n \
+  --argjson findings "$retagged_findings" \
+  --argjson shadow "$retagged_shadow" \
+  --argjson block "$block_count" \
+  --argjson flag "$flag_count" \
+  --argjson shadow_c "$shadow_count" \
+  --argjson locked "$locked_count" \
+  --argjson per_check "$per_check" \
+  '{
+    version: "1.0.0",
+    findings: $findings,
+    shadow_findings: $shadow,
+    summary: {
+      block_count: $block,
+      flag_count: $flag,
+      shadow_count: $shadow_c,
+      locked_count: $locked
+    },
+    per_check_summary: $per_check
+  }'
+
+rm -f "$merged"

.claude/scripts/check-bdd.sh

@@ -64,16 +64,28 @@ while IFS= read -r mod; do
   [ -z "$mod" ] && continue
 
   # Path for THIS repo's feature file
+  # FP-C-01: BDD-01 must accept ANY .feature file under the module's integration
+  # dir, not just `<module>.feature`.
   if [ "$LANGUAGE" = "python" ]; then
-    feature_path="$REPO_ROOT/tests/$mod/integration/$mod.feature"
+    feature_dir="$REPO_ROOT/tests/$mod/integration"
+    feature_path="$feature_dir/$mod.feature"      # kept for BDD-02 sibling comparison
     mod_dir="$REPO_ROOT/src/sap_cloud_sdk/$mod"
   else
-    feature_path="$REPO_ROOT/src/test/resources/com/sap/applicationfoundation/$mod/integration/$mod.feature"
+    feature_dir="$REPO_ROOT/src/test/resources/com/sap/applicationfoundation/$mod/integration"
+    feature_path="$feature_dir/$mod.feature"      # kept for BDD-02 sibling comparison
     mod_dir="$REPO_ROOT/src/main/java/com/sap/cloud/sdk/$mod"
   fi
 
-  # BDD-01: feature file exists (only fire if module has any source files)
-  if [ ! -f "$feature_path" ] && [ -d "$mod_dir" ]; then
+  # BDD-01: at least one .feature file exists in the module's integration dir
+  # (only fire if module has any source files and PR creates the module).
+  has_any_feature=false
+  if [ -d "$feature_dir" ]; then
+    if compgen -G "$feature_dir/*.feature" > /dev/null 2>&1; then
+      has_any_feature=true
+    fi
+  fi
+
+  if [ "$has_any_feature" = "false" ] && [ -d "$mod_dir" ]; then
     # Detect if this PR creates any new file INSIDE the module.
     # `new file mode` is on its own line before the `+++ b/<path>` header,
     # so we scan block-by-block to link them.
@@ -89,9 +101,9 @@ while IFS= read -r mod; do
       }
     ')
     if [ "$is_new_module" = "true" ]; then
-      emit_finding "BDD-01" "BLOCK" "tests/$mod/integration/$mod.feature" 1 \
-        "New module '$mod' has no BDD feature file" \
-        "Create $feature_path with cross-language-consistent scenarios" >> "$findings"
+      emit_finding "BDD-01" "BLOCK" "tests/$mod/integration/" 1 \
+        "New module '$mod' has no .feature files under integration/" \
+        "Create at least one $feature_dir/*.feature with cross-language-consistent scenarios" >> "$findings"
     fi
   fi
 

.claude/scripts/check-concurrency.sh

@@ -4,6 +4,7 @@
 set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/lib/json-emit.sh"
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
@@ -27,7 +28,7 @@ echo "$diff_content" | awk '
   if echo "$content" | grep -qE 'asyncio\.Queue\('; then
     # Check if same file has a Lock or set() nearby
     if [ -f "$REPO_ROOT/$file" ] && ! grep -qE 'asyncio\.Lock|set\(\)' "$REPO_ROOT/$file" 2>/dev/null; then
-      emit_finding "CC-01" "FLAG" "$file" "$line_num" \
+      emit_finding_if_touched "CC-01" "FLAG" "$file" "$line_num" \
         "asyncio.Queue without dedup — if items must be unique, add a set() + asyncio.Lock" "" >> "$findings"
     fi
   fi

.claude/scripts/check-constants.sh

@@ -4,6 +4,7 @@
 set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/lib/json-emit.sh"
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
@@ -25,8 +26,20 @@ if [ "$LANGUAGE" = "python" ]; then
     filtered="$filtered $f"
   done <<< "$changed"
   if [ -n "$filtered" ]; then
+    raw_con=$(mktemp); trap 'rm -f "$raw_con"' EXIT
     # shellcheck disable=SC2086
-    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" con-01 $filtered 2>/dev/null >> "$findings" || true
+    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" con-01 $filtered 2>/dev/null > "$raw_con" || true
+    # FP-A-01: filter to touched lines only
+    while IFS= read -r finding; do
+      [ -z "$finding" ] && continue
+      f=$(echo "$finding" | python3 -c "import json,sys;o=json.loads(sys.stdin.read());print(o.get('file',''))")
+      ln=$(echo "$finding" | python3 -c "import json,sys;o=json.loads(sys.stdin.read());print(o.get('line',0))")
+      [ -z "$f" ] && continue
+      if is_line_touched "$f" "$ln"; then
+        echo "$finding" >> "$findings"
+      fi
+    done < "$raw_con"
+    rm -f "$raw_con"
   fi
 fi
 

.claude/scripts/check-deletion-hygiene.sh

@@ -4,6 +4,10 @@
 set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/lib/json-emit.sh"
+# DEL-01 uses synthetic path "src/:1" — hunk-filter is sourced for
+# emit_finding_if_touched (via is_meta_finding, which treats non-file paths
+# as metadata and passes them through). No filtering applied here.
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
 DIFF_FILE="${DIFF_FILE:-/dev/stdin}"

.claude/scripts/check-disclosure.sh

@@ -6,6 +6,8 @@ set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # shellcheck source=lib/json-emit.sh
 source "$SCRIPT_DIR/lib/json-emit.sh"
+# shellcheck source=lib/hunk-filter.sh
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 # shellcheck source=lib/skill-self-skip.sh
 source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
@@ -39,16 +41,16 @@ echo "$diff_content" | awk '
   # DIS-02: Internal Jira URL (checked first — more specific)
   dis02_fired=false
   if echo "$content" | grep -qEi 'jira\.tools\.sap|jira\.wdf\.sap\.corp'; then
-    emit_finding "DIS-02" "$(sev_public)" "$file" "$line_num" "Internal Jira URL — remove or move to internal-only docs" "" >> "$findings"
+    emit_finding_if_touched "DIS-02" "$(sev_public)" "$file" "$line_num" "Internal Jira URL — remove or move to internal-only docs" "" >> "$findings"
     dis02_fired=true
   fi
   # DIS-01: SAP-internal hostnames (skip if DIS-02 already covers the same line)
   if [ "$dis02_fired" = "false" ] && echo "$content" | grep -qEi '(int\.repositories\.cloud\.sap|\.tools\.sap|\.wdf\.sap\.corp|\.mo\.sap\.corp)'; then
-    emit_finding "DIS-01" "$(sev_public)" "$file" "$line_num" "SAP-internal hostname detected in code" "" >> "$findings"
+    emit_finding_if_touched "DIS-01" "$(sev_public)" "$file" "$line_num" "SAP-internal hostname detected in code" "" >> "$findings"
   fi
   # DIS-06: Internal artifactory index-url
   if echo "$content" | grep -qEi '\-\-index-url[[:space:]]+https?://int\.repositories\.cloud\.sap'; then
-    emit_finding "DIS-06" "BLOCK" "$file" "$line_num" "Internal artifactory --index-url — must not appear in public/shared configs" "" >> "$findings"
+    emit_finding_if_touched "DIS-06" "BLOCK" "$file" "$line_num" "Internal artifactory --index-url — must not appear in public/shared configs" "" >> "$findings"
   fi
 done
 

.claude/scripts/check-errors-logging.sh

@@ -4,6 +4,7 @@
 set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/lib/json-emit.sh"
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
 LANGUAGE="${LANGUAGE:-python}"
@@ -19,10 +20,22 @@ if [ "$LANGUAGE" = "python" ]; then
   changed=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/.*\.py' | sed 's|^+++ b/||' | sort -u)
   if [ -n "$changed" ]; then
     # AST-based chaining / swallow — only reports FLAGs when body ends non-Raise
+    # FP-A-01: filter AST hits through hunk attribution
+    raw_el=$(mktemp); trap 'rm -f "$raw_el"' EXIT
     # shellcheck disable=SC2086
-    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-01 $changed 2>/dev/null >> "$findings" || true
+    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-01 $changed 2>/dev/null > "$raw_el" || true
     # shellcheck disable=SC2086
-    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-02 $changed 2>/dev/null >> "$findings" || true
+    python3 "$SCRIPT_DIR/lib/ast_python_checks.py" el-02 $changed 2>/dev/null >> "$raw_el" || true
+    while IFS= read -r finding; do
+      [ -z "$finding" ] && continue
+      f=$(echo "$finding" | python3 -c "import json,sys;o=json.loads(sys.stdin.read());print(o.get('file',''))")
+      ln=$(echo "$finding" | python3 -c "import json,sys;o=json.loads(sys.stdin.read());print(o.get('line',0))")
+      [ -z "$f" ] && continue
+      if is_line_touched "$f" "$ln"; then
+        echo "$finding" >> "$findings"
+      fi
+    done < "$raw_el"
+    rm -f "$raw_el"
   fi
 
   # EL-04: secret-like variable name in raise args (grep-based, added lines only)
@@ -37,7 +50,7 @@ if [ "$LANGUAGE" = "python" ]; then
   if [ "$(is_skill_file "$file")" = "true" ]; then continue; fi
     # match raise ...({...token|secret|password|api_key|client_secret...})
     if echo "$content" | grep -qE 'raise[[:space:]].*[fF]?"[^"]*\{[^}]*(token|secret|password|api_key|client_secret)[^}]*\}'; then
-      emit_finding "EL-04" "BLOCK" "$file" "$line_num" \
+      emit_finding_if_touched "EL-04" "BLOCK" "$file" "$line_num" \
         "Exception message includes secret-like variable — leaks sensitive data" "" >> "$findings"
     fi
   done

.claude/scripts/check-hardcode.sh

@@ -5,6 +5,8 @@ set -euo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 # shellcheck source=lib/json-emit.sh
 source "$SCRIPT_DIR/lib/json-emit.sh"
+# shellcheck source=lib/hunk-filter.sh
+source "$SCRIPT_DIR/lib/hunk-filter.sh"
 # shellcheck source=lib/skill-self-skip.sh
 source "$SCRIPT_DIR/lib/skill-self-skip.sh"
 
@@ -18,10 +20,12 @@ trap 'rm -f "$findings"' EXIT
 diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "")
 
 # Determine ignore patterns per language
+# FP-B-01: HC-01 must not fire on XML/YAML/POM files where URL-like strings
+# are namespace declarations (http://maven.apache.org/POM/4.0.0 etc.).
 if [ "$LANGUAGE" = "python" ]; then
-  ignore_files='^(tests?/|mocks?/|docs?/|.*/spec/|.*/constants\.py|.*/user-guide\.md|README\.md)'
+  ignore_files='^(tests?/|mocks?/|docs?/|.*/spec/|.*/constants\.py|.*/user-guide\.md|README\.md|.*\.md$|.*\.ya?ml$|.*\.xml$|.*\.properties$|.*pom\.xml$)'
 else
-  ignore_files='^(src/test/|mocks?/|docs?/|.*Constants\.java|.*/constants/|.*/user-guide\.md|README\.md)'
+  ignore_files='^(src/test/|mocks?/|docs?/|.*Constants\.java|.*/constants/|.*/user-guide\.md|README\.md|.*\.md$|.*\.ya?ml$|.*\.xml$|.*\.properties$|.*pom\.xml$)'
 fi
 
 echo "$diff_content" | awk '
@@ -43,30 +47,35 @@ echo "$diff_content" | awk '
     [ -z "$url" ] && continue
     # allow-list: only IANA-reserved test/example TLDs and localhost
     # `.example` must be the terminal label (RFC 2606) — anchor at path/port/end.
+    # FP-B-01: also allowlist standard XML/POM/W3C namespace URLs which appear
+    # as identifiers in build files, not as network endpoints.
     if echo "$url" | grep -qE '^https?://(localhost|127\.0\.0\.1|example\.(com|org|net)|[^/]+\.example(/|:|$)|reserved\.)'; then
       continue
     fi
-    emit_finding "HC-01" "BLOCK" "$file" "$line_num" "Hardcoded URL '$url' in implementation — externalize to config" "" >> "$findings"
+    if echo "$url" | grep -qE '^https?://(maven\.apache\.org/POM/|www\.w3\.org/[0-9]+/XMLSchema|schemas\.xmlsoap\.org/)'; then
+      continue
+    fi
+    emit_finding_if_touched "HC-01" "BLOCK" "$file" "$line_num" "Hardcoded URL '$url' in implementation — externalize to config" "" >> "$findings"
     break  # only one finding per line to avoid duplicate reports
   done < <(echo "$content" | grep -oE 'https?://[A-Za-z0-9][A-Za-z0-9._~:/?#@!$&*+,;=%-]*' || true)
   # HC-02: Authorization Bearer
   if echo "$content" | grep -qE 'Authorization[[:space:]]*:[[:space:]]*Bearer[[:space:]]+[A-Za-z0-9]'; then
-    emit_finding "HC-02" "BLOCK" "$file" "$line_num" "Hardcoded Authorization header value" "" >> "$findings"
+    emit_finding_if_touched "HC-02" "BLOCK" "$file" "$line_num" "Hardcoded Authorization header value" "" >> "$findings"
   fi
   # HC-04: direct os.environ / System.getenv
   if [ "$LANGUAGE" = "python" ]; then
     if echo "$content" | grep -qE 'os\.environ\["[A-Z]'; then
-      emit_finding "HC-04" "FLAG" "$file" "$line_num" "Direct os.environ access — prefer secret_resolver / config layer" "" >> "$findings"
+      emit_finding_if_touched "HC-04" "FLAG" "$file" "$line_num" "Direct os.environ access — prefer secret_resolver / config layer" "" >> "$findings"
     fi
   else
     if echo "$content" | grep -qE 'System\.getenv\('; then
-      emit_finding "HC-04" "FLAG" "$file" "$line_num" "Direct System.getenv() — prefer SecretResolver" "" >> "$findings"
+      emit_finding_if_touched "HC-04" "FLAG" "$file" "$line_num" "Direct System.getenv() — prefer SecretResolver" "" >> "$findings"
     fi
   fi
   # HC-06: hardcoded timeout numeric literal
   # Use word boundary via (^|[^A-Za-z0-9_]) so 'default_timeout' or 'my_timeout' don't match
   if echo "$content" | grep -qiE '(^|[^A-Za-z0-9_])(timeout|retries|max_retries)[[:space:]]*=[[:space:]]*[0-9]+'; then
-    emit_finding "HC-06" "FLAG" "$file" "$line_num" "Hardcoded timeout/retry number — externalize to config" "" >> "$findings"
+    emit_finding_if_touched "HC-06" "FLAG" "$file" "$line_num" "Hardcoded timeout/retry number — externalize to config" "" >> "$findings"
   fi
 done