diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 330db3d7fcc..f01283d34f7 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -2,6 +2,6 @@ ## Claude Code workflow -- Before creating a pull request, run the [Review Guidelines](../AGENTS.md#review-guidelines) checks over the branch changes. +- Before creating a pull request, run `./review.sh` (see [Review Guidelines](../AGENTS.md#review-guidelines)) over the branch changes. - After running `/migrate-groovy-to-java`, run `/review-groovy-migration` on the migrated files before opening a PR. - Always write new unit tests using JUnit 5 and Java. If the target test file is `.groovy`, run the `/migrate-groovy-to-java` skill on it first. diff --git a/AGENTS.md b/AGENTS.md index 95831148819..e632256617f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,9 @@ docs/ Developer documentation (see below) ## Review Guidelines +Run `./review.sh` before pushing. It runs your harness's code review over the branch changes against these guidelines, then `/perf-review`. It is advisory: findings are printed and nothing is enforced. Use `./review.sh --help` for options. + +- **Correctness**: the review applies the [Critical constraints](#critical-constraints) below and the docs they link. - **Technical debt**: run `/techdebt` over branch changes before marking a PR ready to catch code duplication, unnecessary complexity, and dead code (refactor-only, never changes behavior) — see [.agents/skills/techdebt/SKILL.md](.agents/skills/techdebt/SKILL.md). - **Performance**: run `/perf-review` over branch changes before marking a PR ready (advisory, not a merge gate) — see [.agents/skills/perf-review/SKILL.md](.agents/skills/perf-review/SKILL.md). diff --git a/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data b/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data index 0c467473aa8..2c0c8d55f66 160000 --- a/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data +++ b/dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data @@ -1 +1 @@ -Subproject commit 0c467473aa8dc1f1cbfc0ef9a60d5c1c7a8826e6 +Subproject commit 2c0c8d55f665fe4a847fc0ded3abf59b7e2896fc diff --git a/review.sh b/review.sh new file mode 120000 index 00000000000..1cd09efb287 --- /dev/null +++ b/review.sh @@ -0,0 +1 @@ +tooling/review.sh \ No newline at end of file diff --git a/tooling/review.sh b/tooling/review.sh new file mode 100755 index 00000000000..a4ffcff947e --- /dev/null +++ b/tooling/review.sh @@ -0,0 +1,132 @@ +#!/usr/bin/env bash +# Pre-push review entry point for dd-trace-java. +# +# Runs the coding harness's built-in code review over the branch changes, then the +# repo-specific review extensions (currently /perf-review). Review expectations live +# in AGENTS.md (see "Review Guidelines"). Advisory only: findings are printed and the +# script exits 0 regardless of what was found. +set -euo pipefail + +usage() { + cat < Compare against (default: merge-base with upstream master) + --harness Harness to run: claude, codex (default: first one found on PATH) + --no-extensions Only run the built-in review, skip repo extensions (/perf-review) + --out Also write the review output to + -h, --help Show this help +EOF +} + +BASE="" +HARNESS="" +RUN_EXTENSIONS=true +OUT="" + +require_value() { + if [ $# -lt 2 ] || [ -z "$2" ]; then + echo "❌ Missing value for $1" >&2 + usage >&2 + exit 1 + fi +} + +while [ $# -gt 0 ]; do + case "$1" in + --base) require_value "$@"; BASE="$2"; shift 2 ;; + --harness) require_value "$@"; HARNESS="$2"; shift 2 ;; + --no-extensions) RUN_EXTENSIONS=false; shift ;; + --out) require_value "$@"; OUT="$2"; shift 2 ;; + -h|--help) usage; exit 0 ;; + *) echo "❌ Unknown option: $1" >&2; usage >&2; exit 1 ;; + esac +done + +cd "$(git rev-parse --show-toplevel)" + +find_base() { + local upstream + upstream=$(git remote -v | grep -E 'DataDog/[^/]+(\.git)?\s' | head -1 | awk '{print $1}') + git merge-base HEAD "${upstream:-origin}/master" +} + +detect_harness() { + for candidate in claude codex; do + if command -v "$candidate" &> /dev/null; then + echo "$candidate" + return + fi + done + echo "❌ No supported harness found on PATH (claude, codex)." >&2 + exit 1 +} + +if [ -z "$BASE" ]; then + BASE=$(find_base) +else + BASE=$(git merge-base HEAD "$BASE") +fi +case "${HARNESS:=$(detect_harness)}" in + claude|codex) ;; + *) echo "❌ Unsupported harness: $HARNESS (supported: claude, codex)" >&2; exit 1 ;; +esac + +if git diff --quiet "$BASE" HEAD; then + echo "No changes since $BASE; nothing to review." + exit 0 +fi + +echo "🔎 Reviewing changes since $(git rev-parse --short "$BASE") with $HARNESS" +git diff --stat "$BASE" HEAD +echo + +REVIEW_PROMPT="Review the changes on this branch since $BASE (git diff $BASE...HEAD) \ +for correctness bugs. Apply the Review Guidelines and Critical constraints in AGENTS.md \ +and the docs they link. Report only findings you are confident about, with \ +file:line references. Do not edit any files." +PERF_PROMPT="/perf-review the changes on this branch since $BASE" + +# Read-only tool set for headless runs, so nothing stalls on a permission prompt. +CLAUDE_ALLOWED_TOOLS="Read Grep Glob Agent Skill Bash(git diff:*) Bash(git log:*) Bash(git show:*) Bash(git merge-base:*) Bash(git remote:*) Bash(git rev-parse:*) Bash(git status:*) Bash(git blame:*) Bash(grep:*) Bash(find:*) Bash(ls:*) Bash(cat:*) Bash(head:*) Bash(sed -n:*) Bash(wc:*)" + +run_harness() { + local prompt="$1" + case "$HARNESS" in + claude) + claude -p "$prompt" \ + --permission-mode dontAsk \ + --allowedTools "$CLAUDE_ALLOWED_TOOLS" \ + --disallowedTools "Edit Write NotebookEdit" + ;; + codex) + codex exec --sandbox read-only "$prompt" + ;; + esac +} + +run_reviews() { + echo "## Code review" + echo + run_harness "$REVIEW_PROMPT" || echo "⚠️ Code review failed" + if [ "$RUN_EXTENSIONS" = true ]; then + echo + echo "## Performance review" + echo + if [ "$HARNESS" = claude ]; then + run_harness "$PERF_PROMPT" || echo "⚠️ Performance review failed" + else + run_harness "Follow .agents/skills/perf-review/SKILL.md to review the changes on this branch since $BASE" \ + || echo "⚠️ Performance review failed" + fi + fi +} + +if [ -n "$OUT" ]; then + run_reviews | tee "$OUT" +else + run_reviews +fi