diff --git a/SCORING.md b/SCORING.md index 066bcc1..4640a61 100644 --- a/SCORING.md +++ b/SCORING.md @@ -356,11 +356,13 @@ figure. Keep the cap high enough that legitimate runs never hit it; if several d raise it rather than let clipped runs contaminate the numbers. The runs are independent, so `run-eval-matrix.sh --jobs N` executes N at a time -(default 1). Because each run is its own fresh container with a unique id, this -changes **wall-time only — not results or cost** (same runs, same tokens). Runs -are API-latency-bound, so 2–3 roughly halves/thirds the generation phase at -negligible local cost; keep N small (≥4 risks API rate limits). The judge stage -is unaffected. +(default 1). Each run is its own fresh container with a unique per-invocation id, +so at the recommended **2–3** concurrency this changes **wall-time only — not +results or cost** (same runs, same tokens). Runs are API-latency-bound, so 2–3 +roughly halves/thirds the generation phase at negligible local cost. Do not push +it higher: at ≥4 you risk API rate limits, and a rate-limited run *can* change +results — so the wall-time-only guarantee holds only in the 2–3 range. The judge +stage is unaffected. Confirm the skill-install step and `--permission-mode dontAsk` work on the pinned CLI version before the first scored run — verify an installed skill actually shows diff --git a/scripts/scoring/run-eval-matrix.sh b/scripts/scoring/run-eval-matrix.sh index fe42b66..5121e0a 100755 --- a/scripts/scoring/run-eval-matrix.sh +++ b/scripts/scoring/run-eval-matrix.sh @@ -46,6 +46,11 @@ LIMIT="0" # wall-time. Keep small: >=4 risks hitting API rate limits. JOBS="1" DATE_TAG="$(date -u +%Y%m%d)" +# Stamped once per invocation. Appended to each run id so ids are deterministic +# *within* a run but unique *across* invocations — re-running (e.g. after a +# partial failure) into the same out-dir can't clobber a prior run's transcript +# or double-count its manifest rows. +RUN_STAMP="$(date -u +%Y%m%dT%H%M%SZ)" usage() { cat <<'USAGE' @@ -154,11 +159,16 @@ echo # Each task writes its manifest row to its own file here; they are concatenated # (sorted) into manifest.csv after all workers finish, so concurrent writes never -# race on a single file. +# race on a single file. Worker skill-staging dirs live under STAGE_BASE so an +# interrupt can clean them up in one sweep. MANIFEST_DIR="$OUT_DIR/.manifest.d" +STAGE_BASE="$OUT_DIR/.stage" if [[ "$DRY_RUN" != "1" ]]; then - mkdir -p "$OUT_DIR" "$MANIFEST_DIR" + # Start from a clean slate: stale .row files from an interrupted prior run must + # not leak into this run's manifest. + rm -rf "$MANIFEST_DIR" "$STAGE_BASE" + mkdir -p "$OUT_DIR" "$MANIFEST_DIR" "$STAGE_BASE" if [[ ! -f "$MANIFEST" ]]; then echo "prompt,skill_family,skill_leaf,model,condition,rep,run_id,exit_code,skills_sha,timestamp" > "$MANIFEST" fi @@ -190,18 +200,22 @@ run_one() { [[ -n "$ALLOWED_TOOLS" ]] && args+=(--extra-args "--allowedTools $ALLOWED_TOOLS --") [[ -n "$MAX_BUDGET_USD" ]] && args+=(--max-budget-usd "$MAX_BUDGET_USD") + # Descriptive, unique-per-invocation run id. Slugified so an odd char in the + # prompt .name (space, :, /) can't reach --run-id raw; RUN_STAMP makes it unique + # across invocations so a re-run into the same out-dir never clobbers a prior + # run's transcript or double-counts its manifest row. + local run_id + run_id="$(printf '%s' "$name-$model-$condition-r$rep-$RUN_STAMP" | tr -c 'A-Za-z0-9._-' '_')" + local stage="" if [[ "$condition" == "with-skill" ]]; then - stage="$(mktemp -d)" + stage="$(mktemp -d "$STAGE_BASE/stage.XXXXXX")" # Copy the whole family subtree under its real name so progressive-disclosure # relative links resolve and the container installs it as ~/.claude/skills/. cp -R "$SKILLS_ROOT/$family" "$stage/$family" args+=(--skills-dir "$stage") fi - # Descriptive, unique-per-task run id — passed to the harness via --run-id so - # concurrent runs never share a dir or Docker --name. All chars are safe. - local run_id="$name-$model-$condition-r$rep" args+=(--run-id "$run_id") local ts exit_code tmplog @@ -215,20 +229,27 @@ run_one() { return 0 fi - # Capture harness output per-task so parallel workers don't interleave on the console. + # Capture harness output per-task so parallel workers don't interleave on the + # console; keep it as a diagnostic if the run failed, discard it otherwise. tmplog="$(mktemp)" set +e "$HARNESS" "${args[@]}" "$prompt_file" >"$tmplog" 2>&1 exit_code=$? set -e - rm -f "$tmplog" + if [[ "$exit_code" -ne 0 ]]; then + mv "$tmplog" "$OUT_DIR/$run_id.harness.log" + else + rm -f "$tmplog" + fi [[ -n "$stage" ]] && rm -rf "$stage" # One row per task, written to its own file — concatenated after the pool drains. echo "$name,$family,$leaf,$model,$condition,$rep,$run_id,$exit_code,$skills_sha,$ts" \ > "$MANIFEST_DIR/$run_id.row" - printf ' ok %-42s %-7s %-11s rep=%s run_id=%s exit=%s\n' \ - "$name" "$model" "$condition" "$rep" "$run_id" "$exit_code" + local status_word="ok " + [[ "$exit_code" -ne 0 ]] && status_word="FAIL" + printf ' %s %-42s %-7s %-11s rep=%s run_id=%s exit=%s\n' \ + "$status_word" "$name" "$model" "$condition" "$rep" "$run_id" "$exit_code" return 0 } @@ -247,6 +268,31 @@ done # Rolling PID pool: keep up to $JOBS workers in flight (bash 3.2-safe — no # `wait -n`). Poll for any finished worker before launching the next. pids=() + +# Recursively SIGTERM a process and all its descendants (children first). A worker +# subshell has a harness child which in turn has a `docker run` child; killing only +# the tracked worker pid would orphan those, so walk the whole tree. SIGTERM (not +# KILL) lets `docker run` forward the signal so its `--rm` container stops cleanly. +kill_tree() { + local pid="$1" child + for child in $(pgrep -P "$pid" 2>/dev/null); do kill_tree "$child"; done + kill "$pid" 2>/dev/null || true +} + +# On Ctrl-C / termination, stop the whole pool and clear temp state rather than +# leaving orphaned workers, containers, and half-written temp dirs. +cleanup_interrupt() { + trap - INT TERM + echo >&2 + echo "Interrupted — stopping workers and cleaning up..." >&2 + local p + for p in "${pids[@]:-}"; do [[ -n "$p" ]] && kill_tree "$p"; done + wait 2>/dev/null || true + rm -rf "$MANIFEST_DIR" "$STAGE_BASE" + exit 130 +} +trap cleanup_interrupt INT TERM + reap_one() { while :; do local i @@ -275,10 +321,11 @@ done # Assemble the manifest from per-task rows in a deterministic order. if [[ "$DRY_RUN" != "1" ]]; then + trap - INT TERM if compgen -G "$MANIFEST_DIR/*.row" >/dev/null; then cat "$MANIFEST_DIR"/*.row | sort >> "$MANIFEST" fi - rm -rf "$MANIFEST_DIR" + rm -rf "$MANIFEST_DIR" "$STAGE_BASE" fi echo diff --git a/skill-test/README.md b/skill-test/README.md index f91e332..388c4a9 100644 --- a/skill-test/README.md +++ b/skill-test/README.md @@ -285,10 +285,11 @@ to set it explicitly instead: scripts/run-claude-test.sh --run-id my-unique-id prompts/qdrant-smoke.md ``` -`ID` must be filesystem/Docker-safe (letters, digits, `.`, `_`, `-`). This lets a -batch runner give each run a unique, descriptive id so that **concurrent** runs -never collide on a directory or a Docker `--name` (two runs of the same prompt in -the same second would otherwise clash). +`ID` must start with a letter or digit, then letters, digits, `.`, `_`, `-` +(Docker's `--name` rule). This lets a batch runner give each run a unique, +descriptive id so that **concurrent** runs never collide on a directory or a +Docker `--name` (two runs of the same prompt in the same second would otherwise +clash). For tests that intentionally need Claude Code to execute commands or edit a throwaway workspace, use a disposable workspace and pass a more permissive mode, diff --git a/skill-test/scripts/run-claude-test.sh b/skill-test/scripts/run-claude-test.sh index cc2ed23..04533e1 100755 --- a/skill-test/scripts/run-claude-test.sh +++ b/skill-test/scripts/run-claude-test.sh @@ -59,9 +59,10 @@ Options: --choose-model Interactively choose a model from a menu. --max-budget-usd USD Stop once this print-mode budget is reached. --run-id ID Use this exact run id (dir name + Docker --name) instead of - deriving one from the timestamp and prompt name. Must be - filesystem/Docker-safe (letters, digits, . _ -). Lets a - parallel batch runner give each concurrent run a unique id. + deriving one from the timestamp and prompt name. Must start + with a letter/digit, then letters, digits, . _ - (Docker's + --name rule). Lets a parallel batch runner give each + concurrent run a unique id. --extra-args "ARGS" Advanced Claude Code flags passed through by the container. --allow-missing-auth Skip local auth preflight checks. --no-render Do not generate readable.md after the run. @@ -360,8 +361,10 @@ if [[ -n "$RUN_ID" ]]; then # Caller-supplied run id (e.g. a parallel batch runner assigning a unique, # descriptive id per task so concurrent runs never share a dir or Docker # --name). Must be filesystem/Docker-name safe: letters, digits, ., _, - only. - if [[ ! "$RUN_ID" =~ ^[A-Za-z0-9._-]+$ ]]; then - echo "Invalid --run-id '$RUN_ID' (allowed: letters, digits, . _ -)" >&2 + # Must start with a letter or digit (Docker's --name rule) and contain only + # safe chars. Rejects a leading -/., and `..` path traversal. + if [[ ! "$RUN_ID" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]]; then + echo "Invalid --run-id '$RUN_ID' (must start with a letter/digit; allowed: letters, digits, . _ -)" >&2 exit 64 fi run_id="$RUN_ID"