Skip to content

Commit 5e49ec6

Browse files
Copilotmnriem
andauthored
fix: address all PR review comments for git extension
- Fix bash common.sh sourcing: check .specify/scripts/bash/ first, then scripts/bash/, with explicit error if neither found - Fix PowerShell common.ps1 sourcing: use $fallbackRoot for reliable path resolution, with explicit error if not found - Remove undocumented branch_template and auto_fetch from extension.yml defaults and config-template.yml (scripts don't use them yet) - Remove unused ExtensionError import in _install_bundled_git_extension - Remove undocumented SPECKIT_GIT_BRANCH_NUMBERING env var from README - Fix specify.md: skip step 2 when before_specify hook already executed - Fix specify.md: add explicit FEATURE_DIR/SPEC_FILE in disabled-git path - Fix specify.md: add PowerShell path to script resolution - Add tests for git extension auto-install during specify init Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/spec-kit/sessions/008835a0-7778-40bb-bdb2-4182b22be315
1 parent 312c37b commit 5e49ec6

File tree

8 files changed

+121
-44
lines changed

8 files changed

+121
-44
lines changed

extensions/git/README.md

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,6 @@ Configuration is stored in `.specify/extensions/git/git-config.yml`:
3232
```yaml
3333
# Branch numbering strategy: "sequential" or "timestamp"
3434
branch_numbering: sequential
35-
36-
# Branch name template
37-
branch_template: "{number}-{short_name}"
38-
39-
# Whether to fetch remotes before computing next branch number
40-
auto_fetch: true
41-
```
42-
43-
### Environment Variable Override
44-
45-
Set `SPECKIT_GIT_BRANCH_NUMBERING` to override the `branch_numbering` config value:
46-
47-
```bash
48-
export SPECKIT_GIT_BRANCH_NUMBERING=timestamp
4935
```
5036
5137
## Installation

extensions/git/config-template.yml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,3 @@
33

44
# Branch numbering strategy: "sequential" (001, 002, ...) or "timestamp" (YYYYMMDD-HHMMSS)
55
branch_numbering: sequential
6-
7-
# Branch name template (used with sequential numbering)
8-
# Available placeholders: {number}, {short_name}
9-
branch_template: "{number}-{short_name}"
10-
11-
# Whether to run `git fetch --all --prune` before computing the next branch number
12-
auto_fetch: true

extensions/git/extension.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,3 @@ tags:
4545

4646
defaults:
4747
branch_numbering: sequential
48-
branch_template: "{number}-{short_name}"
49-
auto_fetch: true

extensions/git/scripts/bash/create-new-feature.sh

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -176,21 +176,39 @@ clean_branch_name() {
176176
# were initialised with --no-git.
177177
SCRIPT_DIR="$(CDPATH="" cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
178178

179-
# Source common.sh: try the core scripts directory first (standard layout),
180-
# then fall back to the extension's sibling copy.
179+
# Source common.sh using the following priority:
180+
# 1. common.sh next to this script (source checkout layout)
181+
# 2. .specify/scripts/bash/common.sh under the project root (installed project)
182+
# 3. scripts/bash/common.sh under the project root (source checkout fallback)
183+
# 4. git-common.sh next to this script (minimal fallback)
184+
_common_loaded=false
185+
181186
if [ -f "$SCRIPT_DIR/common.sh" ]; then
182187
source "$SCRIPT_DIR/common.sh"
188+
_common_loaded=true
183189
else
184190
# When running from an extension install (.specify/extensions/git/scripts/bash/),
185-
# resolve common.sh from the project's core scripts directory.
186-
_ext_repo_root="$(cd "$SCRIPT_DIR/../../../../.." 2>/dev/null && pwd)"
187-
if [ -f "$_ext_repo_root/scripts/bash/common.sh" ]; then
188-
source "$_ext_repo_root/scripts/bash/common.sh"
191+
# resolve to .specify/ (4 levels up), then to the project root (5 levels up).
192+
_dot_specify="$(cd "$SCRIPT_DIR/../../../.." 2>/dev/null && pwd)"
193+
_project_root="$(cd "$SCRIPT_DIR/../../../../.." 2>/dev/null && pwd)"
194+
195+
if [ -n "$_dot_specify" ] && [ -f "$_dot_specify/scripts/bash/common.sh" ]; then
196+
source "$_dot_specify/scripts/bash/common.sh"
197+
_common_loaded=true
198+
elif [ -n "$_project_root" ] && [ -f "$_project_root/scripts/bash/common.sh" ]; then
199+
source "$_project_root/scripts/bash/common.sh"
200+
_common_loaded=true
189201
elif [ -f "$SCRIPT_DIR/git-common.sh" ]; then
190202
source "$SCRIPT_DIR/git-common.sh"
203+
_common_loaded=true
191204
fi
192205
fi
193206

207+
if [ "$_common_loaded" != "true" ]; then
208+
echo "Error: Could not locate common.sh or git-common.sh. Please ensure the Specify core scripts are installed." >&2
209+
exit 1
210+
fi
211+
194212
if git rev-parse --show-toplevel >/dev/null 2>&1; then
195213
REPO_ROOT=$(git rev-parse --show-toplevel)
196214
HAS_GIT=true

extensions/git/scripts/powershell/create-new-feature.ps1

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,22 +146,39 @@ if (-not $fallbackRoot) {
146146
}
147147

148148
# Load common functions (includes Resolve-Template).
149-
# Try the core scripts directory first (standard layout), then fall back
150-
# to the extension's sibling copy.
149+
# Search locations in priority order:
150+
# 1. common.ps1 next to this script (source checkout layout)
151+
# 2. .specify/scripts/powershell/common.ps1 under the project root (installed project)
152+
# 3. scripts/powershell/common.ps1 under the project root (source checkout fallback)
153+
# 4. git-common.ps1 next to this script (minimal fallback)
154+
$commonLoaded = $false
155+
151156
if (Test-Path "$PSScriptRoot/common.ps1") {
152157
. "$PSScriptRoot/common.ps1"
158+
$commonLoaded = $true
153159
} else {
154-
# When running from an extension install (.specify/extensions/git/scripts/powershell/),
155-
# resolve common.ps1 from the project's core scripts directory.
156-
$extRepoRoot = (Resolve-Path (Join-Path $PSScriptRoot "../../../../..") -ErrorAction SilentlyContinue)
157-
$coreCommon = if ($extRepoRoot) { Join-Path $extRepoRoot "scripts/powershell/common.ps1" } else { "" }
158-
if ($coreCommon -and (Test-Path $coreCommon)) {
159-
. $coreCommon
160-
} elseif (Test-Path "$PSScriptRoot/git-common.ps1") {
161-
. "$PSScriptRoot/git-common.ps1"
160+
$coreCommonCandidates = @()
161+
162+
if ($fallbackRoot) {
163+
$coreCommonCandidates += (Join-Path $fallbackRoot ".specify/scripts/powershell/common.ps1")
164+
$coreCommonCandidates += (Join-Path $fallbackRoot "scripts/powershell/common.ps1")
165+
}
166+
167+
$coreCommonCandidates += "$PSScriptRoot/git-common.ps1"
168+
169+
foreach ($candidate in $coreCommonCandidates) {
170+
if ($candidate -and (Test-Path $candidate)) {
171+
. $candidate
172+
$commonLoaded = $true
173+
break
174+
}
162175
}
163176
}
164177

178+
if (-not $commonLoaded) {
179+
throw "Unable to locate common script file. Please ensure the Specify core scripts are installed."
180+
}
181+
165182
try {
166183
$repoRoot = git rev-parse --show-toplevel 2>$null
167184
if ($LASTEXITCODE -eq 0) {

src/specify_cli/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,7 @@ def _install_bundled_git_extension(project_path: Path) -> bool:
11951195
return False
11961196

11971197
try:
1198-
from .extensions import ExtensionManager, ExtensionError
1198+
from .extensions import ExtensionManager
11991199
manager = ExtensionManager(project_path)
12001200

12011201
# Skip if already installed (e.g. via preset)

templates/commands/specify.md

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,28 @@ Given that feature description, do this:
7373
- "Create a dashboard for analytics" → "analytics-dashboard"
7474
- "Fix payment processing timeout bug" → "fix-payment-timeout"
7575
76-
2. **Create the feature branch** by running the script with `--short-name` (and `--json`). In sequential mode, do NOT pass `--number` — the script auto-detects the next available number. In timestamp mode, the script generates a `YYYYMMDD-HHMMSS` prefix automatically:
76+
2. **Create the feature branch** (unless already handled by a `before_specify` hook — see Pre-Execution Checks above). If a mandatory `before_specify` hook for `speckit.git.feature` already executed and created the branch, **skip this step entirely** and use the branch/spec information from the hook result. Otherwise:
7777
7878
**Git extension check**: Before running the branch creation script, check if the git extension is enabled:
7979
- Check if `.specify/extensions/.registry` exists (a single JSON file tracking all extensions)
8080
- If it exists, read the JSON and look for `extensions.git`; verify its `"enabled"` field is `true` (or not explicitly `false`)
81-
- If the git extension is **disabled** (explicitly `"enabled": false`), **skip branch creation entirely** — proceed directly to step 3 using a spec directory name derived from the short name (e.g., `specs/<short-name>/`)
81+
- If the git extension is **disabled** (explicitly `"enabled": false`), **skip branch creation entirely** — do **not** run the branch creation script. Instead:
82+
- Derive a spec directory name from the short name, e.g. `specs/<short-name>/`
83+
- Explicitly set the following variables so later steps can use them:
84+
- `FEATURE_DIR="specs/<short-name>"`
85+
- `SPEC_FILE="$FEATURE_DIR/spec.md"`
86+
- Ensure the directory and spec file exist:
87+
- Bash:
88+
- `mkdir -p "$FEATURE_DIR"`
89+
- `: > "$SPEC_FILE"`
90+
- PowerShell:
91+
- `New-Item -ItemType Directory -Path $env:FEATURE_DIR -Force | Out-Null`
92+
- `New-Item -ItemType File -Path $env:SPEC_FILE -Force | Out-Null`
93+
- Then proceed directly to step 3 using `FEATURE_DIR` and `SPEC_FILE`
8294
- If the registry file does not exist, proceed with branch creation using the default behavior (backward compatibility)
8395
96+
Run the script with `--short-name` (and `--json`). In sequential mode, do NOT pass `--number` — the script auto-detects the next available number. In timestamp mode, the script generates a `YYYYMMDD-HHMMSS` prefix automatically:
97+
8498
**Branch numbering mode**: Before running the script, determine the branch numbering strategy:
8599
1. Check `.specify/extensions/git/git-config.yml` for `branch_numbering` value (extension config takes precedence)
86100
2. If not found, check `.specify/init-options.json` for `branch_numbering` value (backward compatibility)
@@ -89,8 +103,8 @@ Given that feature description, do this:
89103
- If `"sequential"` or absent, do not add any extra flag (default behavior)
90104
91105
**Script resolution**: Use the extension's bundled scripts when available, falling back to core scripts:
92-
- If `.specify/extensions/git/scripts/bash/create-new-feature.sh` exists, use it
93-
- Otherwise, fall back to `{SCRIPT}`
106+
- **Bash**: If `.specify/extensions/git/scripts/bash/create-new-feature.sh` exists, use it; otherwise, fall back to `{SCRIPT}`
107+
- **PowerShell**: If `.specify/extensions/git/scripts/powershell/create-new-feature.ps1` exists, use it; otherwise, fall back to `{SCRIPT}`
94108
95109
- Bash example: `{SCRIPT} --json --short-name "user-auth" "Add user authentication"`
96110
- Bash (timestamp): `{SCRIPT} --json --timestamp --short-name "user-auth" "Add user authentication"`

tests/test_branch_numbering.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,54 @@ def _fake_download(project_path, *args, **kwargs):
8787
result = runner.invoke(app, ["init", str(tmp_path / "proj"), "--ai", "claude", "--branch-numbering", "timestamp", "--ignore-agent-tools"])
8888
assert result.exit_code == 0
8989
assert "Invalid --branch-numbering" not in (result.output or "")
90+
91+
92+
class TestGitExtensionAutoInstall:
93+
"""Tests for bundled git extension auto-install during specify init."""
94+
95+
def test_git_extension_installed_during_init(self, tmp_path: Path, monkeypatch):
96+
"""verify that `specify init` auto-installs the bundled git extension."""
97+
from typer.testing import CliRunner
98+
from specify_cli import app
99+
100+
def _fake_download(project_path, *args, **kwargs):
101+
Path(project_path).mkdir(parents=True, exist_ok=True)
102+
103+
monkeypatch.setattr("specify_cli.download_and_extract_template", _fake_download)
104+
105+
project_dir = tmp_path / "proj"
106+
runner = CliRunner()
107+
result = runner.invoke(app, ["init", str(project_dir), "--ai", "claude", "--ignore-agent-tools"])
108+
assert result.exit_code == 0
109+
110+
# Extension files should exist
111+
ext_dir = project_dir / ".specify" / "extensions" / "git"
112+
assert ext_dir.is_dir(), "git extension directory not created"
113+
assert (ext_dir / "extension.yml").is_file(), "extension.yml not installed"
114+
115+
# Registry should contain the git extension
116+
registry_file = project_dir / ".specify" / "extensions" / ".registry"
117+
assert registry_file.is_file(), "extension registry not created"
118+
registry = json.loads(registry_file.read_text())
119+
assert "git" in registry.get("extensions", {}), "git not in registry"
120+
assert registry["extensions"]["git"]["enabled"] is True
121+
122+
def test_git_extension_noop_when_already_installed(self, tmp_path: Path):
123+
"""_install_bundled_git_extension should no-op if git is already installed."""
124+
from specify_cli import _install_bundled_git_extension
125+
from specify_cli.extensions import ExtensionManager
126+
127+
project_dir = tmp_path / "proj"
128+
(project_dir / ".specify").mkdir(parents=True)
129+
130+
# First install
131+
result1 = _install_bundled_git_extension(project_dir)
132+
assert result1 is True
133+
134+
# Second install should also succeed (no-op)
135+
result2 = _install_bundled_git_extension(project_dir)
136+
assert result2 is True
137+
138+
# Only one entry in registry
139+
manager = ExtensionManager(project_dir)
140+
assert manager.registry.is_installed("git")

0 commit comments

Comments
 (0)