Skip to content

fix: auto-correct prefix conflicts in create-new-feature scripts#1829

Open
daniel-graham wants to merge 1 commit into
github:mainfrom
daniel-graham:fix/auto-correct-prefix-conflicts
Open

fix: auto-correct prefix conflicts in create-new-feature scripts#1829
daniel-graham wants to merge 1 commit into
github:mainfrom
daniel-graham:fix/auto-correct-prefix-conflicts

Conversation

@daniel-graham

@daniel-graham daniel-graham commented Mar 13, 2026

Copy link
Copy Markdown

Problem

Passing an explicit feature number can reuse a prefix already owned by another directory under specs/. Downstream prefix resolution then sees multiple matching feature directories and fails.

Solution

Treat an explicit --number / -Number as a preferred value:

  • If no spec directory uses that formatted prefix, keep the requested number.
  • If a matching spec directory exists, choose the next unused prefix above max(spec prefixes), skipping prefixes also owned by timestamp directories, and warn on stderr.
  • Match directories only; similarly named files do not trigger correction.
  • Preserve exact-directory reuse when --allow-existing-branch / -AllowExistingBranch is supplied, including canonical truncated names and directories with same-prefix siblings.

The guardrail intentionally checks local spec directories only. It does not fetch or scan git branches.

Changes

  • scripts/bash/create-new-feature.sh: track explicit numbers, detect spec-prefix conflicts, and update help text.
  • scripts/powershell/create-new-feature.ps1: mirror Bash behavior with directory-only matching.
  • scripts/python/create_new_feature.py: keep the current Python script variant behaviorally aligned, including unreadable-directory handling.
  • tests/test_timestamp_branches.py: cover correction, 4+ digit continuation, matching files, dry-run behavior, and exact-directory reuse.
  • tests/test_create_new_feature_python_parity.py: verify Bash, PowerShell, and Python parity, including timestamp-prefix collision skipping.

Current main behavior remains intact for 64-bit feature numbers, timestamp prefixes, automatic numbering, dry-run output, and allow-existing reuse.

Validation

  • 131 passed, 1 skipped in the two affected test modules.
  • 5,255 passed, 6 skipped in the full pytest suite.
  • Ruff checks passed for changed Python files.
  • Bash ShellCheck passed after excluding three pre-existing warnings outside the changed lines.
  • PowerShell parser validation passed.
  • git diff --check passed.

Agent disclosure

This revision was authored autonomously on behalf of @daniel-graham by Codex (model: GPT-5).

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

Updates the feature creation Bash script to prevent duplicate spec-directory prefixes when --number is provided and conflicts with an existing specs/NNN-* directory, by auto-correcting to the next available global number.

Changes:

  • Added a guardrail that detects existing specs/${FEATURE_NUM}-* directories and recomputes the feature number from the global maximum across specs and git branches.
  • Emits a warning to stderr when auto-correction occurs.

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

Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/bash/create-new-feature.sh Outdated

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for tackling this — the problem is real and the guardrail idea is the right direction. Before we can merge, there are a few things we need to address: the --number flag's contract changes from a hard override to a suggestion, which is a silent breaking change for any automation relying on deterministic output; the recalculation path skips git fetch so the corrected number could still collide with unfetched remote branches; the check only covers spec directories but not existing branches, leaving an inconsistent experience; the help text still says "overrides auto-detection"; and the PowerShell script doesn't get the same guardrail, breaking cross-platform parity. Could you scope the guardrail to only fire when --number was explicitly passed, add a fetch before recalculating, extend the collision check to branches, update the help text, and port it to the PowerShell side as well?

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

Adds a guardrail to feature creation scripts so an explicitly requested feature number (--number / -Number) is treated as a validated suggestion and auto-corrected when its NNN- prefix already exists, preventing duplicate spec directories/branch prefixes that break downstream workflows.

Changes:

  • Add “explicit number” tracking and a conflict check that recalculates the next available number when a requested prefix already exists.
  • Update help text to clarify that --number / -Number is preferred and may be auto-corrected.

Reviewed changes

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

File Description
scripts/bash/create-new-feature.sh Adds an explicit---number guardrail that detects existing NNN- prefixes and recalculates from global max.
scripts/powershell/create-new-feature.ps1 Mirrors the guardrail behavior in PowerShell and updates CLI help text accordingly.

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

Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/bash/create-new-feature.sh Outdated
@daniel-graham daniel-graham changed the title fix: auto-correct prefix conflicts in create-new-feature.sh fix: auto-correct prefix conflicts in create-new-feature scripts Mar 13, 2026
@daniel-graham

Copy link
Copy Markdown
Author

Changes made in response to review

Thanks for the thorough review @mnriem! All feedback addressed across 3 follow-up commits:

@mnriem review (requested changes)

Feedback Resolution
--number contract changes silently from override to suggestion Guardrail now only fires when --number was explicitly passed (NUMBER_EXPLICIT flag in Bash, $PSBoundParameters.ContainsKey in PowerShell)
Recalculation skips git fetch Guardrail now calls git fetch --all --prune before checking branches, and delegates recalculation to check_existing_branches / Get-NextBranchNumber which also fetch
Collision check only covers spec dirs, not branches Trigger now checks both specs/NNN-* dirs and git branch -a for matching prefixes
Help text still says "overrides auto-detection" Updated to "Preferred branch number (auto-corrected if prefix already exists in specs or branches)"
PowerShell missing parity Full guardrail ported to create-new-feature.ps1 with identical logic

@copilot review (suggestions)

Feedback Resolution
PR title only mentions .sh Updated to reference both scripts
Use $PSBoundParameters.ContainsKey instead of sentinel Adopted — idiomatic PowerShell explicit-parameter detection
git branch -a without fetch (PS) Added git fetch --all --prune before branch check
git branch -a without fetch (Bash) Same fix applied

Also fixed

  • Removed unrelated templates/commands/specify.md changes that had leaked into the PR

All changes validated on macOS with both Bash and PowerShell (pwsh):

  • Explicit --number with conflict → warns + auto-corrects
  • No --number (auto-detect) → guardrail does not fire

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

Adds guardrails to the “create new feature” scripts so an explicitly suggested feature number won’t silently collide with existing specs/NNN-* directories or existing branch prefixes, preventing downstream prefix-ambiguity errors.

Changes:

  • Add prefix-conflict detection and auto-correction when --number / -Number is explicitly provided.
  • Update CLI help text to describe --number / -Number as a preferred (validated) number rather than a hard override.
  • Fix/adjust the specify.md command template validation step reference (proceed to step 7).

Reviewed changes

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

File Description
templates/commands/specify.md Updates the command template’s instructions/step reference and removes an unused header.
scripts/powershell/create-new-feature.ps1 Adds -Number explicit-tracking and a guardrail to auto-correct number conflicts; updates help text.
scripts/bash/create-new-feature.sh Adds --number explicit-tracking and a guardrail to auto-correct number conflicts; updates help text.

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

Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread templates/commands/specify.md
@mnriem
mnriem self-requested a review March 13, 2026 16:23

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please pull in latest from main and address Copilot feedback. If not applicable, please explain why.

@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from ebbfc54 to f6dbbe2 Compare March 13, 2026 16:30
@daniel-graham

Copy link
Copy Markdown
Author

Follow-up: rebased on main + addressed remaining feedback

What changed

  1. Rebased on latest main — picks up PR Add specify doctor command for project health diagnostics #1828 ("Add specify doctor command")
  2. Fixed compgen to only match directories — previously compgen -G "specs/NNN-*" would also match files (e.g., specs/001-notes.md), causing false positives. Now iterates matches and only triggers on -d (directories)
  3. Confirmed templates/commands/specify.md is clean — no diff vs upstream/main. The bot's earlier comment was from a stale review before the revert was pushed

Final PR diff

Only 2 files changed (82 additions, 2 deletions):

  • scripts/bash/create-new-feature.sh
  • scripts/powershell/create-new-feature.ps1

No other files are modified.

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

Updates the “create new feature” scripts to prevent duplicate numeric prefixes (e.g., 001-*) when a caller explicitly suggests a feature number, aligning behavior with downstream tooling that expects unique 3-digit prefixes.

Changes:

  • Treat --number / -Number as a validated “preferred” value and auto-correct it when the prefix already exists in specs/ or git branches.
  • Track whether the number was explicitly provided to preserve existing auto-detection behavior.
  • Update help text to reflect the new auto-correction semantics.

Reviewed changes

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

File Description
scripts/bash/create-new-feature.sh Adds an explicit-number guardrail to detect prefix conflicts in specs/ and branches and auto-correct to the next available number.
scripts/powershell/create-new-feature.ps1 Mirrors the guardrail behavior in PowerShell and updates CLI help text accordingly.

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

Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/bash/create-new-feature.sh
@mnriem
mnriem self-requested a review March 13, 2026 16:48

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address the additional Copilot comments. Thank for staying with it!

@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from f6dbbe2 to 4fae020 Compare March 13, 2026 16:54
@daniel-graham

Copy link
Copy Markdown
Author

Latest update (rebased + squashed to 1 commit)

Addressed the remaining Copilot feedback and cleaned up from internal review:

  1. Bash: --number range validation — now rejects values outside [1,999] with a clear error, since downstream tooling expects a 3-digit prefix
  2. PowerShell: [ValidateRange(1, 999)] — added to the -Number parameter for parity with Bash
  3. Double-fetch eliminated — Bash inlines the max-finding using already-fetched local branch data; PowerShell defers the single fetch to Get-NextBranchNumber (only called when a conflict is found)
  4. Stale comments cleaned up — removed references to check_existing_branches in the inlined code path

Note for @mnriem on range validation

We added [ValidateRange(1, 999)] to the PowerShell -Number parameter (and equivalent validation in Bash) to prevent malformed prefixes from values outside the 3-digit range. This means -Number 0 now errors instead of silently triggering auto-detection — users should simply omit -Number for auto-detect behavior.

If there's an intentional reason to allow values outside [1, 999] (e.g., future support for 4-digit prefixes), let us know and we'll adjust the bounds accordingly.

Final diff stat

 scripts/bash/create-new-feature.sh    | 62 ++++++++++-
 scripts/powershell/create-new-feature.ps1 | 44 +++++++-
 2 files changed, 104 insertions(+), 2 deletions(-)

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

Improves the “create new feature” scripts (bash + PowerShell) so an explicitly provided feature number is treated as a validated suggestion and auto-corrected when the numeric prefix already exists in specs/ or in git branches, preventing duplicate NNN-* spec directories.

Changes:

  • Add explicit-number tracking and a conflict guardrail that recomputes the next available global feature number on collision.
  • Update CLI help text to clarify --number/-Number is auto-corrected on conflicts.
  • Add input validation for --number / -Number (but see blocking issues in review comments).

Reviewed changes

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

File Description
scripts/powershell/create-new-feature.ps1 Adds -Number conflict guardrail + help text update; introduces a parameter validation issue with the default sentinel.
scripts/bash/create-new-feature.sh Adds --number validation and a conflict guardrail to auto-correct prefix collisions after number resolution.

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

Comment thread scripts/powershell/create-new-feature.ps1
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/bash/create-new-feature.sh
@mnriem
mnriem self-requested a review March 13, 2026 21:56

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would prefer the range to allow for 5 digits so it takes a longer time to run out. Now that we are using AI we probably will go through number more quickly so 3 digits seems a bit small. And please address Copilot feedback. if not applicable please explain why.

@daniel-graham

Copy link
Copy Markdown
Author

I would prefer the range to allow for 5 digits so it takes a longer time to run out. Now that we are using AI we probably will go through number more quickly so 3 digits seems a bit small. And please address Copilot feedback. if not applicable please explain why.

Agreed, I imagine we may want to either add some padding 00NNN to existing directories, or leave that up to the user, and have the script simply progress naturally to 5 digits from 999 -> 01000?

@mnriem

mnriem commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Probably should keep the prepending for the current scheme and once it overflows no more prepending as that will keep it backwards compatible

@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from 4fae020 to 05bf2bf Compare July 22, 2026 22:38
@daniel-graham

Copy link
Copy Markdown
Author

Rebased this PR onto current main and rebuilt the guardrail around current behavior.

The revised implementation checks spec directories only and contains no git fetch or branch-scanning path, so the earlier fetch-order and stale-remote comments no longer apply. Current core auto-numbering also derives from spec directories. The earlier 3-digit/5-digit range discussion is superseded by current main, which supports prefixes through signed 64-bit maximum values and formats them with a minimum width of three digits.

Bash, PowerShell, and the current Python variant now share these contracts:

  • explicit number stays unchanged when its spec prefix is unused;
  • existing spec-directory prefix advances to max(spec prefixes) + 1;
  • matching files are ignored;
  • dry-run computes the corrected result without writes;
  • exact-directory reuse remains available through the allow-existing option;
  • timestamp and automatic-number paths remain unchanged.

Validation on commit 05bf2bff971ed1df074e05d9eb2eb197532e0b9f:

  • affected modules: 127 passed, 1 skipped;
  • full suite: 5,251 passed, 6 skipped;
  • Ruff, Bash ShellCheck for changed behavior, PowerShell parser, and git diff --check passed;
  • independent final diff review found no actionable issues.

Fresh GitHub workflow runs were created but currently report action_required before executing jobs, consistent with the approval gate for fork-origin pull requests rather than a test failure.

Posted on behalf of @daniel-graham by Codex (model: GPT-5, autonomous).

@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from 05bf2bf to 83e6df7 Compare July 22, 2026 23:02
@daniel-graham

daniel-graham commented Jul 22, 2026

Copy link
Copy Markdown
Author

Local review loop complete for 12756f5.

Fixed five cross-implementation defects:

  • Canonical branch-name truncation now happens before exact allow-existing comparison.
  • An exact allow-existing target wins even when a sibling directory shares its numeric prefix.
  • Python matches Bash and PowerShell when the specs directory cannot be listed.
  • Corrected explicit numbers advance past prefixes already owned by timestamp directories, addressing Copilot review 4763816309.
  • Empty --number remains omitted in Bash, matching PowerShell and Python even when automatic numbering lands on a timestamp-owned prefix, addressing Copilot review 4764870736.

Validation:

  • Affected modules: 131 passed, 1 skipped.
  • Full suite: 5,255 passed, 6 skipped.
  • Ruff, Python compilation, Bash syntax and ShellCheck, PowerShell parser, and git diff --check: passed.
  • Independent post-fix review and direct Bash/PowerShell/Python probes: clean.

The four workflow runs are awaiting maintainer approval and have not executed the code.

Posted on behalf of @daniel-graham by Codex (model: GPT-5, autonomous).

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.

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/powershell/create-new-feature.ps1 Outdated
Comment thread scripts/python/create_new_feature.py Outdated
@mnriem

mnriem commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from 83e6df7 to 8df0359 Compare July 23, 2026 13:46
@mnriem
mnriem requested a review from Copilot July 23, 2026 14:02

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.

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread scripts/bash/create-new-feature.sh Outdated
exit 1
fi
BRANCH_NUMBER="$next_arg"
NUMBER_EXPLICIT=true
@mnriem

mnriem commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Please address Copilot feedback

Treat an explicit feature number as a preference when an existing spec directory already uses that prefix. Advance to the next available spec prefix and warn without fetching or scanning git branches.

Keep Bash, PowerShell, and Python variants aligned. Preserve 64-bit numbering, timestamp mode, dry-run output, matching-file behavior, and exact-directory reuse through the allow-existing option.

Assisted-by: Codex (model: GPT-5, autonomous)
@daniel-graham
daniel-graham force-pushed the fix/auto-correct-prefix-conflicts branch from 8df0359 to 12756f5 Compare July 23, 2026 20:31
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.

3 participants