Regression in #418, on dev only. main is unaffected — v0.6.0 predates the hook.
What happens
The pre-push hook runs commitlore sync, and sync publishes with git push. A git push inside a pre-push hook fires pre-push again, which syncs again, which pushes again.
Measured with an instrumented hook:
40 seconds, 1,240 hook invocations, never returned
Any user who installs this and runs git push hangs.
Why the existing tests did not catch it
test/sync.test.ts has eleven cases and calls syncNotes directly in every one of them. The recursion is not a property of the function — it only exists when git invokes the hook. A hook was shipped and never exercised through a hook.
Fix
git push --no-verify in syncRemote.
It belongs in sync rather than in the hook: the recursion is a property of this push, not of the caller, and nothing is served by a notes push running a hook whose only job is to push notes.
What the regression test has to be
Against a real git push, not a function, with a deadline — the defect is a hang, so a test that waits for it forever fails the suite instead of the assertion. Four cases: it does not recurse, it publishes the mirror, a repository with nothing to publish still pushes cleanly, and a failing sync does not fail the push (the contract that outranks publishing).
Adjacent, filed separately
Setting up the test surfaced that COMMITLORE_BIN is exec'd directly by the stub, so a non-executable .js — dist/cli.js is exactly that — makes the hook die with 126 and take the git operation with it. The recorded-path branch does not have this shape: it runs exec "$recorded_node" "$recorded" and checks the interpreter is executable first.
Regression in #418, on
devonly.mainis unaffected — v0.6.0 predates the hook.What happens
The
pre-pushhook runscommitlore sync, andsyncpublishes withgit push. Agit pushinside apre-pushhook firespre-pushagain, which syncs again, which pushes again.Measured with an instrumented hook:
Any user who installs this and runs
git pushhangs.Why the existing tests did not catch it
test/sync.test.tshas eleven cases and callssyncNotesdirectly in every one of them. The recursion is not a property of the function — it only exists when git invokes the hook. A hook was shipped and never exercised through a hook.Fix
git push --no-verifyinsyncRemote.It belongs in
syncrather than in the hook: the recursion is a property of this push, not of the caller, and nothing is served by a notes push running a hook whose only job is to push notes.What the regression test has to be
Against a real
git push, not a function, with a deadline — the defect is a hang, so a test that waits for it forever fails the suite instead of the assertion. Four cases: it does not recurse, it publishes the mirror, a repository with nothing to publish still pushes cleanly, and a failing sync does not fail the push (the contract that outranks publishing).Adjacent, filed separately
Setting up the test surfaced that
COMMITLORE_BINisexec'd directly by the stub, so a non-executable.js—dist/cli.jsis exactly that — makes the hook die with 126 and take the git operation with it. The recorded-path branch does not have this shape: it runsexec "$recorded_node" "$recorded"and checks the interpreter is executable first.