Skip to content

fix(hooks): sync after rebase and diagnose disabled git hooks - #983

Merged
jeff-r2026 merged 4 commits into
Tencent:mainfrom
SaulMoro:fix/git-hook-rebase-disabled-status
Oct 7, 2026
Merged

jeff-r2026 merged 4 commits into
Tencent:mainfrom
SaulMoro:fix/git-hook-rebase-disabled-status

Conversation

@SaulMoro

@SaulMoro SaulMoro commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

 git pull
   merge  -> post-merge -> sync
-  rebase -> no sync
+  rebase -> post-rewrite(rebase) -> sync
+  fast-forward rebase, Git <= 2.32 + autostash
+         -> post-checkout(old ancestor of new, pull/rebase action) -> sync

 doctor
-  registered hook -> pass
+  registered + enabled hook -> pass
+  disabled hook/event -> actionable failure, advice matches the disabling scope

Rebase reuses the existing capped inline pull and detached retry. Amend does not sync; pull preserves explicit disable settings. The fast-forward path adds no hook: it reuses the installed post-checkout.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature
  • Breaking change
  • Documentation only
  • Refactor / internal cleanup

Evidence

  • Before: the built-CLI divergent-rebase regression exited 0 but failed with a missing delivered rule. A named hook set to enabled=false still returned installed: true.
    After: real Git/CLI checks deliver resources before merge and rebase return, deliver only after conflict resolution, ignore amend, and diagnose disabled hooks. Reactivation restores new-worktree delivery; pull keeps enabled=false.

  • Legacy Git, real teamai through the installed .git/hooks scripts (Docker, node:20-alpine with upstream Git 2.32.7 and 2.26.3 built from source; self mode; same results on both versions):

    Case b801e39f 9a24dd0d
    ff pull --rebase, rebase.autoStash=true, dirty tree post-checkout skipped, not delivered post-checkout synced, delivered, dirty file kept
    ff pull --rebase --autostash, dirty tree not delivered delivered
    ff pull --rebase, no autostash post-merge synced same
    divergent pull --rebase --autostash post-checkout skipped, post-rewrite synced same (one sync)
    divergent with conflict, then rebase --continue nothing until continue, then post-rewrite synced same
    plain git checkout --detach origin/main skipped skipped

