Refactor git commit hooks

- the typescript side drives the hook runs
- some very basic tests

Author: mtsgrd

Cargo.lock

@@ -1,10 +1,13 @@
 <script lang="ts">
 	import CommitMessageInput from './CommitMessageInput.svelte';
+	import { PostHogWrapper } from '$lib/analytics/posthog';
 	import ContextMenuItem from '$lib/components/contextmenu/ContextMenuItem.svelte';
 	import ContextMenuSection from '$lib/components/contextmenu/ContextMenuSection.svelte';
 	import { persistedCommitMessage } from '$lib/config/config';
 	import { cloudCommunicationFunctionality } from '$lib/config/uiFeatureFlags';
 	import { SyncedSnapshotService } from '$lib/history/syncedSnapshotService';
+	import { HooksService } from '$lib/hooks/hooksService';
+	import { showError } from '$lib/notifications/toasts';
 	import DropDownButton from '$lib/shared/DropDownButton.svelte';
 	import { intersectionObserver } from '$lib/utils/intersectionObserver';
 	import { BranchController } from '$lib/vbranches/branchController';
@@ -25,30 +28,63 @@
 	const { projectId, expanded, hasSectionsAfter }: Props = $props();
 
 	const branchController = getContext(BranchController);
+	const hooksService = getContext(HooksService);
+	const posthog = getContext(PostHogWrapper);
 	const syncedSnapshotService = getContext(SyncedSnapshotService);
 	const canTakeSnapshot = syncedSnapshotService.canTakeSnapshot;
 	const selectedOwnership = getContextStore(SelectedOwnership);
 	const stack = getContextStore(BranchStack);
 	const commitMessage = persistedCommitMessage(projectId, $stack.id);
+	const canShowCommitAndPublish = $derived($cloudCommunicationFunctionality && $canTakeSnapshot);
 
 	let commitMessageInput = $state<CommitMessageInput>();
 	let isCommitting = $state(false);
 	let commitMessageValid = $state(false);
 	let isInViewport = $state(false);
 
+	let commitAndPublish = $state(false);
+	let commitButton = $state<DropDownButton>();
+
 	async function commit() {
-		const message = $commitMessage;
 		isCommitting = true;
-		try {
-			await branchController.commitBranch($stack.id, message.trim(), $selectedOwnership.toString());
-			$commitMessage = '';
+		const message = $commitMessage;
+		const ownership = $selectedOwnership.toString();
 
-			if (commitAndPublish) {
-				syncedSnapshotService.takeSyncedSnapshot($stack.id);
+		try {
+			const preCommitHook = await hooksService.preCommit(projectId, ownership);
+			if (preCommitHook.status === 'failure') {
+				showError('Pre-commit hook failed', preCommitHook.error);
+				return; // Abort commit if hook failed.
 			}
+			await branchController.commit($stack.id, message.trim(), ownership);
+		} catch (err: unknown) {
+			showError('Failed to commit changes', err);
+			posthog.capture('Commit Failed', { error: err });
+			return;
 		} finally {
 			isCommitting = false;
 		}
+
+		// Run both without awaiting unless commit failed.
+		runPostCommitActions();
+		runPostCommitHook();
+	}
+
+	async function runPostCommitActions() {
+		// Clear the commit message editor.
+		commitMessage.set('');
+
+		// Publishing a snapshot seems to imply posting a bleep.
+		if (commitAndPublish) {
+			await syncedSnapshotService.takeSyncedSnapshot($stack.id);
+		}
+	}
+
+	async function runPostCommitHook() {
+		const postCommitHook = await hooksService.postCommit(projectId);
+		if (postCommitHook.status === 'failure') {
+			showError('Post-commit hook failed', postCommitHook.error);
+		}
 	}
 
 	function close() {
@@ -60,11 +96,6 @@
 		await tick();
 		commitMessageInput?.focus();
 	}
-
-	const canShowCommitAndPublish = $derived($cloudCommunicationFunctionality && $canTakeSnapshot);
-
-	let commitAndPublish = $state(false);
-	let commitButton = $state<DropDownButton>();
 </script>
 
 <div

apps/desktop/src/lib/commit/CommitDialog.svelte

@@ -9,6 +9,7 @@
 		projectCommitGenerationExtraConcise,
 		projectCommitGenerationUseEmojis
 	} from '$lib/config/config';
+	import { HooksService } from '$lib/hooks/hooksService';
 	import { showError } from '$lib/notifications/toasts';
 	import { isFailure } from '$lib/result';
 	import DropDownButton from '$lib/shared/DropDownButton.svelte';
@@ -47,12 +48,14 @@
 	const stack = getContextStore(BranchStack);
 	const project = getContext(Project);
 	const promptService = getContext(PromptService);
+	const hooksService = getContext(HooksService);
 
 	const aiGenEnabled = projectAiGenEnabled(project.id);
 	const commitGenerationExtraConcise = projectCommitGenerationExtraConcise(project.id);
 	const commitGenerationUseEmojis = projectCommitGenerationUseEmojis(project.id);
 
 	let aiLoading = $state(false);
+	let hookRunning = $state(false);
 	let aiConfigurationValid = $state(false);
 
 	let titleTextArea: HTMLTextAreaElement | undefined = $state();
@@ -134,6 +137,22 @@
 		aiLoading = false;
 	}
 
+	async function runMessageHook() {
+		hookRunning = true;
+		try {
+			const hook_result = await hooksService.message(project.id, commitMessage);
+			if (hook_result.status === 'message') {
+				commitMessage = hook_result.message;
+			} else if (hook_result.status === 'failure') {
+				showError('Message hook failed', hook_result.error);
+			}
+		} catch (err: unknown) {
+			showError('Message hook failed', err);
+		} finally {
+			hookRunning = false;
+		}
+	}
+
 	onMount(async () => {
 		aiConfigurationValid = await aiService.validateConfiguration();
 	});
@@ -237,6 +256,7 @@
 			}}
 			onblur={() => {
 				isTitleFocused = false;
+				runMessageHook();
 			}}
 			oninput={(e: Event & { currentTarget: EventTarget & HTMLTextAreaElement }) => {
 				const target = e.currentTarget;
@@ -263,6 +283,7 @@
 				}}
 				onblur={() => {
 					isDescriptionFocused = false;
+					runMessageHook();
 				}}
 				oninput={(e: Event & { currentTarget: EventTarget & HTMLTextAreaElement }) => {
 					const target = e.currentTarget;

apps/desktop/src/lib/commit/CommitMessageInput.svelte

@@ -0,0 +1,41 @@
+import type { Tauri } from '$lib/backend/tauri';
+
+export type HookStatus =
+	| {
+			status: 'success';
+	  }
+	| {
+			status: 'message';
+			message: string;
+	  }
+	| {
+			status: 'notfound';
+	  }
+	| {
+			status: 'failure';
+			error: string;
+	  };
+
+export class HooksService {
+	constructor(private tauri: Tauri) {}
+
+	async preCommit(projectId: string, ownership: string | undefined = undefined) {
+		return await this.tauri.invoke<HookStatus>('pre_commit_hook', {
+			projectId,
+			ownership
+		});
+	}
+
+	async postCommit(projectId: string) {
+		return await this.tauri.invoke<HookStatus>('post_commit_hook', {
+			projectId
+		});
+	}
+
+	async message(projectId: string, message: string) {
+		return await this.tauri.invoke<HookStatus>('message_hook', {
+			projectId,
+			message
+		});
+	}
+}

apps/desktop/src/lib/hooks/hooksService.ts

@@ -52,32 +52,14 @@ export class BranchController {
 		}
 	}
 
-	async runHooks(stackId: string, ownership: string) {
-		await invoke<void>('run_hooks', {
+	async commit(branchId: string, message: string, ownership: string | undefined = undefined) {
+		await invoke<void>('commit_virtual_branch', {
 			projectId: this.projectId,
-			stackId,
+			branch: branchId,
+			message,
 			ownership
 		});
-	}
-
-	async commitBranch(branchId: string, message: string, ownership: string | undefined = undefined) {
-		try {
-			await invoke<void>('commit_virtual_branch', {
-				projectId: this.projectId,
-				branch: branchId,
-				message,
-				ownership
-			});
-			this.posthog.capture('Commit Successful');
-		} catch (err: any) {
-			if (err.code === 'errors.commit.signing_failed') {
-				showSignError(err);
-			} else {
-				showError('Failed to commit changes', err);
-				throw err;
-			}
-			this.posthog.capture('Commit Failed', err);
-		}
+		this.posthog.capture('Commit Successful');
 	}
 
 	async integrateUpstreamForSeries(

apps/desktop/src/lib/vbranches/branchController.ts

@@ -29,6 +29,7 @@
 		GitHubUserService
 	} from '$lib/forge/github/githubUserService';
 	import { octokitFromAccessToken } from '$lib/forge/github/octokit';
+	import { HooksService } from '$lib/hooks/hooksService';
 	import ToastController from '$lib/notifications/ToastController.svelte';
 	import { platformName } from '$lib/platform/platform';
 	import { DesktopDispatch, DesktopState } from '$lib/redux/store.svelte';
@@ -79,6 +80,7 @@
 	setContext(OrganizationService, organizationService);
 	setContext(CloudUserService, cloudUserService);
 	setContext(CloudProjectService, cloudProjectService);
+	setContext(HooksService, data.hooksService);
 
 	// Setters do not need to be reactive since `data` never updates
 	setSecretsService(data.secretsService);

apps/desktop/src/routes/+layout.svelte

@@ -11,6 +11,7 @@ import { Tauri } from '$lib/backend/tauri';
 import { UpdaterService } from '$lib/backend/updater';
 import { loadAppSettings } from '$lib/config/appSettings';
 import { FileService } from '$lib/files/fileService';
+import { HooksService } from '$lib/hooks/hooksService';
 import { RemotesService } from '$lib/remotes/service';
 import { RustSecretService } from '$lib/secrets/secretsService';
 import { TokenMemoryService } from '$lib/stores/tokenMemoryService';
@@ -62,6 +63,7 @@ export const load: LayoutLoad = async () => {
 	const lineManagerFactory = new LineManagerFactory();
 	const stackingLineManagerFactory = new StackingLineManagerFactory();
 	const fileService = new FileService(tauri);
+	const hooksService = new HooksService(tauri);
 
 	return {
 		commandService,
@@ -82,6 +84,7 @@ export const load: LayoutLoad = async () => {
 		secretsService,
 		posthog,
 		tauri,
-		fileService
+		fileService,
+		hooksService
 	};
 };

apps/desktop/src/routes/+layout.ts

@@ -79,4 +79,5 @@ pub use branch::{
 
 pub use integration::GITBUTLER_WORKSPACE_COMMIT_TITLE;
 
+pub mod ownership;
 pub mod stack;

crates/gitbutler-branch-actions/src/lib.rs

@@ -0,0 +1,36 @@
+use std::path::PathBuf;
+
+use anyhow::{anyhow, Result};
+use gitbutler_diff::{DiffByPathMap, GitHunk};
+use gitbutler_stack::BranchOwnershipClaims;
+use itertools::Itertools;
+
+pub fn filter_hunks_by_ownership(
+    diffs: &DiffByPathMap,
+    ownership: &BranchOwnershipClaims,
+) -> Result<Vec<(PathBuf, Vec<GitHunk>)>> {
+    ownership
+        .claims
+        .iter()
+        .map(|claim| {
+            if let Some(diff) = diffs.get(&claim.file_path) {
+                let hunks = claim
+                    .hunks
+                    .iter()
+                    .filter_map(|claimed_hunk| {
+                        diff.hunks
+                            .iter()
+                            .find(|diff_hunk| {
+                                claimed_hunk.start == diff_hunk.new_start
+                                    && claimed_hunk.end == diff_hunk.new_start + diff_hunk.new_lines
+                            })
+                            .cloned()
+                    })
+                    .collect_vec();
+                Ok((claim.file_path.clone(), hunks))
+            } else {
+                Err(anyhow!("Claim not found in workspace diff"))
+            }
+        })
+        .collect::<Result<Vec<(_, Vec<_>)>>>()
+}

crates/gitbutler-branch-actions/src/ownership.rs

@@ -257,7 +257,7 @@ pub(crate) fn reset_files(
 ) -> Result<()> {
     ctx.assure_resolved()?;
 
-    let stack = ctx
+    let branch = ctx
         .project()
         .virtual_branches()
         .list_stacks_in_workspace()
@@ -267,7 +267,7 @@ pub(crate) fn reset_files(
         .with_context(|| {
             format!("could not find applied branch with id {stack_id} to reset files from")
         })?;
-    let claims: Vec<_> = stack
+    let claims: Vec<_> = branch
         .ownership
         .claims
         .into_iter()
@@ -729,6 +729,7 @@ pub fn commit(
     message: &str,
     ownership: Option<&BranchOwnershipClaims>,
 ) -> Result<git2::Oid> {
+    let mut stack = ctx.project().virtual_branches().get_stack(stack_id)?;
     // get the files to commit
     let diffs = gitbutler_diff::workdir(ctx.repo(), get_workspace_head(ctx)?)?;
     let statuses = get_applied_status_cached(ctx, None, &diffs)
@@ -781,8 +782,9 @@ pub fn commit(
 
     let git_repository = ctx.repo();
     let parent_commit = git_repository
-        .find_commit(branch.head())
-        .context(format!("failed to find commit {:?}", branch.head()))?;
+        .find_commit(stack.head())
+        .context(format!("failed to find commit {:?}", stack.head()))?;
+
     let tree = git_repository
         .find_tree(tree_oid)
         .context(format!("failed to find tree {:?}", tree_oid))?;
@@ -807,7 +809,7 @@ pub fn commit(
     };
 
     let vb_state = ctx.project().virtual_branches();
-    branch.set_stack_head(ctx, commit_oid, Some(tree_oid))?;
+    stack.set_stack_head(ctx, commit_oid, Some(tree_oid))?;
 
     crate::integration::update_workspace_commit(&vb_state, ctx)
         .context("failed to update gitbutler workspace")?;