mirror of
https://github.com/github/spec-kit
synced 2026-06-21 13:51:39 +00:00
copilot/fix-code-comments
12 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
f684305e51 |
fix(powershell): ensure UTF-8 templates are written without BOM (#2280)
* fix(powershell): strip BOM from templates and ensure No-BOM output * fix: address review feedback on encoding and naming for all ps scripts * fix: address copilot feedback (encoding detection and variable naming) * fix: remove duplicate comments in setup-plan.ps1 * test: verify spec.md is written without UTF-8 BOM * test: also verify BOM-free output under Windows PowerShell 5.1 * fix * fix: resolve merge conflict with main, add TestDescriptionQuoting * fix: resolve TestDescriptionQuoting string quoting conflict with main * test: restore PowerShell prefix-stripping parity test in TestGetFeaturePathsSinglePrefix * fix: remove trailing whitespace from module docstring blank lines * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * test: seed ps_git_repo with BOM-prefixed template to exercise WriteAllText fix * fix: remove duplicate ps_git_repo fixture, restore ext_ps_git_repo * fix: remove unrelated TestDescriptionQuoting and restore original test_ps_specify_feature_prefixed_resolves_by_prefix --------- Co-authored-by: Nimraakram22 <nimra.akram123451@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
|
13d88d22a6 |
fix: replace xargs trim with sed to handle quotes in descriptions (#2351)
xargs re-parses stdin as shell tokens, causing 'unterminated quote' errors when feature descriptions contain apostrophes, double quotes, or backslashes. Replace with sed-based whitespace trim that preserves input verbatim. Add regression tests for special characters in descriptions (core and extension scripts), plus a negative test for whitespace-only input. Fixes #2339 |
||
|
|
9988a46d96 |
ci: add windows-latest to test matrix (#2233)
* ci: add windows-latest to test matrix Add windows-latest to the pytest job OS matrix so tests run on both Ubuntu and Windows for all Python versions. Closes #2232 * test: skip bash-specific tests on Windows Add sys.platform skip markers to all test classes and methods that execute bash scripts via subprocess, so they are skipped on Windows where bash is not available. Mixed classes with both bash and pwsh tests have markers on individual bash methods only. * test: fix 3 Windows-specific test failures - test_manifest: use platform-appropriate absolute path (C:\ on Windows vs /tmp on POSIX) since /tmp is not absolute on Windows - test_extensions: add agent_scripts.ps entry and platform-conditional assertions for codex skill fallback variant test - test_timestamp_branches: use json.dumps() instead of f-string to properly escape Windows backslash paths in feature.json * test: extract requires_bash marker and fix PS test skip Address PR review feedback: - Define a reusable requires_bash marker in conftest.py and use it across all 3 test files instead of repeating the skipif inline - Move test_powershell_scanner_uses_long_tryparse_for_large_prefixes into its own TestSequentialBranchPowerShell class so it is not incorrectly skipped on Windows by the class-level bash marker * test: use runtime bash check instead of platform check Replace sys.platform == 'win32' with an actual bash invocation test to handle environments where bash exists but is non-functional (e.g., WSL stub on Windows without an installed distro). * test: reject WSL bash, accept only MSYS/MINGW on Windows On Windows, verify uname -s reports MSYS, MINGW, or CYGWIN so the WSL launcher (System32\bash.exe) is rejected — it cannot handle native Windows paths used by test fixtures. Add SPECKIT_TEST_BASH=1 env var escape hatch to force-enable bash tests in non-standard setups. * ci: add comment explaining Windows bash test behavior * test: early-reject WSL launcher, fix remaining f-string JSON - Check resolved bash path for System32 before spawning any subprocess to avoid WSL init prompts and timeout during test collection - Convert remaining feature_json f-string writes to json.dumps() so paths with backslashes produce valid JSON on Windows * test: use bare 'bash' for detection to match test invocation On Windows, subprocess.run(['bash', ...]) uses CreateProcess which searches System32 before PATH — finding WSL bash even when shutil.which('bash') returns Git-for-Windows. Probe with bare 'bash' (same as test helpers) so the detection matches actual test behavior. |
||
|
|
4687c33b0f |
feat(scripts): optional single-segment branch prefix for gitflow (#2202)
* feat(scripts): optional single-segment branch prefix for gitflow - Add spec_kit_effective_branch_name / Get-SpecKitEffectiveBranchName: when branch matches prefix/rest with exactly one slash, validate and resolve specs/ using only the rest (e.g. feat/001-my-feature). - Wire into check_feature_branch, find_feature_dir_by_prefix (bash) and Test-FeatureBranch, Find-FeatureDirByPrefix + Get-FeaturePathsEnv (PS). - Align git extension git-common with core validation; remove unused get_feature_dir / Get-FeatureDir helpers. - Extend tests in test_timestamp_branches.py and test_git_extension.py. Made-with: Cursor * Update scripts/powershell/common.ps1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(ps): align feature-dir resolution errors with bash (no throw under Stop) Find-FeatureDirByPrefix: on ambiguous prefix matches, write errors to stderr and return $null instead of throwing, matching find_feature_dir_by_prefix. Get-FeaturePathsEnv: narrow try/catch to ConvertFrom-Json only; add Get-FeatureDirFromBranchPrefixOrExit to mirror bash get_feature_paths (stderr + exit 1) when prefix lookup fails, avoiding unhandled terminating errors under $ErrorActionPreference = 'Stop' in check-prerequisites, setup-plan, and update-agent-context. Made-with: Cursor * Update tests/test_timestamp_branches.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update extensions/git/scripts/powershell/git-common.ps1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update scripts/powershell/common.ps1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |
||
|
|
2972dec85c |
feat: Git extension stage 2 — GIT_BRANCH_NAME override, --force for existing dirs, auto-install tests (#1940) (#2117)
* feat: Git extension stage 2 — GIT_BRANCH_NAME override, --force for existing dirs, auto-install tests (#1940) - Add GIT_BRANCH_NAME env var override to create-new-feature.sh/.ps1 for exact branch naming (bypasses all prefix/suffix generation) - Fix --force flag for 'specify init <dir>' into existing directories - Add TestGitExtensionAutoInstall tests (auto-install, --no-git skip, commands registered) - Add TestFeatureDirectoryResolution tests (env var, feature.json, priority, branch fallback) - Document GIT_BRANCH_NAME in speckit.git.feature.md and specify.md * fix: remove unused Tuple import (ruff F401) * fix: address Copilot review feedback (#2117) - Fix timestamp regex ordering: check YYYYMMDD-HHMMSS before generic numeric prefix in both bash and PowerShell - Set BRANCH_SUFFIX in GIT_BRANCH_NAME override path so 244-byte truncation logic works correctly - Add 244-byte length check for GIT_BRANCH_NAME in PowerShell - Use existing_items for non-empty dir warning with --force - Skip git extension install if already installed (idempotent --force) - Wrap PowerShell feature.json parsing in try/catch for malformed JSON - Fix PS comment: 'prefix lookup' -> 'exact mapping via Get-FeatureDir' - Remove non-functional SPECIFY_SPEC_DIRECTORY from specify.md template * fix: address second round of Copilot review feedback (#2117) - Guard shutil.rmtree on init failure: skip cleanup when --force merged into a pre-existing directory (prevents data loss) - Bash: error on GIT_BRANCH_NAME >244 bytes instead of broken truncation - Fix malformed numbered list in specify.md (restore missing step 1) - Add claude_skills.exists() assert before iterdir() in test * fix: use UTF-8 byte count for 244-byte branch name limit (#2117) - Bash: use LC_ALL=C wc -c for byte length instead of ${#VAR} - PowerShell: use [System.Text.Encoding]::UTF8.GetByteCount() instead of .Length (UTF-16 code units) * fix: address third round of review feedback (#2117) - Update --dry-run help text in bash and PowerShell (branch name only) - Fix specify.md JSON example: use concrete path, not literal variable - Add TestForceExistingDirectory tests (merge + error without --force) - Add PowerShell Get-FeaturePathsEnv tests (env var + feature.json) * fix: normalize relative paths and fix Test-HasGit compat (#2117) - Bash common.sh: normalize SPECIFY_FEATURE_DIRECTORY and feature.json relative paths to absolute under repo root - PowerShell common.ps1: same normalization using IsPathRooted + Join-Path - PowerShell create-new-feature.ps1: call Test-HasGit without -RepoRoot for compatibility with core common.ps1 (no param) and git-common.ps1 (optional param with default) * test: add GIT_BRANCH_NAME automated tests for bash and PowerShell (#2117) - TestGitBranchNameOverrideBash: 5 tests (exact name, sequential prefix, timestamp prefix, overlong rejection, dry-run) - TestGitBranchNameOverridePowerShell: 4 tests (exact name, sequential prefix, timestamp prefix, overlong rejection) - Tests use extension scripts (not core) via new ext_git_repo and ext_ps_git_repo fixtures * fix: restore git init during specify init + review fixes (#2117) - Restore is_git_repo() and init_git_repo() functions removed in stage 2 - specify init now runs git init AND installs git extension (not just extension install alone) - Add is_dir() guard for non-here path to prevent uncontrolled error when target exists but is a file - Add python3 JSON fallback in common.sh for multi-line feature.json (grep pipeline fails on pretty-printed JSON without jq) * fix: use init_git_repo error_msg in failure output (#2117) * fix: ensure_executable_scripts also covers .specify/extensions/ (#2117) Extension .sh scripts (e.g. create-new-feature.sh, initialize-repo.sh) may lack execute bits after install. Scan both .specify/scripts/ and .specify/extensions/ for permission fixing. * fix: move chmod after extension install + sanitize error_msg (#2117) - ensure_executable_scripts() now runs after git extension install so extension .sh files get execute bits in the same init run - Sanitize init_git_repo error_msg to single line (replace newlines, truncate to 120 chars) to prevent garbled StepTracker output * fix: use tracker.error for git init/extension failures (#2117) Git init failure and extension install failure were reported as tracker.complete (showing green) even on error. Now track a git_has_error flag and call tracker.error when any step fails, so the UI correctly reflects the failure state. * fix: sanitize ext_err in git step tracker for consistent rendering (#2117) |
||
|
|
838bd0fedc | fix(git): surface checkout errors for existing branches (#2122) | ||
|
|
f92e7e8096 |
fix: accept 4+ digit spec numbers in tests and docs (#2094)
Two test assertions in test_timestamp_branches.py used the regex
`\d{3}` (exactly 3 digits) instead of `\d{3,}` (3 or more digits).
While the underlying shell scripts already handle spec numbers ≥ 1000
correctly — printf "%03d" and PowerShell '{0:000}' both expand naturally
beyond 3 digits, and all detection regexes use {3,} — the overly-strict
test assertions would fail with a misleading error if a fixture ever
contained 1000+ spec directories.
Documentation in README.md, spec-driven.md, and the CLI --branch-numbering
help text implied that sequential spec numbers are always 3 digits, which
could lead users to believe a hard limit of 999 exists.
Changes:
- tests/test_timestamp_branches.py: change two \d{3} assertions to \d{3,}
- src/specify_cli/__init__.py: clarify help text to show numbers expand past 999
- README.md: update --branch-numbering docs to note numbers expand beyond 3 digits
- spec-driven.md: update feature numbering description to include 4-digit example
Fixes #2093
Co-authored-by: alex-zwingli <alex-zwingli@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
||
|
|
b44ffc0101 |
feat(scripts): add --dry-run flag to create-new-feature (#1998)
* feat(scripts): add --dry-run flag to create-new-feature scripts Add a --dry-run / -DryRun flag to both bash and PowerShell create-new-feature scripts that computes the next branch name, spec file path, and feature number without creating any branches, directories, or files. This enables external tools to query the next available name before running the full specify workflow. When combined with --json, the output includes a DRY_RUN field. Without --dry-run, behavior is completely unchanged. Closes #1931 Assisted-By: 🤖 Claude Code * fix(scripts): gate specs/ dir creation behind dry-run check Dry-run was unconditionally creating the root specs/ directory via mkdir -p / New-Item before the dry-run guard. This violated the documented contract of zero side effects. Also adds returncode assertion on git branch --list in tests and adds PowerShell dry-run test coverage (skipped when pwsh unavailable). Addresses review comments on #1998. Assisted-By: 🤖 Claude Code * fix: address PR review feedback - Gate `mkdir -p $SPECS_DIR` behind DRY_RUN check (bash + PowerShell) so dry-run creates zero directories - Add returncode assertion on `git branch --list` in test - Strengthen spec dir test to verify root `specs/` is not created - Add PowerShell dry-run test class (5 tests, skipped without pwsh) - Fix run_ps_script to use temp repo copy instead of project root Assisted-By: 🤖 Claude Code * fix: use git ls-remote for remote-aware dry-run numbering Dry-run now queries remote branches via `git ls-remote --heads` (read-only, no fetch) to account for remote-only branches when computing the next sequential number. This prevents dry-run from returning a number that already exists on a remote. Added test verifying dry-run sees remote-only higher-numbered branches and adjusts numbering accordingly. Assisted-By: 🤖 Claude Code * fix(scripts): deduplicate number extraction and branch scanning logic Extract shared _extract_highest_number helper (bash) and Get-HighestNumberFromNames (PowerShell) to eliminate duplicated number extraction patterns between local branch and remote ref scanning. Add SkipFetch/skip_fetch parameter to check_existing_branches / Get-NextBranchNumber so dry-run reuses the same function instead of inlining duplicate max-of-branches-and-specs logic. Assisted-By: 🤖 Claude Code * fix(tests): use isolated paths for remote branch test Move remote.git and second_clone directories under git_repo instead of git_repo.parent to prevent path collisions with parallel test workers. Assisted-By: 🤖 Claude Code * fix: address PR review feedback - Set GIT_TERMINAL_PROMPT=0 for git ls-remote calls to prevent credential prompts from blocking dry-run in automation scenarios - Add returncode assertion to test_dry_run_with_timestamp git branch --list check Assisted-By: 🤖 Claude Code |
||
|
|
8e14ab1935 |
fix: support feature branch numbers with 4+ digits (#2040)
* fix: support feature branch numbers with 4+ digits in common.sh and common.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 #344
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: narrow timestamp guard and use [long] to prevent overflow
- 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>
* test: add regression tests for 4+ digit feature branch support
Cover check_feature_branch and find_feature_dir_by_prefix with 4-digit
sequential prefixes, as requested in PR review #2040.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix: reject timestamp-like branches without trailing slug
Branches like "20260319-143022" (no "-<name>" suffix) were incorrectly
accepted as sequential prefixes. Add explicit rejection for 7-or-8
digit date + 6-digit time patterns with no trailing slug, in both
common.sh and common.ps1.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
|
f8da535d71 |
feat(scripts): add --allow-existing-branch flag to create-new-feature (#1999)
* feat(scripts): add --allow-existing-branch flag to create-new-feature Add an --allow-existing-branch / -AllowExistingBranch flag to both bash and PowerShell create-new-feature scripts. When the target branch already exists, the script switches to it instead of failing. The spec directory and template are still created if missing, but existing spec.md files are not overwritten (prevents data loss on re-runs). The flag is opt-in, so existing behavior is completely unchanged without it. This enables worktree-based workflows and CI/CD pipelines that create branches externally before running speckit.specify. Relates to #1931. Also addresses #1680, #841, #1921. Assisted-By: 🤖 Claude Code * fix: address PR review feedback for allow-existing-branch - Make checkout failure fatal instead of suppressing with || true (bash) - Check $LASTEXITCODE after git checkout in PowerShell - Use Test-Path -PathType Leaf for spec file existence check (PS) - Add PowerShell static assertion test for -AllowExistingBranch flag Assisted-By: 🤖 Claude Code |
||
|
|
ccc44dd00a |
Unify Kimi/Codex skill naming and migrate legacy dotted Kimi dirs (#1971)
* fix: unify hyphenated skills and migrate legacy kimi dotted dirs * fix: preserve legacy kimi dotted preset skill overrides * fix: migrate kimi legacy dotted skills without ai-skills flag * fix: harden kimi migration and cache hook init options * fix: apply kimi preset skill overrides without ai-skills flag * fix: keep sequential branch numbering beyond 999 * test: align kimi scaffold skill path with hyphen naming * chore: align hook typing and preset skill comment * fix: restore AGENT_SKILLS_DIR_OVERRIDES compatibility export * refactor: remove AGENT_SKILLS_DIR_OVERRIDES and update callers * fix(ps1): support sequential branch numbers above 999 * fix: resolve preset skill placeholders for skills agents * Fix legacy kimi migration safety and preset skill dir checks * Harden TOML rendering and consolidate preset skill restore parsing * Fix PowerShell overflow and hook message fallback for empty invocations * Restore preset skills from extensions * Refine preset skill restore helpers * Harden skill path and preset checks * Guard non-dict init options * Avoid deleting unmanaged preset skill dirs * Unify extension skill naming with hooks * Harden extension native skill registration * Normalize preset skill titles |
||
|
|
65ecd5321d |
feat: add timestamp-based branch naming option for specify init (#1911)
* feat: add timestamp-based branch naming option for specify init Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Copilot feedback * Fix test * Copilot feedback * Update tests/test_branch_numbering.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |