fix: honor updateRef when creating signed commits

src/commit.rs

@@ -434,19 +434,50 @@ impl Repository {
       .ok_or(crate::Error::SignatureNotFound)?;
 
     let oid = if let Some(signature_str) = signature {
+      // `commit_signed()` only writes the commit object, unlike `commit()` which
+      // also validates parents against the ref tip and moves the ref inside
+      // libgit2. Mirror that behavior here so both paths behave the same.
+      let update_target = update_ref
+        .as_deref()
+        .map(|name| self.resolve_commit_update_ref(name))
+        .transpose()?;
+
+      let parents = parents.unwrap_or_default();
+      if let Some(CommitUpdateRef::Resolved(reference)) = &update_target {
+        // Validate before writing the object so a failure leaves no orphaned
+        // commit in the odb, matching libgit2's ordering.
+        let first_parent = parents.first().map(|parent| parent.id());
+        if reference.target() != first_parent {
+          return Err(
+            git2::Error::new(
+              git2::ErrorCode::Modified,
+              git2::ErrorClass::Object,
+              "failed to create commit: current tip is not the first parent",
+            )
+            .into(),
+          );
+        }
+      }
+
       let commit_content = self.inner.commit_create_buffer(
         &author,
         &committer,
         &message,
         &tree.inner,
-        &parents.unwrap_or_default().iter().collect::<Vec<_>>(),
+        &parents.iter().collect::<Vec<_>>(),
       )?;
 
       let commit_content_str = std::str::from_utf8(&commit_content)?.to_string();
 
-      self
+      let oid = self
         .inner
-        .commit_signed(&commit_content_str, &signature_str, signature_field.as_deref())?
+        .commit_signed(&commit_content_str, &signature_str, signature_field.as_deref())?;
+
+      if let Some(update_target) = update_target {
+        self.update_ref_for_commit(update_target, oid)?;
+      }
+
+      oid
     } else {
       self.inner.commit(
         update_ref.as_deref(),
@@ -461,3 +492,59 @@ impl Repository {
     Ok(oid.to_string())
   }
 }
+
+/// Resolution of an `updateRef` name for commit creation, mirroring how libgit2
+/// treats the ref on its unsigned commit path (`git_commit__create_internal`).
+enum CommitUpdateRef<'repo> {
+  /// The name resolved to an existing direct reference.
+  Resolved(git2::Reference<'repo>),
+  /// The name (or the branch its symbolic ref points to) does not exist yet;
+  /// a reference with this name should be created.
+  Create(String),
+}
+
+impl Repository {
+  /// Mirrors `git_reference_lookup_resolved` as used by libgit2 when creating
+  /// a commit: a missing ref or a symbolic ref to an unborn branch (e.g. HEAD
+  /// in a fresh repository) is not an error, anything else propagates.
+  fn resolve_commit_update_ref(&self, name: &str) -> crate::Result<CommitUpdateRef<'_>> {
+    match self.inner.find_reference(name) {
+      Ok(reference) => match reference.resolve() {
+        Ok(resolved) => Ok(CommitUpdateRef::Resolved(resolved)),
+        Err(e) if e.code() == git2::ErrorCode::NotFound => {
+          let target = reference.symbolic_target().unwrap_or(name).to_string();
+          Ok(CommitUpdateRef::Create(target))
+        }
+        Err(e) => Err(e.into()),
+      },
+      Err(e) if e.code() == git2::ErrorCode::NotFound => Ok(CommitUpdateRef::Create(name.to_string())),
+      Err(e) => Err(e.into()),
+    }
+  }
+
+  /// Points the resolved ref at the new commit, mirroring
+  /// `git_reference__update_for_commit`: same reflog message format, and
+  /// updating an existing ref asserts its target has not moved since it was
+  /// resolved. The reflog identity falls back to the repository default since
+  /// git2 does not accept a signature on reference updates.
+  fn update_ref_for_commit(&self, target: CommitUpdateRef<'_>, oid: git2::Oid) -> crate::Result<()> {
+    let commit = self.inner.find_commit(oid)?;
+    let commit_type = match commit.parent_count() {
+      0 => " (initial)",
+      1 => "",
+      _ => " (merge)",
+    };
+    let reflog_message = format!("commit{}: {}", commit_type, commit.summary().unwrap_or_default());
+    match target {
+      CommitUpdateRef::Resolved(mut reference) => {
+        reference.set_target(oid, &reflog_message)?;
+      }
+      CommitUpdateRef::Create(name) => {
+        // Create-only (no force), like libgit2's `git_reference__update_terminal`:
+        // if the ref appeared in the meantime this errors instead of clobbering it.
+        self.inner.reference(&name, oid, false, &reflog_message)?;
+      }
+    }
+    Ok(())
+  }
+}

tests/commit.spec.ts

@@ -1,11 +1,14 @@
 import fs from 'node:fs/promises';
 import path from 'node:path';
 import { describe, expect, it } from 'vitest';
-import { isValidOid, openRepository } from '../index';
+import { initRepository, isValidOid, openRepository } from '../index';
 import { useFixture } from './fixtures';
+import { makeTmpDir } from './tmp';
 
 describe('commit', () => {
   const signature = { name: 'Seokju Na', email: 'seokju.me@gmail.com' };
+  const gpgSignature =
+    '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----';
 
   it('get commit', async () => {
     const p = await useFixture('commits');
@@ -77,18 +80,16 @@ describe('commit', () => {
       author: signature,
       committer: signature,
       parents: [repo.head().target()!],
-      signature:
-        '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----',
+      signature: gpgSignature,
     });
     expect(isValidOid(oid)).toBe(true);
+    expect(repo.head().target()).toEqual(oid);
     const signatureInfo = repo.extractSignature(oid);
     expect(signatureInfo).not.toBeNull();
 
     const { signature: extractedSignature = '', signedData = '' } = signatureInfo || {};
 
-    expect(extractedSignature).toEqual(
-      '-----BEGIN PGP SIGNATURE-----\\nVersion: GnuPG v1\\n\\niQEcBAABAgAGBQJTest123\\n-----END PGP SIGNATURE-----'
-    );
+    expect(extractedSignature).toEqual(gpgSignature);
 
     expect(signedData).toContain('tree ab9abf28de846b5968a8f12156f1d5ce3f4a198e');
     expect(signedData).toContain('parent a01e9888e46729ef4aa68953ba19b02a7a64eb82');
@@ -97,6 +98,191 @@ describe('commit', () => {
     expect(signedData).toContain('signed commit');
   });
 
+  it('signed commit records reflog entry', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'signed commit\n\nbody the reflog entry must not contain', {
+      updateRef: 'HEAD',
+      author: signature,
+      committer: signature,
+      parents: [repo.head().target()!],
+      signature: gpgSignature,
+    });
+    const entry = repo.reflog('HEAD').get(0);
+    expect(entry?.idNew()).toEqual(oid);
+    expect(entry?.message()).toEqual('commit: signed commit');
+  });
+
+  it('signed commit updates an existing branch ref', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const tip = repo.head().target()!;
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'signed commit on main', {
+      updateRef: 'refs/heads/main',
+      author: signature,
+      committer: signature,
+      parents: [tip],
+      signature: gpgSignature,
+    });
+    expect(repo.findReference('refs/heads/main')?.target()).toEqual(oid);
+    expect(repo.reflog('refs/heads/main').get(0)?.message()).toEqual('commit: signed commit on main');
+  });
+
+  it('create signed commit on unborn HEAD', async () => {
+    const p = await makeTmpDir('signed-unborn');
+    const repo = await initRepository(p, { initialHead: 'main' });
+    await fs.writeFile(path.join(p, 'first'), 'first');
+    const index = repo.index();
+    index.addPath('first');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'initial signed commit', {
+      updateRef: 'HEAD',
+      author: signature,
+      committer: signature,
+      signature: gpgSignature,
+    });
+    expect(repo.head().name()).toEqual('refs/heads/main');
+    expect(repo.head().target()).toEqual(oid);
+    expect(repo.reflog('HEAD').get(0)?.message()).toEqual('commit (initial): initial signed commit');
+  });
+
+  it('create signed commit updating a new ref', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const headBefore = repo.head().target()!;
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'signed commit on new branch', {
+      updateRef: 'refs/heads/sign-target',
+      author: signature,
+      committer: signature,
+      parents: [headBefore],
+      signature: gpgSignature,
+    });
+    expect(repo.findReference('refs/heads/sign-target')?.target()).toEqual(oid);
+    expect(repo.head().target()).toEqual(headBefore);
+  });
+
+  it('signed commit rejects updateRef when first parent is not the current tip', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const headBefore = repo.head().target();
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    expect(() =>
+      repo.commit(tree, 'orphaned signed commit', {
+        updateRef: 'HEAD',
+        author: signature,
+        committer: signature,
+        signature: gpgSignature,
+      })
+    ).toThrowError(/current tip is not the first parent/);
+    const revwalk = repo.revwalk();
+    revwalk.pushHead();
+    revwalk.next();
+    const older = revwalk.next()!;
+    expect(() =>
+      repo.commit(tree, 'orphaned signed commit', {
+        updateRef: 'HEAD',
+        author: signature,
+        committer: signature,
+        parents: [older],
+        signature: gpgSignature,
+      })
+    ).toThrowError(/current tip is not the first parent/);
+    expect(repo.head().target()).toEqual(headBefore);
+  });
+
+  it('signed merge commit records merge reflog message', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const revwalk = repo.revwalk();
+    revwalk.pushHead();
+    const tip = revwalk.next()!;
+    const older = revwalk.next()!;
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'signed merge commit', {
+      updateRef: 'HEAD',
+      author: signature,
+      committer: signature,
+      parents: [tip, older],
+      signature: gpgSignature,
+    });
+    expect(repo.head().target()).toEqual(oid);
+    expect(repo.reflog('HEAD').get(0)?.message()).toEqual('commit (merge): signed merge commit');
+  });
+
+  it('signed commit skips first-parent validation for a nonexistent ref', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const headBefore = repo.head().target();
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'rootless signed commit', {
+      updateRef: 'refs/heads/no-validate',
+      author: signature,
+      committer: signature,
+      signature: gpgSignature,
+    });
+    expect(repo.findReference('refs/heads/no-validate')?.target()).toEqual(oid);
+    expect(repo.head().target()).toEqual(headBefore);
+  });
+
+  it('signed commit rejects an invalid updateRef name like unsigned commits', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    expect(() =>
+      repo.commit(tree, 'signed commit', {
+        updateRef: 'not a valid ref name',
+        author: signature,
+        committer: signature,
+        parents: [repo.head().target()!],
+        signature: gpgSignature,
+      })
+    ).toThrowError(/not valid/);
+  });
+
+  it('signed commit updates detached HEAD', async () => {
+    const p = await useFixture('commits');
+    const repo = await openRepository(p);
+    const tip = repo.head().target()!;
+    repo.setHeadDetached(repo.getCommit(tip));
+    await fs.writeFile(path.join(p, 'signed'), 'signed');
+    const index = repo.index();
+    index.addPath('signed');
+    const tree = repo.getTree(index.writeTree());
+    const oid = repo.commit(tree, 'detached signed commit', {
+      updateRef: 'HEAD',
+      author: signature,
+      committer: signature,
+      parents: [tip],
+      signature: gpgSignature,
+    });
+    expect(repo.head().target()).toEqual(oid);
+    expect(repo.findReference('refs/heads/main')?.target()).toEqual(tip);
+  });
+
   it('extract signature from unsigned commit', async () => {
     const p = await useFixture('commits');
     const repo = await openRepository(p);