fix(lefthook): remove duplicate git-lfs pre-push command — resolves pre-push deadlock #23

Merged
bert.hausmans merged 1 commits from fix/lefthook-lfs-deadlock into main 2026-05-11 00:33:45 +02:00

⚠️ Reviewer-attentie: this fix removes a redundant config entry; the heavy lifting is in dev-docs/ADR-LEFTHOOK-LFS-INTEGRATION.md. Read the ADR before approving — it documents two misconceptions that would re-introduce the bug if accepted at face value.

Root cause

Lefthook v2 automatically runs git lfs pre-push for every push when git-lfs is installed (per docs/usage/features/git-lfs.md, confirmed in internal/run/controller/lfs.go). Our lefthook.yml had a manual git-lfs: command running the same thing — a duplicate invocation. The internal call read buffered stdin and uploaded LFS objects cleanly; the manual call either hung on pseudo-TTY-without-EOF or got tangled with the Gitea LFS server on a redundant verify request.

Historical context

Both tools are recent additions. Lefthook landed in WS-3 (April) for post-commit .claude-sync/ regeneration. Git-LFS landed in TEST-INFRA-001 (May, two days ago) for visual regression baseline PNGs. The deadlock surfaced only after both were active together — explains why the first weeks of project work had no issue.

Fix

  • Removed manual git-lfs: command from lefthook.yml
  • Removed piped: true (placebo from earlier failed attempt commit 1b06804piped is fail-fast semantics, not stdin/sequencing)
  • Lefthook's built-in pre-push LFS handler does the upload
  • sync-check remains the only user-defined pre-push command

Empirical verification

  • Phase B sandbox test: confirmed duplicate execution is real and visible in logs. Sandbox did NOT reproduce the hang (filesystem remote, no LFS server).
  • Phase E real push: this branch pushed to Gitea in ~8 seconds without --no-verify. Exactly one [git-lfs] executing hook line in log; no duplicate. Confirms fix works against network-y remote where the hang originally manifested.

