fix: address review feedback on evaluate.sh (#380)
- container_name(): derive separator from compose version (_COMPOSE_SEP), so v1 (underscores) and v2 (hyphens) both resolve the right container name - Drop buggy grep fallback for JSON branch extraction; python3 path is sufficient and safe; grep cannot reliably target only head.ref in the nested Gitea response - Validate KRAIKEN/STAKE/LIQUIDITY_MANAGER after sourcing contracts.env to catch renamed variables with a clear infra_error instead of a cryptic 'unbound variable' abort - wait_exited: handle Docker 'dead' state alongside 'exited' to fail fast instead of polling the full timeout - Ponder /ready timeout is now infra_error (was a logged warning); scenarios must not run against a partially-indexed chain - Avoid double git fetch: only fetch the specific branch if the earlier --prune fetch (fallback path) has not already retrieved all remote refs - Use mktemp -u so git worktree add creates the directory itself, avoiding failures on older git versions that reject a pre-existing target path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
openhands
parent
43c8d79afd
commit
1d6dd8c628
1 changed files with 38 additions and 11 deletions
|
|
@ -12,6 +12,10 @@
|
|||
# Environment overrides:
|
||||
# HARB_REPO_REMOTE git remote to fetch from (default: origin)
|
||||
# CODEBERG_REPO Gitea/Codeberg repo path (default: johba/harb)
|
||||
#
|
||||
# NOTE: host port isolation — docker-compose.yml binds fixed host ports
|
||||
# (8545, 42069, 5173, 8081, 5100). Concurrent evaluation runs on the same
|
||||
# host will collide on those ports. This script is designed for sequential use.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
|
|
@ -56,8 +60,12 @@ PR_NUMBER="$1"
|
|||
# ── Prerequisites ──────────────────────────────────────────────────────
|
||||
if docker compose version &>/dev/null 2>&1; then
|
||||
COMPOSE_CMD="docker compose"
|
||||
# Compose v2 uses hyphens in container names: PROJECT-SERVICE-1
|
||||
_COMPOSE_SEP="-"
|
||||
elif command -v docker-compose &>/dev/null; then
|
||||
COMPOSE_CMD="docker-compose"
|
||||
# Compose v1 uses underscores in container names: PROJECT_SERVICE_1
|
||||
_COMPOSE_SEP="_"
|
||||
else
|
||||
infra_error "docker compose not found. Install Docker with the compose plugin."
|
||||
fi
|
||||
|
|
@ -68,6 +76,7 @@ command -v curl &>/dev/null || infra_error "curl not found"
|
|||
# ── Fetch PR branch name ───────────────────────────────────────────────
|
||||
# Try the Codeberg REST API first (requires ~/.netrc with credentials).
|
||||
PR_BRANCH=""
|
||||
_FETCHED_ALL=false
|
||||
if [[ -f "$HOME/.netrc" ]]; then
|
||||
log "Resolving PR #$PR_NUMBER branch via Codeberg API..."
|
||||
api_json="$(curl --netrc --silent --max-time 10 \
|
||||
|
|
@ -77,10 +86,11 @@ if [[ -f "$HOME/.netrc" ]]; then
|
|||
if command -v jq &>/dev/null; then
|
||||
PR_BRANCH="$(echo "$api_json" | jq -r '.head.ref // empty' 2>/dev/null)" || true
|
||||
else
|
||||
# jq not available — extract with python3 or grep+sed
|
||||
# jq not available — use python3 for reliable nested key extraction.
|
||||
# grep+sed is intentionally omitted: the Gitea response has multiple "ref"
|
||||
# keys in nested objects and grep cannot safely target only head.ref.
|
||||
PR_BRANCH="$(echo "$api_json" | \
|
||||
python3 -c "import json,sys; print(json.load(sys.stdin)['head']['ref'])" 2>/dev/null)" || \
|
||||
PR_BRANCH="$(echo "$api_json" | grep -o '"ref":"[^"]*"' | head -n1 | sed 's/"ref":"//;s/"//')" || true
|
||||
python3 -c "import json,sys; print(json.load(sys.stdin)['head']['ref'])" 2>/dev/null)" || true
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
|
|
@ -90,6 +100,7 @@ if [[ -z "$PR_BRANCH" ]]; then
|
|||
log "API lookup skipped or failed; scanning remote branches..."
|
||||
cd "$REPO_ROOT"
|
||||
git fetch "$REPO_REMOTE" --prune 2>/dev/null || infra_error "git fetch $REPO_REMOTE failed"
|
||||
_FETCHED_ALL=true
|
||||
PR_BRANCH="$(git branch -r 2>/dev/null | \
|
||||
grep -E "(fix|feat|chore|refactor|hotfix)/.*[-/]${PR_NUMBER}[^0-9]?$|issue-${PR_NUMBER}$" | \
|
||||
head -n1 | sed 's|.*/||' | tr -d ' ')" || true
|
||||
|
|
@ -99,7 +110,10 @@ fi
|
|||
log "PR #$PR_NUMBER => branch: $PR_BRANCH"
|
||||
|
||||
# ── Create isolated worktree ───────────────────────────────────────────
|
||||
WORKTREE_DIR="$(mktemp -d /tmp/harb-eval-${PR_NUMBER}-XXXXXX)"
|
||||
# Use mktemp -u to generate a unique path without creating the directory;
|
||||
# git worktree add creates it. This avoids failures on older git that
|
||||
# rejects a pre-existing (even empty) target directory.
|
||||
WORKTREE_DIR="$(mktemp -u /tmp/harb-eval-${PR_NUMBER}-XXXXXX)"
|
||||
# Use a project name that is unique per PR and safe for Docker labels.
|
||||
COMPOSE_PROJECT="harb-eval-${PR_NUMBER}"
|
||||
|
||||
|
|
@ -117,10 +131,12 @@ cleanup() {
|
|||
}
|
||||
trap cleanup EXIT INT TERM
|
||||
|
||||
# Ensure the branch is locally available before adding the worktree.
|
||||
# Fetch the specific branch only if we haven't already fetched everything above.
|
||||
cd "$REPO_ROOT"
|
||||
git fetch "$REPO_REMOTE" "$PR_BRANCH" 2>/dev/null || \
|
||||
infra_error "Could not fetch branch '$PR_BRANCH' from $REPO_REMOTE"
|
||||
if [[ "$_FETCHED_ALL" != "true" ]]; then
|
||||
git fetch "$REPO_REMOTE" "$PR_BRANCH" 2>/dev/null || \
|
||||
infra_error "Could not fetch branch '$PR_BRANCH' from $REPO_REMOTE"
|
||||
fi
|
||||
|
||||
log "Creating worktree at $WORKTREE_DIR (branch: $PR_BRANCH)..."
|
||||
git worktree add "$WORKTREE_DIR" "remotes/$REPO_REMOTE/$PR_BRANCH" \
|
||||
|
|
@ -137,8 +153,9 @@ log "Starting containerised stack (project: $COMPOSE_PROJECT)..."
|
|||
$COMPOSE_CMD -p "$COMPOSE_PROJECT" up -d \
|
||||
|| infra_error "docker compose up failed"
|
||||
|
||||
# Helper: get status/health of a named container in this project.
|
||||
container_name() { echo "${COMPOSE_PROJECT}-$1-1"; }
|
||||
# Helper: resolve the container name for a service in this project.
|
||||
# Compose v2 uses hyphens (PROJECT-SERVICE-1); v1 uses underscores (PROJECT_SERVICE_1).
|
||||
container_name() { echo "${COMPOSE_PROJECT}${_COMPOSE_SEP}$1${_COMPOSE_SEP}1"; }
|
||||
|
||||
wait_healthy() {
|
||||
local service="$1" timeout="$2"
|
||||
|
|
@ -168,7 +185,7 @@ wait_exited() {
|
|||
while (( SECONDS < deadline )); do
|
||||
local status
|
||||
status="$(docker inspect --format='{{.State.Status}}' "$container" 2>/dev/null || echo "missing")"
|
||||
if [[ "$status" == "exited" ]]; then
|
||||
if [[ "$status" == "exited" || "$status" == "dead" ]]; then
|
||||
local exit_code
|
||||
exit_code="$(docker inspect --format='{{.State.ExitCode}}' "$container" 2>/dev/null || echo "1")"
|
||||
if [[ "$exit_code" != "0" ]]; then
|
||||
|
|
@ -198,6 +215,16 @@ CONTRACTS_ENV="$WORKTREE_DIR/tmp/containers/contracts.env"
|
|||
log "Reading contract addresses from contracts.env..."
|
||||
# shellcheck source=/dev/null
|
||||
source "$CONTRACTS_ENV"
|
||||
|
||||
# Validate expected variables after sourcing — guards against set -u crashes
|
||||
# if a future bootstrap refactor renames any of these.
|
||||
KRAIKEN="${KRAIKEN:-}"
|
||||
STAKE="${STAKE:-}"
|
||||
LIQUIDITY_MANAGER="${LIQUIDITY_MANAGER:-}"
|
||||
[[ -n "$KRAIKEN" ]] || infra_error "KRAIKEN not set in contracts.env"
|
||||
[[ -n "$STAKE" ]] || infra_error "STAKE not set in contracts.env"
|
||||
[[ -n "$LIQUIDITY_MANAGER" ]] || infra_error "LIQUIDITY_MANAGER not set in contracts.env"
|
||||
|
||||
log " KRAIKEN=$KRAIKEN"
|
||||
log " STAKE=$STAKE"
|
||||
log " LIQUIDITY_MANAGER=$LIQUIDITY_MANAGER"
|
||||
|
|
@ -219,7 +246,7 @@ while (( SECONDS < ponder_deadline )); do
|
|||
sleep "$POLL_INTERVAL"
|
||||
done
|
||||
if [[ "$ponder_ready" != "true" ]]; then
|
||||
log "WARNING: Ponder not fully indexed after ${PONDER_READY_TIMEOUT}s — continuing anyway"
|
||||
infra_error "Ponder did not finish indexing within ${PONDER_READY_TIMEOUT}s"
|
||||
fi
|
||||
|
||||
# ── Export stack endpoints for scenario scripts ────────────────────────
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue