fix(agents-vault): surface commit-hook rejection as exit 1
_agents_repo_sync fell off the end of its final if-block when git commit failed, e.g. a pre-commit or commit-msg hook rejecting it (this repo runs ggshield and Git-LFS hooks). fish's if construct sets status 0 when the condition is false and there is no else branch, so a rejected commit was being reported as success rather than as the documented "not a git repository" exit 1 it was assumed to fall through to. Add an explicit else branch that emits a stderr diagnostic and returns 1, and widen the EXIT STATUS/DESCRIPTION docs to cover this path under the existing code 1 rather than adding a fourth code, since later tasks already consume the 0/1/2 contract.
This commit is contained in:
@@ -14,7 +14,9 @@
|
|||||||
# left clean at local HEAD for the user to resolve by hand.
|
# left clean at local HEAD for the user to resolve by hand.
|
||||||
#
|
#
|
||||||
# Commits are made with commit.gpgsign=false so a pinentry prompt can
|
# Commits are made with commit.gpgsign=false so a pinentry prompt can
|
||||||
# never block a shell or an agent launch.
|
# never block a shell or an agent launch. If a pre-commit or commit-msg
|
||||||
|
# hook rejects the commit (e.g. a secret scanner), that failure is
|
||||||
|
# surfaced too: nothing is committed and a diagnostic goes to stderr.
|
||||||
#
|
#
|
||||||
# ARGUMENTS
|
# ARGUMENTS
|
||||||
# dir Absolute path to the git repository
|
# dir Absolute path to the git repository
|
||||||
@@ -22,7 +24,8 @@
|
|||||||
#
|
#
|
||||||
# EXIT STATUS
|
# EXIT STATUS
|
||||||
# 0 Committed, or nothing needed committing
|
# 0 Committed, or nothing needed committing
|
||||||
# 1 <dir> is not a git repository
|
# 1 <dir> is not a git repository, arguments were missing, or the commit
|
||||||
|
# itself failed (e.g. a pre-commit/commit-msg hook rejected it)
|
||||||
# 2 Rebase conflict; aborted, nothing committed
|
# 2 Rebase conflict; aborted, nothing committed
|
||||||
#
|
#
|
||||||
# RETURNS
|
# RETURNS
|
||||||
@@ -51,5 +54,9 @@ function _agents_repo_sync --argument-names dir msg
|
|||||||
set -l sha (git -C "$dir" rev-parse --short HEAD 2>/dev/null)
|
set -l sha (git -C "$dir" rev-parse --short HEAD 2>/dev/null)
|
||||||
set -l subject (git -C "$dir" log -1 --pretty=%s 2>/dev/null)
|
set -l subject (git -C "$dir" log -1 --pretty=%s 2>/dev/null)
|
||||||
echo "→ Committed ($sha) $subject"
|
echo "→ Committed ($sha) $subject"
|
||||||
|
return 0
|
||||||
|
else
|
||||||
|
echo "_agents_repo_sync: commit failed in $dir (hook rejected it?); nothing committed" >&2
|
||||||
|
return 1
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -183,6 +183,25 @@ check "conflict commits nothing" $before_count (git -C $s rev-list --count HEAD)
|
|||||||
check "conflict content survives" ours (cat $s/a.md)
|
check "conflict content survives" ours (cat $s/a.md)
|
||||||
check "conflict left no markers" false (grep -q '<<<<<<<' $s/a.md; and echo true; or echo false)
|
check "conflict left no markers" false (grep -q '<<<<<<<' $s/a.md; and echo true; or echo false)
|
||||||
|
|
||||||
|
# Commit-hook rejection: the commit call itself fails (e.g. a secret
|
||||||
|
# scanner in a pre-commit hook), distinct from "not a git repository" --
|
||||||
|
# both currently map to exit 1, so this must not fall through silently or
|
||||||
|
# report success.
|
||||||
|
set -l h (new_repo)
|
||||||
|
echo first >$h/a.md
|
||||||
|
_agents_repo_sync $h "chore: init" >/dev/null
|
||||||
|
|
||||||
|
set -l hooks (mktemp -d); set -ga TMPDIRS $hooks
|
||||||
|
printf '#!/bin/sh\nexit 1\n' >$hooks/pre-commit
|
||||||
|
chmod +x $hooks/pre-commit
|
||||||
|
git -C $h config core.hooksPath $hooks
|
||||||
|
|
||||||
|
echo second >$h/a.md
|
||||||
|
set -l before_hook_count (git -C $h rev-list --count HEAD)
|
||||||
|
set -l hrc (_agents_repo_sync $h "chore: blocked" 2>/dev/null; echo $status)
|
||||||
|
check "commit hook rejection returns 1" 1 "$hrc"
|
||||||
|
check "commit hook rejection commits nothing" $before_hook_count (git -C $h rev-list --count HEAD)
|
||||||
|
|
||||||
cleanup
|
cleanup
|
||||||
echo ""
|
echo ""
|
||||||
echo (math $TESTS_RUN - $TESTS_FAILED)"/$TESTS_RUN passed"
|
echo (math $TESTS_RUN - $TESTS_FAILED)"/$TESTS_RUN passed"
|
||||||
|
|||||||
Reference in New Issue
Block a user