Regression protection

  • dev-docs/ADR-LEFTHOOK-LFS-INTEGRATION.md — full investigation record + two named misconceptions:
    1. The git lfs install --skip-repo comment in the original lefthook.yml was technically correct but conceptually misleading (the "delegate" framing was wrong)
    2. piped: true does NOT control sequencing or stdin routing (it's fail-fast semantics)
  • scripts/test-lefthook-pre-push.sh — disposable-sandbox smoke test asserting exactly one internal LFS call, zero manual ones, LFS upload present. Run after any lefthook.yml change.

What this does NOT touch

  • LFS itself stays (visual regression baselines require it)
  • Lefthook itself stays (sync-check + post-commit hook)
  • .githooks/pre-push content unchanged (it's stdin-innocent, per Phase A finding A5)

Post-merge

No special action. Future pushes work without --no-verify. If the hang ever recurs: run scripts/test-lefthook-pre-push.sh and consult the ADR's "Alternative scenarios" section.

🤖 Generated with Claude Code

> ⚠️ **Reviewer-attentie**: this fix removes a redundant config entry; the heavy lifting is in `dev-docs/ADR-LEFTHOOK-LFS-INTEGRATION.md`. Read the ADR before approving — it documents two misconceptions that would re-introduce the bug if accepted at face value. ## Root cause Lefthook v2 automatically runs `git lfs pre-push` for every push when git-lfs is installed (per `docs/usage/features/git-lfs.md`, confirmed in `internal/run/controller/lfs.go`). Our `lefthook.yml` had a manual `git-lfs:` command running the same thing — a duplicate invocation. The internal call read buffered stdin and uploaded LFS objects cleanly; the manual call either hung on pseudo-TTY-without-EOF or got tangled with the Gitea LFS server on a redundant verify request. ## Historical context Both tools are recent additions. Lefthook landed in WS-3 (April) for post-commit `.claude-sync/` regeneration. Git-LFS landed in TEST-INFRA-001 (May, two days ago) for visual regression baseline PNGs. The deadlock surfaced only after both were active together — explains why the first weeks of project work had no issue. ## Fix - Removed manual `git-lfs:` command from `lefthook.yml` - Removed `piped: true` (placebo from earlier failed attempt commit `1b06804` — `piped` is fail-fast semantics, not stdin/sequencing) - Lefthook's built-in pre-push LFS handler does the upload - `sync-check` remains the only user-defined pre-push command ## Empirical verification - **Phase B sandbox test**: confirmed duplicate execution is real and visible in logs. Sandbox did NOT reproduce the hang (filesystem remote, no LFS server). - **Phase E real push**: this branch pushed to Gitea in ~8 seconds without `--no-verify`. Exactly one `[git-lfs] executing hook` line in log; no duplicate. Confirms fix works against network-y remote where the hang originally manifested. ## Regression protection - `dev-docs/ADR-LEFTHOOK-LFS-INTEGRATION.md` — full investigation record + two named misconceptions: 1. The `git lfs install --skip-repo` comment in the original `lefthook.yml` was technically correct but conceptually misleading (the "delegate" framing was wrong) 2. `piped: true` does NOT control sequencing or stdin routing (it's fail-fast semantics) - `scripts/test-lefthook-pre-push.sh` — disposable-sandbox smoke test asserting exactly one internal LFS call, zero manual ones, LFS upload present. Run after any `lefthook.yml` change. ## What this does NOT touch - LFS itself stays (visual regression baselines require it) - Lefthook itself stays (sync-check + post-commit hook) - `.githooks/pre-push` content unchanged (it's stdin-innocent, per Phase A finding A5) ## Post-merge No special action. Future pushes work without `--no-verify`. If the hang ever recurs: run `scripts/test-lefthook-pre-push.sh` and consult the ADR's "Alternative scenarios" section. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
bert.hausmans added 1 commit 2026-05-11 00:33:30 +02:00
Lefthook v2 runs `git lfs pre-push` internally for pre-push hooks (per
docs/usage/features/git-lfs.md; confirmed in internal/run/controller/
lfs.go where the internal handler invokes `git lfs pre-push <remote>
<url>` with a buffered `cachedStdin`). Our manual `git-lfs:` command
in lefthook.yml was a second invocation against the same remote; the
duplicate is directly visible in `LEFTHOOK_VERBOSE=1` output as
`[git-lfs] executing hook` (internal) followed by `[lefthook] run:
git lfs pre-push` (manual).

The previous fix attempt (piped: true, commit 1b06804) was based on a
wrong understanding of `piped`'s semantics — `piped` controls
fail-fast behavior, not stdin routing or sequencing. Default lefthook
behavior is already sequential per docs/configuration/parallel.md.
That "fix" was placebo; incident 2 (F2 push, zero LFS objects, commit
99eedb6) proved it.

Phase A investigation: documentary + source confirmation that lefthook
owns the LFS pre-push call. Phase B sandbox test against a filesystem
remote confirmed the duplicate execution in logs but did NOT reproduce
the production hang — likely because the duplicate manual call against
a local remote has no LFS server to interact with. A network-y remote
(Gitea over SSH/HTTPS) appears to be part of the trigger. Two
mechanisms remain plausible (H1: PTY-stdin without EOF in
`while read` loop per docs/configuration/use_stdin.md; H4: server-side
LFS interaction on the duplicate call). Both are eliminated by the
same fix: remove the manual command. LFS uploads continue to work via
lefthook's internal handler (verified in sandbox post-fix).

Regression coverage: scripts/test-lefthook-pre-push.sh asserts exactly
one internal LFS invocation, zero manual ones, and `Uploading LFS
objects: 100%` present, against a disposable sandbox.

See dev-docs/ADR-LEFTHOOK-LFS-INTEGRATION.md for full context, both
misconceptions to prevent regression, and the alternative-scenarios
playbook if Phase E ever regresses.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
bert.hausmans merged commit d99f9567c3 into main 2026-05-11 00:33:45 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bert.hausmans/crewli#23