feat(ui): use backend normalize method to validate user-entered branch names by nshcr · Pull Request #12488 · gitbutlerapp/gitbutler

nshcr · 2026-02-22T02:17:19Z

🧢 Changes

Before:

Screen.Recording.2026-02-21.at.12.07.02.mov

After:

Screen.Recording.2026-02-21.at.21.10.04.mov

Screen.Recording.2026-02-21.at.21.13.45.mov

This PR updates the frontend logic for branch name normalization when creating or renaming branches via modal dialogs. Instead of relying on the frontend slugify function, it now calls a backend API to perform normalized form input validation, and prevents submission early if the branch name is invalid.

(Since the submit handler also invokes the same normalize function for a second check, failing to block invalid input during validation would result in an error being thrown on submit.)

References:

Frontend slugify function:

gitbutler/packages/ui/src/lib/utils/string.ts

Lines 39 to 47 in 36b7994

    
           export function slugify(input: string) { 
        
           	return String(input) 
        
           		.normalize('NFKD') 
        
           		.replace(/[\u0300-\u036f]/g, '') 
        
           		.trim() 
        
           		.replace(/[^A-Za-z0-9._/ -]/g, '') 
        
           		.replace(/\s+/g, '-') 
        
           		.replace(/-+/g, '-'); 
        
           }

Backend normalize function:

gitbutler/crates/but-core/src/branch/normalize.rs

Lines 4 to 6 in 36b7994

    
           /// Assuming `name` is a short-name like `feat/hi`, `main` or `something the user typed`, convert it into 
        
           /// a version of this which would be a valid *short* reference name. 
        
           pub fn normalize_short_name<'a>(name: impl Into<&'a BStr>) -> anyhow::Result<BString> {

☕️ Reasoning

This change primarily helps allow users to create or rename branches using characters beyond just alphanumeric ones, aligning the behavior with the existing logic used when renaming a branch by clicking the branch header (not using slugify, allowing a broader range of characters).

With this unified approach, branch name normalization will rely solely on backend functions. This makes it easier to extend normalization logic in the future from the backend side (e.g., filtering special characters, enforcing lowercase rules, etc.).

🎫 Affected issues

Related PR: fix: preserve # in branch names during frontend slugification #12322

This PR addresses the broader problem targeted in #12322. Instead of continuously modifying the frontend slugify function to support more special characters, it shifts the responsibility to backend-based normalization.

Related issue: Branch names should be lower case #4786

Moving branch name normalization entirely to the backend will also make it easier to enforce an all-lowercase rule in the future. This would primarily be a backend-driven feature, with the frontend simply reflecting the consistent behavior in the UI.

Fixes: Removal of hashtag in branch name #9076

vercel · 2026-02-22T02:17:24Z

@nshcr is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

Copilot

Pull request overview

This PR refactors branch name validation in the frontend to use a backend API call instead of the frontend slugify function. The change allows users to create and rename branches using a wider range of characters (such as # and @) that are valid in Git ref names but were previously stripped by the frontend's restrictive character allowlist.

Changes:

Replaced frontend slugify validation with async backend normalizeBranchName API calls
Added debounced validation (300ms) with loading state and error handling
Updated all branch name input modals to use the new validation state

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
apps/desktop/src/components/BranchNameTextbox.svelte	Replaced synchronous `slugify` with async backend normalization, added validation state tracking, loading spinner, and race condition protection
apps/desktop/src/components/CreateBranchModal.svelte	Added `isBranchNameValid` state and updated button disabled logic to use validation state instead of checking for non-empty input
apps/desktop/src/components/ChangedFilesContextMenu.svelte	Added `isStashBranchNameValid` state for stash-into-branch modal and updated button disabled logic
apps/desktop/src/components/BranchRenameModal.svelte	Added `isBranchNameValid` state and updated button disabled logic
apps/desktop/src/components/AddDependentBranchModal.svelte	Added `isBranchNameValid` state and updated button disabled logic

apps/desktop/src/components/BranchNameTextbox.svelte

apps/desktop/src/components/CreateBranchModal.svelte

apps/desktop/src/components/ChangedFilesContextMenu.svelte

apps/desktop/src/components/BranchRenameModal.svelte

apps/desktop/src/components/AddDependentBranchModal.svelte

Copilot · 2026-02-22T02:22:09Z

apps/desktop/src/components/BranchNameTextbox.svelte

+		} finally {
+			if (value === inputValue) {
+				isValidating = false;
+			}
+		}


There's a potential race condition in the validation state management. When a user types quickly, multiple API calls can be in flight simultaneously. When an outdated API call completes, the finally block sets isValidating = false even though a newer validation is still pending. This could cause the spinner to briefly disappear and the button to become enabled prematurely. Consider tracking a unique identifier for each validation request or canceling previous requests when a new one starts.

This race condition was resolved by adding a counter to the debounced function call and checking whether the counter has changed before actually updating the state, ensuring that no additional async calls are still in progress.
I believe this implementation is sound. Leaving this thread unresolved for further review.

…lidation Applied changes from PR review gitbutlerapp#12488: - Renamed onslugifiedvalue callback to onnormalizedvalue - Renamed slugifiedRefName variables to normalizedRefName across all components - Fixed race condition in validation state using validationCounter pattern Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-22T02:57:37Z

apps/desktop/src/components/BranchNameTextbox.svelte

+		} catch {
+			if (value === inputValue && currentValidation === validationCounter) {
+				normalizedResult = undefined;
+				onnormalizedvalue?.(undefined);
+				validationError = 'Invalid branch name';
+			}


Consider logging the caught error for debugging purposes. Currently, when the normalization API call fails, the error is silently caught and only a generic 'Invalid branch name' message is shown. Logging the actual error would help diagnose issues with branch name validation. For example: catch (err) { console.error('Branch name normalization failed:', err); ... }

Since validation runs dynamically as the user types, I think it's acceptable to silently catch the error and display a generic error message (IPC errors are printed to the console by default and it's difficult to directly retrieve the message from the error object).
Leaving this conversation as is.

