diff --git a/scripts/bash/check-prerequisites.sh b/scripts/bash/check-prerequisites.sh index 1eadd991b2..e802113bc0 100644 --- a/scripts/bash/check-prerequisites.sh +++ b/scripts/bash/check-prerequisites.sh @@ -148,7 +148,13 @@ if [[ ! -f "$IMPL_PLAN" ]]; then exit 1 fi -if [[ ! -s "$IMPL_PLAN" ]]; then +if [[ ! -r "$IMPL_PLAN" ]]; then + echo "ERROR: plan.md is not readable in $FEATURE_DIR" >&2 + echo "Check its file permissions and rerun the prerequisite check." >&2 + exit 1 +fi + +if ! file_has_non_whitespace_content "$IMPL_PLAN"; then echo "ERROR: plan.md is empty in $FEATURE_DIR" >&2 echo "Populate it, or remove the empty file and rerun $(format_speckit_command plan "$REPO_ROOT") with a valid plan template." >&2 exit 1 diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index ff18cba4d9..9096689c25 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -1,6 +1,13 @@ #!/usr/bin/env bash # Common functions and variables for all scripts +# Match Unicode White_Space, as PowerShell does, even under LC_ALL=C. +# Remove only a file-leading UTF-8 BOM for the check; never rewrite the file. +file_has_non_whitespace_content() { + LC_ALL=C sed $'1s/^\357\273\277//' "$1" | + LC_ALL=C grep -aEv $'^([[:space:]]|\302[\205\240]|\341\232\200|\342\200[\200-\212\250\251\257]|\342\201\237|\343\200\200)*$' >/dev/null +} + # Find repository root by searching upward for .specify directory # This is the primary marker for spec-kit projects find_specify_root() { diff --git a/scripts/bash/setup-plan.sh b/scripts/bash/setup-plan.sh index 9c8d163409..3bd09f712c 100644 --- a/scripts/bash/setup-plan.sh +++ b/scripts/bash/setup-plan.sh @@ -46,7 +46,7 @@ else template_file=$(mktemp) trap 'rm -f "$template_file"' EXIT if resolve_template_content "plan-template" "$REPO_ROOT" > "$template_file"; then - if [[ ! -s "$template_file" ]]; then + if ! file_has_non_whitespace_content "$template_file"; then echo "ERROR: Plan template is empty; no new plan.md was created." >&2 echo "Restore a valid plan template and rerun plan setup." >&2 exit 1 diff --git a/scripts/powershell/check-prerequisites.ps1 b/scripts/powershell/check-prerequisites.ps1 index 04814e6283..7060ad5805 100644 --- a/scripts/powershell/check-prerequisites.ps1 +++ b/scripts/powershell/check-prerequisites.ps1 @@ -108,7 +108,7 @@ if (-not (Test-Path -LiteralPath $paths.IMPL_PLAN -PathType Leaf)) { exit 1 } -if ((Get-Item -LiteralPath $paths.IMPL_PLAN).Length -eq 0) { +if ([string]::IsNullOrWhiteSpace([System.IO.File]::ReadAllText($paths.IMPL_PLAN))) { [Console]::Error.WriteLine("ERROR: plan.md is empty in $($paths.FEATURE_DIR)") $planCommand = Format-SpecKitCommand -CommandName 'plan' -RepoRoot $paths.REPO_ROOT [Console]::Error.WriteLine( diff --git a/scripts/powershell/setup-plan.ps1 b/scripts/powershell/setup-plan.ps1 index 88b342c8bb..2d0c85a134 100644 --- a/scripts/powershell/setup-plan.ps1 +++ b/scripts/powershell/setup-plan.ps1 @@ -48,7 +48,7 @@ if (Test-Path -LiteralPath $paths.IMPL_PLAN -PathType Leaf) { } else { $content = Resolve-TemplateContent -TemplateName 'plan-template' -RepoRoot $paths.REPO_ROOT if ($null -ne $content) { - if ($content.Length -eq 0) { + if ([string]::IsNullOrWhiteSpace($content)) { [Console]::Error.WriteLine( "ERROR: Plan template is empty; no new plan.md was created." ) diff --git a/scripts/python/check_prerequisites.py b/scripts/python/check_prerequisites.py index c7834fe456..29e49357b7 100644 --- a/scripts/python/check_prerequisites.py +++ b/scripts/python/check_prerequisites.py @@ -14,6 +14,7 @@ TemplateResolutionError, format_speckit_command, get_feature_paths, + has_non_whitespace_content, resolve_template_content, ) except ImportError: # pragma: no cover - direct execution from unusual cwd @@ -23,6 +24,7 @@ TemplateResolutionError, format_speckit_command, get_feature_paths, + has_non_whitespace_content, resolve_template_content, ) @@ -236,7 +238,9 @@ def main(argv: list[str] | None = None) -> int: ) return 1 - if paths.impl_plan.stat().st_size == 0: + if not has_non_whitespace_content( + paths.impl_plan.read_text(encoding="utf-8", errors="replace") + ): print(f"ERROR: plan.md is empty in {paths.feature_dir}", file=sys.stderr) print( "Populate it, or remove the empty file and rerun " diff --git a/scripts/python/common.py b/scripts/python/common.py index 0cd17e013d..9fb014b7d1 100644 --- a/scripts/python/common.py +++ b/scripts/python/common.py @@ -11,6 +11,15 @@ from pathlib import Path +def has_non_whitespace_content(content: str) -> bool: + """Check Unicode White_Space like PowerShell, ignoring one leading BOM.""" + whitespace = ( + "\t\n\v\f\r \u0085\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005" + "\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000" + ) + return bool(content.removeprefix("\ufeff").strip(whitespace)) + + def _trim_trailing_separators(value: Path) -> str: text = str(value) while len(text) > 1 and text.endswith((os.sep, "/")): diff --git a/scripts/python/setup_plan.py b/scripts/python/setup_plan.py index 3eaa9f4a3f..a1d64f761d 100644 --- a/scripts/python/setup_plan.py +++ b/scripts/python/setup_plan.py @@ -11,6 +11,7 @@ from common import ( TemplateResolutionError, get_feature_paths, + has_non_whitespace_content, resolve_template_content, ) except ImportError: # pragma: no cover - direct execution from unusual cwd @@ -18,6 +19,7 @@ from common import ( TemplateResolutionError, get_feature_paths, + has_non_whitespace_content, resolve_template_content, ) @@ -70,7 +72,7 @@ def main(argv: list[str] | None = None) -> int: print(f"Error: {exc}", file=sys.stderr) return 1 if template_content is not None: - if template_content == "": + if not has_non_whitespace_content(template_content): print( "ERROR: Plan template is empty; no new plan.md was created.", file=sys.stderr, diff --git a/tests/test_check_prerequisites_python_parity.py b/tests/test_check_prerequisites_python_parity.py index 3a53887ff8..05327b8079 100644 --- a/tests/test_check_prerequisites_python_parity.py +++ b/tests/test_check_prerequisites_python_parity.py @@ -576,7 +576,16 @@ def test_python_negative_errors_are_stderr_only( ], ) @pytest.mark.parametrize("json_mode", [True, False], ids=["json", "text"]) -@pytest.mark.parametrize("plan_state", ["missing", "empty", "nonempty", "whitespace"]) +@pytest.mark.parametrize( + "plan_state", + [ + "missing", "empty", "nonempty", "padded", + "whitespace", "whitespace_crlf", "whitespace_controls", + "whitespace_nbsp", "whitespace_bom", "whitespace_bom_nbsp", + "whitespace_unicode", "bom_content", "utf16_content", "cp1252_content", + "non_whitespace_control", "internal_bom", "double_bom", + ], +) def test_plan_prerequisite_diagnostics( tmp_path: Path, port: str, json_mode: bool, plan_state: str ) -> None: @@ -589,7 +598,27 @@ def test_plan_prerequisite_diagnostics( feature = repo / "specs" / "001-my-feature" feature.mkdir(parents=True) plan = feature / "plan.md" - bodies = {"empty": b"", "nonempty": b"# Plan\n", "whitespace": b" \t\n"} + bodies = { + "empty": b"", + "nonempty": b"# Plan\n", + "padded": b" \t\n# Plan\n \t\r\n", + "whitespace": b" \t\n", + "whitespace_crlf": b" \t\r\n\r\n", + "whitespace_controls": b"\v\f\n", + "whitespace_nbsp": "\u00a0\n".encode(), + "whitespace_bom": b"\xef\xbb\xbf \t\r\n", + "whitespace_bom_nbsp": "\ufeff\u00a0\n".encode(), + "whitespace_unicode": ( + "\u0085\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005" + "\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\n" + ).encode(), + "bom_content": b"\xef\xbb\xbf \t# Plan\n", + "utf16_content": "# Plan\n".encode("utf-16"), + "cp1252_content": b"# \x93Plan\x94\n", + "non_whitespace_control": b"\x1c", + "internal_bom": b"\n\xef\xbb\xbf\n", + "double_bom": b"\xef\xbb\xbf\xef\xbb\xbf\n", + } if plan_state != "missing": plan.write_bytes(bodies[plan_state]) before = plan.stat().st_mtime_ns @@ -599,9 +628,9 @@ def test_plan_prerequisite_diagnostics( command_fn = _bash_cmd if port == "bash" else _py_cmd command = command_fn(repo, *(("--json",) if json_mode else ())) - result = _run(command, repo) + result = _run(command, repo, env={**_clean_env(), "LC_ALL": "C"}) - if plan_state in {"missing", "empty"}: + if plan_state in {"missing", "empty"} or plan_state.startswith("whitespace"): assert result.returncode == 1 assert result.stdout == "" lines = result.stderr.replace("\\", "/").splitlines() @@ -626,6 +655,47 @@ def test_plan_prerequisite_diagnostics( assert plan.stat().st_mtime_ns == before +@requires_bash +@pytest.mark.skipif(os.name == "nt", reason="requires POSIX read permissions") +@pytest.mark.parametrize("json_mode", [True, False], ids=["json", "text"]) +def test_bash_unreadable_plan_is_not_reported_as_empty( + prereq_repo: Path, json_mode: bool +) -> None: + _write_feature_json(prereq_repo) + feature = prereq_repo / "specs" / "001-my-feature" + feature.mkdir(parents=True) + plan = feature / "plan.md" + content = b"# Existing implementation plan\n" + plan.write_bytes(content) + before = plan.stat().st_mtime_ns + mode = plan.stat().st_mode + args = ("--json",) if json_mode else () + + try: + plan.chmod(0) + if os.access(plan, os.R_OK): + pytest.skip("current user can read files with no read permissions") + + result = _run(_bash_cmd(prereq_repo, *args), prereq_repo) + paths_only = _run( + _bash_cmd(prereq_repo, *args, "--paths-only"), prereq_repo + ) + finally: + plan.chmod(mode) + + assert result.returncode == 1 + assert result.stdout == "" + assert result.stderr.splitlines() == [ + f"ERROR: plan.md is not readable in {feature}", + "Check its file permissions and rerun the prerequisite check.", + ] + assert paths_only.returncode == 0, paths_only.stderr + assert plan.read_bytes() == content + assert plan.stat().st_mtime_ns == before + readable = _run(_bash_cmd(prereq_repo, *args), prereq_repo) + assert readable.returncode == 0, readable.stderr + + def test_python_branch_falls_back_to_feature_dir_basename(prereq_repo: Path) -> None: (prereq_repo / "specs" / "001-my-feature").mkdir(parents=True) _write_feature_json(prereq_repo) diff --git a/tests/test_setup_plan_python_parity.py b/tests/test_setup_plan_python_parity.py index 0ac08cf626..00442552a7 100644 --- a/tests/test_setup_plan_python_parity.py +++ b/tests/test_setup_plan_python_parity.py @@ -62,7 +62,14 @@ def repo(tmp_path: Path) -> Path: @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize("json_mode", [True, False], ids=["json", "text"]) -@pytest.mark.parametrize("template_state", ["missing", "empty", "empty_override"]) +@pytest.mark.parametrize( + "template_state", + [ + "missing", "empty", "empty_override", "whitespace", "whitespace_override", + "whitespace_nbsp", "whitespace_bom", "whitespace_bom_override", + "whitespace_bom_nbsp", "whitespace_unicode", + ], +) def test_plan_template_failure_and_retry( tmp_path: Path, port: str, json_mode: bool, template_state: str ) -> None: @@ -70,15 +77,26 @@ def test_plan_template_failure_and_retry( template = repo / ".specify" / "templates" / "plan-template.md" if template_state != "missing": template.parent.mkdir(parents=True) - template.write_bytes(b"") - if template_state == "empty_override": + bodies = { + "whitespace": b" \t\r\n\v\f\n", + "whitespace_nbsp": "\u00a0\n".encode(), + "whitespace_bom": b"\xef\xbb\xbf \t\r\n", + "whitespace_bom_nbsp": "\ufeff\u00a0\n".encode(), + "whitespace_unicode": ( + "\u0085\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005" + "\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\n" + ).encode(), + } + body = bodies.get(template_state.removesuffix("_override"), b"") + template.write_bytes(body) + if template_state.endswith("_override"): template.write_bytes(TEMPLATE_BODY.encode("utf-8")) template = template.parent / "overrides" / template.name template.parent.mkdir() - template.write_bytes(b"") + template.write_bytes(body) plan = repo / "specs" / "001-my-feature" / "plan.md" - result = run(_command(repo, port, json_mode), repo) + result = run(_command(repo, port, json_mode), repo, env={**clean_env(), "LC_ALL": "C"}) diagnostic = "not found" if template_state == "missing" else "is empty" assert result.returncode == 1 @@ -106,7 +124,11 @@ def test_plan_template_failure_and_retry( @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize("json_mode", [True, False], ids=["json", "text"]) @pytest.mark.parametrize("template_state", ["missing", "empty", "valid"]) -@pytest.mark.parametrize("body", [b"", b"# Existing plan\n"], ids=["empty", "nonempty"]) +@pytest.mark.parametrize( + "body", + [b"", b"# Existing plan\n", b" \t\r\n"], + ids=["empty", "nonempty", "whitespace"], +) def test_existing_plan_is_preserved( tmp_path: Path, port: str, json_mode: bool, template_state: str, body: bytes ) -> None: @@ -133,16 +155,29 @@ def test_existing_plan_is_preserved( @pytest.mark.parametrize("port", PORTS) @pytest.mark.parametrize("json_mode", [True, False], ids=["json", "text"]) -def test_whitespace_plan_template_is_not_empty( - repo: Path, port: str, json_mode: bool +@pytest.mark.parametrize( + "body", + [ + b" \t\n# Plan Template\n\nBody.\n \t\n", + b"\xef\xbb\xbf \t\n# Plan Template\n\nBody.\n", + "\u00a0# Plan Template\n\u3000".encode(), + b"\x1c", + b"\n\xef\xbb\xbf\n", + b"\xef\xbb\xbf\xef\xbb\xbf\n", + ], + ids=["ascii", "bom", "unicode", "non_whitespace_control", "internal_bom", "double_bom"], +) +def test_plan_template_with_padded_content_is_preserved( + repo: Path, port: str, json_mode: bool, body: bytes ) -> None: - body = b" \t\n\n" (repo / ".specify" / "templates" / "plan-template.md").write_bytes(body) - result = run(_command(repo, port, json_mode), repo) + result = run(_command(repo, port, json_mode), repo, env={**clean_env(), "LC_ALL": "C"}) assert result.returncode == 0, result.stderr - assert (repo / "specs" / "001-my-feature" / "plan.md").read_bytes() == body + # PowerShell's existing resolver consumes the file BOM before returning text. + expected = body.removeprefix(b"\xef\xbb\xbf") if port == "powershell" else body + assert (repo / "specs" / "001-my-feature" / "plan.md").read_bytes() == expected @requires_bash