diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3461943ea..acbba7e12 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -4,4 +4,9 @@ set -euo pipefail repo_root="$(git rev-parse --show-toplevel)" -"$repo_root/contrib/dev-tools/git/hooks/pre-commit.sh" \ No newline at end of file +# Use human-friendly text format when stdout is a terminal; JSON for non-interactive / agent runs. +if [[ -t 1 ]]; then + "$repo_root/contrib/dev-tools/git/hooks/pre-commit.sh" --format=text +else + "$repo_root/contrib/dev-tools/git/hooks/pre-commit.sh" --format=json +fi \ No newline at end of file diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 000000000..a2586e43b --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" + +# Use human-friendly text format when stdout is a terminal; JSON for non-interactive / agent runs. +if [[ -t 1 ]]; then + "$repo_root/contrib/dev-tools/git/hooks/pre-push.sh" --format=text +else + "$repo_root/contrib/dev-tools/git/hooks/pre-push.sh" --format=json +fi \ No newline at end of file diff --git a/.github/agents/committer.agent.md b/.github/agents/committer.agent.md index 7c5612c80..f961fa865 100644 --- a/.github/agents/committer.agent.md +++ b/.github/agents/committer.agent.md @@ -37,12 +37,22 @@ Treat every commit request as a review-and-verify workflow, not as a blind reque 5. Check for obvious repository-policy violations in the diff (for example missing required spec progress updates, missing documented rationale where required, or similar policy blockers). If found, stop and return to the Implementer/Reviewer before committing. -6. Run `./contrib/dev-tools/git/hooks/pre-commit.sh` when feasible. For AI execution, use - `--format=json` first and retry with `--format=text --verbosity=verbose` if needed. If it fails: - - **You may fix**: formatting, linting, spell-check, import organization, and similar - metadata-only issues that are direct artifacts of the commit scope. - - **You must not fix**: build failures, test failures, logic errors, or runtime issues. - These are implementation defects; stop and return them to the **Implementer** to resolve. +6. **Check if the pre-commit git hook is already installed** before running checks manually: + + ```bash + [[ -x "$(git rev-parse --git-path hooks)/pre-commit" ]] && echo "installed" || echo "not installed" + ``` + + - **If installed**: do NOT run the script manually — `git commit -S` will trigger it + automatically. Running it first would execute every check twice. + - **If not installed**: run `./contrib/dev-tools/git/hooks/pre-commit.sh` manually. + For AI execution, use `--format=json` first and retry with + `--format=text --verbosity=verbose` if needed. If it fails: + - **You may fix**: formatting, linting, spell-check, import organization, and similar + metadata-only issues that are direct artifacts of the commit scope. + - **You must not fix**: build failures, test failures, logic errors, or runtime issues. + These are implementation defects; stop and return them to the **Implementer** to resolve. + 7. Propose a precise Conventional Commit message. 8. Create the commit with `git commit -S` only after the scope is clear and blockers are resolved. 9. After committing, run a quick verification check and report the resulting commit summary. diff --git a/.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md b/.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md index 2d45d3c71..8216e17f8 100644 --- a/.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md +++ b/.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md @@ -20,6 +20,15 @@ automatically on every `git commit`. Install it once after cloning: After installation the hook fires automatically; you do not need to invoke the script manually before each commit. +> **For AI agents**: before invoking the script manually, check whether the hook is installed: +> +> ```bash +> [[ -x "$(git rev-parse --git-path hooks)/pre-commit" ]] && echo "installed" || echo "not installed" +> ``` +> +> If installed, skip the manual run — `git commit` will trigger it automatically. +> Running both would execute every check twice. + ## Automated Checks > **⏱️ Expected runtime: ~1 minute** on a modern developer machine with warm caches. @@ -68,12 +77,12 @@ Flag behavior: - Duplicate `--format`/`--verbosity` flags: last value wins - Invalid values or unknown flags exit with code `2` and print usage guidance to stderr - In `--format=json`, structured output remains JSON regardless of verbosity value -- Per-step logs are written to `PRE_COMMIT_LOG_DIR` (default: `/tmp`) +- Per-step logs are written to `TORRUST_GIT_HOOKS_LOG_DIR` (default: `/tmp`) For restricted agent environments that cannot write outside the workspace, run with: ```bash -PRE_COMMIT_LOG_DIR=.tmp ./contrib/dev-tools/git/hooks/pre-commit.sh +TORRUST_GIT_HOOKS_LOG_DIR=.tmp ./contrib/dev-tools/git/hooks/pre-commit.sh ``` The `.tmp/` directory is git-ignored. diff --git a/.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md b/.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md new file mode 100644 index 000000000..38d958b37 --- /dev/null +++ b/.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md @@ -0,0 +1,113 @@ +--- +name: run-pre-push-checks +description: Run all mandatory pre-push verification steps for the torrust-tracker project. Covers the pre-push script (automated checks), output modes, and log-directory configuration. Use before pushing or when running the comprehensive developer gate including nightly checks and E2E tests. Triggers on "pre-push checks", "run pre-push", "verify before push", or "push checks". +metadata: + author: torrust + version: "1.0" +--- + +# Run Pre-push Checks + +## Git Hook (Recommended Setup) + +The repository ships a `pre-push` Git hook that runs `./contrib/dev-tools/git/hooks/pre-push.sh` +automatically on every `git push`. Install it once after cloning: + +```bash +./contrib/dev-tools/git/install-git-hooks.sh +``` + +After installation the hook fires automatically; you do not need to invoke the script +manually before each push. + +> **For AI agents**: before invoking the script manually, check whether the hook is installed: +> +> ```bash +> [[ -x "$(git rev-parse --git-path hooks)/pre-push" ]] && echo "installed" || echo "not installed" +> ``` +> +> If installed, skip the manual run — `git push` will trigger it automatically. +> Running both would execute every check twice. + +## Automated Checks + +> **⏱️ Expected runtime: ~15 minutes** on a modern developer machine with warm caches. +> AI agents should set a command timeout of **at least 30 minutes** before invoking +> `./contrib/dev-tools/git/hooks/pre-push.sh`. + +Run the pre-push script. **It must exit with code `0` before every push.** + +```bash +./contrib/dev-tools/git/hooks/pre-push.sh +``` + +The script runs these steps in order: + +1. `cargo +stable machete` - unused dependency check +2. `linter all` - all linters (markdown, YAML, TOML, clippy, rustfmt, shellcheck, cspell) +3. `cargo +nightly fmt --check` - nightly format check +4. `cargo +nightly check ...` - nightly workspace check +5. `cargo +nightly doc ...` - nightly documentation build +6. `cargo +stable test --doc --workspace` - documentation tests +7. `cargo +stable test --tests --benches --examples --workspace --all-targets --all-features` - all tests +8. `cargo +stable run --bin e2e_tests_runner ...` - end-to-end tests + +## Output Modes + +The pre-push script supports concise human output, verbose human output, and JSON output for +automation. + +```bash +# Default: text + concise +./contrib/dev-tools/git/hooks/pre-push.sh + +# Explicit text + concise +./contrib/dev-tools/git/hooks/pre-push.sh --format=text --verbosity=concise + +# Text + verbose streaming command output +./contrib/dev-tools/git/hooks/pre-push.sh --format=text --verbosity=verbose + +# Compatibility alias +./contrib/dev-tools/git/hooks/pre-push.sh --format=text --verbose + +# Structured output (single JSON document to stdout) +./contrib/dev-tools/git/hooks/pre-push.sh --format=json +``` + +Flag behavior: + +- `--format=` defaults to `text` +- `--verbosity=` defaults to `concise` +- `--verbose` is an alias for `--verbosity=verbose` +- Duplicate `--format`/`--verbosity` flags: last value wins +- Invalid values or unknown flags exit with code `2` and print usage guidance to stderr +- In `--format=json`, structured output remains JSON regardless of verbosity value +- Per-step logs are written to `TORRUST_GIT_HOOKS_LOG_DIR` (default: `/tmp`) + +For restricted agent environments that cannot write outside the workspace, run with: + +```bash +TORRUST_GIT_HOOKS_LOG_DIR=.tmp ./contrib/dev-tools/git/hooks/pre-push.sh +``` + +The `.tmp/` directory is git-ignored. +Because `.tmp/` is workspace-local, clean stale `pre-push-*.log` files periodically. + +## Check Tier Ownership + +Check ownership is intentionally split by gate: + +- Pre-commit: fast local gate (`cargo machete`, `linter all`, `cargo test --doc --workspace`) +- Pre-push: comprehensive developer gate (nightly format/check/doc + stable tests + E2E) +- CI: merge authority with full validation and E2E matrix jobs + +E2E is intentionally excluded from pre-commit and remains a pre-push/CI responsibility. + +## Troubleshooting Output Modes + +- Concise mode shows high-signal per-step summaries only. On failure, it prints the log path and + a short failure tail. +- Verbose mode streams full command output to the terminal. Use this for deep local debugging. +- JSON mode emits one structured document to stdout; diagnostics and usage errors go to stderr. +- If concise output is too short for debugging, re-run the same command with + `--format=text --verbosity=verbose`. diff --git a/AGENTS.md b/AGENTS.md index 43df09238..3caf50ea9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -153,11 +153,11 @@ Pre-commit defaults to concise text output and runs the fast local profile: Use `--format=text --verbosity=verbose` for full streaming output, or `--format=json` for a single structured JSON payload. -Pre-commit per-step logs are written to `PRE_COMMIT_LOG_DIR` (default: `/tmp`). -In restricted AI-agent sandboxes, set `PRE_COMMIT_LOG_DIR=.tmp` to keep temporary logs inside the -workspace (`.tmp/` is git-ignored). -When using `.tmp`, periodically clean old logs (for example, remove stale `pre-commit-*.log` -files) because OS-managed `/tmp` cleanup does not apply. +Both hooks write per-step logs to `TORRUST_GIT_HOOKS_LOG_DIR` (default: `/tmp`). +In restricted AI-agent sandboxes, set `TORRUST_GIT_HOOKS_LOG_DIR=.tmp` to keep temporary logs +inside the workspace for both hooks (`.tmp/` is git-ignored). +When using `.tmp`, periodically clean old logs (for example, remove stale `pre-commit-*.log` and +`pre-push-*.log` files) because OS-managed `/tmp` cleanup does not apply. Gate ownership: @@ -175,6 +175,7 @@ Primary skill references: - `run-linters`: `.github/skills/dev/git-workflow/run-linters/SKILL.md` - `run-pre-commit-checks`: `.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md` +- `run-pre-push-checks`: `.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md` - `setup-dev-environment`: `.github/skills/dev/maintenance/setup-dev-environment/SKILL.md` Supporting docs: diff --git a/contrib/dev-tools/git/hooks/pre-commit.sh b/contrib/dev-tools/git/hooks/pre-commit.sh index c710a3eaf..8c3ab1430 100755 --- a/contrib/dev-tools/git/hooks/pre-commit.sh +++ b/contrib/dev-tools/git/hooks/pre-commit.sh @@ -26,7 +26,7 @@ declare -a STEPS=( FORMAT="text" VERBOSITY="concise" FAILURE_TAIL_LINES=10 -LOG_DIR="${PRE_COMMIT_LOG_DIR:-/tmp}" +LOG_DIR="${TORRUST_GIT_HOOKS_LOG_DIR:-/tmp}" declare -a STEP_NAMES=() declare -a STEP_COMMANDS=() @@ -60,7 +60,7 @@ Options: -h, --help Show this help Environment: - PRE_COMMIT_LOG_DIR Directory for per-step log files. Default: /tmp + TORRUST_GIT_HOOKS_LOG_DIR Directory for per-step log files (shared by all git hooks). Default: /tmp EOF } @@ -165,11 +165,13 @@ run_step() { local safe_name safe_name=$(sanitize_name_for_log "${description}") - local log_path - if ! log_path=$(mktemp "${LOG_DIR%/}/pre-commit-${safe_name}-XXXXXX"); then + local _tmp log_path + if ! _tmp=$(mktemp "${LOG_DIR%/}/pre-commit-${safe_name}-XXXXXX"); then echo "Error: failed to create a temporary log file in '${LOG_DIR}'." >&2 return 2 fi + log_path="${_tmp}.log" + mv "$_tmp" "$log_path" run_command "${command}" "${log_path}" local command_exit_code=$? diff --git a/contrib/dev-tools/git/hooks/pre-push.sh b/contrib/dev-tools/git/hooks/pre-push.sh index f03c6d5cd..2e8dea4a9 100755 --- a/contrib/dev-tools/git/hooks/pre-push.sh +++ b/contrib/dev-tools/git/hooks/pre-push.sh @@ -4,31 +4,42 @@ # validation and end-to-end tests. # # Usage: -# ./contrib/dev-tools/git/hooks/pre-push.sh +# ./contrib/dev-tools/git/hooks/pre-push.sh [--format=] [--verbosity=] [--verbose] # # Expected runtime: ~15 minutes on a modern developer machine. # AI agents: set a per-command timeout of at least 30 minutes before invoking this script. # # All steps must pass (exit 0) before pushing. -set -euo pipefail +set -uo pipefail # ============================================================================ # STEPS # ============================================================================ -# Each step: "description|success_message|command" +# Each step: "description|command" declare -a STEPS=( - "Checking for unused dependencies (cargo machete)|No unused dependencies found|cargo +stable machete" - "Running all linters|All linters passed|linter all" - "Checking format with nightly toolchain|Nightly format check passed|cargo +nightly fmt --check" - "Checking workspace with nightly toolchain|Nightly check passed|cargo +nightly check --tests --benches --examples --workspace --all-targets --all-features" - "Building documentation with nightly toolchain|Nightly documentation built|cargo +nightly doc --no-deps --bins --examples --workspace --all-features" - "Running documentation tests|Documentation tests passed|cargo +stable test --doc --workspace" - "Running all tests|All tests passed|cargo +stable test --tests --benches --examples --workspace --all-targets --all-features" - "Running E2E tests|E2E tests passed|cargo +stable run --bin e2e_tests_runner -- --config-toml-path ./share/default/config/tracker.e2e.container.sqlite3.toml" + "Checking for unused dependencies (cargo machete)|cargo +stable machete" + "Running all linters|linter all" + "Checking format with nightly toolchain|cargo +nightly fmt --check" + "Checking workspace with nightly toolchain|cargo +nightly check --tests --benches --examples --workspace --all-targets --all-features" + "Building documentation with nightly toolchain|cargo +nightly doc --no-deps --bins --examples --workspace --all-features" + "Running documentation tests|cargo +stable test --doc --workspace" + "Running all tests|cargo +stable test --tests --benches --examples --workspace --all-targets --all-features" + "Running E2E tests|cargo +stable run --bin e2e_tests_runner -- --config-toml-path ./share/default/config/tracker.e2e.container.sqlite3.toml" ) +FORMAT="text" +VERBOSITY="concise" +FAILURE_TAIL_LINES=10 +LOG_DIR="${TORRUST_GIT_HOOKS_LOG_DIR:-/tmp}" + +declare -a STEP_NAMES=() +declare -a STEP_COMMANDS=() +declare -a STEP_STATUSES=() +declare -a STEP_ELAPSED_SECONDS=() +declare -a STEP_LOG_PATHS=() + # ============================================================================ # HELPER FUNCTIONS # ============================================================================ @@ -44,26 +55,269 @@ format_time() { fi } +print_usage() { + cat >&2 <<'EOF' +Usage: ./contrib/dev-tools/git/hooks/pre-push.sh [--format=] [--verbosity=] [--verbose] + +Options: + --format= Output format. Default: text + --verbosity= Text output verbosity. Default: concise + --verbose Compatibility alias for --verbosity=verbose + -h, --help Show this help + +Environment: + TORRUST_GIT_HOOKS_LOG_DIR Shared directory for per-step log files (used by all git hooks). Default: /tmp +EOF +} + +prepare_log_dir() { + if ! mkdir -p "${LOG_DIR}"; then + echo "Error: cannot create log directory '${LOG_DIR}'." >&2 + exit 2 + fi + + if [[ ! -d "${LOG_DIR}" || ! -w "${LOG_DIR}" ]]; then + echo "Error: log directory '${LOG_DIR}' is not writable." >&2 + exit 2 + fi +} + +json_escape() { + local input=$1 + input=${input//\\/\\\\} + input=${input//\"/\\\"} + input=${input//$'\b'/\\b} + input=${input//$'\f'/\\f} + input=${input//$'\n'/\\n} + input=${input//$'\r'/\\r} + input=${input//$'\t'/\\t} + input=$(printf '%s' "${input}" | tr -d '\000-\010\013\016-\037') + printf '%s' "${input}" +} + +strip_ansi() { + sed -E 's/\x1B\[[0-9;]*[A-Za-z]//g' +} + +sanitize_name_for_log() { + local raw_name=$1 + local normalized + normalized=$(printf '%s' "${raw_name}" | tr '[:upper:]' '[:lower:]' | tr -cs 'a-z0-9' '-') + normalized=${normalized#-} + normalized=${normalized%-} + if [[ -z "${normalized}" ]]; then + normalized="step" + fi + printf '%s' "${normalized}" +} + +print_step_summary() { + local step_number=$1 + local total_steps=$2 + local description=$3 + local status=$4 + local elapsed_seconds=$5 + local log_path=$6 + + if [[ "${status}" == "pass" ]]; then + printf '[Step %d/%d] %s ... PASS (%s)\n' "${step_number}" "${total_steps}" "${description}" "$(format_time "${elapsed_seconds}")" + return + fi + + printf '[Step %d/%d] %s ... FAIL (%s) log: %s\n' \ + "${step_number}" \ + "${total_steps}" \ + "${description}" \ + "$(format_time "${elapsed_seconds}")" \ + "${log_path}" + + local -a tail_lines=() + while IFS= read -r line; do + tail_lines+=("${line}") + done < <(tail -n "${FAILURE_TAIL_LINES}" "${log_path}" | strip_ansi) + + local shown_count=${#tail_lines[@]} + for line in "${tail_lines[@]}"; do + printf ' %s\n' "${line}" + done + + printf ' (%d lines shown - full log: %s)\n' "${shown_count}" "${log_path}" +} + +run_command() { + local command=$1 + local log_path=$2 + + if [[ "${FORMAT}" == "text" && "${VERBOSITY}" == "verbose" ]]; then + bash -o pipefail -c "${command}" 2>&1 | tee "${log_path}" + local command_exit_code=${PIPESTATUS[0]} + return "${command_exit_code}" + fi + + bash -o pipefail -c "${command}" >"${log_path}" 2>&1 +} + run_step() { local step_number=$1 local total_steps=$2 local description=$3 - local success_message=$4 - local command=$5 + local command=$4 - echo "[Step ${step_number}/${total_steps}] ${description}..." + if [[ "${FORMAT}" == "text" && "${VERBOSITY}" == "verbose" ]]; then + printf '[Step %d/%d] %s...\n' "${step_number}" "${total_steps}" "${description}" + fi local step_start=$SECONDS - local -a cmd_array - read -ra cmd_array <<< "${command}" - "${cmd_array[@]}" + + local safe_name + safe_name=$(sanitize_name_for_log "${description}") + local _tmp log_path + if ! _tmp=$(mktemp "${LOG_DIR%/}/pre-push-${safe_name}-XXXXXX"); then + echo "Error: failed to create a temporary log file in '${LOG_DIR}'." >&2 + return 2 + fi + log_path="${_tmp}.log" + mv "$_tmp" "$log_path" + + run_command "${command}" "${log_path}" + local command_exit_code=$? + local step_elapsed=$((SECONDS - step_start)) - echo "PASSED: ${success_message} ($(format_time "${step_elapsed}"))" - echo + STEP_NAMES+=("${description}") + STEP_COMMANDS+=("${command}") + STEP_ELAPSED_SECONDS+=("${step_elapsed}") + STEP_LOG_PATHS+=("${log_path}") + + if [[ "${command_exit_code}" -eq 0 ]]; then + STEP_STATUSES+=("pass") + else + STEP_STATUSES+=("fail") + fi + + local step_status=${STEP_STATUSES[$(( ${#STEP_STATUSES[@]} - 1 ))]} + + if [[ "${FORMAT}" == "text" ]]; then + print_step_summary \ + "${step_number}" \ + "${total_steps}" \ + "${description}" \ + "${step_status}" \ + "${step_elapsed}" \ + "${log_path}" + if [[ "${VERBOSITY}" == "verbose" ]]; then + echo + fi + fi + + return "${command_exit_code}" } -trap 'echo ""; echo "=========================================="; echo "FAILED: Pre-push checks failed!"; echo "Fix the errors above before pushing."; echo "=========================================="; exit 1' ERR +emit_json_result() { + local overall_status=$1 + local exit_code=$2 + local total_elapsed=$3 + local failed_step_name=$4 + + printf '{\n' + printf ' "schema_version": 1,\n' + printf ' "status": "%s",\n' "${overall_status}" + printf ' "exit_code": %d,\n' "${exit_code}" + printf ' "elapsed_seconds": %d' "${total_elapsed}" + + if [[ -n "${failed_step_name}" ]]; then + printf ',\n "failed_step": "%s"' "$(json_escape "${failed_step_name}")" + fi + + printf ',\n "steps": [\n' + + local steps_count=${#STEP_NAMES[@]} + for ((index = 0; index < steps_count; index++)); do + local name=${STEP_NAMES[$index]} + local command=${STEP_COMMANDS[$index]} + local status=${STEP_STATUSES[$index]} + local elapsed=${STEP_ELAPSED_SECONDS[$index]} + local log_path=${STEP_LOG_PATHS[$index]} + + printf ' {\n' + printf ' "name": "%s",\n' "$(json_escape "${name}")" + printf ' "command": "%s",\n' "$(json_escape "${command}")" + printf ' "status": "%s",\n' "${status}" + printf ' "elapsed_seconds": %d' "${elapsed}" + + if [[ "${status}" == "fail" ]]; then + printf ',\n "log_path": "%s",\n' "$(json_escape "${log_path}")" + printf ' "failure_tail": [' + + local -a tail_lines=() + while IFS= read -r line; do + tail_lines+=("${line}") + done < <(tail -n "${FAILURE_TAIL_LINES}" "${log_path}" | strip_ansi) + + local tail_count=${#tail_lines[@]} + for ((tail_index = 0; tail_index < tail_count; tail_index++)); do + if [[ "${tail_index}" -gt 0 ]]; then + printf ', ' + fi + printf '"%s"' "$(json_escape "${tail_lines[$tail_index]}")" + done + printf ']' + fi + + if [[ "${index}" -lt $((steps_count - 1)) ]]; then + printf '\n },\n' + else + printf '\n }\n' + fi + done + + printf ' ]\n' + printf '}\n' +} + +parse_args() { + for arg in "$@"; do + case "${arg}" in + --format=text) + FORMAT="text" + ;; + --format=json) + FORMAT="json" + ;; + --verbosity=concise) + VERBOSITY="concise" + ;; + --verbosity=verbose) + VERBOSITY="verbose" + ;; + --verbose) + VERBOSITY="verbose" + ;; + -h|--help) + print_usage + exit 0 + ;; + --format=*) + echo "Error: invalid --format value in '${arg}'. Expected --format=text or --format=json." >&2 + print_usage + exit 2 + ;; + --verbosity=*) + echo "Error: invalid --verbosity value in '${arg}'. Expected --verbosity=concise or --verbosity=verbose." >&2 + print_usage + exit 2 + ;; + *) + echo "Error: unknown option '${arg}'." >&2 + print_usage + exit 2 + ;; + esac + done +} + +parse_args "$@" +prepare_log_dir # ============================================================================ # MAIN @@ -71,18 +325,48 @@ trap 'echo ""; echo "=========================================="; echo "FAILED: TOTAL_START=$SECONDS TOTAL_STEPS=${#STEPS[@]} +overall_status="pass" +exit_code=0 +failed_step_name="" -echo "Running pre-push checks..." -echo +if [[ "${FORMAT}" == "text" ]]; then + echo "Running pre-push checks..." + echo +fi for i in "${!STEPS[@]}"; do - IFS='|' read -r description success_message command <<< "${STEPS[$i]}" - run_step $((i + 1)) "${TOTAL_STEPS}" "${description}" "${success_message}" "${command}" + IFS='|' read -r description command <<< "${STEPS[$i]}" + run_step_rc=0 + run_step $((i + 1)) "${TOTAL_STEPS}" "${description}" "${command}" || run_step_rc=$? + if [[ $run_step_rc -ne 0 ]]; then + overall_status="fail" + # exit_code 2 = infrastructure/script error (e.g. mktemp failed); 1 = check failure. + # Normalize any non-zero, non-2 command exit code to 1 so consumers see a stable contract. + exit_code=$(( run_step_rc == 2 ? 2 : 1 )) + failed_step_name="${description}" + break + fi done TOTAL_ELAPSED=$((SECONDS - TOTAL_START)) + +if [[ "${FORMAT}" == "json" ]]; then + emit_json_result "${overall_status}" "${exit_code}" "${TOTAL_ELAPSED}" "${failed_step_name}" + exit "${exit_code}" +fi + +if [[ "${overall_status}" == "pass" ]]; then + echo "==========================================" + echo "SUCCESS: All pre-push checks passed! ($(format_time "${TOTAL_ELAPSED}"))" + echo "==========================================" + echo + echo "You can now safely push your changes." + exit 0 +fi + +echo echo "==========================================" -echo "SUCCESS: All pre-push checks passed! ($(format_time "${TOTAL_ELAPSED}"))" +echo "FAILED: Pre-push checks failed!" +echo "Fix the errors above before pushing." echo "==========================================" -echo -echo "You can now safely push your changes." +exit 1 diff --git a/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md b/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md index 24e5dbc2f..0d6ed5926 100644 --- a/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md +++ b/docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md @@ -7,7 +7,7 @@ github-issue: 1780 spec-path: docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md branch: "1780-refactor-pre-push-checks-performance-and-verbosity" related-pr: null -last-updated-utc: 2026-05-13 17:55 +last-updated-utc: 2026-05-13 21:00 semantic-links: skill-links: - create-issue @@ -16,6 +16,7 @@ semantic-links: - contrib/dev-tools/git/hooks/pre-commit.sh - .github/workflows/testing.yaml - .github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md + - .github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md --- @@ -51,9 +52,14 @@ while improving clarity, observability, and parity with pre-commit. - Keep `--verbose` as alias for `--verbosity=verbose`. - Add concise failure summaries (step, status, elapsed, log path, failure tail). - Add JSON output mode with one structured payload to stdout. -- Add configurable per-step log directory env var (follow pre-commit contract). +- Add `TORRUST_GIT_HOOKS_LOG_DIR` env var for configurable per-step log directory (see + [Design Decisions](#design-decisions)). +- Update `pre-commit.sh` to use `TORRUST_GIT_HOOKS_LOG_DIR` (replacing the script-specific + `PRE_COMMIT_LOG_DIR` var) so all hooks share the same env var. - Preserve existing pre-push validation steps, including E2E. -- Update docs/skills so pre-commit and pre-push behavior is consistent. +- Create a new `run-pre-push-checks` skill (parallel structure to `run-pre-commit-checks`). +- Update `run-pre-commit-checks` skill to document `TORRUST_GIT_HOOKS_LOG_DIR`. +- Update `AGENTS.md` to reference the new env var and pre-push output modes. ### Out of Scope @@ -62,26 +68,43 @@ while improving clarity, observability, and parity with pre-commit. - CI workflow redesign. - Broader hook framework rewrite into Rust CLI (future option only). +## Design Decisions + +Decisions agreed with maintainer during planning (2026-05-13): + +| Decision | Choice | Rationale | +| --------------------------------------- | ----------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | +| Log directory env var | `TORRUST_GIT_HOOKS_LOG_DIR` (shared across all hooks, default `/tmp`) | `TORRUST_` prefix keeps tracker namespace clean; `GIT_HOOKS_` infix distinguishes from tracker runtime vars | +| `pre-commit.sh` updated | Replace script-specific `PRE_COMMIT_LOG_DIR` with `TORRUST_GIT_HOOKS_LOG_DIR` | Single env var for all hooks; simpler mental model for developers | +| Skill docs strategy | New `run-pre-push-checks` skill (parallel to `run-pre-commit-checks`) | Keeps skills focused; mirrors pre-commit/pre-push symmetry | +| `--format=json` + `--verbosity=verbose` | JSON only; verbosity flag silently ignored in JSON mode | Consistent with pre-commit behavior; keeps JSON output machine-parseable | +| Failure behavior | Fail-fast — stop on first failure | Consistent with pre-commit; saves time on a broken state | + ## Implementation Plan Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. -| ID | Status | Task | Notes / Expected Output | -| --- | ------ | ---------------------------------------- | -------------------------------------------------------------- | -| T1 | TODO | Define pre-push CLI/output contract | Final behavior matrix and error handling documented | -| T2 | TODO | Implement hook refactor | `pre-push.sh` supports format/verbosity/log-dir parity | -| T3 | TODO | Validate behavior in pass and fail paths | Text concise/verbose + JSON tested with exit-code verification | -| T4 | TODO | Update docs and skills | Workflow docs aligned with pre-push capabilities | -| T5 | TODO | Run quality checks and finalize evidence | `linter all` and targeted checks pass | +| ID | Status | Task | Notes / Expected Output | +| --- | ------ | ------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------- | +| T1 | DONE | Define pre-push CLI/output contract | Decisions captured in [Design Decisions](#design-decisions) | +| T2 | DONE | Refactor `pre-push.sh` | Adds format/verbosity/log-dir parity; mirrors `pre-commit.sh` implementation | +| T3 | DONE | Update `pre-commit.sh` for `TORRUST_GIT_HOOKS_LOG_DIR` | Replaced `PRE_COMMIT_LOG_DIR` with `TORRUST_GIT_HOOKS_LOG_DIR`; all hooks now share one env var | +| T4 | DONE | Create `run-pre-push-checks` skill | `.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md` created | +| T5 | DONE | Update `run-pre-commit-checks` skill | `TORRUST_GIT_HOOKS_LOG_DIR` fallback documented | +| T6 | DONE | Update `AGENTS.md` | Log-dir env var and pre-push skill reference added | +| T7 | DONE | Validate behavior in pass and fail paths | shellcheck clean; all output modes (text+concise, text+verbose, json) verified on pass and fail paths | +| T8 | DONE | Run quality checks and finalize evidence | `linter all` exits `0`; shellcheck passes on both hook scripts | +| T9 | DONE | Add `.githooks/pre-push` hook dispatcher | Mirrors `.githooks/pre-commit`; registered via `install-git-hooks.sh` | +| T10 | DONE | Explicit output mode in `.githooks/` dispatchers | Both dispatchers use TTY detection: `--format=text` for interactive terminals, `--format=json` for non-interactive/agent runs | ## Progress Tracking ### Workflow Checkpoints - [x] Spec drafted in `docs/issues/drafts/` -- [ ] Spec reviewed and approved by user/maintainer +- [x] Spec reviewed and approved by user/maintainer - [ ] GitHub issue created and issue number added to this spec -- [ ] Implementation completed +- [x] Implementation completed - [ ] Reviewer validated acceptance criteria and updated checkboxes - [ ] Committer verified spec progress is up to date before commit - [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` @@ -89,29 +112,61 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. ### Progress Log - 2026-05-13 13:00 UTC - Copilot - Drafted follow-up issue for pre-push parity with #1769 (output modes, summaries, JSON, log-dir configurability). +- 2026-05-13 19:00 UTC - Copilot - Agreed design decisions with maintainer: `TORRUST_GIT_HOOKS_LOG_DIR` shared env var, new `run-pre-push-checks` skill, JSON-only in `--format=json`, fail-fast behavior. Implementation plan refined into T1–T8. +- 2026-05-13 19:30 UTC - Copilot - Implemented T2–T8: refactored `pre-push.sh`, updated `pre-commit.sh`, created `run-pre-push-checks` skill, updated `run-pre-commit-checks` skill and `AGENTS.md`. All pre-commit checks pass; shellcheck clean. +- 2026-05-13 20:00 UTC - Copilot - Manually verified all output modes (pass+fail paths for text+concise, text+verbose, json; TORRUST_GIT_HOOKS_LOG_DIR log file creation). Added `.githooks/pre-push` dispatcher (T9) and installed via `install-git-hooks.sh`. +- 2026-05-13 20:30 UTC - Copilot - Added explicit `--format=text --verbosity=concise` to both `.githooks/` dispatchers (T10); added manual verification test matrix to spec. +- 2026-05-13 21:00 UTC - Copilot - Changed `.githooks/` dispatchers to use `--format=json` as the explicit default (updated T10). ## Acceptance Criteria -- [ ] AC1: `pre-push.sh` supports `--format=` and `--verbosity=` with `--verbose` alias. -- [ ] AC2: `--format=text --verbosity=concise` prints high-signal per-step summary; failures include log path and short tail. -- [ ] AC3: `--format=json` emits one valid JSON document to stdout with step-level status and timing. -- [ ] AC4: Invalid/unknown flags fail with exit code `2`, usage hint, and stderr diagnostics. -- [ ] AC5: Existing pre-push check ownership is preserved (including E2E in pre-push). -- [ ] AC6: Log-directory override env var is supported and documented (parity with pre-commit behavior). -- [ ] `linter all` exits with code `0` +- [x] AC1: `pre-push.sh` supports `--format=` and `--verbosity=` with `--verbose` alias. +- [x] AC2: `--format=text --verbosity=concise` prints high-signal per-step summary; failures include log path and short tail. +- [x] AC3: `--format=json` emits one valid JSON document to stdout with step-level status and timing. +- [x] AC4: Invalid/unknown flags fail with exit code `2`, usage hint, and stderr diagnostics. +- [x] AC5: Existing pre-push check ownership is preserved (including E2E in pre-push). +- [x] AC6: `TORRUST_GIT_HOOKS_LOG_DIR` is the shared log-directory env var for all hooks, defaulting to + `/tmp`. Both `pre-push.sh` and `pre-commit.sh` use it. Both hooks document it in their usage + text and in skill docs. +- [x] AC7: `--format=json` emits JSON only regardless of `--verbosity` value (verbosity silently + ignored in JSON mode). +- [x] AC8: On first step failure, the hook stops immediately (fail-fast) and reports the failing + step; subsequent steps are not run. +- [x] `linter all` exits with code `0` - [ ] Relevant tests pass -- [ ] Documentation is updated when behavior/workflow changes +- [x] Documentation is updated when behavior/workflow changes + +### Manual Verification Test Matrix + +Tested with a fast-step stub (2–3 no-op steps), `TORRUST_GIT_HOOKS_LOG_DIR=.tmp`. + +| Test case | Expected | Result | +| ----------------------------------------------- | ------------------------------------------------------------------------- | ------ | +| `--help` / `-h` | exit 0, usage text on stderr | PASS | +| `--format=bad` | exit 2, error + usage on stderr | PASS | +| `--verbosity=bad` | exit 2, error + usage on stderr | PASS | +| `--unknown` | exit 2, error + usage on stderr | PASS | +| `text concise` pass path | `[Step N/M] … PASS (Xs)` per step + SUCCESS footer, exit 0 | PASS | +| `text verbose` pass path | step header + streaming stdout + PASS summary + blank line, exit 0 | PASS | +| `--format=json` pass path | valid JSON, `status: pass`, `exit_code: 0`, all steps in array | PASS | +| `text concise` fail path | FAIL line + log path + tail lines; subsequent steps skipped; exit 1 | PASS | +| `--format=json` fail path | valid JSON, `status: fail`, `exit_code: 1`, `failed_step`, `failure_tail` | PASS | +| `--format=json --verbose` | JSON only — verbosity silently ignored | PASS | +| `TORRUST_GIT_HOOKS_LOG_DIR` in pre-push | log files created in `.tmp/pre-push-*` | PASS | +| `TORRUST_GIT_HOOKS_LOG_DIR` fallback pre-commit | logs in `.tmp/pre-commit-*`, JSON output valid | PASS | ### Acceptance Verification -| AC ID | Status (`TODO`/`DONE`) | Evidence | -| ----- | ---------------------- | -------- | -| AC1 | TODO | | -| AC2 | TODO | | -| AC3 | TODO | | -| AC4 | TODO | | -| AC5 | TODO | | -| AC6 | TODO | | +| AC ID | Status (`TODO`/`DONE`) | Evidence | +| ----- | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| AC1 | DONE | `--format`, `--verbosity`, `--verbose` parsed in `parse_args`; invalid values exit `2` | +| AC2 | DONE | `print_step_summary` in concise mode; failure path prints log path + tail | +| AC3 | DONE | `emit_json_result` outputs one JSON doc to stdout on `--format=json` | +| AC4 | DONE | `--format=bad` → exit `2` + usage; `--verbosity=bad` → exit `2`; `--unknown` → exit `2` (all manually verified) | +| AC5 | DONE | All 8 original steps preserved unchanged in `STEPS` array | +| AC6 | DONE | Both hooks use `TORRUST_GIT_HOOKS_LOG_DIR`; log files written to `.tmp/` in tests; usage texts and skills updated; `.githooks/pre-push` dispatcher installed | +| AC7 | DONE | `emit_json_result` is called regardless of `VERBOSITY` when `FORMAT=json` | +| AC8 | DONE | `break` on first `run_step` failure in main loop | ## Risks and Trade-offs diff --git a/project-words.txt b/project-words.txt index e022264ff..75ccd808d 100644 --- a/project-words.txt +++ b/project-words.txt @@ -195,6 +195,7 @@ ostr Pando parallelise parallelised +parseable peekable peerlist peersld @@ -230,8 +231,8 @@ repomix repr reqs reqwest -rescope rerequests +rescope reuseaddr ringbuf ringsize