mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
* fix: fall back to feature dir basename for empty CURRENT_BRANCH (#3026) When a feature is resolved via SPECIFY_FEATURE_DIRECTORY or .specify/feature.json without SPECIFY_FEATURE set, get_current_branch() returns empty, so get_feature_paths / Get-FeaturePathsEnv emitted CURRENT_BRANCH= (empty) even though the feature directory was resolvable. Downstream scripts and agents that expect a non-empty identifier got misleading output. Fall back to the basename of the resolved feature directory when the branch is empty, in both the bash (`${feature_dir##*/}`) and PowerShell (`Split-Path -Leaf`) resolvers. An explicit SPECIFY_FEATURE still takes precedence, so this only fills the previously-empty case. Add bash + PowerShell regression tests: the basename fallback fires when SPECIFY_FEATURE is unset, and an explicit SPECIFY_FEATURE still overrides it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: address Copilot feedback — PS 5.1 compat + parametrize bash test - common.ps1: replace [System.IO.Path]::TrimEndingDirectorySeparator (a .NET Core-only method that throws MethodNotFound on Windows PowerShell 5.1 / .NET Framework) with a portable String.TrimEnd, so the trailing-slash trim actually works on 5.1. - tests: parametrize the bash fallback test to cover feature.json, SPECIFY_FEATURE_DIRECTORY, and the explicit SPECIFY_FEATURE override (mirrors the PowerShell test), folding in the old explicit-override test; add the missing blank line before the next test (PEP 8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -198,6 +198,15 @@ get_feature_paths() {
|
|||||||
return 1
|
return 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# When no branch context exists (no SPECIFY_FEATURE, feature resolved via
|
||||||
|
# SPECIFY_FEATURE_DIRECTORY or feature.json), fall back to the feature
|
||||||
|
# directory basename so CURRENT_BRANCH is a usable identifier rather than
|
||||||
|
# an empty, misleading value (issue #3026).
|
||||||
|
if [[ -z "$current_branch" ]]; then
|
||||||
|
local feature_dir_trimmed="${feature_dir%/}"
|
||||||
|
current_branch="${feature_dir_trimmed##*/}"
|
||||||
|
fi
|
||||||
|
|
||||||
# Use printf '%q' to safely quote values, preventing shell injection
|
# Use printf '%q' to safely quote values, preventing shell injection
|
||||||
# via crafted branch names or paths containing special characters
|
# via crafted branch names or paths containing special characters
|
||||||
printf 'REPO_ROOT=%q\n' "$repo_root"
|
printf 'REPO_ROOT=%q\n' "$repo_root"
|
||||||
|
|||||||
@@ -192,6 +192,17 @@ function Get-FeaturePathsEnv {
|
|||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# When no branch context exists (no SPECIFY_FEATURE, feature resolved via
|
||||||
|
# SPECIFY_FEATURE_DIRECTORY or feature.json), fall back to the feature
|
||||||
|
# directory basename so CURRENT_BRANCH is a usable identifier rather than
|
||||||
|
# an empty, misleading value (issue #3026).
|
||||||
|
if (-not $currentBranch) {
|
||||||
|
# TrimEnd (not [Path]::TrimEndingDirectorySeparator, which is .NET Core
|
||||||
|
# only) keeps this working on Windows PowerShell 5.1 / .NET Framework.
|
||||||
|
$featureDirTrimmed = $featureDir.TrimEnd('/', '\')
|
||||||
|
$currentBranch = Split-Path -Leaf $featureDirTrimmed
|
||||||
|
}
|
||||||
|
|
||||||
[PSCustomObject]@{
|
[PSCustomObject]@{
|
||||||
REPO_ROOT = $repoRoot
|
REPO_ROOT = $repoRoot
|
||||||
CURRENT_BRANCH = $currentBranch
|
CURRENT_BRANCH = $currentBranch
|
||||||
|
|||||||
@@ -121,6 +121,45 @@ def test_paths_only_succeeds_on_spec_branch(prereq_repo: Path) -> None:
|
|||||||
assert "001-my-feature" in data.get("BRANCH", "")
|
assert "001-my-feature" in data.get("BRANCH", "")
|
||||||
|
|
||||||
|
|
||||||
|
@requires_bash
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("use_env_var", "specify_feature", "expected_branch"),
|
||||||
|
[
|
||||||
|
(False, None, "001-my-feature"),
|
||||||
|
(True, None, "001-my-feature"),
|
||||||
|
(False, "my-explicit-branch", "my-explicit-branch"),
|
||||||
|
],
|
||||||
|
ids=["feature_json", "env_var", "explicit_feature"],
|
||||||
|
)
|
||||||
|
def test_current_branch_falls_back_to_feature_dir_basename(
|
||||||
|
prereq_repo: Path, use_env_var: bool, specify_feature: str | None, expected_branch: str
|
||||||
|
) -> None:
|
||||||
|
"""With no SPECIFY_FEATURE, BRANCH falls back to the feature directory
|
||||||
|
basename (from feature.json or SPECIFY_FEATURE_DIRECTORY) instead of being
|
||||||
|
emitted empty. If SPECIFY_FEATURE is set, it remains authoritative (#3026)."""
|
||||||
|
feat = prereq_repo / "specs" / "001-my-feature"
|
||||||
|
feat.mkdir(parents=True, exist_ok=True)
|
||||||
|
env = _clean_env()
|
||||||
|
if specify_feature:
|
||||||
|
env["SPECIFY_FEATURE"] = specify_feature
|
||||||
|
if use_env_var:
|
||||||
|
env["SPECIFY_FEATURE_DIRECTORY"] = "specs/001-my-feature"
|
||||||
|
else:
|
||||||
|
_write_feature_json(prereq_repo)
|
||||||
|
script = prereq_repo / ".specify" / "scripts" / "bash" / "check-prerequisites.sh"
|
||||||
|
result = subprocess.run(
|
||||||
|
["bash", str(script), "--json", "--paths-only"],
|
||||||
|
cwd=prereq_repo,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
env=env,
|
||||||
|
)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
data = json.loads(result.stdout)
|
||||||
|
assert data["BRANCH"] == expected_branch
|
||||||
|
|
||||||
|
|
||||||
@requires_bash
|
@requires_bash
|
||||||
def test_paths_only_text_mode_on_non_spec_branch(prereq_repo: Path) -> None:
|
def test_paths_only_text_mode_on_non_spec_branch(prereq_repo: Path) -> None:
|
||||||
"""--paths-only without --json must return text paths from feature.json."""
|
"""--paths-only without --json must return text paths from feature.json."""
|
||||||
@@ -249,6 +288,46 @@ def test_ps_paths_only_succeeds_on_non_spec_branch(prereq_repo: Path) -> None:
|
|||||||
assert "FEATURE_DIR" in data
|
assert "FEATURE_DIR" in data
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(not (HAS_PWSH or _WINDOWS_POWERSHELL), reason="no PowerShell available")
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("use_env_var", "specify_feature", "expected_branch"),
|
||||||
|
[
|
||||||
|
(False, None, "001-my-feature"),
|
||||||
|
(True, None, "001-my-feature"),
|
||||||
|
(False, "my-explicit-branch", "my-explicit-branch"),
|
||||||
|
],
|
||||||
|
ids=["feature_json", "env_var", "explicit_feature"],
|
||||||
|
)
|
||||||
|
def test_ps_current_branch_falls_back_to_feature_dir_basename(
|
||||||
|
prereq_repo: Path, use_env_var: bool, specify_feature: str | None, expected_branch: str
|
||||||
|
) -> None:
|
||||||
|
"""With no SPECIFY_FEATURE, BRANCH falls back to the feature directory
|
||||||
|
basename (from feature.json or SPECIFY_FEATURE_DIRECTORY) instead of being
|
||||||
|
emitted empty. If SPECIFY_FEATURE is set, it remains authoritative (#3026)."""
|
||||||
|
feat = prereq_repo / "specs" / "001-my-feature"
|
||||||
|
feat.mkdir(parents=True, exist_ok=True)
|
||||||
|
env = _clean_env()
|
||||||
|
if specify_feature:
|
||||||
|
env["SPECIFY_FEATURE"] = specify_feature
|
||||||
|
if use_env_var:
|
||||||
|
env["SPECIFY_FEATURE_DIRECTORY"] = "specs/001-my-feature"
|
||||||
|
else:
|
||||||
|
_write_feature_json(prereq_repo)
|
||||||
|
script = prereq_repo / ".specify" / "scripts" / "powershell" / "check-prerequisites.ps1"
|
||||||
|
exe = "pwsh" if HAS_PWSH else _WINDOWS_POWERSHELL
|
||||||
|
result = subprocess.run(
|
||||||
|
[exe, "-NoProfile", "-File", str(script), "-Json", "-PathsOnly"],
|
||||||
|
cwd=prereq_repo,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
env=env,
|
||||||
|
)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
data = json.loads(result.stdout)
|
||||||
|
assert data["BRANCH"] == expected_branch
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not (HAS_PWSH or _WINDOWS_POWERSHELL), reason="no PowerShell available")
|
@pytest.mark.skipif(not (HAS_PWSH or _WINDOWS_POWERSHELL), reason="no PowerShell available")
|
||||||
def test_ps_paths_only_succeeds_on_spec_branch(prereq_repo: Path) -> None:
|
def test_ps_paths_only_succeeds_on_spec_branch(prereq_repo: Path) -> None:
|
||||||
"""-PathsOnly must also work when feature.json and SPECIFY_FEATURE agree."""
|
"""-PathsOnly must also work when feature.json and SPECIFY_FEATURE agree."""
|
||||||
|
|||||||
Reference in New Issue
Block a user