Skip to content

feat(cloud-agent): add git_signed_rewrite tool#2486

Merged
tatoalo merged 1 commit into
mainfrom
git-signed-rewrite
Jun 4, 2026
Merged

feat(cloud-agent): add git_signed_rewrite tool#2486
tatoalo merged 1 commit into
mainfrom
git-signed-rewrite

Conversation

@tatoalo
Copy link
Copy Markdown
Contributor

@tatoalo tatoalo commented Jun 4, 2026

Problem

cloud agents can create GitHub-signed (commits via git_signed_commit, but had no way to rewrite history so after a local rebase or conflict fix the agent could not force-update an existing PR branch and would open a brand-new PR instead.

Changes

Adds git_signed_rewrite, the force-push counterpart to git_signed_commit:

  • The agent rebases/merges locally with normal git (finishing with git rebase --continue, not git commit), then calls git_signed_rewrite.
  • It replays the branch's commits as GitHub-Verified onto a fresh scratch ref (via createCommitOnBranch), atomically force-moves the real branch onto the result, then deletes the scratch ref — so an existing PR updates in place, every commit stays Verified, and no local signing key is needed. Works for both bot- and user-authored runs

@tatoalo tatoalo self-assigned this Jun 4, 2026
@tatoalo tatoalo marked this pull request as ready for review June 4, 2026 15:42
@tatoalo tatoalo requested a review from sampennington June 4, 2026 15:43
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Comments Outside Diff (1)

  1. packages/git/src/signed-commit.ts, line 178-182 (link)

    P2 The error message is hardcoded to reference git_signed_commit, but resolveCurrentBranch is now also reachable from createSignedRewrite (via resolveBranchName). A user calling git_signed_rewrite on a detached HEAD without supplying branch will see a confusing directive to use the wrong tool.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/git/src/signed-commit.ts
    Line: 178-182
    
    Comment:
    The error message is hardcoded to reference `git_signed_commit`, but `resolveCurrentBranch` is now also reachable from `createSignedRewrite` (via `resolveBranchName`). A user calling `git_signed_rewrite` on a detached HEAD without supplying `branch` will see a confusing directive to use the wrong tool.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
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.ts:178-182
The error message is hardcoded to reference `git_signed_commit`, but `resolveCurrentBranch` is now also reachable from `createSignedRewrite` (via `resolveBranchName`). A user calling `git_signed_rewrite` on a detached HEAD without supplying `branch` will see a confusing directive to use the wrong tool.

```suggestion
  if (!current || current === "HEAD") {
    throw new Error(
      "Detached HEAD — pass a `branch` to the tool (e.g. posthog-code/...).",
    );
  }
```

### Issue 2 of 2
packages/git/src/signed-commit.test.ts:60-95
**No tests for `createSignedRewrite`**

The new function contains non-trivial logic: scratch-ref lifecycle, empty-commit skipping, the TOCTOU staleness check, and the `onto` resolution heuristic. Only `splitCommitMessage` (a pure helper) gained tests. Given the complexity and the cost of a mis-fired `forceUpdateRef`, exercising the rewrite path — even with a mock `execGh`/`execGhWithRetry` — would give meaningful protection.

Reviews (1): Last reviewed commit: "feat(cloud-agent): add git_signed_rewrit..." | Re-trigger Greptile

Comment thread packages/git/src/signed-commit.test.ts
@tatoalo tatoalo merged commit 73fd6ad into main Jun 4, 2026
18 checks passed
@tatoalo tatoalo deleted the git-signed-rewrite branch June 4, 2026 16:46
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