Copilot · 2026-02-22T02:57:37Z

apps/desktop/src/components/CreateBranchModal.svelte

@@ -91,22 +92,22 @@
 			await createNewStack({
 				projectId,
 				branch: {
-					name: slugifiedRefName,
+					name: normalizedRefName,
 					// If addToLeftmost is true, place at position 0 (leftmost)
 					// Otherwise, leave undefined to append to the right
 					order: $addToLeftmost ? 0 : undefined
 				}
 			});
 			createRefModal?.close();


The stack creation path should include a defensive check for normalizedRefName similar to the dependent branch path on line 103. While the submit button is disabled when the branch name is invalid, adding this check would make the code more defensive and consistent with other similar functions in the codebase (e.g., AddDependentBranchModal line 25, BranchRenameModal line 44, and the dependent branch case on line 103).

I don't think the defensive check here is necessary. The createNewStack method accepts a name of type string | undefined, and normalizedRefName conforms to that type definition. If something goes wrong, the backend will panic.
Leaving this conversation as is.

Byron · 2026-02-22T08:33:27Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ae61c2656

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

apps/desktop/src/components/BranchNameTextbox.svelte

Byron · 2026-02-22T09:50:01Z

Thanks so much for contributing this fix! This seems to be a follow-up to one of my recent modifications to this code where I was thinking the same - the backend should be in control of handling this and act as source of truth.

I tried it locally and could see it working, but found the debouncing to make it feel a little sluggish. With this patch it felt alright.

diff --git a/apps/desktop/src/components/BranchNameTextbox.svelte b/apps/desktop/src/components/BranchNameTextbox.svelte
index 1b7fcf79b6..ecd31ac3d5 100644
--- a/apps/desktop/src/components/BranchNameTextbox.svelte
+++ b/apps/desktop/src/components/BranchNameTextbox.svelte
@@ -78,7 +78,7 @@
 				isValidating = false;
 			}
 		}
