Skip to content

Commit f6da5aa

Browse files
committed
Changes graph commits to honor VS Code's commit signing setting
Adds an explicit `-S` to the commit operation when the host-level signing override (`git.enableCommitSigning`) is enabled, matching the built-in Source Control commit behavior — previously only the repo's `commit.gpgsign` Git config was respected. Fires the `onSigned` hook on explicitly-signed commits (parity with the patch provider) and threads a `graph` telemetry source through the commit RPC.
1 parent 46bef5e commit f6da5aa

6 files changed

Lines changed: 108 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
2525
- Format string examples are rendered by the real formatter, so the preview always matches what GitLens displays
2626
- Adds _Cloud Integrations_ and _AI_ categories — view and connect hosting and issue service integrations, and manage the AI provider and model, GitKraken MCP, default coding agent, and Claude Code hooks
2727
- Shows connection-aware cues in the category rail — a connected/total count for _Cloud Integrations_ and a rule count for _Autolinks_
28+
- Changes commits created from the _Commit Graph_'s working changes (WIP) commit box to honor VS Code's `git.enableCommitSigning` setting — matching the built-in Source Control commit behavior; previously only the repo's `commit.gpgsign` Git config was respected
2829

2930
### Fixed
3031

packages/git-cli/src/providers/__tests__/integration/helpers.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, wr
1010
import { readFile } from 'node:fs/promises';
1111
import { tmpdir } from 'node:os';
1212
import { join } from 'node:path';
13-
import type { FileSystemProvider, GitServiceContext, GitServiceHooks } from '@gitlens/git/context.js';
13+
import type { FileSystemProvider, GitServiceConfig, GitServiceContext, GitServiceHooks } from '@gitlens/git/context.js';
1414
import { Logger } from '@gitlens/utils/logger.js';
1515
import { toFsPath } from '@gitlens/utils/uri.js';
1616
import { CliGitProvider } from '../../../cliGitProvider.js';
@@ -61,10 +61,11 @@ function ensureLogger() {
6161
});
6262
}
6363

