From 3812b98927147b34e838bd3a5596e33cef8264d9 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Mon, 10 Aug 2026 01:55:46 +0100 Subject: [PATCH] fix(ship): verify pre-commit execution (sc-1537) Fixes Story #1537. Root cause: ship projected the Husky hook when core.hooksPath was unset, but git commit still used Git's empty default hooks directory. The unconditional success banner therefore claimed gates ran without evidence. This change shares hook resolution between preparation and commit, fails closed when no executable pre-commit hook exists, and requires an attempt-specific execution marker before publishing. Private direct-exec wrappers preserve Husky's real path semantics for sibling hooks such as commit-msg. Setup and proof failures now emit accurate terminal telemetry. Validation: - 125 affected ship tests passed - full suite: 3,740 passed, 5 skipped - typecheck and lint passed - GitNexus scope low; duplication, correctness, and commit-guard reviews passed --- .../commit-with-gate-capture.test.mts | 169 ++++++++++++++++++ .../review-private-dependencies.test.mts | 2 + cli/lib/ship/commit-with-gate-capture.sh | 162 ++++++++++++----- cli/lib/ship/prepare-gate-worktree.sh | 37 ++-- dist/cli/lib/ship/commit-with-gate-capture.sh | 162 ++++++++++++----- dist/cli/lib/ship/prepare-gate-worktree.sh | 37 ++-- 6 files changed, 459 insertions(+), 110 deletions(-) create mode 100644 cli/__tests__/commit-with-gate-capture.test.mts diff --git a/cli/__tests__/commit-with-gate-capture.test.mts b/cli/__tests__/commit-with-gate-capture.test.mts new file mode 100644 index 00000000..178e7cb1 --- /dev/null +++ b/cli/__tests__/commit-with-gate-capture.test.mts @@ -0,0 +1,169 @@ +import { execFileSync, spawnSync } from 'node:child_process'; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it } from 'vitest'; + +const here = dirname(fileURLToPath(import.meta.url)); +const helper = resolve(here, '../lib/ship/commit-with-gate-capture.sh'); +const created: string[] = []; +const gitEnv = { + ...process.env, + GIT_AUTHOR_NAME: 'Devkit Test', + GIT_AUTHOR_EMAIL: 'devkit@example.com', + GIT_COMMITTER_NAME: 'Devkit Test', + GIT_COMMITTER_EMAIL: 'devkit@example.com', +}; + +function git(cwd: string, ...args: string[]) { + return execFileSync('git', args, { cwd, env: gitEnv, encoding: 'utf8' }).trim(); +} + +function fixture(withHooks: boolean) { + const root = mkdtempSync(join(tmpdir(), 'ship-hook-proof-root-')); + const wt = mkdtempSync(join(tmpdir(), 'ship-hook-proof-wt-')); + rmSync(wt, { recursive: true, force: true }); + created.push(root, wt); + + git(root, 'init', '-q', '-b', 'main'); + git(root, 'config', 'user.email', 'devkit@example.com'); + git(root, 'config', 'user.name', 'Devkit Test'); + mkdirSync(join(root, '.husky/_'), { recursive: true }); + writeFileSync(join(root, '.husky/.keep'), ''); + if (withHooks) { + const huskyShim = '#!/usr/bin/env sh\n. "$(dirname "$0")/h"\n'; + writeFileSync( + join(root, '.husky/_/h'), + '#!/bin/sh\nn=$(basename "$0")\ns=$(dirname "$(dirname "$0")")/$n\n' + + '[ ! -f "$s" ] && exit 0\nexec sh -e "$s" "$@"\n', + ); + writeFileSync(join(root, '.husky/_/pre-commit'), huskyShim); + writeFileSync(join(root, '.husky/_/commit-msg'), huskyShim); + writeFileSync(join(root, '.husky/pre-commit'), "echo 'REAL_PRE_COMMIT_RAN' >&2\n"); + writeFileSync(join(root, '.husky/commit-msg'), 'echo "REAL_COMMIT_MSG_RAN:$1" >&2\n'); + chmodSync(join(root, '.husky/_/h'), 0o755); + chmodSync(join(root, '.husky/_/pre-commit'), 0o755); + chmodSync(join(root, '.husky/_/commit-msg'), 0o755); + } + git(root, 'add', '.husky'); + git(root, '-c', 'core.hooksPath=/dev/null', 'commit', '-qm', 'base'); + const base = git(root, 'rev-parse', 'HEAD'); + git(root, 'worktree', 'add', '-q', '--detach', wt, base); + writeFileSync(join(wt, 'note.txt'), 'changed\n'); + git(wt, 'add', 'note.txt'); + return { root, wt, base }; +} + +function runCommit(root: string, wt: string, base: string, hideHookProof = false) { + const telemetry = join(root, 'telemetry', 'gate-events.jsonl'); + const script = ` +set -e +. "$1" +if [ "$6" = hide-proof ]; then + grep() { + if [ "$1" = -qF ] && [[ "$2" = devkit-ship-hook-start:* ]]; then return 1; fi + command grep "$@" + } +fi +export DEVKIT_GATE_EVENTS="$2" +export DEVKIT_SHIP_BASE_SHA="$3" +export DEVKIT_SHIP_ID=sc1537-test +export SHIP_COMMIT_TIMEOUT=10 +commit_with_gate_capture "$4" "$5" feat/sc1537 "test title" "test body" +`; + return spawnSync( + '/bin/bash', + [ + '-c', + script, + 'ship-hook-test', + helper, + telemetry, + base, + wt, + root, + hideHookProof ? 'hide-proof' : '', + ], + { + cwd: root, + env: gitEnv, + encoding: 'utf8', + }, + ); +} + +afterEach(() => { + for (const path of created.splice(0).reverse()) { + if (existsSync(path)) rmSync(path, { recursive: true, force: true }); + } +}); + +describe('commit_with_gate_capture — executable hook proof', () => { + it('runs the projected Husky hook when core.hooksPath is unset and captures proof', () => { + const { root, wt, base } = fixture(true); + + expect(() => git(root, 'config', '--get', 'core.hooksPath')).toThrow(); + const result = runCommit(root, wt, base); + + expect(result.status, result.stderr).toBe(0); + expect(result.stderr).toContain('devkit-ship-hook-start:sc1537-test'); + expect(result.stderr).toContain('REAL_PRE_COMMIT_RAN'); + expect(result.stderr).toMatch(/REAL_COMMIT_MSG_RAN:.*COMMIT_EDITMSG/); + expect(result.stderr).toContain('pre-commit gates ran in the ship worktree'); + const log = join(root, '.devkit/last-ship-gates-feat-sc1537.log'); + expect(existsSync(log)).toBe(true); + expect(git(wt, 'rev-parse', 'HEAD')).not.toBe(base); + expect(readdirSync(wt).some((name) => name.startsWith('.devkit-ship-hooks.'))).toBe(false); + }); + + it('fails closed before committing when no executable pre-commit hook resolves', () => { + const { root, wt, base } = fixture(false); + + const result = runCommit(root, wt, base); + + expect(result.status).not.toBe(0); + expect(result.stderr).toMatch(/no executable pre-commit hook/); + expect(result.stderr).toMatch(/gates must not fail open/); + expect(result.stderr).not.toMatch(/pre-commit gates ran/); + expect(git(wt, 'rev-parse', 'HEAD')).toBe(base); + const events = readFileSync(join(root, 'telemetry/gate-events.jsonl'), 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events.at(-1)).toMatchObject({ + type: 'ship_result', + exit_code: 1, + blocked_gate: 'hook_setup', + }); + }); + + it('rewinds and records a failed result when the execution proof is not observed', () => { + const { root, wt, base } = fixture(true); + + const result = runCommit(root, wt, base, true); + + expect(result.status).not.toBe(0); + expect(result.stderr).toMatch(/NO pre-commit execution proof/); + expect(result.stderr).not.toMatch(/pre-commit gates ran/); + expect(git(wt, 'rev-parse', 'HEAD')).toBe(base); + const events = readFileSync(join(root, 'telemetry/gate-events.jsonl'), 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(events.at(-1)).toMatchObject({ + type: 'ship_result', + exit_code: 1, + blocked_gate: 'hook_proof', + }); + }); +}); diff --git a/cli/__tests__/review-private-dependencies.test.mts b/cli/__tests__/review-private-dependencies.test.mts index edd4896e..c6047486 100644 --- a/cli/__tests__/review-private-dependencies.test.mts +++ b/cli/__tests__/review-private-dependencies.test.mts @@ -384,6 +384,7 @@ describe('private review dependency runtime', () => { it('preserves the shipping dependency link and refreshes packaged reviewer assets', () => { const { source, destination } = fixture('prepare-shipping'); write(source, '.husky/_/pre-commit', 'runner\n'); + chmodSync(join(source, '.husky/_/pre-commit'), 0o755); write(source, 'node_modules/pkg/index.js'); write(source, '.claude/agents/api-security-reviewer.md', 'stale agent\n'); write(source, '.claude/skills/api-security/scripts/checklist.mjs', 'stale checklist\n'); @@ -411,6 +412,7 @@ describe('private review dependency runtime', () => { it('projects packaged reviewer assets when the ship caller has no .claude projection', () => { const { source, destination } = fixture('prepare-ship-without-projection'); write(source, '.husky/_/pre-commit', 'runner\n'); + chmodSync(join(source, '.husky/_/pre-commit'), 0o755); write(source, 'node_modules/pkg/index.js'); const result = prepare(source, destination, 'shipping'); diff --git a/cli/lib/ship/commit-with-gate-capture.sh b/cli/lib/ship/commit-with-gate-capture.sh index 440a9deb..e895ffe9 100644 --- a/cli/lib/ship/commit-with-gate-capture.sh +++ b/cli/lib/ship/commit-with-gate-capture.sh @@ -45,6 +45,7 @@ commit_with_gate_capture() { local progress="$root/.devkit/review-progress-${br//\//-}.json" . "$(dirname "${BASH_SOURCE[0]}")/run-gates-with-capture.sh" . "$(dirname "${BASH_SOURCE[0]}")/telemetry.sh" + . "$(dirname "${BASH_SOURCE[0]}")/prepare-gate-worktree.sh" # Both callers already source this, but the object probe below is this function's own evidence — # don't inherit it by luck of call order. . "$(dirname "${BASH_SOURCE[0]}")/assert-staged-set.sh" @@ -77,22 +78,8 @@ commit_with_gate_capture() { local ship_log="$ship_logs_dir/${ship_id_safe}.log" mkdir -p "$ship_logs_dir" 2>/dev/null || true - # Overlay mode: force core.hooksPath at the .devkit overlay hook so the FULL gate chain runs in the - # ship worktree in EVERY state. A plain `git commit` otherwise honours the husky-reclaimed - # core.hooksPath=.husky/_ and runs only the team's committed hook — the overlay chain (devkit's - # gates) silently no-ops (the bug this fixes). ship-branch/reship linked .devkit in, so the relative - # path resolves to $wt/.devkit/hooks via the symlink; the overlay hook then execs the repo's own - # committed hook too. Non-overlay repos have no such file → empty array → unchanged behaviour. - # gc.auto=0: this commit is the one ship-owned git call that can trip auto-gc, and it fires at the - # END of a multi-minute gate chain in a repo that may hold dozens of worktrees. Auto-gc cannot - # delete a minutes-old object under git's default pruneExpire, so this is hygiene rather than the - # sc-1420 fix — it just keeps ship from starting repository maintenance at its most fragile moment. - # APPEND the overlay flag below; assigning here would confine gc.auto=0 to overlay installs only. - local hookcfg=(-c gc.auto=0) - [ -x "$root/.devkit/hooks/pre-commit" ] && hookcfg+=(-c core.hooksPath=.devkit/hooks) - - # Ship attempt telemetry — one line per commit attempt; count-per-branch = the number of times the - # root agent re-shipped after a gate blocked it. mode ('ship'|'reship') is set by the caller. + # Start the attempt before hook resolution so a fail-closed setup error still has a terminal + # ship_result row instead of disappearing from telemetry. local dur_start; dur_start=$(date +%s) printf '{"type":"ship_attempt","ship_id":"%s","repo":"%s","branch":"%s","devkit_version":"%s","mode":"%s","log_path":"%s","ts":"%s"}\n' \ "$(devkit_json_escape "$DEVKIT_SHIP_ID")" "$(devkit_json_escape "$repo_name")" "$(devkit_json_escape "$br")" \ @@ -100,6 +87,78 @@ commit_with_gate_capture() { "$(devkit_json_escape "${DEVKIT_SHIP_MODE:-ship}")" "$(devkit_json_escape "$ship_log")" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ >> "$DEVKIT_GATE_EVENTS" 2>/dev/null || true + # $log is a REUSED per-branch path ("last-ship-gates-*"), so this attempt starts from empty even + # when hook setup fails before run_gates_with_capture gets a chance to own the log. + mkdir -p "$(dirname "$log")" 2>/dev/null || true + : > "$log" 2>/dev/null || true + local rc=0 hook_setup_failed=0 hook_setup_error="" ship_hook_dir="" + + # Resolve the hook ship intends to run, then put ship-owned wrappers in front of the real hook + # directory. The pre-commit wrapper emits an attempt-specific proof marker; every wrapper directly + # execs its real counterpart with the original args/stdin. Direct execution is load-bearing for + # Husky's generated shims: they locate `.husky/` from their own $0, so symlinking those shims + # into the private directory would silently no-op sibling hooks such as commit-msg. This closes two + # fail-open paths at once: + # - core.hooksPath is unset in a clean clone, even though prepare-gate-worktree projected the + # package-mode .husky/_ runner into the disposable worktree; and + # - git commit returns zero without proving that any hook actually ran. + # Hook resolution is shared with prepare_gate_worktree so projection and execution cannot drift. + local real_hooks_dir real_pre_commit + real_pre_commit=$(gate_worktree_pre_commit "$wt" "$root") + real_hooks_dir=${real_pre_commit%/pre-commit} + if [ -z "$real_hooks_dir" ] || [ ! -x "$real_pre_commit" ]; then + hook_setup_error="ship: no executable pre-commit hook for the ship worktree (resolved: ${real_pre_commit:-none}) — gates must not fail open" + hook_setup_failed=1 + rc=1 + fi + + local ship_hook_rel="" ship_hook_marker source_hook source_name + ship_hook_marker="devkit-ship-hook-start:$ship_id_safe" + if [ "$hook_setup_failed" -eq 0 ]; then + ship_hook_dir=$(mktemp -d "$wt/.devkit-ship-hooks.XXXXXX") || { + hook_setup_error="ship: could not create the private hook wrapper — gates must not fail open" + hook_setup_failed=1 + rc=1 + } + fi + if [ "$hook_setup_failed" -eq 0 ]; then + ship_hook_rel=${ship_hook_dir#"$wt/"} + for source_hook in "$real_hooks_dir"/*; do + [ -x "$source_hook" ] || continue + source_name=${source_hook##*/} + if ! (umask 077; cat > "$ship_hook_dir/$source_name" <<'SHIP_HOOK_WRAPPER' +#!/bin/sh +hook_name=${0##*/} +if [ "$hook_name" = pre-commit ]; then + printf '%s\n' "$DEVKIT_SHIP_HOOK_MARKER" >&2 +fi +exec "$DEVKIT_SHIP_REAL_HOOKS_DIR/$hook_name" "$@" +SHIP_HOOK_WRAPPER + chmod 700 "$ship_hook_dir/$source_name"); then + hook_setup_error="ship: could not project the real hook chain into the private wrapper — gates must not fail open" + hook_setup_failed=1 + rc=1 + break + fi + done + fi + if [ "$hook_setup_failed" -eq 0 ] && [ ! -x "$ship_hook_dir/pre-commit" ]; then + hook_setup_error="ship: private hook projection omitted pre-commit — gates must not fail open" + hook_setup_failed=1 + rc=1 + fi + if [ "$hook_setup_failed" -eq 0 ]; then + export DEVKIT_SHIP_HOOK_MARKER="$ship_hook_marker" + export DEVKIT_SHIP_REAL_HOOKS_DIR="$real_hooks_dir" + fi + + # gc.auto=0: this commit is the one ship-owned git call that can trip auto-gc, and it fires at the + # END of a multi-minute gate chain in a repo that may hold dozens of worktrees. Auto-gc cannot + # delete a minutes-old object under git's default pruneExpire, so this is hygiene rather than the + # sc-1420 fix — it just keeps ship from starting repository maintenance at its most fragile moment. + # APPEND the overlay flag below; assigning here would confine gc.auto=0 to overlay installs only. + local hookcfg=(-c gc.auto=0 -c core.hooksPath="$ship_hook_rel") + # sc-1442: the composed message exists BEFORE `git commit` runs — hand it to the pre-commit # reviewers as ADVISORY intent via a temp file (NEVER .git/COMMIT_EDITMSG: at pre-commit that # holds the PREVIOUS commit's message). Best-effort throughout: any failure degrades to the @@ -116,14 +175,17 @@ commit_with_gate_capture() { fi fi - local rc=0 - # $log is a REUSED per-branch path ("last-ship-gates-*"), so this attempt must start from empty. - # The capture appends now (it must not erase devkit review's preflight progress), which makes - # clearing the caller's job — without this every ship on a branch would pile onto the last one. - mkdir -p "$(dirname "$log")" 2>/dev/null || true - : > "$log" 2>/dev/null || true - DEVKIT_GATE_ARCHIVE_LOG="$ship_log" run_gates_with_capture "$wt" "$root" ship "$log" "$progress" -- \ - git -C "$wt" ${hookcfg[@]+"${hookcfg[@]}"} commit -m "$title" -m "$body" || rc=$? + if [ "$hook_setup_failed" -eq 1 ]; then + printf '%s\n' "$hook_setup_error" | tee -a "$log" "$ship_log" >&2 + else + DEVKIT_GATE_ARCHIVE_LOG="$ship_log" run_gates_with_capture "$wt" "$root" ship "$log" "$progress" -- \ + git -C "$wt" ${hookcfg[@]+"${hookcfg[@]}"} commit -m "$title" -m "$body" || rc=$? + fi + + local ship_hook_proved=0 + grep -qF "$ship_hook_marker" "$log" 2>/dev/null && ship_hook_proved=1 + [ -z "$ship_hook_dir" ] || rm -rf -- "$ship_hook_dir" + unset DEVKIT_SHIP_HOOK_MARKER DEVKIT_SHIP_REAL_HOOKS_DIR # sc-1442 cleanup — sits ABOVE both return sites, so every exit path is already clean. A Ctrl-C # mid-gate can leak the mode-600 temp file; accepted — its content is the message the author is @@ -131,6 +193,32 @@ commit_with_gate_capture() { if [ -n "$msgf" ]; then rm -f -- "$msgf" 2>/dev/null || true; fi unset DEVKIT_COMMIT_MSG_FILE + # A zero-exit commit is provisional until ship proves its wrapper ran and, for sentinel-aware + # overlays, that the real gate chain emitted output. Rewind before telemetry so the terminal row + # records the actual failed ship rather than a success that callers will never publish. + local ship_abort_reported=0 blocked_override="" + if [ "$rc" -eq 0 ] && [ "$ship_hook_proved" -ne 1 ]; then + git -C "$wt" reset --soft HEAD~1 2>/dev/null || true + { + echo "⚠️ ship: NO pre-commit execution proof was captured — ship aborted; nothing pushed" + echo " Expected marker: $ship_hook_marker. Full log: $log" + } >&2 + rc=1 + blocked_override='"hook_proof"' + ship_abort_reported=1 + elif [ "$rc" -eq 0 ] && [ -x "$root/.devkit/hooks/pre-commit" ] \ + && grep -q 'devkit-gates: chain start' "$root/.devkit/hooks/pre-commit" \ + && ! grep -q 'devkit-gates: chain start' "$log"; then + git -C "$wt" reset --soft HEAD~1 2>/dev/null || true + { + echo "⚠️ ship: NO gate output captured — overlay hook chain appears to have no-op'd" + echo " (expected .devkit/hooks/pre-commit to run). Ship aborted; nothing pushed. Log: $log" + } >&2 + rc=1 + blocked_override='"overlay_no_output"' + ship_abort_reported=1 + fi + # Did OUR outer `git commit` die on its own HEAD finalize, or did a GATE merely PRINT the same git # error? The captured log is a COMBINED stream (`2>&1 | tee` above folds hook output in), so the two # are textually indistinguishable — and devkit's own suite emits this string deliberately, so a gate @@ -165,7 +253,9 @@ commit_with_gate_capture() { # decisions → review, and each hook step is `|| exit`, so exactly one gate blocks; grep in that # order attributes it. qavis is advisory (never blocks a ship) so it is not a blocked_gate value. local blocked_json timed_out - if [ "$rc" -eq 0 ]; then blocked_json=null; timed_out=false + if [ -n "$blocked_override" ]; then blocked_json=$blocked_override; timed_out=false + elif [ "$hook_setup_failed" -eq 1 ]; then blocked_json='"hook_setup"'; timed_out=false + elif [ "$rc" -eq 0 ]; then blocked_json=null; timed_out=false elif [ "$rc" -eq 124 ] || [ "$rc" -eq 137 ]; then blocked_json='"timeout"'; timed_out=true # NOT a blocked gate: every gate PASSED and `git commit` then died on its finalize ref-update # because something moved the ship worktree's HEAD mid-commit. Must be tested BEFORE the gate @@ -189,27 +279,9 @@ commit_with_gate_capture() { if [ "$rc" -eq 124 ] || [ "$rc" -eq 137 ]; then : # run_gates_with_capture already emitted the attributed timeout + retry guidance + elif [ "$ship_abort_reported" -eq 1 ]; then + : # Proof/sentinel failure was already reported before terminal telemetry was emitted. elif [ "$rc" -eq 0 ]; then - # Honest banner: a zero-exit commit is NOT proof the gates ran. In overlay mode the chain can - # silently no-op (see the core.hooksPath forcing above); if that ever happens the log holds no - # gate output and reporting "✓ gates ran" would be a lie. Gate enforcement on the overlay hook - # FILE emitting the sentinel — `devkit update` re-pins the package but does NOT regenerate the - # git-ignored on-disk hook, so a consumer on a new ship.sh + an old sentinel-less hook still - # runs its gates correctly; holding it to a sentinel it can't emit would falsely abort a fully - # gated ship. Only sentinel-emitting hooks are held to fail-closed enforcement. - if [ -x "$root/.devkit/hooks/pre-commit" ] \ - && grep -q 'devkit-gates: chain start' "$root/.devkit/hooks/pre-commit" \ - && ! grep -q 'devkit-gates: chain start' "$log"; then - # The commit already succeeded (rc=0) but the chain produced no sentinel → undo it so the - # caller's cleanup reclaims the branch (else tip≠BASE keeps it, blocking a retry). HEAD~1==BASE - # (branch created at BASE, exactly one commit); the worktree is discarded next, so --soft suffices. - git -C "$wt" reset --soft HEAD~1 2>/dev/null || true - { - echo "⚠️ ship: NO gate output captured — overlay hook chain appears to have no-op'd" - echo " (expected .devkit/hooks/pre-commit to run). Ship aborted; nothing pushed. Log: $log" - } >&2 - return 1 - fi { echo "✓ pre-commit gates ran in the ship worktree — full output: $log" # Was: "(e.g. coverage is NOT gated in the ship worktree)" — false since prepare-gate-worktree.sh diff --git a/cli/lib/ship/prepare-gate-worktree.sh b/cli/lib/ship/prepare-gate-worktree.sh index 12195395..e55a7491 100644 --- a/cli/lib/ship/prepare-gate-worktree.sh +++ b/cli/lib/ship/prepare-gate-worktree.sh @@ -220,13 +220,31 @@ gate_node_modules_source() { return 2 } -# The hook git will ACTUALLY run in the ephemeral worktree, or '' when nothing is configured. -# Resolved, never hardcoded: husky points core.hooksPath at `.husky/_`, an overlay install points it -# at `.devkit/hooks` (overlay.mts), and it may be unset entirely. +# The pre-commit hook ship will run in the ephemeral worktree. This is the single resolver shared by +# preparation and commit: overlay wins when projected, an explicit core.hooksPath is honoured, an +# unset path falls back to the projected package-mode Husky runner, and Git's default hooks directory +# is the final candidate. Returning a non-executable candidate is intentional — callers fail closed +# with a useful path instead of treating absence as an opt-out. gate_worktree_pre_commit() { - local wt=$1 hooks_path + local wt=$1 root=${2:-} hooks_path default_hooks_dir + if [ -n "$root" ] && [ -x "$root/.devkit/hooks/pre-commit" ]; then + printf '%s\n' "$wt/.devkit/hooks/pre-commit" + return 0 + fi hooks_path=$(git -C "$wt" config --get core.hooksPath 2>/dev/null) || hooks_path='' - [ -n "$hooks_path" ] || return 0 + if [ -z "$hooks_path" ]; then + if [ -e "$wt/.husky/_/pre-commit" ] || [ -L "$wt/.husky/_/pre-commit" ]; then + printf '%s\n' "$wt/.husky/_/pre-commit" + return 0 + fi + default_hooks_dir=$(git -C "$wt" rev-parse --git-path hooks 2>/dev/null) || default_hooks_dir='' + [ -n "$default_hooks_dir" ] || return 0 + case $default_hooks_dir in + /*) printf '%s\n' "$default_hooks_dir/pre-commit" ;; + *) printf '%s\n' "$wt/$default_hooks_dir/pre-commit" ;; + esac + return 0 + fi case $hooks_path in /*) printf '%s\n' "$hooks_path/pre-commit" ;; *) printf '%s\n' "$wt/$hooks_path/pre-commit" ;; @@ -315,12 +333,11 @@ prepare_gate_worktree() { echo " ↳ $purpose: linked $d ← $source" >&2 done - # Postcondition, not a resolution question: a `.husky/_` that is populated but carries no pre-commit - # shim passes every test above, and the ship then commits and opens a PR with ZERO gates — the exact - # fail-open the .husky/_ preflight exists to prevent. Skipped when no hooksPath is configured, which - # leaves that case exactly as it was. + # Postcondition, not a second resolution question: a projected hook directory that carries no + # executable pre-commit shim must never reach `git commit`. The shared resolver includes the + # hooksPath-unset package-mode fallback, closing the clean-clone fail-open before reviewers run. local pre_commit - pre_commit=$(gate_worktree_pre_commit "$wt") + pre_commit=$(gate_worktree_pre_commit "$wt" "$root") if [ -n "$pre_commit" ] && [ ! -x "$pre_commit" ]; then echo "no executable pre-commit hook at $pre_commit — the $purpose worktree would commit with NO gate chain (gates must not fail open)" >&2 return 1 diff --git a/dist/cli/lib/ship/commit-with-gate-capture.sh b/dist/cli/lib/ship/commit-with-gate-capture.sh index 440a9deb..e895ffe9 100644 --- a/dist/cli/lib/ship/commit-with-gate-capture.sh +++ b/dist/cli/lib/ship/commit-with-gate-capture.sh @@ -45,6 +45,7 @@ commit_with_gate_capture() { local progress="$root/.devkit/review-progress-${br//\//-}.json" . "$(dirname "${BASH_SOURCE[0]}")/run-gates-with-capture.sh" . "$(dirname "${BASH_SOURCE[0]}")/telemetry.sh" + . "$(dirname "${BASH_SOURCE[0]}")/prepare-gate-worktree.sh" # Both callers already source this, but the object probe below is this function's own evidence — # don't inherit it by luck of call order. . "$(dirname "${BASH_SOURCE[0]}")/assert-staged-set.sh" @@ -77,22 +78,8 @@ commit_with_gate_capture() { local ship_log="$ship_logs_dir/${ship_id_safe}.log" mkdir -p "$ship_logs_dir" 2>/dev/null || true - # Overlay mode: force core.hooksPath at the .devkit overlay hook so the FULL gate chain runs in the - # ship worktree in EVERY state. A plain `git commit` otherwise honours the husky-reclaimed - # core.hooksPath=.husky/_ and runs only the team's committed hook — the overlay chain (devkit's - # gates) silently no-ops (the bug this fixes). ship-branch/reship linked .devkit in, so the relative - # path resolves to $wt/.devkit/hooks via the symlink; the overlay hook then execs the repo's own - # committed hook too. Non-overlay repos have no such file → empty array → unchanged behaviour. - # gc.auto=0: this commit is the one ship-owned git call that can trip auto-gc, and it fires at the - # END of a multi-minute gate chain in a repo that may hold dozens of worktrees. Auto-gc cannot - # delete a minutes-old object under git's default pruneExpire, so this is hygiene rather than the - # sc-1420 fix — it just keeps ship from starting repository maintenance at its most fragile moment. - # APPEND the overlay flag below; assigning here would confine gc.auto=0 to overlay installs only. - local hookcfg=(-c gc.auto=0) - [ -x "$root/.devkit/hooks/pre-commit" ] && hookcfg+=(-c core.hooksPath=.devkit/hooks) - - # Ship attempt telemetry — one line per commit attempt; count-per-branch = the number of times the - # root agent re-shipped after a gate blocked it. mode ('ship'|'reship') is set by the caller. + # Start the attempt before hook resolution so a fail-closed setup error still has a terminal + # ship_result row instead of disappearing from telemetry. local dur_start; dur_start=$(date +%s) printf '{"type":"ship_attempt","ship_id":"%s","repo":"%s","branch":"%s","devkit_version":"%s","mode":"%s","log_path":"%s","ts":"%s"}\n' \ "$(devkit_json_escape "$DEVKIT_SHIP_ID")" "$(devkit_json_escape "$repo_name")" "$(devkit_json_escape "$br")" \ @@ -100,6 +87,78 @@ commit_with_gate_capture() { "$(devkit_json_escape "${DEVKIT_SHIP_MODE:-ship}")" "$(devkit_json_escape "$ship_log")" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ >> "$DEVKIT_GATE_EVENTS" 2>/dev/null || true + # $log is a REUSED per-branch path ("last-ship-gates-*"), so this attempt starts from empty even + # when hook setup fails before run_gates_with_capture gets a chance to own the log. + mkdir -p "$(dirname "$log")" 2>/dev/null || true + : > "$log" 2>/dev/null || true + local rc=0 hook_setup_failed=0 hook_setup_error="" ship_hook_dir="" + + # Resolve the hook ship intends to run, then put ship-owned wrappers in front of the real hook + # directory. The pre-commit wrapper emits an attempt-specific proof marker; every wrapper directly + # execs its real counterpart with the original args/stdin. Direct execution is load-bearing for + # Husky's generated shims: they locate `.husky/` from their own $0, so symlinking those shims + # into the private directory would silently no-op sibling hooks such as commit-msg. This closes two + # fail-open paths at once: + # - core.hooksPath is unset in a clean clone, even though prepare-gate-worktree projected the + # package-mode .husky/_ runner into the disposable worktree; and + # - git commit returns zero without proving that any hook actually ran. + # Hook resolution is shared with prepare_gate_worktree so projection and execution cannot drift. + local real_hooks_dir real_pre_commit + real_pre_commit=$(gate_worktree_pre_commit "$wt" "$root") + real_hooks_dir=${real_pre_commit%/pre-commit} + if [ -z "$real_hooks_dir" ] || [ ! -x "$real_pre_commit" ]; then + hook_setup_error="ship: no executable pre-commit hook for the ship worktree (resolved: ${real_pre_commit:-none}) — gates must not fail open" + hook_setup_failed=1 + rc=1 + fi + + local ship_hook_rel="" ship_hook_marker source_hook source_name + ship_hook_marker="devkit-ship-hook-start:$ship_id_safe" + if [ "$hook_setup_failed" -eq 0 ]; then + ship_hook_dir=$(mktemp -d "$wt/.devkit-ship-hooks.XXXXXX") || { + hook_setup_error="ship: could not create the private hook wrapper — gates must not fail open" + hook_setup_failed=1 + rc=1 + } + fi + if [ "$hook_setup_failed" -eq 0 ]; then + ship_hook_rel=${ship_hook_dir#"$wt/"} + for source_hook in "$real_hooks_dir"/*; do + [ -x "$source_hook" ] || continue + source_name=${source_hook##*/} + if ! (umask 077; cat > "$ship_hook_dir/$source_name" <<'SHIP_HOOK_WRAPPER' +#!/bin/sh +hook_name=${0##*/} +if [ "$hook_name" = pre-commit ]; then + printf '%s\n' "$DEVKIT_SHIP_HOOK_MARKER" >&2 +fi +exec "$DEVKIT_SHIP_REAL_HOOKS_DIR/$hook_name" "$@" +SHIP_HOOK_WRAPPER + chmod 700 "$ship_hook_dir/$source_name"); then + hook_setup_error="ship: could not project the real hook chain into the private wrapper — gates must not fail open" + hook_setup_failed=1 + rc=1 + break + fi + done + fi + if [ "$hook_setup_failed" -eq 0 ] && [ ! -x "$ship_hook_dir/pre-commit" ]; then + hook_setup_error="ship: private hook projection omitted pre-commit — gates must not fail open" + hook_setup_failed=1 + rc=1 + fi + if [ "$hook_setup_failed" -eq 0 ]; then + export DEVKIT_SHIP_HOOK_MARKER="$ship_hook_marker" + export DEVKIT_SHIP_REAL_HOOKS_DIR="$real_hooks_dir" + fi + + # gc.auto=0: this commit is the one ship-owned git call that can trip auto-gc, and it fires at the + # END of a multi-minute gate chain in a repo that may hold dozens of worktrees. Auto-gc cannot + # delete a minutes-old object under git's default pruneExpire, so this is hygiene rather than the + # sc-1420 fix — it just keeps ship from starting repository maintenance at its most fragile moment. + # APPEND the overlay flag below; assigning here would confine gc.auto=0 to overlay installs only. + local hookcfg=(-c gc.auto=0 -c core.hooksPath="$ship_hook_rel") + # sc-1442: the composed message exists BEFORE `git commit` runs — hand it to the pre-commit # reviewers as ADVISORY intent via a temp file (NEVER .git/COMMIT_EDITMSG: at pre-commit that # holds the PREVIOUS commit's message). Best-effort throughout: any failure degrades to the @@ -116,14 +175,17 @@ commit_with_gate_capture() { fi fi - local rc=0 - # $log is a REUSED per-branch path ("last-ship-gates-*"), so this attempt must start from empty. - # The capture appends now (it must not erase devkit review's preflight progress), which makes - # clearing the caller's job — without this every ship on a branch would pile onto the last one. - mkdir -p "$(dirname "$log")" 2>/dev/null || true - : > "$log" 2>/dev/null || true - DEVKIT_GATE_ARCHIVE_LOG="$ship_log" run_gates_with_capture "$wt" "$root" ship "$log" "$progress" -- \ - git -C "$wt" ${hookcfg[@]+"${hookcfg[@]}"} commit -m "$title" -m "$body" || rc=$? + if [ "$hook_setup_failed" -eq 1 ]; then + printf '%s\n' "$hook_setup_error" | tee -a "$log" "$ship_log" >&2 + else + DEVKIT_GATE_ARCHIVE_LOG="$ship_log" run_gates_with_capture "$wt" "$root" ship "$log" "$progress" -- \ + git -C "$wt" ${hookcfg[@]+"${hookcfg[@]}"} commit -m "$title" -m "$body" || rc=$? + fi + + local ship_hook_proved=0 + grep -qF "$ship_hook_marker" "$log" 2>/dev/null && ship_hook_proved=1 + [ -z "$ship_hook_dir" ] || rm -rf -- "$ship_hook_dir" + unset DEVKIT_SHIP_HOOK_MARKER DEVKIT_SHIP_REAL_HOOKS_DIR # sc-1442 cleanup — sits ABOVE both return sites, so every exit path is already clean. A Ctrl-C # mid-gate can leak the mode-600 temp file; accepted — its content is the message the author is @@ -131,6 +193,32 @@ commit_with_gate_capture() { if [ -n "$msgf" ]; then rm -f -- "$msgf" 2>/dev/null || true; fi unset DEVKIT_COMMIT_MSG_FILE + # A zero-exit commit is provisional until ship proves its wrapper ran and, for sentinel-aware + # overlays, that the real gate chain emitted output. Rewind before telemetry so the terminal row + # records the actual failed ship rather than a success that callers will never publish. + local ship_abort_reported=0 blocked_override="" + if [ "$rc" -eq 0 ] && [ "$ship_hook_proved" -ne 1 ]; then + git -C "$wt" reset --soft HEAD~1 2>/dev/null || true + { + echo "⚠️ ship: NO pre-commit execution proof was captured — ship aborted; nothing pushed" + echo " Expected marker: $ship_hook_marker. Full log: $log" + } >&2 + rc=1 + blocked_override='"hook_proof"' + ship_abort_reported=1 + elif [ "$rc" -eq 0 ] && [ -x "$root/.devkit/hooks/pre-commit" ] \ + && grep -q 'devkit-gates: chain start' "$root/.devkit/hooks/pre-commit" \ + && ! grep -q 'devkit-gates: chain start' "$log"; then + git -C "$wt" reset --soft HEAD~1 2>/dev/null || true + { + echo "⚠️ ship: NO gate output captured — overlay hook chain appears to have no-op'd" + echo " (expected .devkit/hooks/pre-commit to run). Ship aborted; nothing pushed. Log: $log" + } >&2 + rc=1 + blocked_override='"overlay_no_output"' + ship_abort_reported=1 + fi + # Did OUR outer `git commit` die on its own HEAD finalize, or did a GATE merely PRINT the same git # error? The captured log is a COMBINED stream (`2>&1 | tee` above folds hook output in), so the two # are textually indistinguishable — and devkit's own suite emits this string deliberately, so a gate @@ -165,7 +253,9 @@ commit_with_gate_capture() { # decisions → review, and each hook step is `|| exit`, so exactly one gate blocks; grep in that # order attributes it. qavis is advisory (never blocks a ship) so it is not a blocked_gate value. local blocked_json timed_out - if [ "$rc" -eq 0 ]; then blocked_json=null; timed_out=false + if [ -n "$blocked_override" ]; then blocked_json=$blocked_override; timed_out=false + elif [ "$hook_setup_failed" -eq 1 ]; then blocked_json='"hook_setup"'; timed_out=false + elif [ "$rc" -eq 0 ]; then blocked_json=null; timed_out=false elif [ "$rc" -eq 124 ] || [ "$rc" -eq 137 ]; then blocked_json='"timeout"'; timed_out=true # NOT a blocked gate: every gate PASSED and `git commit` then died on its finalize ref-update # because something moved the ship worktree's HEAD mid-commit. Must be tested BEFORE the gate @@ -189,27 +279,9 @@ commit_with_gate_capture() { if [ "$rc" -eq 124 ] || [ "$rc" -eq 137 ]; then : # run_gates_with_capture already emitted the attributed timeout + retry guidance + elif [ "$ship_abort_reported" -eq 1 ]; then + : # Proof/sentinel failure was already reported before terminal telemetry was emitted. elif [ "$rc" -eq 0 ]; then - # Honest banner: a zero-exit commit is NOT proof the gates ran. In overlay mode the chain can - # silently no-op (see the core.hooksPath forcing above); if that ever happens the log holds no - # gate output and reporting "✓ gates ran" would be a lie. Gate enforcement on the overlay hook - # FILE emitting the sentinel — `devkit update` re-pins the package but does NOT regenerate the - # git-ignored on-disk hook, so a consumer on a new ship.sh + an old sentinel-less hook still - # runs its gates correctly; holding it to a sentinel it can't emit would falsely abort a fully - # gated ship. Only sentinel-emitting hooks are held to fail-closed enforcement. - if [ -x "$root/.devkit/hooks/pre-commit" ] \ - && grep -q 'devkit-gates: chain start' "$root/.devkit/hooks/pre-commit" \ - && ! grep -q 'devkit-gates: chain start' "$log"; then - # The commit already succeeded (rc=0) but the chain produced no sentinel → undo it so the - # caller's cleanup reclaims the branch (else tip≠BASE keeps it, blocking a retry). HEAD~1==BASE - # (branch created at BASE, exactly one commit); the worktree is discarded next, so --soft suffices. - git -C "$wt" reset --soft HEAD~1 2>/dev/null || true - { - echo "⚠️ ship: NO gate output captured — overlay hook chain appears to have no-op'd" - echo " (expected .devkit/hooks/pre-commit to run). Ship aborted; nothing pushed. Log: $log" - } >&2 - return 1 - fi { echo "✓ pre-commit gates ran in the ship worktree — full output: $log" # Was: "(e.g. coverage is NOT gated in the ship worktree)" — false since prepare-gate-worktree.sh diff --git a/dist/cli/lib/ship/prepare-gate-worktree.sh b/dist/cli/lib/ship/prepare-gate-worktree.sh index 12195395..e55a7491 100644 --- a/dist/cli/lib/ship/prepare-gate-worktree.sh +++ b/dist/cli/lib/ship/prepare-gate-worktree.sh @@ -220,13 +220,31 @@ gate_node_modules_source() { return 2 } -# The hook git will ACTUALLY run in the ephemeral worktree, or '' when nothing is configured. -# Resolved, never hardcoded: husky points core.hooksPath at `.husky/_`, an overlay install points it -# at `.devkit/hooks` (overlay.mts), and it may be unset entirely. +# The pre-commit hook ship will run in the ephemeral worktree. This is the single resolver shared by +# preparation and commit: overlay wins when projected, an explicit core.hooksPath is honoured, an +# unset path falls back to the projected package-mode Husky runner, and Git's default hooks directory +# is the final candidate. Returning a non-executable candidate is intentional — callers fail closed +# with a useful path instead of treating absence as an opt-out. gate_worktree_pre_commit() { - local wt=$1 hooks_path + local wt=$1 root=${2:-} hooks_path default_hooks_dir + if [ -n "$root" ] && [ -x "$root/.devkit/hooks/pre-commit" ]; then + printf '%s\n' "$wt/.devkit/hooks/pre-commit" + return 0 + fi hooks_path=$(git -C "$wt" config --get core.hooksPath 2>/dev/null) || hooks_path='' - [ -n "$hooks_path" ] || return 0 + if [ -z "$hooks_path" ]; then + if [ -e "$wt/.husky/_/pre-commit" ] || [ -L "$wt/.husky/_/pre-commit" ]; then + printf '%s\n' "$wt/.husky/_/pre-commit" + return 0 + fi + default_hooks_dir=$(git -C "$wt" rev-parse --git-path hooks 2>/dev/null) || default_hooks_dir='' + [ -n "$default_hooks_dir" ] || return 0 + case $default_hooks_dir in + /*) printf '%s\n' "$default_hooks_dir/pre-commit" ;; + *) printf '%s\n' "$wt/$default_hooks_dir/pre-commit" ;; + esac + return 0 + fi case $hooks_path in /*) printf '%s\n' "$hooks_path/pre-commit" ;; *) printf '%s\n' "$wt/$hooks_path/pre-commit" ;; @@ -315,12 +333,11 @@ prepare_gate_worktree() { echo " ↳ $purpose: linked $d ← $source" >&2 done - # Postcondition, not a resolution question: a `.husky/_` that is populated but carries no pre-commit - # shim passes every test above, and the ship then commits and opens a PR with ZERO gates — the exact - # fail-open the .husky/_ preflight exists to prevent. Skipped when no hooksPath is configured, which - # leaves that case exactly as it was. + # Postcondition, not a second resolution question: a projected hook directory that carries no + # executable pre-commit shim must never reach `git commit`. The shared resolver includes the + # hooksPath-unset package-mode fallback, closing the clean-clone fail-open before reviewers run. local pre_commit - pre_commit=$(gate_worktree_pre_commit "$wt") + pre_commit=$(gate_worktree_pre_commit "$wt" "$root") if [ -n "$pre_commit" ] && [ ! -x "$pre_commit" ]; then echo "no executable pre-commit hook at $pre_commit — the $purpose worktree would commit with NO gate chain (gates must not fail open)" >&2 return 1