-	}, 300);
+	}, 100);
 
 	$effect(() => {
 		debouncedNormalize(value || '');

With that said, I don't know enough to be able to merge it, so let's have a separate review.

nshcr · 2026-02-23T07:08:46Z

Recording the issue pointed out during the Codex review (#12488 (comment)), along with the problem in the current (uncommitted) fix.

Steps to reproduce:

Screen.Recording.2026-02-23.at.14.58.55.mov

A potential fix, and the issues with that fix:

Screen.Recording.2026-02-23.at.14.59.37.mov

Specific fix implementation code:

diff --git a/apps/desktop/src/components/BranchNameTextbox.svelte b/apps/desktop/src/components/BranchNameTextbox.svelte
index 1b7fcf79b..6e865f005 100644
--- a/apps/desktop/src/components/BranchNameTextbox.svelte
+++ b/apps/desktop/src/components/BranchNameTextbox.svelte
@@ -30,7 +30,11 @@
 	let normalizedResult = $state<{ fromValue: string; normalized: string } | undefined>();
 
 	const isValidState = $derived(
-		!isValidating && !validationError && !!value && !!normalizedResult?.normalized
+		!isValidating &&
+			!validationError &&
+			!!value &&
+			!!normalizedResult?.normalized &&
+			normalizedResult.fromValue === value
 	);
 	$effect(() => {
 		onvalidationchange?.(isValidState);
@@ -78,7 +82,7 @@
 				isValidating = false;
 			}
 		}
-	}, 300);
+	}, 100);
 
 	$effect(() => {
 		debouncedNormalize(value || '');

Byron · 2026-02-23T11:14:10Z

I am glad you could reproduce the race and have a fix for it!

Also, I put it back to draft while there are conflicts due to the style changes. Please feel free to take it out of draft once it can get a final review.

…h names refactor: rename slugified to normalized and fix race condition in validation Applied changes from PR review gitbutlerapp#12488: - Renamed onslugifiedvalue callback to onnormalizedvalue - Renamed slugifiedRefName variables to normalizedRefName across all components - Fixed race condition in validation state using validationCounter pattern Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-23T12:19:05Z

apps/desktop/src/components/BranchNameTextbox.svelte

 	$effect(() => {
-		onslugifiedvalue?.(slugifiedName);
+		debouncedNormalize(value || "");
 	});


The $effect that calls debouncedNormalize doesn't provide a cleanup function to clear pending timeouts when the component unmounts or when the value changes. This could lead to memory leaks or stale API calls completing after the component is destroyed.

Based on the pattern used elsewhere in the codebase (e.g., UnassignedViewForgePrompt.svelte:47), the effect should return a cleanup function. However, the current debounce utility doesn't expose a way to cancel pending calls.

Consider either:

Enhancing the debounce utility to return an object with both the debounced function and a cancel() method, or

Creating a custom debounced effect that manages its own timeout state within the component and returns a cleanup function that clears the timeout.

The effect here doesn't need cleanup. The debounced function already has an internal mechanism to ensure that only the last invocation is executed, and the code also prevents concurrent state updates.
Adding a cleanup function would only increase code complexity in exchange for eliminating just one extra 20ms invocation.

Copilot · 2026-02-23T12:19:06Z

apps/desktop/src/components/CreateBranchModal.svelte

@@ -145,7 +146,8 @@
 			id={ElementId.NewBranchNameInput}
 			value={createRefName}


The value prop should be a two-way binding using bind:value={createRefName} instead of just value={createRefName}. Without the bind: directive, the createRefName variable won't update as the user types, and the component will only display the initial value fetched from the backend. This is inconsistent with other usages of BranchNameTextbox in the codebase (e.g., BranchRenameModal.svelte:54, AddDependentBranchModal.svelte:54, ChangedFilesContextMenu.svelte:652).

Suggested change

value={createRefName}

bind:value={createRefName}

This variable is mainly used to provide an initial value when opening the modal for creating a new branch. It does not participate in subsequent updates, so two-way binding isn't necessary.

nshcr · 2026-02-23T12:46:20Z

I've included the fix mentioned in my previous comment in the latest commit.
It's not perfect - it guards against passing stale state during rapid interactions, but it also introduces a button flickering issue. More review and suggestions are needed here.

Copilot AI review requested due to automatic review settings February 22, 2026 02:17

Copilot started reviewing on behalf of nshcr February 22, 2026 02:17 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 22, 2026 02:52

nshcr force-pushed the feat-normalize-branch-name branch from 9a01610 to 3ae61c2 Compare February 22, 2026 02:52

Copilot started reviewing on behalf of nshcr February 22, 2026 02:53 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Byron assigned Byron and unassigned Byron Feb 22, 2026

chatgpt-codex-connector bot reviewed Feb 22, 2026

View reviewed changes

apps/desktop/src/components/BranchNameTextbox.svelte Show resolved Hide resolved

Byron requested a review from mtsgrd February 22, 2026 09:37

Byron mentioned this pull request Feb 22, 2026

fix: preserve # in branch names during frontend slugification #12322

Closed

4 tasks

Byron marked this pull request as draft February 23, 2026 11:12

nshcr force-pushed the feat-normalize-branch-name branch from 3ae61c2 to 4ff0f76 Compare February 23, 2026 12:11

nshcr marked this pull request as ready for review February 23, 2026 12:13

Copilot AI review requested due to automatic review settings February 23, 2026 12:13

Copilot started reviewing on behalf of nshcr February 23, 2026 12:14 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

	export function slugify(input: string) {
	return String(input)
	.normalize('NFKD')
	.replace(/[\u0300-\u036f]/g, '')
	.trim()
	.replace(/[^A-Za-z0-9._/ -]/g, '')
	.replace(/\s+/g, '-')
	.replace(/-+/g, '-');
	}

	/// Assuming `name` is a short-name like `feat/hi`, `main` or `something the user typed`, convert it into
	/// a version of this which would be a valid short reference name.
	pub fn normalize_short_name<'a>(name: impl Into<&'a BStr>) -> anyhow::Result<BString> {

		@@ -145,7 +146,8 @@
		id={ElementId.NewBranchNameInput}
		value={createRefName}

Conversation

nshcr commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧢 Changes

☕️ Reasoning

🎫 Affected issues

Uh oh!

vercel bot commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Byron commented Feb 22, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Byron commented Feb 22, 2026

Uh oh!

nshcr commented Feb 23, 2026

Uh oh!

Byron commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

nshcr commented Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nshcr commented Feb 22, 2026 •

edited

Loading