64-
function createMinimalContext(hooks?: GitServiceHooks): GitServiceContext {
64+
function createMinimalContext(hooks?: GitServiceHooks, config?: GitServiceConfig): GitServiceContext {
6565
return {
6666
fs: createNodeFs(),
6767
hooks: hooks,
68+
config: config,
6869
};
6970
}
7071

@@ -99,7 +100,11 @@ function createNodeFs(): FileSystemProvider {
99100
*
100101
* Call `cleanup()` in your `teardown()` / `suiteTeardown()`.
101102
*/
102-
export function createTestRepo(options?: { hooks?: GitServiceHooks; gitOptions?: GitOptions }): TestRepo {
103+
export function createTestRepo(options?: {
104+
hooks?: GitServiceHooks;
105+
gitOptions?: GitOptions;
106+
config?: GitServiceConfig;
107+
}): TestRepo {
103108
ensureLogger();
104109

105110
const dir = mkdtempSync(join(tmpdir(), 'gitlens-test-'));
@@ -123,7 +128,7 @@ export function createTestRepo(options?: { hooks?: GitServiceHooks; gitOptions?:
123128
env: { ...process.env, GIT_COMMITTER_DATE: '2024-01-01T00:00:00Z', GIT_AUTHOR_DATE: '2024-01-01T00:00:00Z' },
124129
});
125130

126-
const context = createMinimalContext(options?.hooks);
131+
const context = createMinimalContext(options?.hooks, options?.config);
127132
const provider = new CliGitProvider({
128133
context: context,
129134
locator: getGitLocation,

packages/git-cli/src/providers/__tests__/integration/operations.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,80 @@ suite('OperationsGitSubProvider signing', () => {
254254
r.cleanup();
255255
}
256256
});
257+
258+
test('host signing override adds -S even when commit.gpgsign is false', async () => {
259+
const failedCalls: Array<{ reason: SigningErrorReason; format: SigningFormat }> = [];
260+
const signedCalls: unknown[] = [];
261+
const r = createTestRepo({
262+
config: { commits: {}, signing: { enabled: true } },
263+
hooks: {
264+
commits: {
265+
onSigned: (...args) => signedCalls.push(args),
266+
onSigningFailed: (reason, format) => failedCalls.push({ reason: reason, format: format }),
267+
},
268+
},
269+
});
270+
try {
271+
// The helper leaves `commit.gpgsign=false` — without an explicit `-S` from the host
272+
// override, git would never invoke the (broken) gpg program and the commit would
273+
// succeed. A SigningError here is the proof that `-S` was passed.
274+
execFileSync('git', ['config', 'gpg.format', 'openpgp'], { cwd: r.path, stdio: 'pipe' });
275+
execFileSync('git', ['config', 'gpg.program', 'node --eval process.exit(1)'], {
276+
cwd: r.path,
277+
stdio: 'pipe',
278+
});
279+
280+
writeFileSync(join(r.path, 'override.txt'), 'content\n');
281+
execFileSync('git', ['add', 'override.txt'], { cwd: r.path, stdio: 'pipe' });
282+
283+
await assert.rejects(
284+
() => r.provider.ops.commit(r.path, 'should attempt to sign via override'),
285+
ex => SigningError.is(ex),
286+
'Expected a SigningError — the override should force `-S` despite commit.gpgsign=false',
287+
);
288+
289+
assert.strictEqual(failedCalls.length, 1, 'Expected onSigningFailed hook to fire exactly once');
290+
assert.strictEqual(failedCalls[0].format, 'openpgp');
291+
assert.strictEqual(signedCalls.length, 0, 'onSigned must not fire when signing fails');
292+
} finally {
293+
r.cleanup();
294+
}
295+
});
296+
297+
test('without the host override, broken signer config does not affect commits', async () => {
298+
const calls: unknown[] = [];
299+
const r = createTestRepo({
300+
config: { commits: {}, signing: { enabled: false } },
301+
hooks: {
302+
commits: {
303+
onSigned: (...args) => calls.push(args),
304+
onSigningFailed: (...args) => calls.push(args),
305+
},
306+
},
307+
});
308+
try {
309+
// Same broken gpg program as above, but no override and `commit.gpgsign=false` —
310+
// the commit must go through without ever invoking the signer.
311+
execFileSync('git', ['config', 'gpg.program', 'node --eval process.exit(1)'], {
312+
cwd: r.path,
313+
stdio: 'pipe',
314+
});
315+
316+
writeFileSync(join(r.path, 'plain.txt'), 'content\n');
317+
execFileSync('git', ['add', 'plain.txt'], { cwd: r.path, stdio: 'pipe' });
318+
319+
await r.provider.ops.commit(r.path, 'unsigned commit');
320+
321+
const log = execFileSync('git', ['log', '-1', '--format=%s'], {
322+
cwd: r.path,
323+
encoding: 'utf-8',
324+
}).trim();
325+
assert.strictEqual(log, 'unsigned commit');
326+
assert.strictEqual(calls.length, 0, 'No signing hooks should fire for an unsigned commit');
327+
} finally {
328+
r.cleanup();
329+
}
330+
});
257331
});
258332

259333
suite('OperationsGitSubProvider.push', () => {

packages/git-cli/src/providers/operations.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ export class OperationsGitSubProvider implements GitOperationsSubProvider {
183183
if (options?.date) {
184184
params.push(`--date=${options.date}`);
185185
}
186+
// Host-level override (e.g. VS Code's `git.enableCommitSigning`) — request signing explicitly
187+
// via `-S`. Implicit repo-config signing (`commit.gpgsign=true`) needs no flag, and `-S`
188+
// alongside it is harmless, so the override can only enable signing, never force it off.
189+
const sign = this.context.config?.signing?.enabled === true;
190+
if (sign) {
191+
params.push('-S');
192+
}
186193
// Read commit message from stdin via -F - to avoid shell escaping issues
187194
params.push('-F', '-');
188195

@@ -193,6 +200,15 @@ export class OperationsGitSubProvider implements GitOperationsSubProvider {
193200
);
194201
this.context.hooks?.cache?.onReset?.(repoPath, 'branches', 'status');
195202
this.context.hooks?.repository?.onChanged?.(repoPath, ['head', 'heads', 'index']);
203+
if (sign) {
204+
// `-S` was passed and the commit succeeded, so signing is confirmed — fire the
205+
// explicit-sign hook (same contract as the patch provider's `commit-tree -S` path).
206+
let format: SigningFormat | undefined;
207+
try {
208+
format = (await this.provider.config.getSigningConfig?.(repoPath))?.format;
209+
} catch {}
210+
this.context.hooks?.commits?.onSigned?.(format ?? 'gpg', options?.source);
211+
}
196212
} catch (ex) {
197213
scope?.error(ex);
198214
await this.throwIfSigningError(repoPath, params, ex, options?.source);

packages/git/src/context.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,10 @@ export interface GitServiceHooks {
137137
/**
138138
* Called when a commit is signed successfully.
139139
*
140-
* Note: currently fired only from explicit-sign paths in the patch provider
141-
* (where signing is requested via `-S` and confirmed up front). Operations
142-
* that sign implicitly via `commit.gpgsign=true` (`commit`, `merge`, `pull`,
140+
* Note: currently fired only from explicit-sign paths — the patch provider's
141+
* `commit-tree -S` and the commit operation when the host signing override
142+
* (`config.signing.enabled`) adds `-S`. Operations that sign implicitly via
143+
* `commit.gpgsign=true` (`commit` without the override, `merge`, `pull`,
143144
* `rebase`, `revert`, `cherryPick`) do not fire this hook because the
144145
* library cannot cheaply confirm that signing actually occurred without an
145146
* extra `git config`/`log --show-signature` call.

src/webviews/rpc/services/repository.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { normalizePath } from '@gitlens/utils/path.js';
2525
import { getSettledValue } from '@gitlens/utils/promise.js';
2626
import { pluralize } from '@gitlens/utils/string.js';
2727
import type { DiffWithCommandArgs } from '../../../commands/diffWith.js';
28+
import type { Source } from '../../../constants.telemetry.js';
2829
import type { Container } from '../../../container.js';
2930
import { ProviderNotSupportedError } from '../../../errors.js';
3031
import type { FeatureAccess, PlusFeatures } from '../../../features.js';
@@ -835,7 +836,9 @@ export class RepositoryService {
835836
options?: { all?: boolean; amend?: boolean },
836837
): Promise<CommitResult> {
837838
try {
838-
await this.container.git.getRepositoryService(repoPath).ops?.commit(message, options);
839+
await this.container.git
840+
.getRepositoryService(repoPath)
841+
.ops?.commit(message, { ...options, source: { source: 'graph' } satisfies Source });
839842
return { status: 'committed' };
840843
} catch (ex) {
841844
const failure = classifyCommitFailure(ex);

0 commit comments

Comments
 (0)