-
Notifications
You must be signed in to change notification settings - Fork 7.8k
fix(plan): use .specify/feature.json to allow /speckit.plan on custom git branches (#2305) #2349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -164,6 +164,44 @@ function Test-FeatureBranch { | |||||
| return $true | ||||||
| } | ||||||
|
|
||||||
| # True when .specify/feature.json pins an existing feature directory that matches the | ||||||
| # active FEATURE_DIR from Get-FeaturePathsEnv (so /speckit.plan can skip git branch pattern checks). | ||||||
| function Test-FeatureJsonMatchesFeatureDir { | ||||||
| param( | ||||||
| [Parameter(Mandatory = $true)][string]$RepoRoot, | ||||||
| [Parameter(Mandatory = $true)][string]$ActiveFeatureDir | ||||||
| ) | ||||||
|
|
||||||
| $featureJson = Join-Path $RepoRoot '.specify\feature.json' | ||||||
|
||||||
| $featureJson = Join-Path $RepoRoot '.specify\feature.json' | |
| $featureJson = Join-Path (Join-Path $RepoRoot '.specify') 'feature.json' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,9 +23,11 @@ if ($Help) { | |
| # Get all paths and variables from common functions | ||
| $paths = Get-FeaturePathsEnv | ||
|
|
||
| # Check if we're on a proper feature branch (only for git repos) | ||
| if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) { | ||
| exit 1 | ||
| # If feature.json pins an existing feature directory, branch naming is not required. | ||
| if (-not (Test-FeatureJsonMatchesFeatureDir -RepoRoot $paths.REPO_ROOT -ActiveFeatureDir $paths.FEATURE_DIR)) { | ||
| if (-not (Test-FeatureBranch -Branch $paths.CURRENT_BRANCH -HasGit $paths.HAS_GIT)) { | ||
| exit 1 | ||
| } | ||
| } | ||
|
Comment on lines
+26
to
31
|
||
|
|
||
| # Ensure the feature directory exists | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,181 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """Tests for setup-plan bypassing branch-pattern checks when feature.json is valid.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import shutil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| from tests.conftest import requires_bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| PROJECT_ROOT = Path(__file__).resolve().parent.parent | ||||||||||||||||||||||||||||||||||||||||||||||||||
| COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SETUP_PLAN_SH = PROJECT_ROOT / "scripts" / "bash" / "setup-plan.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| COMMON_PS = PROJECT_ROOT / "scripts" / "powershell" / "common.ps1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SETUP_PLAN_PS = PROJECT_ROOT / "scripts" / "powershell" / "setup-plan.ps1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| PLAN_TEMPLATE = PROJECT_ROOT / "templates" / "plan-template.md" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| HAS_PWSH = shutil.which("pwsh") is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _POWERSHELL = shutil.which("powershell.exe") or shutil.which("powershell") | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def _install_bash_scripts(repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| d = repo / ".specify" / "scripts" / "bash" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| d.mkdir(parents=True, exist_ok=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copy(COMMON_SH, d / "common.sh") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copy(SETUP_PLAN_SH, d / "setup-plan.sh") | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def _install_ps_scripts(repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| d = repo / ".specify" / "scripts" / "powershell" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| d.mkdir(parents=True, exist_ok=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copy(COMMON_PS, d / "common.ps1") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copy(SETUP_PLAN_PS, d / "setup-plan.ps1") | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def _minimal_templates(repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| tdir = repo / ".specify" / "templates" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| tdir.mkdir(parents=True, exist_ok=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| shutil.copy(PLAN_TEMPLATE, tdir / "plan-template.md") | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def _git_init(repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run(["git", "init", "-q"], cwd=repo, check=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "config", "user.email", "test@example.com"], cwd=repo, check=True | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run(["git", "config", "user.name", "Test User"], cwd=repo, check=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "commit", "--allow-empty", "-m", "init", "-q"], cwd=repo, check=True | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @pytest.fixture | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def plan_repo(tmp_path: Path) -> Path: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| repo = tmp_path / "proj" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| repo.mkdir() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _git_init(repo) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (repo / ".specify").mkdir() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _minimal_templates(repo) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _install_bash_scripts(repo) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| _install_ps_scripts(repo) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return repo | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @requires_bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_setup_plan_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat = plan_repo / "specs" / "001-tiny-notes-app" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat.mkdir(parents=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (feat / "spec.md").write_text("# spec\n", encoding="utf-8") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (plan_repo / ".specify" / "feature.json").write_text( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| json.dumps({"feature_directory": "specs/001-tiny-notes-app"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| encoding="utf-8", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["bash", str(script)], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert result.returncode == 0, result.stderr + result.stdout | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert (feat / "plan.md").is_file() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @requires_bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_setup_plan_fails_custom_branch_without_feature_json(plan_repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["bash", str(script)], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert result.returncode != 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert "Not on a feature branch" in result.stderr | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @requires_bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_setup_plan_numbered_branch_unchanged_without_feature_json( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| plan_repo: Path, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "checkout", "-q", "-b", "001-tiny-notes-app"], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat = plan_repo / "specs" / "001-tiny-notes-app" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat.mkdir(parents=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (feat / "spec.md").write_text("# spec\n", encoding="utf-8") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| script = plan_repo / ".specify" / "scripts" / "bash" / "setup-plan.sh" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["bash", str(script)], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert result.returncode == 0, result.stderr + result.stdout | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert (feat / "plan.md").is_file() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| @pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_setup_plan_ps_passes_custom_branch_when_feature_json_valid(plan_repo: Path) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat = plan_repo / "specs" / "001-tiny-notes-app" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| feat.mkdir(parents=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (feat / "spec.md").write_text("# spec\n", encoding="utf-8") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| (plan_repo / ".specify" / "feature.json").write_text( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| json.dumps({"feature_directory": "specs/001-tiny-notes-app"}), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| encoding="utf-8", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| exe = "pwsh" if HAS_PWSH else _POWERSHELL | ||||||||||||||||||||||||||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [exe, "-NoProfile", "-File", str(script)], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cwd=plan_repo, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| check=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert result.returncode == 0, result.stderr + result.stdout | ||||||||||||||||||||||||||||||||||||||||||||||||||
| assert (feat / "plan.md").is_file() | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| assert (feat / "plan.md").is_file() | |
| assert (feat / "plan.md").is_file() | |
| @pytest.mark.skipif(not (HAS_PWSH or _POWERSHELL), reason="no PowerShell available") | |
| def test_setup_plan_ps_fails_custom_branch_without_feature_json( | |
| plan_repo: Path, | |
| ) -> None: | |
| subprocess.run( | |
| ["git", "checkout", "-q", "-b", "feature/my-feature-branch"], | |
| cwd=plan_repo, | |
| check=True, | |
| ) | |
| script = plan_repo / ".specify" / "scripts" / "powershell" / "setup-plan.ps1" | |
| exe = "pwsh" if HAS_PWSH else _POWERSHELL | |
| result = subprocess.run( | |
| [exe, "-NoProfile", "-File", str(script)], | |
| cwd=plan_repo, | |
| capture_output=True, | |
| text=True, | |
| check=False, | |
| ) | |
| assert result.returncode != 0 | |
| assert "Not on a feature branch" in result.stderr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feature_json_matches_feature_dirre-implements feature.json parsing/normalization logic that already exists inget_feature_paths(jq/python/grep fallbacks). This duplication risks the two code paths drifting (e.g., future changes to parsing rules need to be updated twice). Consider extracting a single “read feature_directory from feature.json” helper used by both places, or havingget_feature_pathssurface whether the resolvedFEATURE_DIRcame from a valid existing feature.json directory.