feat(repository): expose commit signing buffer APIs

[poketopa]

Summary

• Expose Repository#commitCreateBuffer to obtain unsigned commit content without writing an object, and Repository#commitSigned to store externally signed content without moving a reference.
• Document commit() with signature and updateRef as the branch-updating signing flow, using fixed author/committer timestamps and an executable GPG invocation in the English and Korean guides.
• Preserve explicit timezone offsets, share commit-content generation, validate signature field names, and remove exactly one trailing LF from supplied signatures.
• Preserve main's signed-commit reference and reflog behavior, with cryptographic round-trip and regression coverage.

Behavior changes

These changes also affect existing Repository#commit() callers compared with 0.7.x:

• Invalid explicit author or committer identities now throw instead of silently falling back to the repository default. Callers that build identities from potentially empty environment variables must provide valid values or omit the identity to use the configured default.
• Unsigned commits now preserve timeOptions.offset instead of writing UTC (+0000). A nonzero offset such as 540 is written as +0900, so the same inputs can produce different commit bytes and OIDs.
• Signed commit() calls remove exactly one trailing LF (\n) from the supplied signature. Raw GPG output can therefore produce a different OID than in 0.7.x. This removes an extra signature-header continuation line without changing the unsigned content covered by the signature.

Validation

• just test: 242 passed, 1 existing Windows-only test skipped on macOS; Rust test target passed (0 tests).
• just lint and just typecheck.
• cargo fmt --all --check and Taplo checks for all tracked TOML files.
• just docs-gen reference --pattern='Repository/Methods/commit*'; repeated generation and builds produce identical index.js, index.d.ts, and English commit reference pages.
• yarn workspace docs build.
• Real SSH signing smoke test verified with git verify-commit, including +0900, trailing-LF normalization, matching object IDs, and the HEAD update.

Closes #219

[vercel]

@poketopa is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

[marshallku]

Nice feature, and the round-trip test (commitCreateBuffer → real RSA signature → verify) is a good way to pin it down. On this branch, cargo fmt/clippy/test, biome, tsc and commit.spec.ts (9/9) pass, and index.d.ts matches a fresh build.

Merge conflict. This now conflicts with main in tests/commit.spec.ts because of #225 (updateRef on signed commits). src/commit.rs merges cleanly. The resolution I tested, with commit.spec.ts passing 18/18:

1. Keep both constant blocks: this PR's fixture helpers and main's gpgSignature.
2. In create signed commit, keep this PR's body but add updateRef: 'HEAD' back. The auto-merged expect(repo.head().target()).toEqual(oid) needs it.
3. Keep this PR's final assertions (externalSignature, signedData === content, verify) and drop main's toEqual(gpgSignature).

How it interacts with #225. Since #225, commit({ signature, updateRef }) writes the object, moves the ref and records a reflog entry, while commitSigned only writes the object. The docs should lead with the flow that moves a branch. See the inline comment on docs/usage/commit.md.

Severity: P1 must fix before merge · P2 should fix before merge · P3 worth fixing · P4 suggestion.

[marshallku]

P3 · merge — This is the spot that conflicts with #225. When resolving, add updateRef: 'HEAD' back to these options: main now asserts repo.head().target() equals oid right after this call. The full resolution is in the review summary.

[poketopa]

Merged current main in fb816ec without rewriting the existing contribution. The resolution retains both fixture blocks, restores updateRef: 'HEAD', keeps the HEAD assertion and the cryptographic round-trip assertions, and preserves all #225 ref/reflog tests. The final commit suite passes 31/31; the full suite passes 242 tests with one existing Windows-only test skipped on macOS.

[poketopa]

@marshallku I addressed the eight inline comments in 740b3d2 and merged current main in fb816ec without rewriting the existing contribution. Each thread has a reply with the change and validation. Follow-up 2a63bc2 removes a Git CLI dependency from the custom-field regression test after Linux CI exposed it.

The commit tests pass 31/31 even without Git on PATH; the full local suite passes 242 tests with one existing Windows-only test skipped on macOS. Lint, typecheck, documentation build, and generated-file reproducibility checks pass. A real SSH-signed commit also passes git verify-commit.

The CI run for 2a63bc2 now passes all 37 jobs, including all Linux, macOS, and Windows test combinations. Vercel remains blocked on Toss team deployment authorization.

Could you take another look? GitHub rejected the formal review request because my account lacks RequestReviews permission.

[marshallku]

Thanks for the thorough follow-up. I re-verified each thread against 2a63bc2, and every one is addressed: the merge resolution keeps all #225 tests, cargo fmt/clippy/test, biome, tsc and commit.spec.ts (31/31) pass locally, the bindings match a fresh build, and CI is green. It also merges cleanly with #217.

One thing before merge. The new helpers change the behaviour of the existing commit(), not only the new APIs, and none of it is visible in the docs or in what ends up in the changelog. Release notes here are generated from the squash-merge subject alone (cliff.toml), so a feat(repository): expose commit signing buffer APIs entry won't tell anyone about these:

1. Invalid explicit identities now throw (behaviour change). On main, commit() swallowed a conversion failure with .ok() and silently committed as the repository's default identity, or threw SignatureNotFound if none was configured. Now commit(tree, 'm', { author: { name: '', email: 'x@y' } }) throws failed to parse signature - Signature cannot have an empty name or email; a name containing </> or a NUL byte throws too. The new behaviour is the right one, since silently committing as someone else was a bug, but code that builds identities from possibly-empty env vars will start failing.
2. Unsigned commit() now honours timeOptions.offset. +0900 used to be written as +0000. This is a fix of a documented field, but the same inputs now produce a different OID.
3. commit({ signature }) strips one trailing LF. The object bytes now match what the git CLI writes for a signed commit, and signatures stay valid, but callers passing raw gpg output get a different OID than on 0.7.x.

Requested:

• Add a @throws (and the missing @params) to commit(). Suggested text is inline.
• Call these out so they reach users: either a ! in the title (e.g. feat(repository)!: expose commit signing buffer APIs), or a short "Behavior changes" section in the PR description that we can carry into the squash subject and release notes.

Follow-up, not for this PR: commit.amend() still falls back silently on an invalid identity and still drops the offset, as do note, stash and rebase. After this PR and #217, commit() and amend() behave differently for the same payload. A follow-up issue to move them onto the new SignaturePayload → git2::Signature conversion would close that gap.

Severity: P1 must fix before merge · P2 should fix before merge · P3 worth fixing · P4 suggestion.

[marshallku]

P2 · docs — commit() documents neither its parameters nor its errors, and with this PR it gains a new failure mode for invalid identities. This comment is the source of the reference page, so suggest:

  /// @param {Tree} tree - Tree of the commit.
  /// @param {string} message - Full commit message.
  /// @param {CommitOptions} [options] - Options for creating the commit.
  /// @returns ID(SHA1) of created commit.
  /// @throws If `author` or `committer` is given but is not a valid signature (for example an empty
  /// name or email, or one containing `<`, `>` or a NUL byte), if no identity is given and no default
  /// signature is configured, if a parent commit does not exist, if `signatureField` is empty or
  /// contains whitespace or NUL bytes, or if `updateRef` cannot be updated.

Then regenerate docs/reference/Repository/Methods/commit.md and the ko page.

[poketopa]

Added the three @param entries and @throws to commit() in a71ff3b. The errors cover invalid explicit identities, missing repository defaults for omitted identities, missing parents, ref-update failures, and invalid signature fields for signed commits. The text explicitly says invalid identities do not fall back to the repository default. Rebuilt index.d.ts, regenerated the English and Korean commit references, and retained the existing Korean translations. just test (242 passed, one existing Windows-only skip on macOS), just lint, just typecheck, the documentation build, and generated-file reproducibility checks passed.

[marshallku]

P2 · compatibility — This is where the existing commit() changes behaviour. On main the equivalent code was:

author
  .and_then(|x| git2::Signature::try_from(x).ok())
  .or_else(|| self.inner.signature().ok())
  .ok_or(crate::Error::SignatureNotFound)?

So an invalid explicit identity silently became the repository default. Now it propagates the libgit2 error. I agree this is the correct behaviour; please just make it explicit (see the @throws suggestion and the review summary) so it isn't a surprise in a feat: release.

[poketopa]

Added a Behavior changes section to the PR description covering all three effects on existing commit() callers: invalid explicit identities now throw, unsigned commits preserve nonzero offsets and can have different OIDs, and signed commits remove one trailing LF from the signature and can have different OIDs. The new @throws and both language references also document the identity error. This follow-up changes documentation only; the other identity-consuming APIs remain outside this PR.

[poketopa]

@marshallku The documentation follow-up is in a71ff3b. commit() now documents all three parameters and its identity, default-identity, parent, reference-update, and signed-field errors. The English and Korean reference pages have been updated.

The PR description now has a Behavior changes section covering invalid explicit identities throwing instead of falling back, unsigned commits preserving offsets, and signed commits removing one trailing LF, including the OID compatibility effects. The other identity-consuming APIs remain outside this PR.

Local validation passed: 242 tests with one existing Windows-only skip on macOS, lint, typecheck, Rust formatting, documentation build, and generated-file reproducibility. CI for a71ff3b passes all 37 jobs. Vercel still requires Toss team deployment authorization.

Could you review the documentation follow-up?

[marshallku]

Thanks. I checked a71ff3b against the two remaining threads:

• commit() now documents all three parameters and the new failure modes, and index.d.ts plus the English and Korean reference pages were regenerated from it. The Korean page also picks up the updated parameter descriptions.
• The Behavior changes section in the description covers all three effects on existing commit() callers, including the OID differences.

CI is green on all 37 jobs. When squash-merging, please keep a ! in the subject (e.g. feat(repository)!: expose commit signing buffer APIs) or carry the Behavior changes section into the commit body, since the release notes are generated from the commit, not the PR description.

The amend/note/stash/rebase identity handling stays a follow-up, as discussed.