fix(ship): verify pre-commit execution (sc-1537)

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',
+    });
+  });
+});

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');

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,29 +78,87 @@ 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")" \
     "$(devkit_json_escape "$DEVKIT_TELEMETRY_VERSION")" \
     "$(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/<hook>` 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,21 +175,50 @@ 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
   # about to publish anyway.
   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