From 8268d80d55601d9cf33e98f2c2d19773d101306a Mon Sep 17 00:00:00 2001 From: Tiago Kochenborger Date: Mon, 6 Jul 2026 12:00:44 -0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(sdk-review):=20batch=204/6=20=E2=80=94?= =?UTF-8?q?=20structural=20checks=20+=20BREAKING=20family?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 5 checks that enforce structural conventions: ## Checks (.claude/scripts/) - check-docs.sh — DC-01..16 (user-guide.md sections; BTP dependency documentation for Destination/Fragments/Certificates/Regional Availability) - check-bdd.sh — BDD-01..05 (feature files + cross-language parity via module-aliases.yaml) - check-patterns.sh — PT-01..11 (factory pattern, py.typed, exception hierarchy, type hints). Scope-gated to client modules. - check-versioning.sh — VER-01..07 + BREAKING-01..04 family. Uses breaking-detector.py (from batch 1) for AST-based diff. BREAKING-01/02 BLOCK_LOCKED — cannot be suppressed. - check-commits.sh — COM-01 Conventional Commits. Merge commits skipped via parent-count detection (robust regardless of message shape). Part 4 of 6. Ref: AFSDK-3937 --- .claude/scripts/check-bdd.sh | 95 ++++++++++++++++++++++++ .claude/scripts/check-commits.sh | 35 +++++++++ .claude/scripts/check-docs.sh | 102 ++++++++++++++++++++++++++ .claude/scripts/check-patterns.sh | 85 ++++++++++++++++++++++ .claude/scripts/check-versioning.sh | 109 ++++++++++++++++++++++++++++ 5 files changed, 426 insertions(+) create mode 100755 .claude/scripts/check-bdd.sh create mode 100755 .claude/scripts/check-commits.sh create mode 100755 .claude/scripts/check-docs.sh create mode 100755 .claude/scripts/check-patterns.sh create mode 100755 .claude/scripts/check-versioning.sh diff --git a/.claude/scripts/check-bdd.sh b/.claude/scripts/check-bdd.sh new file mode 100755 index 00000000..5f09a735 --- /dev/null +++ b/.claude/scripts/check-bdd.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# check-bdd.sh — feature file existence + cross-language parity via alias map. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/lib/json-emit.sh" + +LANGUAGE="${LANGUAGE:-python}" +REPO_ROOT="${REPO_ROOT:-.}" +DIFF_FILE="${DIFF_FILE:-/dev/stdin}" +SDK_SIBLING_PATH="${SDK_SIBLING_PATH:-}" +CONFIG_DIR="${CONFIG_DIR:-$REPO_ROOT/.claude/config}" + +STARTED=$(now_iso) +findings=$(mktemp); trap 'rm -f "$findings"' EXIT + +diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "") + +# Extract new/modified modules from diff +if [ "$LANGUAGE" = "python" ]; then + modules=$(echo "$diff_content" | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' 2>/dev/null | sed 's|src/sap_cloud_sdk/||; s|/$||' | grep -v '^core$' | sort -u || true) +else + modules=$(echo "$diff_content" | grep -oE 'src/main/java/com/sap/cloud/sdk/[a-z_]+/' 2>/dev/null | sed 's|src/main/java/com/sap/cloud/sdk/||; s|/$||' | grep -v '^core$' | sort -u || true) +fi + +# resolve alias mapping (python name → java name and vice versa) +resolve_sibling_name() { + local mod="$1" dir="$LANGUAGE" + local aliases="$CONFIG_DIR/module-aliases.yaml" + if [ ! -f "$aliases" ]; then echo "$mod"; return; fi + # Simple parser: "- python: X\n java: Y" pairs + if [ "$dir" = "python" ]; then + # look for "python: $mod" then get "java: " + awk -v mod="$mod" ' + /python:/ { p_name=$2 } + /java:/ { j_name=$2; if (p_name == mod) { print j_name; exit } } + ' "$aliases" | head -1 + return + else + awk -v mod="$mod" ' + /python:/ { p_name=$2 } + /java:/ { j_name=$2; if (j_name == mod) { print p_name; exit } } + ' "$aliases" | head -1 + return + fi +} + +while IFS= read -r mod; do + [ -z "$mod" ] && continue + + # Path for THIS repo's feature file + if [ "$LANGUAGE" = "python" ]; then + feature_path="$REPO_ROOT/tests/$mod/integration/$mod.feature" + mod_dir="$REPO_ROOT/src/sap_cloud_sdk/$mod" + else + feature_path="$REPO_ROOT/src/test/resources/com/sap/applicationfoundation/$mod/integration/$mod.feature" + 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 + # check if it's a "new" module (created in this PR) + if echo "$diff_content" | grep -qE "new file mode.*($mod/)"; 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" + fi + fi + + # BDD-02: sibling repo parity + sibling_name=$(resolve_sibling_name "$mod") + [ -z "$sibling_name" ] && sibling_name="$mod" + + if [ -n "$SDK_SIBLING_PATH" ] && [ -d "$SDK_SIBLING_PATH" ]; then + if [ "$LANGUAGE" = "python" ]; then + # Python repo — sibling is Java + sibling_feature="$SDK_SIBLING_PATH/src/test/resources/com/sap/applicationfoundation/$sibling_name/integration/$sibling_name.feature" + sibling_module_dir="$SDK_SIBLING_PATH/src/main/java/com/sap/cloud/sdk/$sibling_name" + else + sibling_feature="$SDK_SIBLING_PATH/tests/$sibling_name/integration/$sibling_name.feature" + sibling_module_dir="$SDK_SIBLING_PATH/src/sap_cloud_sdk/$sibling_name" + fi + # If the sibling module dir exists, and sibling has no feature but we do (or vice versa) + if [ -d "$sibling_module_dir" ] && [ ! -f "$sibling_feature" ] && [ -f "$feature_path" ]; then + emit_finding "BDD-02" "FLAG" "$feature_path" 1 \ + "Sibling SDK ($sibling_name) has module but no BDD feature — parity broken" "" >> "$findings" + fi + if [ -d "$sibling_module_dir" ] && [ -f "$sibling_feature" ] && [ ! -f "$feature_path" ] && [ -d "$mod_dir" ]; then + emit_finding "BDD-02" "BLOCK" "tests/.../$mod.feature" 1 \ + "Module '$mod' exists in sibling SDK ($sibling_name) with BDD feature — this repo must have equivalent" "" >> "$findings" + fi + fi +done <<< "$modules" + +status=$(status_from_findings < "$findings") +emit_report "bdd" "$LANGUAGE" "$status" "$STARTED" < "$findings" diff --git a/.claude/scripts/check-commits.sh b/.claude/scripts/check-commits.sh new file mode 100755 index 00000000..6c4859a5 --- /dev/null +++ b/.claude/scripts/check-commits.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# check-commits.sh — Conventional Commits enforcement. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/lib/json-emit.sh" + +LANGUAGE="${LANGUAGE:-python}" +BASE_SHA="${BASE_SHA:-HEAD~10}" +HEAD_SHA="${HEAD_SHA:-HEAD}" + +STARTED=$(now_iso) +findings=$(mktemp); trap 'rm -f "$findings"' EXIT + +# List commit subjects +commits=$(git log "${BASE_SHA}..${HEAD_SHA}" --format='%H %s' 2>/dev/null || echo "") +prefix_regex='^(feat|fix|refactor|chore|docs|test|ci|build|perf|style|revert)(\([a-z0-9_/-]+\))?!?:[[:space:]]+.+' + +echo "$commits" | while IFS= read -r line; do + [ -z "$line" ] && continue + sha=$(echo "$line" | cut -d' ' -f1) + subject=$(echo "$line" | cut -d' ' -f2-) + # Skip merge commits (identified by 2+ parents, robust regardless of message shape) + parents=$(git rev-list --parents -n 1 "$sha" 2>/dev/null | awk '{print NF-1}') + if [ "${parents:-0}" -gt 1 ]; then continue; fi + # Also skip legacy "Merge branch/pull/etc" defaults + if echo "$subject" | grep -qiE '^Merge '; then continue; fi + if ! echo "$subject" | grep -qE "$prefix_regex"; then + emit_finding "COM-01" "BLOCK" "COMMIT:$sha" 1 \ + "Commit '$subject' does not follow Conventional Commits" \ + "Prefix with feat:/fix:/chore:/docs:/test:/refactor:/ci:/build:/perf:/style:/revert:" >> "$findings" + fi +done + +status=$(status_from_findings < "$findings") +emit_report "commits" "$LANGUAGE" "$status" "$STARTED" < "$findings" diff --git a/.claude/scripts/check-docs.sh b/.claude/scripts/check-docs.sh new file mode 100755 index 00000000..b12e7f95 --- /dev/null +++ b/.claude/scripts/check-docs.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# check-docs.sh — documentation completeness including BTP deps and regional availability. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/lib/json-emit.sh" + +LANGUAGE="${LANGUAGE:-python}" +REPO_ROOT="${REPO_ROOT:-.}" +DIFF_FILE="${DIFF_FILE:-/dev/stdin}" + +STARTED=$(now_iso) +findings=$(mktemp); trap 'rm -f "$findings"' EXIT + +diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "") + +# Detect new module directories (module has new files at top-level) +if [ "$LANGUAGE" = "python" ]; then + new_modules=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/sap_cloud_sdk/[a-z_]+/[^/]+\.py' | sed 's|^+++ b/src/sap_cloud_sdk/||; s|/[^/]*\.py$||' | sort -u) +else + new_modules=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/main/java/com/sap/cloud/sdk/[a-z_]+/[^/]+\.java' | sed 's|^+++ b/src/main/java/com/sap/cloud/sdk/||; s|/[^/]*\.java$||' | sort -u) +fi + +while IFS= read -r mod; do + [ -z "$mod" ] && continue + [ "$mod" = "core" ] && continue + + if [ "$LANGUAGE" = "python" ]; then + user_guide="$REPO_ROOT/src/sap_cloud_sdk/$mod/user-guide.md" + mod_dir="$REPO_ROOT/src/sap_cloud_sdk/$mod" + else + user_guide="$REPO_ROOT/src/main/java/com/sap/cloud/sdk/$mod/user-guide.md" + mod_dir="$REPO_ROOT/src/main/java/com/sap/cloud/sdk/$mod" + fi + + # DC-01: user-guide.md exists + if [ ! -f "$user_guide" ]; then + # Only fire if module dir exists (module is new-ish) + if [ -d "$mod_dir" ]; then + emit_finding "DC-01" "BLOCK" "src/.../$mod/user-guide.md" 1 \ + "Module '$mod' missing user-guide.md" \ + "Create $user_guide with sections: ## Installation, ## Quick Start, ## Configuration" >> "$findings" + fi + continue + fi + + # DC-02: required sections + guide_content=$(cat "$user_guide" 2>/dev/null || echo "") + if ! echo "$guide_content" | grep -qE '^##[[:space:]]+(Installation|Import)'; then + emit_finding "DC-02" "FLAG" "$user_guide" 1 "user-guide.md missing ## Installation section" "" >> "$findings" + fi + if ! echo "$guide_content" | grep -qE '^##[[:space:]]+Quick Start'; then + emit_finding "DC-02" "BLOCK" "$user_guide" 1 "user-guide.md missing ## Quick Start section" "" >> "$findings" + fi + + # DC-11..DC-14 (BTP deps + regional) + # Detect module imports/usages + if [ "$LANGUAGE" = "python" ]; then + has_dest=$(grep -rq "from sap_cloud_sdk\.destination" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_frag=$(grep -rq "FragmentClient\|Fragment[[:space:]]*[,)]" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_cert=$(grep -rq "CertificateClient\|Certificate[[:space:]]*[,)]" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_region=$(grep -rqE "REGION|region_id|SupportedRegion|available_regions" "$mod_dir" 2>/dev/null && echo yes || echo no) + else + has_dest=$(grep -rq "com\.sap\.cloud\.sdk\.destination" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_frag=$(grep -rq "FragmentClient" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_cert=$(grep -rq "CertificateClient" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_region=$(grep -rqE "Region\b|regionId|SupportedRegion" "$mod_dir" 2>/dev/null && echo yes || echo no) + fi + + # DC-11: destination dep must be documented + if [ "$has_dest" = "yes" ]; then + if ! echo "$guide_content" | grep -qEi 'destination service|## Dependencies|## Prerequisites'; then + emit_finding "DC-11" "BLOCK" "$user_guide" 1 \ + "Module imports destination service — must document in ## Dependencies section" \ + "Add: ## Dependencies\\n- SAP BTP Destination Service instance" >> "$findings" + fi + fi + # DC-12: fragments + if [ "$has_frag" = "yes" ]; then + if ! echo "$guide_content" | grep -qEi 'fragments'; then + emit_finding "DC-12" "BLOCK" "$user_guide" 1 \ + "Module uses Fragments — must document Fragments prerequisite" "" >> "$findings" + fi + fi + # DC-13: certificates + if [ "$has_cert" = "yes" ]; then + if ! echo "$guide_content" | grep -qEi 'certificate'; then + emit_finding "DC-13" "BLOCK" "$user_guide" 1 \ + "Module uses Certificates — must document Certificate prerequisite" "" >> "$findings" + fi + fi + # DC-14: regional + if [ "$has_region" = "yes" ]; then + if ! echo "$guide_content" | grep -qEi 'Regional Availability|## Limitations|available in|supported region'; then + emit_finding "DC-14" "BLOCK" "$user_guide" 1 \ + "Module has region-specific constants — must document ## Regional Availability" "" >> "$findings" + fi + fi + +done <<< "$new_modules" + +status=$(status_from_findings < "$findings") +emit_report "docs" "$LANGUAGE" "$status" "$STARTED" < "$findings" diff --git a/.claude/scripts/check-patterns.sh b/.claude/scripts/check-patterns.sh new file mode 100755 index 00000000..c3cc7b83 --- /dev/null +++ b/.claude/scripts/check-patterns.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# check-patterns.sh — idiomatic patterns (factory, exceptions, py.typed). +# Uses module_shape predicate to skip non-client modules. +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/lib/json-emit.sh" +source "$SCRIPT_DIR/lib/predicates.sh" + +LANGUAGE="${LANGUAGE:-python}" +REPO_ROOT="${REPO_ROOT:-.}" +DIFF_FILE="${DIFF_FILE:-/dev/stdin}" + +STARTED=$(now_iso) +findings=$(mktemp); trap 'rm -f "$findings"' EXIT + +diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "") + +# Detect module dirs touched +if [ "$LANGUAGE" = "python" ]; then + modules=$(echo "$diff_content" | grep -oE 'src/sap_cloud_sdk/[a-z_]+/' | sed 's|src/sap_cloud_sdk/||; s|/$||' | grep -v '^core$' | sort -u) +else + modules=$(echo "$diff_content" | grep -oE 'src/main/java/com/sap/cloud/sdk/[a-z_]+/' | sed 's|src/main/java/com/sap/cloud/sdk/||; s|/$||' | grep -v '^core$' | sort -u) +fi + +while IFS= read -r mod; do + [ -z "$mod" ] && continue + + if [ "$LANGUAGE" = "python" ]; then + mod_dir="$REPO_ROOT/src/sap_cloud_sdk/$mod" + else + mod_dir="$REPO_ROOT/src/main/java/com/sap/cloud/sdk/$mod" + fi + + [ -d "$mod_dir" ] || continue + + # PY-PT-01 / JV-PT-01: factory exists — only fire on client modules + shape=$(module_shape "$mod_dir") + if [ "$shape" = "client" ]; then + if [ "$LANGUAGE" = "python" ]; then + if ! grep -rqE '^def create_[a-z_]*client\(' "$mod_dir" 2>/dev/null; then + emit_finding "PY-PT-01" "BLOCK" "src/sap_cloud_sdk/$mod/" 1 \ + "Client module '$mod' missing create_client() factory function" "" >> "$findings" + fi + else + if ! grep -rqE 'class [A-Z][A-Za-z]*ClientFactory' "$mod_dir" 2>/dev/null; then + emit_finding "JV-PT-01" "BLOCK" "src/main/java/com/sap/cloud/sdk/$mod/" 1 \ + "Client module '$mod' missing ClientFactory class" "" >> "$findings" + fi + fi + fi + + # PY-PT-03: py.typed marker + if [ "$LANGUAGE" = "python" ]; then + if [ ! -f "$mod_dir/py.typed" ] && [ ! -f "$REPO_ROOT/src/sap_cloud_sdk/py.typed" ]; then + emit_finding "PY-PT-03" "FLAG" "src/sap_cloud_sdk/$mod/py.typed" 1 \ + "Module missing py.typed marker (PEP 561)" "" >> "$findings" + fi + fi + + # PY-PT-04 / JV-PT-05: module-specific exceptions + if [ "$LANGUAGE" = "python" ]; then + if [ ! -f "$mod_dir/exceptions.py" ]; then + emit_finding "PY-PT-04" "FLAG" "src/sap_cloud_sdk/$mod/exceptions.py" 1 \ + "Module lacks exceptions.py — module-specific exception hierarchy recommended" "" >> "$findings" + fi + else + if [ ! -d "$mod_dir/exceptions" ]; then + emit_finding "JV-PT-05" "FLAG" "src/main/java/com/sap/cloud/sdk/$mod/exceptions/" 1 \ + "Module lacks exceptions/ package" "" >> "$findings" + fi + fi + +done <<< "$modules" + +# PY-PT-08 via AST on changed Python files +if [ "$LANGUAGE" = "python" ]; then + changed_py=$(echo "$diff_content" | grep -oE '^\+\+\+ b/src/sap_cloud_sdk/.*\.py' | sed 's|^+++ b/||' | grep -v '__pycache__' | sort -u) + if [ -n "$changed_py" ]; then + # shellcheck disable=SC2086 + python3 "$SCRIPT_DIR/lib/ast_python_checks.py" pt-08 $changed_py 2>/dev/null >> "$findings" || true + fi +fi + +status=$(status_from_findings < "$findings") +emit_report "patterns" "$LANGUAGE" "$status" "$STARTED" < "$findings" diff --git a/.claude/scripts/check-versioning.sh b/.claude/scripts/check-versioning.sh new file mode 100755 index 00000000..8828834d --- /dev/null +++ b/.claude/scripts/check-versioning.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# check-versioning.sh — SemVer bump + BREAKING family (BREAKING-01..04). +set -uo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$SCRIPT_DIR/lib/json-emit.sh" +source "$SCRIPT_DIR/lib/predicates.sh" + +LANGUAGE="${LANGUAGE:-python}" +REPO_ROOT="${REPO_ROOT:-.}" +DIFF_FILE="${DIFF_FILE:-/dev/stdin}" +PR_BODY_FILE="${PR_BODY_FILE:-}" +BASE_SHA="${BASE_SHA:-}" +HEAD_SHA="${HEAD_SHA:-HEAD}" +BREAKING_JSON="${BREAKING_JSON:-}" # pre-computed via breaking-detector.py + +STARTED=$(now_iso) +findings=$(mktemp); trap 'rm -f "$findings"' EXIT + +diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "") + +# Detect version-bump line change +if [ "$LANGUAGE" = "python" ]; then + version_bumped=$(echo "$diff_content" | grep -E '^\+version\s*=' | head -1) + version_removed=$(echo "$diff_content" | grep -E '^-version\s*=' | head -1) +else + version_bumped=$(echo "$diff_content" | grep -E '^\+\s*' | head -1) + version_removed=$(echo "$diff_content" | grep -E '^-\s*' | head -1) +fi + +# src/ changes present? +if [ "$LANGUAGE" = "python" ]; then + src_changed=$(echo "$diff_content" | grep -qE 'diff --git a/src/sap_cloud_sdk/' && echo yes || echo no) +else + src_changed=$(echo "$diff_content" | grep -qE 'diff --git a/src/main/java/' && echo yes || echo no) +fi + +# commit types (may not find any → allow non-zero exit under set -e) +commit_types=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat,feat!,fix,fix!" 2>/dev/null || echo "false") + +# BREAKING detector output +breaking_detected="false" +if [ -n "$BREAKING_JSON" ] && [ -f "$BREAKING_JSON" ]; then + breaking_detected=$(jq -r '.breaking_detected' "$BREAKING_JSON" 2>/dev/null || echo "false") +fi + +is_feat=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat,feat!" 2>/dev/null || echo "false") + +# VER-01: src/ change without bump — only fires on feat OR breaking +if [ "$src_changed" = "yes" ] && [ -z "$version_bumped" ]; then + if [ "$is_feat" = "true" ] || [ "$breaking_detected" = "true" ]; then + emit_finding "VER-01" "BLOCK" "pyproject.toml" 1 \ + "Feature or breaking change detected but version not bumped" \ + "Bump MINOR version for feat/API change; MAJOR for breaking" >> "$findings" + fi +fi + +# BREAKING-01: if breaking, check PR body has proper declarations +if [ "$breaking_detected" = "true" ]; then + # collect requirements + has_bang=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat!,fix!" || echo "false") + has_bump=$([ -n "$version_bumped" ] && echo "true" || echo "false") + + pr_body="" + if [ -n "$PR_BODY_FILE" ] && [ -f "$PR_BODY_FILE" ]; then + pr_body=$(cat "$PR_BODY_FILE") + fi + has_breaking_section="false" + if echo "$pr_body" | grep -qE '^##[[:space:]]+Breaking Changes' ; then + # section must have non-empty content (not "N/A" or "None" alone) + # Use awk to extract from "## Breaking Changes" to next "## " heading, drop the header line itself + section=$(echo "$pr_body" | awk ' + /^##[[:space:]]+Breaking Changes/ { flag=1; next } + flag && /^##[[:space:]]+[A-Z]/ { exit } + flag { print } + ' | tr -d '[:space:]') + if [ -n "$section" ] && ! echo "$section" | grep -qEi '^(N/A|None|none|--)*$'; then + has_breaking_section="true" + fi + fi + has_checkbox=$(echo "$pr_body" | grep -qE '\-[[:space:]]*\[[xX]\][[:space:]]+Breaking change' && echo "true" || echo "false") + + # if ANY of the 4 requirements is missing → BLOCK + missing="" + [ "$has_bang" != "true" ] && missing="$missing commit-!:-prefix" + [ "$has_bump" != "true" ] && missing="$missing version-bump" + [ "$has_breaking_section" != "true" ] && missing="$missing PR-body-Breaking-Changes-section" + [ "$has_checkbox" != "true" ] && missing="$missing PR-body-checkbox" + + if [ -n "$missing" ]; then + emit_finding "BREAKING-01" "BLOCK" "PR_METADATA" 1 \ + "Breaking change detected — missing declarations:$missing" \ + "Add all 4: (a) feat!:/fix!: commit prefix (b) ## Breaking Changes section (c) checkbox ticked (d) version bump" >> "$findings" + + # BREAKING-02: partial declaration (some declared, others not) + declared_count=0 + [ "$has_bang" = "true" ] && declared_count=$((declared_count+1)) + [ "$has_bump" = "true" ] && declared_count=$((declared_count+1)) + [ "$has_breaking_section" = "true" ] && declared_count=$((declared_count+1)) + [ "$has_checkbox" = "true" ] && declared_count=$((declared_count+1)) + if [ "$declared_count" -gt 0 ] && [ "$declared_count" -lt 4 ]; then + emit_finding "BREAKING-02" "BLOCK" "PR_METADATA" 1 \ + "Breaking-change metadata is inconsistent ($declared_count/4 declared)" \ + "All 4 declarations must agree — reconcile or fully retract" >> "$findings" + fi + fi +fi + +status=$(status_from_findings < "$findings") +emit_report "versioning" "$LANGUAGE" "$status" "$STARTED" < "$findings" From 1a7ccc8d82487d065881afa93858d79845a49e0b Mon Sep 17 00:00:00 2001 From: Tiago Kochenborger Date: Mon, 6 Jul 2026 16:15:43 -0300 Subject: [PATCH 2/2] fix(sdk-review): batch-4 base-ref resolution, scope, and portability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - check-commits/check-versioning: resolve BASE_SHA from origin/${GITHUB_BASE_REF} merge-base rather than HEAD~10 (which fails on branches with <10 commits) - check-versioning: set -e enabled; \s+ → [[:space:]]+ in version regex; BREAKING checkbox accepts *, +, or - bullets and case-insensitive label - check-bdd: alias YAML parser handles indented values and single-quoted scalars; BDD-01 new-module detection walks diff blocks (new file mode + next +++ b/…) instead of matching them on the same line - check-docs: DC-12/13/14 use word-bounded SAP-namespaced references so DocumentFragment, AWSRegion, and 'region_id' in docstrings no longer trigger false positives --- .claude/scripts/check-bdd.sh | 49 +++++++++++++++++++++++------ .claude/scripts/check-commits.sh | 14 ++++++++- .claude/scripts/check-docs.sh | 21 ++++++++----- .claude/scripts/check-versioning.sh | 34 +++++++++++++------- 4 files changed, 89 insertions(+), 29 deletions(-) diff --git a/.claude/scripts/check-bdd.sh b/.claude/scripts/check-bdd.sh index 5f09a735..586002b5 100755 --- a/.claude/scripts/check-bdd.sh +++ b/.claude/scripts/check-bdd.sh @@ -23,23 +23,39 @@ else fi # resolve alias mapping (python name → java name and vice versa) +# The YAML is a simple list of pairs: +# aliases: +# - python: dms +# java: documentmanagement +# awk on `$NF` (last field) extracts the value cleanly even when the key +# column varies. We only pair a python: line with the very next java: line. resolve_sibling_name() { local mod="$1" dir="$LANGUAGE" local aliases="$CONFIG_DIR/module-aliases.yaml" if [ ! -f "$aliases" ]; then echo "$mod"; return; fi - # Simple parser: "- python: X\n java: Y" pairs if [ "$dir" = "python" ]; then - # look for "python: $mod" then get "java: " awk -v mod="$mod" ' - /python:/ { p_name=$2 } - /java:/ { j_name=$2; if (p_name == mod) { print j_name; exit } } - ' "$aliases" | head -1 + /^[[:space:]]*-?[[:space:]]*python:[[:space:]]*/ { + sub(/^[^:]*:[[:space:]]*/, ""); p_name=$0; next + } + /^[[:space:]]*java:[[:space:]]*/ { + sub(/^[^:]*:[[:space:]]*/, ""); j_name=$0 + if (p_name == mod) { print j_name; exit } + p_name="" + } + ' "$aliases" return else awk -v mod="$mod" ' - /python:/ { p_name=$2 } - /java:/ { j_name=$2; if (j_name == mod) { print p_name; exit } } - ' "$aliases" | head -1 + /^[[:space:]]*-?[[:space:]]*python:[[:space:]]*/ { + sub(/^[^:]*:[[:space:]]*/, ""); p_name=$0; next + } + /^[[:space:]]*java:[[:space:]]*/ { + sub(/^[^:]*:[[:space:]]*/, ""); j_name=$0 + if (j_name == mod) { print p_name; exit } + p_name="" + } + ' "$aliases" return fi } @@ -58,8 +74,21 @@ while IFS= read -r mod; do # BDD-01: feature file exists (only fire if module has any source files) if [ ! -f "$feature_path" ] && [ -d "$mod_dir" ]; then - # check if it's a "new" module (created in this PR) - if echo "$diff_content" | grep -qE "new file mode.*($mod/)"; then + # Detect if this PR creates any new file INSIDE the module. + # `new file mode` is on its own line before the `+++ b/` header, + # so we scan block-by-block to link them. + is_new_module=$(echo "$diff_content" | awk -v mod="$mod" -v lang="$LANGUAGE" ' + BEGIN { + if (lang == "python") pat = "src/sap_cloud_sdk/" mod "/" + else pat = "src/main/java/com/sap/cloud/sdk/" mod "/" + } + /^diff --git/ { is_new = 0; next } + /^new file mode/ { is_new = 1; next } + /^\+\+\+ b\// { + if (is_new && index($0, pat) > 0) { print "true"; exit } + } + ') + 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" diff --git a/.claude/scripts/check-commits.sh b/.claude/scripts/check-commits.sh index 6c4859a5..32f17ea8 100755 --- a/.claude/scripts/check-commits.sh +++ b/.claude/scripts/check-commits.sh @@ -5,7 +5,19 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$SCRIPT_DIR/lib/json-emit.sh" LANGUAGE="${LANGUAGE:-python}" -BASE_SHA="${BASE_SHA:-HEAD~10}" +# Resolve BASE_SHA from GITHUB_BASE_REF merge-base when not explicitly set — +# HEAD~10 is a poor fallback (only reachable when the PR has ≥10 commits). +if [ -z "${BASE_SHA:-}" ]; then + base_ref="${GITHUB_BASE_REF:-main}" + # Try origin/, then , then finally HEAD~10 as last resort + if git rev-parse --verify "origin/${base_ref}" >/dev/null 2>&1; then + BASE_SHA=$(git merge-base HEAD "origin/${base_ref}" 2>/dev/null || echo "HEAD~10") + elif git rev-parse --verify "$base_ref" >/dev/null 2>&1; then + BASE_SHA=$(git merge-base HEAD "$base_ref" 2>/dev/null || echo "HEAD~10") + else + BASE_SHA="HEAD~10" + fi +fi HEAD_SHA="${HEAD_SHA:-HEAD}" STARTED=$(now_iso) diff --git a/.claude/scripts/check-docs.sh b/.claude/scripts/check-docs.sh index b12e7f95..56b1233b 100755 --- a/.claude/scripts/check-docs.sh +++ b/.claude/scripts/check-docs.sh @@ -53,17 +53,24 @@ while IFS= read -r mod; do fi # DC-11..DC-14 (BTP deps + regional) - # Detect module imports/usages + # Detect module imports/usages. Use word boundaries and prefer explicit + # SAP-namespaced references to avoid false positives on DocumentFragment, + # AWSRegion, region_id inside docstrings, etc. if [ "$LANGUAGE" = "python" ]; then has_dest=$(grep -rq "from sap_cloud_sdk\.destination" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_frag=$(grep -rq "FragmentClient\|Fragment[[:space:]]*[,)]" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_cert=$(grep -rq "CertificateClient\|Certificate[[:space:]]*[,)]" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_region=$(grep -rqE "REGION|region_id|SupportedRegion|available_regions" "$mod_dir" 2>/dev/null && echo yes || echo no) + # Fragment/Certificate: require the SDK-provided client class specifically, + # or an import from the SDK's fragments/certificates subpackage. + has_frag=$(grep -rqE "\bFragmentClient\b|from[[:space:]]+sap_cloud_sdk\.fragments" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_cert=$(grep -rqE "\bCertificateClient\b|from[[:space:]]+sap_cloud_sdk\.certificates" "$mod_dir" 2>/dev/null && echo yes || echo no) + # Region: constant naming or SDK-provided helper only — plain 'region_id' + # in docstrings should not fire. + has_region=$(grep -rqE "\bSUPPORTED_REGIONS\b|\bSupportedRegion\b|\bavailable_regions\b|from[[:space:]]+sap_cloud_sdk\.regions" "$mod_dir" 2>/dev/null && echo yes || echo no) else has_dest=$(grep -rq "com\.sap\.cloud\.sdk\.destination" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_frag=$(grep -rq "FragmentClient" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_cert=$(grep -rq "CertificateClient" "$mod_dir" 2>/dev/null && echo yes || echo no) - has_region=$(grep -rqE "Region\b|regionId|SupportedRegion" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_frag=$(grep -rqE "\bFragmentClient\b|com\.sap\.cloud\.sdk\.fragments" "$mod_dir" 2>/dev/null && echo yes || echo no) + has_cert=$(grep -rqE "\bCertificateClient\b|com\.sap\.cloud\.sdk\.certificates" "$mod_dir" 2>/dev/null && echo yes || echo no) + # Java word-boundary: require SAP SDK Region type; avoid AWSRegion etc. + has_region=$(grep -rqE "\bSupportedRegion\b|com\.sap\.cloud\.sdk\.regions|\bREGIONAL_AVAILABILITY\b" "$mod_dir" 2>/dev/null && echo yes || echo no) fi # DC-11: destination dep must be documented diff --git a/.claude/scripts/check-versioning.sh b/.claude/scripts/check-versioning.sh index 8828834d..870ecb96 100755 --- a/.claude/scripts/check-versioning.sh +++ b/.claude/scripts/check-versioning.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # check-versioning.sh — SemVer bump + BREAKING family (BREAKING-01..04). -set -uo pipefail +set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "$SCRIPT_DIR/lib/json-emit.sh" source "$SCRIPT_DIR/lib/predicates.sh" @@ -9,7 +9,18 @@ LANGUAGE="${LANGUAGE:-python}" REPO_ROOT="${REPO_ROOT:-.}" DIFF_FILE="${DIFF_FILE:-/dev/stdin}" PR_BODY_FILE="${PR_BODY_FILE:-}" -BASE_SHA="${BASE_SHA:-}" +# Resolve BASE_SHA from the PR base ref rather than HEAD~10 (which fails on +# short branches). Only fall back to HEAD~10 when we can't reach a base ref. +if [ -z "${BASE_SHA:-}" ]; then + base_ref="${GITHUB_BASE_REF:-main}" + if git rev-parse --verify "origin/${base_ref}" >/dev/null 2>&1; then + BASE_SHA=$(git merge-base HEAD "origin/${base_ref}" 2>/dev/null || echo "HEAD~10") + elif git rev-parse --verify "$base_ref" >/dev/null 2>&1; then + BASE_SHA=$(git merge-base HEAD "$base_ref" 2>/dev/null || echo "HEAD~10") + else + BASE_SHA="HEAD~10" + fi +fi HEAD_SHA="${HEAD_SHA:-HEAD}" BREAKING_JSON="${BREAKING_JSON:-}" # pre-computed via breaking-detector.py @@ -20,11 +31,11 @@ diff_content=$(cat "$DIFF_FILE" 2>/dev/null || echo "") # Detect version-bump line change if [ "$LANGUAGE" = "python" ]; then - version_bumped=$(echo "$diff_content" | grep -E '^\+version\s*=' | head -1) - version_removed=$(echo "$diff_content" | grep -E '^-version\s*=' | head -1) + version_bumped=$(echo "$diff_content" | grep -E '^\+version[[:space:]]*=' | head -1) + version_removed=$(echo "$diff_content" | grep -E '^-version[[:space:]]*=' | head -1) else - version_bumped=$(echo "$diff_content" | grep -E '^\+\s*' | head -1) - version_removed=$(echo "$diff_content" | grep -E '^-\s*' | head -1) + version_bumped=$(echo "$diff_content" | grep -E '^\+[[:space:]]*' | head -1) + version_removed=$(echo "$diff_content" | grep -E '^-[[:space:]]*' | head -1) fi # src/ changes present? @@ -34,8 +45,8 @@ else src_changed=$(echo "$diff_content" | grep -qE 'diff --git a/src/main/java/' && echo yes || echo no) fi -# commit types (may not find any → allow non-zero exit under set -e) -commit_types=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat,feat!,fix,fix!" 2>/dev/null || echo "false") +# commit types (BASE_SHA already resolved above; do not fall back to HEAD~10) +commit_types=$(has_commit_type "$BASE_SHA" "$HEAD_SHA" "feat,feat!,fix,fix!" 2>/dev/null || echo "false") # BREAKING detector output breaking_detected="false" @@ -43,7 +54,7 @@ if [ -n "$BREAKING_JSON" ] && [ -f "$BREAKING_JSON" ]; then breaking_detected=$(jq -r '.breaking_detected' "$BREAKING_JSON" 2>/dev/null || echo "false") fi -is_feat=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat,feat!" 2>/dev/null || echo "false") +is_feat=$(has_commit_type "$BASE_SHA" "$HEAD_SHA" "feat,feat!" 2>/dev/null || echo "false") # VER-01: src/ change without bump — only fires on feat OR breaking if [ "$src_changed" = "yes" ] && [ -z "$version_bumped" ]; then @@ -57,7 +68,7 @@ fi # BREAKING-01: if breaking, check PR body has proper declarations if [ "$breaking_detected" = "true" ]; then # collect requirements - has_bang=$(has_commit_type "${BASE_SHA:-HEAD~10}" "$HEAD_SHA" "feat!,fix!" || echo "false") + has_bang=$(has_commit_type "$BASE_SHA" "$HEAD_SHA" "feat!,fix!" 2>/dev/null || echo "false") has_bump=$([ -n "$version_bumped" ] && echo "true" || echo "false") pr_body="" @@ -77,7 +88,8 @@ if [ "$breaking_detected" = "true" ]; then has_breaking_section="true" fi fi - has_checkbox=$(echo "$pr_body" | grep -qE '\-[[:space:]]*\[[xX]\][[:space:]]+Breaking change' && echo "true" || echo "false") + # Checkbox: accept -, *, +, or bullet with either ticked casing + has_checkbox=$(echo "$pr_body" | grep -qE '^[[:space:]]*[-*+][[:space:]]*\[[xX]\][[:space:]]+([Bb]reaking change|BREAKING|Contains breaking)' && echo "true" || echo "false") # if ANY of the 4 requirements is missing → BLOCK missing=""