Skip to content

reconcile: harden replayLocalOntoRemote against merge commits + empty commits #88

Description

@themightychris

Background

PR #86's replayLocalOntoRemote (in apps/api/src/store/reconcile.ts) replaces git rebase with a per-commit merge-tree --write-tree + commit-tree loop. The plan's Risks section flags two edge cases that the implementation accepts silently rather than guarding against:

1. Merge commits get silently flattened

'commit-tree', mergedTreeHash, '-p', newTip, '-m', message

Single -p parent. If a multi-parent commit ever appears in localCommits, the replay collapses it into a single-parent commit. git rebase by default refuses to replay merges (errors out); --rebase-merges preserves them.

Risk: the data repo today is all programmatic single-parent gitsheets transacts, so this never fires. But if a human or a future automation ever lands a merge commit on the data repo, reconcile would silently drop one of the parents and rewrite history in a misleading way.

2. Empty commits are preserved instead of dropped

git commit-tree <existing-tree> happily writes a commit even when the resulting tree equals the parent's tree (no diff). Modern git rebase drops empty commits by default (--no-empty). The replay loop currently writes them through.

Risk: an empty commit (programmatically possible if a gitsheets transact does nothing but still commits) gets preserved across reconcile, making history noisier than git rebase would produce.

Proposed hardening

In replayLocalOntoRemote:

  1. Reject merge commits early. Before the loop, check each commit's parent count via git rev-list --parents or git cat-file -p <commit> | head -1. If any commit has >1 parent, throw RebaseReplayConflictError (routes to the escape-hatch) with a clear message. Operator can then investigate the unexpected merge commit on conflicts/<UTC>.
  2. Skip empty commits. Before calling commit-tree, compare mergedTreeHash to the new tip's tree (git show <newTip> --format=%T -s). If equal, skip the commit and continue with newTip unchanged.

Both checks are cheap. The merge-commit guard is the more important one (data loss avoidance); the empty-commit skip is purely a history-cleanliness improvement.

Tests to add

In apps/api/tests/data-repo-reconcile.test.ts:

  • New case: diverged-with-merge-commit-on-local → expect outcome: 'conflict-escaped' with the merge commit's hash mentioned in the conflict log.
  • New case: diverged-with-empty-commit-on-local → expect outcome: 'rebased', empty commit dropped from the rewritten history.

Why post-cutover

Neither edge case is reachable from current code paths (no merge commits, no empty-commit-producing transacts). Filing as hardening rather than a bug fix.

Filed as follow-up from PR #86.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions