reconcile: harden replayLocalOntoRemote against merge commits + empty commits
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 68/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Quiet
- Tech stack
- git, typescript
Research direction
Start in apps/api/src/store/reconcile.ts at replayLocalOntoRemote, then read the existing reconciliation cases in apps/api/tests/data-repo-reconcile.test.ts. Add coverage for local merge commits and empty commits using the proposed outcomes, and run that test file. Done means merge commits escape with the hash in the conflict log and empty commits are absent from rewritten history.
Written by the indexing model from the issue text.
Description
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:
- Reject merge commits early. Before the loop, check each commit's parent count via
git rev-list --parentsorgit cat-file -p <commit> | head -1. If any commit has>1parent, throwRebaseReplayConflictError(routes to the escape-hatch) with a clear message. Operator can then investigate the unexpected merge commit onconflicts/<UTC>. - Skip empty commits. Before calling
commit-tree, comparemergedTreeHashto the new tip's tree (git show <newTip> --format=%T -s). If equal, skip the commit and continue withnewTipunchanged.
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.
- Dominant language
- TypeScript
- Stars
- 1
- Forks
- 1
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 25
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from CodeForPhilly/codeforphilly-ng
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
MarkdownEditor toolbar: use Radix Toolbar from radix-ui instead of the hand-rolled roving tabindex Openenhancement
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
All issues in CodeForPhilly/codeforphilly-ng
Similar issues
-
Difficulty 1/5 1-3 hours Newbie friendliness 88/100
motiondivision/motion#3849 ·
-
Add: S Play Event HD Opencheck:passed streams:add
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
i-am-bee/beeai-framework#1697 · 1 reaction ·
-
Support bun dedupe Openenhancement
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
antfu/node-modules-inspector#214 ·