feat(pre-push): add format/verbosity/log-dir parity with pre-commit (#1780)#1783
Merged
josecelano merged 9 commits intoMay 14, 2026
Conversation
- Add Design Decisions section (5 agreed decisions: TORRUST_GIT_HOOKS_LOG_DIR shared env var, new run-pre-push-checks skill, JSON-only in --format=json, fail-fast behavior, PRE_COMMIT_LOG_DIR backward compat for pre-commit) - Expand scope section with precise env var name and list of artifacts to update - Refine implementation plan from T1-T5 to T1-T8; mark T1 as DONE - Add AC7 (JSON-only in json mode) and AC8 (fail-fast) to acceptance criteria - Update progress log with planning entry (2026-05-13 19:00 UTC) - Update frontmatter: last-updated-utc, add run-pre-push-checks to related-artifacts - Add "parseable" to project-words.txt (used in Design Decisions table)
Add `.githooks/pre-push` that delegates to `contrib/dev-tools/git/hooks/pre-push.sh`, mirroring the existing `.githooks/pre-commit` pattern. Run `./contrib/dev-tools/git/install-git-hooks.sh` to register it. Also update issue spec torrust#1780: - T7 evidence expanded to cover all verified output modes - T9 added: add .githooks/pre-push dispatcher - AC4 and AC6 evidence updated with manual verification notes - Progress log updated
Pass `--format=text --verbosity=concise` explicitly in both `.githooks/pre-commit` and `.githooks/pre-push` so the output mode is declared rather than relying on implicit script defaults. Also update issue spec torrust#1780: - T10 added: explicit output mode in .githooks/ dispatchers - Manual verification test matrix added to Acceptance Verification section - Progress log updated
…atchers Change both `.githooks/pre-commit` and `.githooks/pre-push` to pass `--format=json` so hook output is machine-readable by default. The underlying scripts still default to `--format=text` when invoked directly from the command line. Update issue spec torrust#1780: T10 note and progress log updated.
Update the Committer agent and run-pre-commit/run-pre-push-checks skills to check whether the corresponding git hook is installed before invoking the pre-commit.sh / pre-push.sh script manually. When the hook is installed, git fires it automatically on commit/push, so a prior manual run would execute every check twice. Agents now use: [[ -x "$(git rev-parse --git-path hooks)/pre-commit" ]] to detect the installed hook and skip the redundant manual invocation.
There was a problem hiding this comment.
Pull request overview
This PR refactors the pre-push git-hook script to match the newer pre-commit hook’s operator contract (text vs JSON output, concise vs verbose modes, per-step log capture, and clearer failure summaries), and updates docs/skills/dispatchers to reflect the unified workflow (including a shared log-dir env var).
Changes:
- Rewrites
contrib/dev-tools/git/hooks/pre-push.shto support--format=<text|json>,--verbosity=<concise|verbose>, fail-fast execution, and per-step log files underTORRUST_GIT_HOOKS_LOG_DIR. - Updates
pre-commit.shto fall back toTORRUST_GIT_HOOKS_LOG_DIRafterPRE_COMMIT_LOG_DIR, and updates workflow docs/skills/agents accordingly. - Adds/updates
.githooks/*dispatchers and introduces a new “run-pre-push-checks” skill doc.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
contrib/dev-tools/git/hooks/pre-push.sh |
Full rewrite adding format/verbosity support, fail-fast behavior, JSON emission, and shared log-dir usage. |
contrib/dev-tools/git/hooks/pre-commit.sh |
Adds TORRUST_GIT_HOOKS_LOG_DIR fallback and updates help text. |
.githooks/pre-push |
New pre-push dispatcher invoking the rewritten script. |
.githooks/pre-commit |
Dispatcher updated to explicitly request JSON output. |
.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md |
New skill documenting pre-push hook usage and output modes. |
.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.md |
Skill updated to document shared log-dir fallback and hook-installed guard. |
.github/agents/committer.agent.md |
Adds guidance to avoid double-running checks when hooks are installed. |
AGENTS.md |
Documents shared log-dir env var and references the new skill. |
docs/issues/open/1780-refactor-pre-push-checks-performance-and-verbosity.md |
Spec updated with design decisions, progress, and verification matrix. |
project-words.txt |
Adds/relocates terms to keep cspell wordlist consistent and sorted. |
Comments suppressed due to low confidence (1)
AGENTS.md:162
- The docs recommend cleaning up
pre-commit-*.log/pre-push-*.log, but the hook scripts currently create log files without a.logsuffix (viamktemp ...-XXXXXX). Update the cleanup glob here (or change the scripts to emit.logfiles) so the guidance matches the actual filenames.
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1783 +/- ##
========================================
Coverage 78.24% 78.24%
========================================
Files 376 376
Lines 28369 28369
Branches 28369 28369
========================================
Hits 22198 22198
Misses 5859 5859
Partials 312 312 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add .log suffix to mktemp in pre-push.sh and pre-commit.sh so log files match the *.log cleanup globs documented in skills/AGENTS.md - Propagate actual exit code from run_step (exit 2 for infra errors, exit 1 for check failures) instead of hardcoding 1 - Replace PRE_COMMIT_LOG_DIR with TORRUST_GIT_HOOKS_LOG_DIR in pre-commit.sh — single env var for all hooks, default /tmp - Use TTY detection in .githooks/ dispatchers: --format=text for interactive terminals, --format=json for non-interactive/agent runs - Update usage text, AGENTS.md, skill docs, and issue spec accordingly
- Fix mktemp portability: create temp file with XXXXXX at end, then mv to add .log suffix (XXXXXX must be last token on BSD/macOS mktemp) - Normalize exit_code to 1 for check failures; keep 2 for infrastructure/script errors so consumers see a stable contract - Update issue spec T10 note to reflect TTY detection in dispatchers rather than hardcoded --format=json
Member
Author
|
ACK a916d0b |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1780
Summary
Refactors
contrib/dev-tools/git/hooks/pre-push.shto align its operator experience with the pre-commit hook introduced in #1769:--format=<text|json>withtextas default--verbosity=<concise|verbose>withconciseas default (--verbosealias)TORRUST_GIT_HOOKS_LOG_DIRenv var for configurable log directory (default/tmp)Also:
pre-commit.shto useTORRUST_GIT_HOOKS_LOG_DIR(replacing the old script-specificPRE_COMMIT_LOG_DIRvar) so all hooks share the same env var.githooks/pre-pushdispatcher (TTY detection:--format=textfor interactive terminals,--format=jsonfor non-interactive/agent runs).githooks/pre-commitdispatcher to use the same TTY detection.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.mdrun-pre-commit-checksskill to documentTORRUST_GIT_HOOKS_LOG_DIRAGENTS.mdwith env var and new skill referenceManual Verification Test Matrix
Tested with a fast-step stub (2–3 no-op steps),
TORRUST_GIT_HOOKS_LOG_DIR=.tmp.--help/-h--format=bad--verbosity=bad--unknowntext concisepass path[Step N/M] … PASS (Xs)per step + SUCCESS footer, exit 0text verbosepass path--format=jsonpass pathstatus: pass,exit_code: 0, all steps in arraytext concisefail path--format=jsonfail pathstatus: fail,exit_code: 1,failed_step,failure_tail--format=json --verboseTORRUST_GIT_HOOKS_LOG_DIRin pre-push.tmp/pre-push-*.logTORRUST_GIT_HOOKS_LOG_DIRin pre-commit.tmp/pre-commit-*.log, JSON output validFiles Changed
contrib/dev-tools/git/hooks/pre-push.shcontrib/dev-tools/git/hooks/pre-commit.shPRE_COMMIT_LOG_DIRwithTORRUST_GIT_HOOKS_LOG_DIR.githooks/pre-push.githooks/pre-commit.github/skills/dev/git-workflow/run-pre-push-checks/SKILL.md.github/skills/dev/git-workflow/run-pre-commit-checks/SKILL.mdTORRUST_GIT_HOOKS_LOG_DIRand agent duplicate-run guidance.github/agents/committer.agent.mdAGENTS.md