Skip to content

fix(scripts): make bash branch-name sanitizing match the Python and PowerShell twins - #4286

Open
SEPURI-SAI-KRISHNA wants to merge 2 commits into
github:mainfrom
SEPURI-SAI-KRISHNA:fix/branch-name-locale-parity
Open

fix(scripts): make bash branch-name sanitizing match the Python and PowerShell twins#4286
SEPURI-SAI-KRISHNA wants to merge 2 commits into
github:mainfrom
SEPURI-SAI-KRISHNA:fix/branch-name-locale-parity

Conversation

@SEPURI-SAI-KRISHNA

@SEPURI-SAI-KRISHNA SEPURI-SAI-KRISHNA commented Aug 23, 2026

Copy link
Copy Markdown

Description

clean_branch_name / generate_branch_name in the bash twins produce branch and
specs/ directory names that differ from the Python and PowerShell twins. Three
independent defects, all in the same sanitizing pipeline, all in two copies of it
(scripts/bash/create-new-feature.sh and
extensions/git/scripts/bash/create-new-feature-branch.sh).

1. [^a-z0-9] is locale-dependent (the user-visible one)

glibc resolves a bracket-expression range through the locale's collation
table, so under en_US.UTF-8 the a-z in sed 's/[^a-z0-9]/-/g' covers
accented lowercase letters. Under C.UTF-8 / POSIX it does not.

Same repo, same command, same description — only LANG differs:

$ LC_ALL=C.UTF-8   create-new-feature.sh --json --dry-run "Ajouter la réservation hôtelière"
  bash   -> 001-ajouter-servation-teli
  python -> 001-ajouter-servation-teli

$ LC_ALL=en_US.UTF-8 create-new-feature.sh --json --dry-run "Ajouter la réservation hôtelière"
  bash   -> 001-ajouter-réservation-hôtelière     <-- diverges
  python -> 001-ajouter-servation-teli

en_US.UTF-8 is the default on macOS Terminal and most Linux desktops; CI
runners generally run in the POSIX locale, which is why this never showed up in
the suite. Two teammates running /speckit.specify on the same description get
two different specs/ directories, and the second one's /speckit.plan cannot
find the first one's feature. The git extension checks out a branch with those
bytes in it.

Same divergence for Spanish (añadir vs adir), German (prüfung vs fung),
Portuguese, Nordic — anything in Latin-1 Supplement / Latin Extended-A.

2. sed 's/-\+/-/g' does not collapse anything on macOS

\+ is a GNU extension, not POSIX BRE. POSIX/BSD sed reads it as a literal
+, so the collapse step is a no-op there:

$ printf 'my-fancy---name' | sed         's/-\+/-/g'   # GNU
my-fancy-name
$ printf 'my-fancy---name' | sed --posix 's/-\+/-/g'   # POSIX/BSD semantics
my-fancy---name

--short-name 'My Fancy!! Name' therefore yields 001-my-fancy---name on macOS
and 001-my-fancy-name on Linux and from the Python/PowerShell twins. The repo
already knew: tests/extensions/git/test_git_extension_python_parity.py carried

# Single separator runs only: the bash twin's collapse step
# (sed 's/-\+/-/g') is a GNU-ism that BSD sed treats literally.

and deliberately tested only inputs that could not hit it. That comment and that
restriction are removed here.

3. echo "$name" eats -n / -e / -E

The raw value went through echo, so those three short names were consumed as
options and produced an empty suffix (001-) where Python produces 001-n.

Fix

LC_ALL=C for the sanitizing pipeline (byte-exact classes, and ASCII word
boundaries for the grep -qw acronym probe, matching the Python twin's
(?<![0-9A-Za-z_]) lookarounds), the portable --* in place of \+, and
printf '%s\n' in place of echo. The same edit in both copies of the
function. Behaviour on ASCII input in the POSIX locale — which is what CI has
been exercising — is unchanged.

local -x scopes and exports the override to the pipeline's children only; it
is restored on return, so nothing else in the script or the caller's environment
sees it.

Follow-up from review: ASCII acronym boundaries

Copilot spotted that scoping LC_ALL=C over the whole of generate_branch_name
also changes the grep -qw acronym probe, and that the extension's Python twin
still used a Unicode \b there. That was correct:

"éDBé cache"  ->  bash: db-cache   python (extension): cache

Under LC_ALL=C an accented letter is a non-word byte, so DB has boundaries
and survives; Python's \b and .NET's \b are Unicode-aware, treat éDBé as
one word, and drop it. Only words shorter than three characters reach this
probe, so the window is narrow, but the twins genuinely disagreed.