Test Plan

  • npx tsc --noEmit
  • npm run lint
  • npx vitest run: 8,107 passed, 20 skipped; 392 test files passed, 1 skipped.
  • Added/updated public-interface regressions, including global/local/worktree disable settings and legacy hook-script ownership/removal.
  • Fast-forward rebase, written failing first: handler-selection unit tests (hook-handlers.test.ts: four sync cases failed before the fix; divergent, non-pull action, unknown action, file checkout, unchanged HEAD and non-OID args stay skipped) and e2e legacy-autostash-fast-forward (runs the rebase step under the pull's GIT_REFLOG_ACTION as Git <= 2.32 does; failed with the rule undelivered, passes now). E2E autostash-fast-forward guards the Git 2.33+ post-merge path.
  • Worktree-scope disable advice, written failing first: git-hook.test.ts disables with git config --worktree and expects git config --worktree --unset <key>; a command-scope case expects "unset or override it there".
  • npm run build
  • Real CLI: npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/git-hook-new-worktree.test.ts --retry 0: 34 passed at 9a24dd0d with Git 2.55, local Git remotes and isolated HOME. Covers merge/rebase, configured rebase, conflict continuation, fast-forward autostash rebase, amend, disabled diagnostics/reactivation, lock failure records and uninstall.
  • Legacy Git in Docker, as in Evidence.
  • git diff --check

Extra providers and agent coverage are left to CI. English/Chinese usage guides and affected skill-data are synchronized.

Related Issues

No linked issue.

Merge Danger

Door: two-way

Revert the code. For projects that already pulled this version, also remove hook.teamai-post-rewrite from local Git config; on older Git remove only its TeamAI-marked block from .git/hooks/post-rewrite. Preserve the owner's script. Normal uninstall removes the new hook too.

Blast Radius: hooks

Project Git synchronization and its doctor check. Named disable settings require Git 2.54+; event disable settings require Git 2.55+. Existing hook managers and explicit disable settings are preserved.

Notes for Reviewers

The new event uses existing install/remove loops and the same synchronization handler. Review the rebase-only guard, effective activation check and preservation of other hooks.

Codex review [P1] (fast-forward pull --rebase fires neither hook) is valid for Git 2.14–2.32 with autostash: those versions skip the merge --ff-only shortcut (git/git f15e7cf5cc, reverted by 340062243a in 2.33) and run a rebase that only checks out the upstream, so only post-checkout fires, with a non-zero old OID that the new-worktree handler ignored. Fixed without adding a hook: the git-pull handler also takes post-checkout when the branch flag is 1, both OIDs are real and differ, GIT_REFLOG_ACTION starts with pull or rebase, and git merge-base --is-ancestor old new succeeds. It keeps the git-pull fetch/lock caps and self-mode gate, and records failures as post-checkout. Divergent rebases fail the ancestry check and keep syncing once on post-rewrite; an unknown reflog action skips. Git 2.33+ still syncs fast-forwards on post-merge.

Codex review [P3] (worktree-scope disable): git config --local <key> true cannot override git config --worktree <key> false. gitHookStatus now records the effective setting's scope (git config --show-scope --get), and doctor advises git config --worktree --unset <key> for worktree scope, keeps the local override for local/global/system, and names any other scope.

@jeff-r2026 jeff-r2026 self-assigned this Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

No findings.

The PR description documents sufficient testing, including a representative real-CLI/e2e verification at head commit 9cdd4f28, satisfying the runtime-change requirement.

@SaulMoro
SaulMoro force-pushed the fix/git-hook-rebase-disabled-status branch from 9cdd4f2 to b801e39 Compare October 6, 2026 15:36
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
  • [P1 blocking] src/hook-handlers.ts:1116 — Relying solely on post-rewrite misses the common git pull --rebase case where the local branch has no commits and Git simply fast-forwards. No commits are rewritten, so post-rewrite does not run; post-merge also does not run for the rebase path. The pull therefore returns without syncing newly published TeamAI resources, contradicting the documented pull.rebase=true behavior. The added e2e test always creates a local commit before rebasing, so it does not cover this scenario. citeturn7search0turn7search1

The PR description otherwise documents sufficient testing, including representative real-CLI verification.

@SaulMoro

SaulMoro commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Re the [P1] on src/hook-handlers.ts:1116 (fast-forward git pull --rebase fires neither hook): valid for Git < 2.33 with autostash on. Fixing it in this PR.

Correction: an earlier version of this comment said no hook fires in that case. That was wrong: rebase discards successful hook output, and my probe read output. Logging hooks to a file shows post-checkout fires:

Case 2.26.3 / 2.32.7 2.34.8 / 2.55.0
fast-forward, pull --rebase post-merge post-merge
fast-forward, --autostash or rebase.autoStash=true post-checkout <old> <new> 1 post-merge
diverged, pull --rebase post-checkout, post-rewrite rebase post-checkout, post-rewrite rebase

Git 2.14 to 2.32 skipped pull's merge --ff-only shortcut under autostash (git/git f15e7cf5cc, reverted by 340062243a in 2.33), so the fast-forward runs through rebase's checkout. teamai already runs on that post-checkout but only acts on new worktrees (all-zero old OID), so it skips the sync.

Fix: on post-checkout with a real old OID, sync when the reflog action is a pull/rebase and the new HEAD descends from the old one. No new hook. A follow-up commit replaces the earlier docs note.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
  • [P3 nit] src/git-hook.ts:140 — The suggested git config --local <key> true does not reactivate a hook disabled in higher-priority per-worktree config (git config --worktree <key> false). git hook list detects that effective disablement, but doctor provides an ineffective fix. Report the setting’s scope or recommend overriding/unsetting it with --worktree.

The earlier fast-forward rebase finding is resolved by the documented Git 2.32-and-older autostash limitation. The PR description includes sufficient representative real-CLI testing; the final commit is documentation-only.

…ckout

Git 2.14-2.32 runs a fast-forward git pull --rebase with autostash as a
rebase that only checks out the upstream: no post-merge, no post-rewrite.
The post-checkout handler only accepted new worktrees, so nothing synced.
Route a post-checkout whose GIT_REFLOG_ACTION is a pull or rebase and whose
old HEAD is an ancestor of the new one through the git-pull sync. Divergent
rebases fail the ancestry check and keep syncing once, on post-rewrite.
A hook disabled with git config --worktree stays disabled after
git config --local <key> true, because worktree config wins. gitHookStatus
now records the scope of the effective setting, and the advice unsets the
worktree value, keeps the local override for local/global/system, and names
any other scope.
@SaulMoro
SaulMoro force-pushed the fix/git-hook-rebase-disabled-status branch from 89132ea to 9a24dd0 Compare October 6, 2026 22:14
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

No findings.

  • The earlier fast-forward rebase issue is resolved by the guarded post-checkout handling in src/git-hook.ts:334 and src/hook-handlers.ts:195.
  • The earlier worktree-config reactivation issue is resolved by scope-aware advice in src/git-hook.ts:127.
  • The PR description documents sufficient representative real-CLI testing. Per instruction, I did not run the PR code or tests.

@jeff-r2026
jeff-r2026 merged commit f502e5a into Tencent:main Oct 7, 2026
13 checks passed
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.

2 participants