[Bug] Post-commit hook can reuse stale pending state after a read failure

[404-Page-Found]

Description

runPostCommitHook() only clears the pending hook entry on successful processing, invalid JSON, an empty commit message, or an appendHistoryEntry() failure. If an earlier operation in the outer try throws — especially checkGitRepo() or readLatestCommitMessage() — the catch block only emits a warning and leaves the pending file behind.

That stale entry can then be consumed by a later successful commit. The later commit's message is paired with the older commit's diff/model/provider metadata.

Location

src/git/hook.ts:220-253 — runPostCommitHook()

Relevant code

try {
  deps.checkGitRepo();
  ...
  const message = deps.readLatestCommitMessage().trim();
  ...
  try {
    await deps.appendHistoryEntry(entry);
  } finally {
    await deps.removePendingEntryFile();
  }
} catch (err) {
  const message = err instanceof Error ? err.message : String(err);
  deps.warn(`commit-echo hook: ${message}`);
}

The outer catch has no cleanup.

Steps to reproduce

1. Let prepare-commit-msg create a valid pending entry for a commit.
2. Make the following post-commit invocation fail before the inner appendHistoryEntry() block (for example, make readLatestCommitMessage() throw in a test).
3. Confirm the pending entry remains.
4. Run another commit whose post-commit hook can read the latest commit message.
5. The old pending entry is now used for the new commit.

A focused regression test can inject the existing PostCommitHookDeps callbacks to reproduce this without filesystem corruption.

Expected behavior

Any terminal error in the post-commit hook should leave no stale pending state that can be attributed to a future commit.

Actual behavior

Some outer-hook failures only warn and return, leaving commit-echo-pending-entry.json behind.

Suggested fix

Make pending-state ownership explicit and guarantee cleanup for the current pending record on all terminal paths. Ideally, key pending state by worktree/commit generation so a later hook invocation cannot consume an earlier invocation's metadata.

Impact

History can become silently incorrect: a later commit may be recorded with the previous commit's diff, model, and provider information. This undermines the learned style data without producing a hard failure.

Reviewed against current main at c67ff967a018d5fd4b032f6ef6a4981ac9d76a12.

[404-Page-Found]

Triage

Priority: P2 — data integrity / reliability

This is an edge-triggered bug, but the consequence is silent history corruption: a later commit can inherit an older pending entry's diff/model/provider metadata.

Disposition: Fix before relying on hook-driven history for long-lived repos. The cleanup guarantee should cover every terminal path, and pending state should ideally be tied to a commit/worktree generation rather than a single shared filename.

Suggested implementation: add a regression test where readLatestCommitMessage (or checkGitRepo) fails after a pending entry exists, then verify the next successful post-commit cannot consume that stale entry.