Skip to content

fix(git): prevent reverting remote commits#2632

Merged
charlesvien merged 3 commits into
mainfrom
posthog-code/guard-stale-signed-commits
Jun 12, 2026
Merged

fix(git): prevent reverting remote commits#2632
charlesvien merged 3 commits into
mainfrom
posthog-code/guard-stale-signed-commits

Conversation

@mp-hog

@mp-hog mp-hog commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Problem

Agent didn't check if there were new commits on remote, possibly overwriting them in the subsequent commit (for example leading to silently dropping CI artifacts).

Changes

  • packages/gitassertNotBehindRemote: after fetching an existing branch, fail closed when the remote tip has commits the local checkout lacks. The error outputs the divergence and gives a work-preserving recovery (stash --include-untrackedfetchreset --hardstash pop). Mirrors the existing merge-in-progress / base-leak refusals; new and up-to-date branches are unaffected.
  • packages/agent — documents the failure mode in the agent's git-workflow prompt and the tool description.

Tests

Real-git + bare-remote fixture reproducing the bug (commit → simulated bot push → second commit from a stale checkout → refused), plus up-to-date, local-ahead, and no-op (staged work preserved) cases. 39/39 pass.

…ranch

git_signed_commit built the new commit on the remote tip while taking file contents from the local staged tree. When the remote branch had advanced (e.g. a CI bot auto-committing regenerated artifacts onto an open PR), committing from a stale checkout silently reverted those commits.

Add assertNotBehindRemote: after fetching an existing branch, fail closed when the remote tip has commits the local checkout lacks, with an actionable recovery message (stash -> fetch -> reset --hard -> stash pop). Mirrors the existing merge-in-progress and base-leak refusals. New and up-to-date branches are unaffected (one rev-parse + merge-base, no extra fetch).

Also document the failure mode in the agent git-workflow system prompt and the tool description, and cover the regression with tests.

Generated-By: PostHog Code
Task-Id: d562b81f-a6b5-425c-9981-b155fb52b612
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit e8e3223.

@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/git/src/signed-commit.test.ts:263-284
**Redundant call + missed parameterisation opportunity**

The two `behindRemoteError` tests call the same pure function twice on slightly different inputs, which violates OnceAndOnlyOnce. The "stays generic" checks (absence of `"tests-posthog"` and `"OpenAPI"`) don't depend on a different input — they could run alongside the presence assertions on the same call. Collapsing to a single test, or using `it.each` if more combinations are ever added, would express the idea once.

### Issue 2 of 2
packages/git/src/signed-commit.test.ts:399-464
**Setup duplication across the last two `assertNotBehindRemote` tests**

The "no-op" and "local ahead" tests each hand-build their own bare-remote + agent fixture (~20 lines each) that reproduces almost exactly what `setupBranchAdvancedByBot` already does — just without the bot step. Extracting a `setupBaseRepo` helper (or passing an option to skip the bot) would satisfy the OnceAndOnlyOnce rule and make each test's intent clearer at a glance.

Reviews (1): Last reviewed commit: "fix(git): refuse signed commits that wou..." | Re-trigger Greptile

Comment thread packages/git/src/signed-commit.test.ts
Comment thread packages/git/src/signed-commit.test.ts Outdated
@mp-hog mp-hog changed the title fix(git): refuse signed commits that would revert a remote-advanced branch fix(git): prevent commits that would revert remote commits Jun 12, 2026
@mp-hog mp-hog changed the title fix(git): prevent commits that would revert remote commits fix(git): prevent reverting remote commits Jun 12, 2026
mp-hog added 2 commits June 12, 2026 13:39
Address greptile review nits: extract setupBaseRepo/setupPushedBranch helpers so the no-op and local-ahead tests stop re-building the bare-remote fixture, and fold the two behindRemoteError assertions onto a single call.

Generated-By: PostHog Code
Task-Id: d562b81f-a6b5-425c-9981-b155fb52b612
Use it.each for the present/absent substring checks (building the message once) instead of a single flat test, matching the file's existing parameterised style and the CLAUDE.md preference.

Generated-By: PostHog Code
Task-Id: d562b81f-a6b5-425c-9981-b155fb52b612
@mp-hog mp-hog requested a review from a team June 12, 2026 16:07
@charlesvien charlesvien merged commit ff42948 into main Jun 12, 2026
23 checks passed
@charlesvien charlesvien deleted the posthog-code/guard-stale-signed-commits branch June 12, 2026 18:39
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