Skip to content

fix: land resolve-conflicts pushes through PR pre-push hooks - #114

Merged
danii1 merged 1 commit into
mainfrom
fix/resolve-conflicts-pre-push-hooks
Sep 1, 2026
Merged

fix: land resolve-conflicts pushes through PR pre-push hooks#114
danii1 merged 1 commit into
mainfrom
fix/resolve-conflicts-pre-push-hooks

Conversation

@danii1

@danii1 danii1 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem

devintern resolve-conflicts <pr-url> resolved conflicts but never pushed them (seen on #98). The PR's failure comments showed push-failed: the push was rejected by the target repo's lefthook pre-push hook running inside the /tmp review worktree.

Root cause chain:

  1. installDependencies (bun install) in the review worktree runs dependency postinstalls; lefthook's postinstall rewrites git hooks via lefthook install. A linked worktree shares .git/hooks with the user's checkout, so the user's real hooks were overwritten with scripts hardcoding the ephemeral worktree's node_modules path (/tmp/devintern-review-worktree-<branch>/...).
  2. Since 5a427fd the worktree is always removed after the run, so those hook paths dangle and every later push fails — both from the worktree and from the user's own checkout.

Changes

Hook isolation (src/lib/utils.ts):

  • New Utils.isolateWorktreeHooks(), called in prepareReviewWorktree before installDependencies (both the reuse and fresh-creation paths).
  • Enables extensions.worktreeConfig and points per-worktree core.hooksPath at the worktree's git admin dir (.git/worktrees/<name>/hooks), seeded with copies of the shared hooks. Postinstall rewrites are confined to the ephemeral worktree, stay valid for its pushes, and vanish with it. The dir lives outside the working tree, so it's invisible to git status, git clean, and git add -A.

Hook-failure retry (src/lib/conflict-resolver.ts):

  • Push failures carrying hookError are handed to the agent (buildHookFixPrompt, up to MAX_HOOK_FIX_ATTEMPTS = 2), mirroring the review flow's hook fixer; leftover changes are amended into the merge commit before each retry.
  • A branch race (leaseSha moved) skips the agent entirely — the existing defer/retry handling stays authoritative, so no agent runs are burned on races.

Testing

  • 5 new tests: hook-fix retry lands the push, amend folding, hook isolation (redirect + copies, postinstall rewrite shielding, isolated hook actually runs on push).
  • Full package suite: 1281 pass, typecheck/lint/format clean.

Follow-up note

Existing local clones whose .git/hooks were already poisoned still point at deleted worktrees (the /node_modules fallback usually still resolves); re-running lefthook install in affected clones restores them.

Review worktree dependency installs (lefthook postinstall) rewrote the
shared .git/hooks with scripts hardcoding the ephemeral worktree's
node_modules path, so pushes failed once the worktree was removed —
resolve-conflicts resolved conflicts locally but never landed them.

- isolate worktree hooks: point core.hooksPath at the worktree's git
  admin dir (per-worktree config) before installDependencies, seeded
  with copies of the shared hooks, so postinstall rewrites stay inside
  the ephemeral worktree and vanish with it
- conflict-resolver: hand pre-push hook failures to the agent and retry
  the push (bounded), folding leftover changes into the merge commit;
  branch races still defer/retry without burning agent runs
@danii1
danii1 merged commit a69c5e9 into main Sep 1, 2026
1 check passed
@danii1
danii1 deleted the fix/resolve-conflicts-pre-push-hooks branch September 1, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant