feat: add SSH commit signing support - #20
Merged
Merged
Conversation
Reuse existing per-profile SSH keys for Git commit signing (Git >= 2.34). Adds interactive prompt during `ghem add` and `ghem edit`, generates gpgsign/gpg.format/signingkey in profile gitconfig, and shows signing status in `ghem status` and `ghem list`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds per-profile SSH commit signing: new Profile field, CLI prompts on add/edit guarded by Git >= 2.34 detection, gitconfig generation including SSH signing entries, list/status display of signing state, i18n additions, and a new git-version helper plus atomic write helper. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/edit.ts (1)
114-121:⚠️ Potential issue | 🔴 CriticalAlways persist
commitSigningto prevent data loss when disabling signing.Line 120 conditionally omits
commitSigningwhen false. SinceupdateProfilereplaces the entire profile object, false values are lost rather than persisted. A user disabling commit signing will have that change silently dropped.Change:
const updatedProfile = { name: profileName, gitUserName, gitUserEmail, sshKeyPath, directories, - ...(commitSigning && { commitSigning }), + commitSigning, };Note:
src/commands/add.tshas the same pattern at line 97 and needs the same fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/edit.ts` around lines 114 - 121, The object literal for updatedProfile currently uses a conditional spread that omits commitSigning when it's falsy, causing false to be lost when updateProfile replaces the entire profile; change the construction to always include the commitSigning property (e.g., set commitSigning: commitSigning) so false persists, and apply the same fix in the analogous object in add.ts (the place where updatedProfile / new profile is built before calling updateProfile).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/add.ts`:
- Around line 70-73: The prompt currently accepts commitSigning without
verifying Git version; add a Git version check (require >= 2.34) before
persisting the user's choice in both the add and edit flows. After you gather
the user's answer (commitSigning from confirm/t().commitSigningPrompt) call a
helper like ensureGitVersionOrThrow or inline a check using spawn/exec to run
`git --version`, parse the semver, and if the installed Git is < 2.34,
override/reject enabling commitSigning (set commitSigning = false) and surface a
clear message/error to the user; update both the code paths that set/persist
commitSigning in src/commands/add (where commitSigning is declared) and
src/commands/edit (where commitSigning is handled) so they use the same
validation function/name. Ensure the version check runs before any persistence
step so unsupported configurations are never saved.
In `@src/commands/list.ts`:
- Around line 47-49: The hardcoded signing badge text is rendered in
src/commands/list.ts where signing is set to profile.commitSigning ?
chalk.cyan(' [signing]') : ''; change this to use the localization/messages API
instead of a literal string: fetch the appropriate localized string key (e.g.,
messages.get('commands.list.signing') or msg('commands.list.signing')) and use
that value inside chalk.cyan when building the signing variable, then keep the
console.log(`${marker}${name}${email}${dirs}${signing}`) usage; ensure the
messages import/lookup used in other command files is reused so the badge is
localized for all locales.
In `@src/core/gitconfig.ts`:
- Line 56: The current direct call to writeFileSync(configPath,
lines.join('\n'), 'utf-8') can corrupt gitconfig-{profile} if interrupted;
update the profile gitconfig generation to perform an atomic write instead
(e.g., write to a temp file and atomically rename or use a library like
write-file-atomic) so the final replace of configPath is atomic; locate the
writeSite by the writeFileSync invocation and the variables configPath and lines
(or the surrounding function that assembles the gitconfig content) and replace
that call with an atomic-write helper that writes the joined lines and then
atomically moves them into place.
---
Outside diff comments:
In `@src/commands/edit.ts`:
- Around line 114-121: The object literal for updatedProfile currently uses a
conditional spread that omits commitSigning when it's falsy, causing false to be
lost when updateProfile replaces the entire profile; change the construction to
always include the commitSigning property (e.g., set commitSigning:
commitSigning) so false persists, and apply the same fix in the analogous object
in add.ts (the place where updatedProfile / new profile is built before calling
updateProfile).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 40891d89-9555-4ab0-8a27-a7abef60f6b9
📒 Files selected for processing (11)
README.mddocs/ko-KR/README.mdsrc/commands/add.tssrc/commands/edit.tssrc/commands/list.tssrc/commands/status.tssrc/core/gitconfig.tssrc/i18n/locales/en.tssrc/i18n/locales/ko.tssrc/i18n/types.tssrc/types/config.ts
- Always persist commitSigning so disabling is not silently dropped - Check Git >= 2.34 before prompting for SSH signing, warn and skip otherwise - Localize list command signing badge via i18n - Write profile gitconfig atomically to prevent corruption on interrupt
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/edit.ts`:
- Around line 82-95: The code wrongly flips commitSigning to false when
supportsSshCommitSigning() returns false, which rewrites a previously-signed
profile; instead, when supportsSshCommitSigning() is false and currentSigning is
true, log the warning via logger.warn(t().commitSigningUnsupported) but do not
change the data—leave commitSigning set to currentSigning so
generateProfileGitconfig() preserves signing config; only prompt and allow
changes when supportsSshCommitSigning() is true (functions/variables referenced:
supportsSshCommitSigning(), currentSigning, commitSigning,
generateProfileGitconfig()).
In `@src/core/gitconfig.ts`:
- Around line 21-24: The writeFileAtomic function currently uses a fixed temp
name `${path}.ghem-tmp` which allows concurrent processes to collide; update
writeFileAtomic to generate a unique temp path per invocation (e.g., include
process.pid plus a random/UUID or use a per-write tmp file in the same directory
via mkdtemp semantics) before writeFileSync and renameSync, ensure the temp file
is created in the same directory as path to avoid cross-device rename issues,
and make sure any temp-file cleanup on error is handled so renameSync in
writeFileAtomic cannot stomp or fail due to another process's temp file;
reference the writeFileAtomic function to implement these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7bf2cc6-4bde-41d3-9325-e39753f14961
📒 Files selected for processing (8)
src/commands/add.tssrc/commands/edit.tssrc/commands/list.tssrc/core/git.tssrc/core/gitconfig.tssrc/i18n/locales/en.tssrc/i18n/locales/ko.tssrc/i18n/types.ts
- edit: keep existing commitSigning value when Git < 2.34 (warn only) so previously-signed profiles are not silently rewritten - gitconfig: use pid+random suffix for atomic temp file and clean up on failure to avoid collisions between concurrent processes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reuse existing per-profile SSH keys for Git commit signing (Git >= 2.34). Adds interactive prompt during
ghem addandghem edit, generates gpgsign/gpg.format/signingkey in profile gitconfig, and shows signing status inghem statusandghem list.Summary by CodeRabbit
Documentation
New Features
Localization