The core Python twin had already settled this the ASCII way
(scripts/python/create_new_feature.py:185, with a comment saying it mirrors
bash's grep -qw). Three files were still using \b and are now aligned to the
same explicit lookarounds:

  • extensions/git/scripts/python/create_new_feature_branch.py
  • extensions/git/scripts/powershell/create-new-feature-branch.ps1
  • scripts/powershell/create-new-feature.ps1

The last one is beyond the reported comment: it had the identical \b against a
core Python twin that already used ASCII lookarounds, so the same divergence
existed in core. Fixing only the extension would have left the two halves
inconsistent with each other.

Covered by test_acronym_adjacent_to_non_ascii_matches_python in the extension
parity suite and an acronym_next_to_non_ascii case in
test_python_branch_name_generation_matches_bash.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

Full suite on this branch: 7151 passed, 181 skipped, 1 failed on Linux /
Python 3.14. On main the same run is 7136 passed, 181 skipped, 1 failed;
this branch adds 15 test cases (7136 + 15 = 7151).

The one failure, tests/contract/test_bundle_cli.py::test_build_escapes_markup_in_output_path,
is unrelated and pre-existing on main: it asserts across a Rich wrap point, so
it passes or fails depending on how long the runner's temp path is. Fixed
separately in #4280.

Fail-before / pass-after, with the sources reverted and the new tests kept:

Test Before After
test_bash_branch_name_ignores_locale_collation (3 params) FAIL pass
test_python_dash_prefixed_short_name_matches_bash (3 params) FAIL pass
TestCreateFeatureBranchParity::test_branch_name_ignores_locale_collation (2 params) FAIL pass
test_bash_collapses_repeated_separators (2 params) pass on GNU sed pass
test_short_name_cleaning[repeated_separators, separator_run] pass on GNU sed pass

The two separator tests are the macOS guard — on a GNU-sed runner they pass
either way, which is exactly why the gap survived. They will exercise the real
thing on the macos-latest leg of the matrix.

The locale tests probe sed under en_US.UTF-8 and skip when the environment
cannot reproduce collation-ordered ranges (locale not installed, non-glibc libc,
Git-for-Windows), rather than asserting against a locale name that may not exist.

Manual test results

Agent: Claude Code | OS/Shell: Ubuntu 24.04.4, bash 5.2.21, LANG=en_IN

Scaffolded from this branch with
specify init <dir> --integration claude --extension git, so the project under
test carries the patched scripts.

Command tested Result
/speckit-specify Ajouter la réservation hôtelière (accented) pass — specs/001-hotel-booking/, branch 001-hotel-booking; spec dir and branch agree
/speckit-git-feature (same description) pass — same name, no divergence
create-new-feature.sh --json --dry-run --short-name "réservation hôtelière" pass — 002-r-servation-h-teli-re
extensions/git/.../create-new-feature-branch.sh --json --short-name "réservation hôtelière" pass — branch 002-r-servation-h-teli-re checked out

Worth noting for reviewers: in the two slash-command runs the agent summarised
the French description into an ASCII short name of its own ("hotel-booking"),
so those runs confirm the commands still work but do not reach the accented
path. The last two rows drive the scripts with a non-ASCII --short-name
directly, which is what the skills do internally, and that is where the
divergence used to appear. en_IN is a stock desktop locale and reproduces it
exactly like en_US.UTF-8.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Code and tests generated with Claude Code. I reviewed the diff, reproduced the
locale divergence, ran the suite, and understand what the change does.

Copilot AI balanced review requested due to automatic review settings August 23, 2026 09:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns Bash branch-name sanitization with Python and PowerShell across locales and platforms.

Changes:

  • Uses locale-independent sanitization, portable sed, and safe printf.
  • Adds parity coverage for locales, repeated separators, and option-like names.
  • Introduces locale probing for tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
scripts/bash/create-new-feature.sh Fixes core Bash sanitization.
extensions/git/scripts/bash/create-new-feature-branch.sh Applies equivalent Git extension changes.
tests/test_create_new_feature_python_parity.py Adds core parity tests.
tests/extensions/git/test_git_extension_python_parity.py Expands extension parity tests.
tests/parity_helpers.py Adds locale detection helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

# LC_ALL=C for the same collation reason documented on clean_branch_name,
# and so the `grep -qw` acronym probe below uses ASCII word boundaries like
# the Python twin's (?<![0-9A-Za-z_]) lookarounds.
local -x LC_ALL=C

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed. "éDBé cache" gave db-cache from bash and cache from the extension's Python twin — LC_ALL=C makes the accent a word boundary for grep -qw,
while Unicode \b treats éDBé as one word.

Aligned to explicit ASCII lookarounds, matching what the core Python twin (scripts/python/create_new_feature.py:185) already did. Also fixed
scripts/powershell/create-new-feature.ps1, which is outside your comment but had the same \b against a core Python twin that already used ASCII boundaries, so core had the
divergence too.

Added test_acronym_adjacent_to_non_ascii_matches_python to the extension parity suite and an acronym_next_to_non_ascii case to test_python_branch_name_generation_matches_bash.

Copilot AI review requested due to automatic review settings August 23, 2026 11:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/extensions/git/test_git_extension_python_parity.py:179

  • This extension parity table covers the separator-collapse fix but not the echo-option fix made in the duplicated extension Bash script. Only the core script gets -n/-e/-E coverage, so the extension copy could regress independently while CI remains green. Add those values here as well.
            ("User_Auth!", "001-user-auth"),
            ("User__Auth!!", "001-user-auth"),
            ("auth -- v2", "001-auth-v2"),
        ],
        ids=["single_separators", "repeated_separators", "separator_run"],

if ($word.Length -ge 3) {
$meaningfulWords += $word
} elseif ($Description -cmatch "\b$($word.ToUpper())\b") {
} elseif ($Description -cmatch "(?<![0-9A-Za-z_])$($word.ToUpper())(?![0-9A-Za-z_])") {
if ($word.Length -ge 3) {
$meaningfulWords += $word
} elseif ($Description -cmatch "\b$($word.ToUpper())\b") {
} elseif ($Description -cmatch "(?<![0-9A-Za-z_])$($word.ToUpper())(?![0-9A-Za-z_])") {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants