Skip to content

fix: support feature branch numbers with 4+ digits#2040

Open
t-sakoda wants to merge 3 commits intogithub:mainfrom
t-sakoda:fix/support-variable-length-feature-numbers
Open

fix: support feature branch numbers with 4+ digits#2040
t-sakoda wants to merge 3 commits intogithub:mainfrom
t-sakoda:fix/support-variable-length-feature-numbers

Conversation

@t-sakoda
Copy link
Copy Markdown

@t-sakoda t-sakoda commented Apr 1, 2026

Description

The sequential feature number regex in common.sh and common.ps1 was hardcoded to exactly 3 digits ({3}), causing branches like 1234-feature-name to be rejected once a project exceeds 999 features.

This PR changes the pattern to {3,} (3 or more digits) so that 4+ digit prefixes are recognized correctly. A guard is also added in check_feature_branch / Test-FeatureBranch to prevent malformed timestamps (e.g. 2026031-143022-feat) from being falsely accepted as sequential prefixes.

Closes #344

Testing

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

All 27 test_timestamp_branches.py tests pass, including the existing test that rejects partial timestamps.

AI Disclosure

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

Code changes generated and reviewed with Claude Code (Claude Opus 4.6).

…ommon.ps1

The sequential feature number pattern was hardcoded to exactly 3 digits
(`{3}`), causing branches like `1234-feature-name` to be rejected.
Changed to `{3,}` (3 or more digits) to support growing projects.
Also added a guard to exclude malformed timestamp patterns from being
accepted as sequential prefixes.

Closes github#344

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 1, 2026 05:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates feature-branch parsing in Spec Kit’s Bash and PowerShell helper scripts to support sequential prefixes beyond 999 (3+ digits) while preventing malformed timestamp-like prefixes from being treated as sequential.

Changes:

  • Allow sequential feature prefixes with 3+ digits ({3,}) when scanning specs/ and resolving feature directories.
  • Update feature-branch validation to accept 4+ digit sequential prefixes.
  • Add a guard intended to reject malformed timestamp-like branches (e.g., 2026031-143022-feat) from being accepted as sequential.

Reviewed changes

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

File Description
scripts/powershell/common.ps1 Expands sequential prefix matching and adds a timestamp-shape guard in Test-FeatureBranch.
scripts/bash/common.sh Expands sequential prefix matching, updates prefix-based directory resolution, and adds the same timestamp-shape guard in check_feature_branch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +86 to 88
} elseif ($_.Name -match '^(\d{3,})-') {
$num = [int]$matches[1]
if ($num -gt $highest) {
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

In Get-CurrentBranch, the sequential prefix is now 3+ digits, but $num = [int]$matches[1] can throw for larger values (e.g., external ticket IDs or very large feature numbers > 2,147,483,647), breaking the non-git fallback branch detection. Consider switching to [long] (or [long]::TryParse) to match the approach used in scripts/powershell/create-new-feature.ps1 and avoid overflow exceptions.

Copilot uses AI. Check for mistakes.
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.

Fixed in ab7c3c1 — changed [int] to [long] to prevent overflow for large feature numbers.

Comment on lines +142 to +143
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (digits-6digits pattern)
$isSequential = ($Branch -match '^[0-9]{3,}-') -and ($Branch -notmatch '^[0-9]+-[0-9]{6}-')
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The new malformed-timestamp guard ($Branch -notmatch '^[0-9]+-[0-9]{6}-') also rejects otherwise-valid sequential branches where the slug starts with 6 digits (e.g., 004-123456-fix-bug). If the intent is only to reject partial timestamps like 2026031-143022-feat, narrow the exclusion pattern (for example, only the 7-digit-date case) so numeric-leading slugs remain valid.

Suggested change
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (digits-6digits pattern)
$isSequential = ($Branch -match '^[0-9]{3,}-') -and ($Branch -notmatch '^[0-9]+-[0-9]{6}-')
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time)
$isSequential = ($Branch -match '^[0-9]{3,}-') -and ($Branch -notmatch '^[0-9]{7}-[0-9]{6}-')

Copilot uses AI. Check for mistakes.
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.

Fixed in ab7c3c1 — narrowed the pattern to ^[0-9]{7}-[0-9]{6}- so only the 7-digit-date + 6-digit-time shape is excluded.

Comment on lines +127 to +129
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (digits-6digits pattern)
local is_sequential=false
if [[ "$branch" =~ ^[0-9]{3,}- ]] && [[ ! "$branch" =~ ^[0-9]+-[0-9]{6}- ]]; then
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The malformed-timestamp exclusion ([[ ! "$branch" =~ ^[0-9]+-[0-9]{6}- ]]) is broad enough to reject valid sequential branches whose slug starts with 6 digits (e.g., 004-123456-fix-bug). If the goal is specifically to block partial timestamps like 2026031-143022-feat, consider narrowing this pattern (e.g., target only the 7-digit-date + 6-digit-time shape) so numeric-leading slugs are still accepted.

Suggested change
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (digits-6digits pattern)
local is_sequential=false
if [[ "$branch" =~ ^[0-9]{3,}- ]] && [[ ! "$branch" =~ ^[0-9]+-[0-9]{6}- ]]; then
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time pattern)
local is_sequential=false
if [[ "$branch" =~ ^[0-9]{3,}- ]] && [[ ! "$branch" =~ ^[0-9]{7}-[0-9]{6}- ]]; then

Copilot uses AI. Check for mistakes.
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.

Fixed in ab7c3c1 — same narrowing applied here: ^[0-9]{7}-[0-9]{6}-.

- Change [int] to [long] in PowerShell Get-CurrentBranch to avoid
  overflow for large feature numbers (>2,147,483,647)
- Narrow malformed-timestamp exclusion from ^[0-9]+-[0-9]{6}- to
  ^[0-9]{7}-[0-9]{6}- so valid sequential branches like
  004-123456-fix-bug are not rejected

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@t-sakoda t-sakoda marked this pull request as ready for review April 1, 2026 06:29
@t-sakoda t-sakoda requested a review from mnriem as a code owner April 1, 2026 06:29
Copilot AI review requested due to automatic review settings April 1, 2026 06:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +127 to 135
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time)
local is_sequential=false
if [[ "$branch" =~ ^[0-9]{3,}- ]] && [[ ! "$branch" =~ ^[0-9]{7}-[0-9]{6}- ]]; then
is_sequential=true
fi
if [[ "$is_sequential" != "true" ]] && [[ ! "$branch" =~ ^[0-9]{8}-[0-9]{6}- ]]; then
echo "ERROR: Not on a feature branch. Current branch: $branch" >&2
echo "Feature branches should be named like: 001-feature-name or 20260319-143022-feature-name" >&2
echo "Feature branches should be named like: 001-feature-name, 1234-feature-name, or 20260319-143022-feature-name" >&2
return 1
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

check_feature_branch now accepts sequential prefixes with 4+ digits, but the existing tests for check_feature_branch only cover a 3-digit sequential branch (004-feat). Add a regression test that asserts check_feature_branch "1234-feat" "true" succeeds, so the new {3,} behavior is covered directly (separate from create-new-feature.sh tests).

Copilot uses AI. Check for mistakes.
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.

Fixed in 49c29c2 — added test_accepts_four_digit_sequential_branch that asserts check_feature_branch "1234-feat" "true" succeeds.

Comment on lines 151 to 155
local prefix=""
if [[ "$branch_name" =~ ^([0-9]{8}-[0-9]{6})- ]]; then
prefix="${BASH_REMATCH[1]}"
elif [[ "$branch_name" =~ ^([0-9]{3})- ]]; then
elif [[ "$branch_name" =~ ^([0-9]{3,})- ]]; then
prefix="${BASH_REMATCH[1]}"
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

find_feature_dir_by_prefix now accepts 4+ digit sequential prefixes (^([0-9]{3,})-), but the existing TestFindFeatureDirByPrefix cases only cover timestamp prefixes. Add a test that creates something like specs/1000-original and asserts that find_feature_dir_by_prefix <repo> "1000-different" resolves to the existing 1000-... directory, so the new {3,} path is covered.

Copilot uses AI. Check for mistakes.
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.

Fixed in 49c29c2 — added test_four_digit_sequential_prefix that creates specs/1000-original-feat and asserts find_feature_dir_by_prefix resolves 1000-different-name to it.

Cover check_feature_branch and find_feature_dir_by_prefix with 4-digit
sequential prefixes, as requested in PR review github#2040.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

tests/test_timestamp_branches.py:197

  • There’s test coverage for rejecting a 7-digit partial date (2026031-143022-feat), but no regression test for the newly-possible case 20260319-143022 (8-digit date + 6-digit time with no final -<slug>). Adding a test that asserts check_feature_branch "20260319-143022" "true" fails would lock in the intended malformed-timestamp guard while still allowing 4+ digit sequential branches.
    def test_accepts_four_digit_sequential_branch(self):
        """check_feature_branch accepts 4+ digit sequential branch."""
        result = source_and_call('check_feature_branch "1234-feat" "true"')
        assert result.returncode == 0

    def test_rejects_partial_timestamp(self):
        """Test 9: check_feature_branch rejects 7-digit date."""
        result = source_and_call('check_feature_branch "2026031-143022-feat" "true"')
        assert result.returncode != 0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +127 to +132
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time)
local is_sequential=false
if [[ "$branch" =~ ^[0-9]{3,}- ]] && [[ ! "$branch" =~ ^[0-9]{7}-[0-9]{6}- ]]; then
is_sequential=true
fi
if [[ "$is_sequential" != "true" ]] && [[ ! "$branch" =~ ^[0-9]{8}-[0-9]{6}- ]]; then
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

With the sequential regex widened to {3,}, branches like 20260319-143022 (8-digit date + 6-digit time but missing the final -<slug> segment) now match the sequential check and will be accepted. This seems to undermine the intent of rejecting malformed timestamps; consider explicitly rejecting ^\d{8}-\d{6}$ / ^\d{8}-\d{6}[^-] (and the analogous 7-digit case) before treating the branch as sequential.

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +144
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time)
$isSequential = ($Branch -match '^[0-9]{3,}-') -and ($Branch -notmatch '^[0-9]{7}-[0-9]{6}-')
if (-not $isSequential -and $Branch -notmatch '^\d{8}-\d{6}-') {
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Because sequential validation now allows 3+ digits, a branch named 20260319-143022 (looks like a timestamp but lacks the trailing -<name>) matches $isSequential and will be accepted. If malformed timestamps should be rejected, add an explicit check for the ^\d{8}-\d{6}$ / missing-third-segment shape (and optionally the 7-digit date variant) so these don’t slip through as “sequential”.

Suggested change
# Accept sequential prefix (3+ digits) but exclude malformed timestamps (7-digit date + 6-digit time)
$isSequential = ($Branch -match '^[0-9]{3,}-') -and ($Branch -notmatch '^[0-9]{7}-[0-9]{6}-')
if (-not $isSequential -and $Branch -notmatch '^\d{8}-\d{6}-') {
# Accept sequential prefix (3+ digits) but exclude malformed timestamps
# Valid timestamp-based feature branches look like: YYYYMMDD-HHMMSS-feature-name
$hasValidTimestampPrefix = ($Branch -match '^\d{8}-\d{6}-')
# Malformed timestamps: date+time with no trailing "-<name>" (optionally 7-digit date variant)
$hasMalformedTimestamp = ($Branch -match '^(?:\d{7}|\d{8})-\d{6}$')
$isSequential = ($Branch -match '^[0-9]{3,}-') -and (-not $hasMalformedTimestamp)
if (-not $isSequential -and -not $hasValidTimestampPrefix) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

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

Please address Copilot feedback

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.

Customize Feature Number to include External Reference

3 participants