mirror of
https://github.com/github/spec-kit.git
synced 2026-07-08 23:59:55 +08:00
fix(scripts): fall through to grep/sed when python3 is a broken stub in feature.json parser (#3312)
* fix(scripts): fall through to grep/sed when python3 is a broken stub in feature.json parser read_feature_json_feature_directory picked its json parser by availability (if jq / elif python3 / else grep-sed). on windows `python3` usually resolves to the microsoft store app execution alias stub: it satisfies `command -v` but fails at runtime (exit 49). the elif selected it, the runtime failure was swallowed to _fd='', and the grep/sed last resort was never reached - so a valid .specify/feature.json read as empty and every setup-plan / setup-tasks / check-prerequisites call errored with "Feature directory not found" right after a successful `specify init --script sh`. change selection from availability to parse success: try jq, then python3 only if still empty, then grep/sed only if still empty. a parser that exists but produces nothing now falls through instead of terminating the chain. the write path (_persist_feature_json) already uses jq-or-printf with no python3, so it was unaffected; only the read path needed this. add a regression test that puts a python3 stub (exit 49, like the store alias) first on PATH and asserts setup-plan.sh still resolves the feature via the grep/sed fallback. fixes #3304 * test: shadow jq so the broken-python3 fallback test actually exercises it the test claimed it dropped jq so the parser chain would reach python3 and then grep/sed, but it only prepended the python3 stub dir to PATH. on a runner with jq installed, read_feature_json_feature_directory parses via jq and never reaches the fallback the test is meant to cover. add a failing jq stub alongside the python3 stub so the chain is forced through jq -> python3 -> grep/sed regardless of what the runner has installed.
This commit is contained in:
@@ -97,17 +97,26 @@ read_feature_json_feature_directory() {
|
||||
local fj="$repo_root/.specify/feature.json"
|
||||
[[ -f "$fj" ]] || { printf '%s' ''; return 0; }
|
||||
|
||||
# Try parsers in order (jq -> python3 -> grep/sed), falling through on
|
||||
# failure. Selection is by *parse success*, not mere availability: on
|
||||
# Windows `python3` commonly resolves to the Microsoft Store App Execution
|
||||
# Alias stub, which passes `command -v` but fails at runtime (exit 49), so
|
||||
# an availability-gated `elif` would pick python3, swallow its failure, and
|
||||
# never reach the grep/sed fallback -- leaving feature.json unreadable even
|
||||
# though it is valid (issue #3304).
|
||||
local _fd=''
|
||||
if command -v jq >/dev/null 2>&1; then
|
||||
if ! _fd=$(jq -r '.feature_directory // empty' "$fj" 2>/dev/null); then
|
||||
_fd=''
|
||||
fi
|
||||
elif command -v python3 >/dev/null 2>&1; then
|
||||
fi
|
||||
if [[ -z "$_fd" ]] && command -v python3 >/dev/null 2>&1; then
|
||||
# Use Python so pretty-printed/multi-line JSON still parses correctly.
|
||||
if ! _fd=$(python3 -c "import json,sys; d=json.load(open(sys.argv[1])); v=d.get('feature_directory'); print(v if v else '')" "$fj" 2>/dev/null); then
|
||||
_fd=''
|
||||
fi
|
||||
else
|
||||
fi
|
||||
if [[ -z "$_fd" ]]; then
|
||||
# Last-resort single-line grep/sed fallback. The `|| true` guards against
|
||||
# grep returning 1 (no match) aborting under `set -e` / `pipefail`.
|
||||
_fd=$( { grep -E '"feature_directory"[[:space:]]*:' "$fj" 2>/dev/null || true; } \
|
||||
|
||||
@@ -126,6 +126,71 @@ def test_setup_plan_errors_without_feature_context(plan_repo: Path) -> None:
|
||||
assert "Feature directory not found" in result.stderr
|
||||
|
||||
|
||||
@requires_bash
|
||||
def test_setup_plan_survives_broken_python3_stub(plan_repo: Path) -> None:
|
||||
"""A `python3` on PATH that exists but fails at runtime must not defeat
|
||||
feature.json parsing.
|
||||
|
||||
On Windows `python3` typically resolves to the Microsoft Store App Execution
|
||||
Alias stub: it satisfies `command -v python3` yet exits non-zero at runtime.
|
||||
The parser must fall through to the grep/sed fallback on that failure instead
|
||||
of selecting python3 by mere availability and swallowing its error (#3304).
|
||||
"""
|
||||
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")
|
||||
_write_feature_json(plan_repo, "specs/001-tiny-notes-app")
|
||||
|
||||
# A stub python3 that mimics the Windows Store alias: on PATH, exits 49.
|
||||
stub_dir = plan_repo / "_stubbin"
|
||||
stub_dir.mkdir()
|
||||
stub = stub_dir / "python3"
|
||||
stub.write_text(
|
||||
"#!/bin/sh\n"
|
||||
'echo "Python was not found; run without arguments to install from the '
|
||||
'Microsoft Store" >&2\n'
|
||||
"exit 49\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
stub.chmod(0o755)
|
||||
|
||||
# A stub jq that shadows any real jq on PATH and also fails, so the parser
|
||||
# cannot short-circuit on jq and must reach the broken python3 stub and then
|
||||
# fall through to grep/sed. Without this, a runner that has jq installed
|
||||
# would parse feature.json via jq and never exercise the fallback this test
|
||||
# is meant to cover.
|
||||
jq_stub = stub_dir / "jq"
|
||||
jq_stub.write_text(
|
||||
"#!/bin/sh\n"
|
||||
'echo "jq: simulated failure" >&2\n'
|
||||
"exit 1\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
jq_stub.chmod(0o755)
|
||||
|
||||
env = _clean_env()
|
||||
# Prepend the stub dir so the failing jq and python3 stubs take precedence
|
||||
# over any real ones; PATH still needs the real bash utilities for grep/sed.
|
||||
env["PATH"] = f"{stub_dir}{os.pathsep}{env.get('PATH', '')}"
|
||||
|
||||
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,
|
||||
env=env,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr + result.stdout
|
||||
assert (feat / "plan.md").is_file()
|
||||
|
||||
|
||||
@requires_bash
|
||||
def test_setup_plan_numbered_branch_works_with_feature_json(
|
||||
plan_repo: Path,
|
||||
|
||||
Reference in New Issue
Block a user