From cbda91f51e34d5d5e1189da3809d34087bb56d0f Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Fri, 3 Jul 2026 10:44:49 +0500 Subject: [PATCH] fix(scripts): resolve invoke_separator by parse success, add awk fallback (#3304) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_invoke_separator` in common.sh selected its JSON parser by tool *availability* (`command -v python3`) rather than parse *success*, and had no text fallback after the python3 branch. On stock Windows + Git Bash — no jq, and `python3` resolving to the Microsoft Store App Execution Alias stub that passes `command -v` but exits 49 at runtime — it silently fell back to "." even for `-`-separator integrations (e.g. forge, cline). The observable result was wrong command hints in error messages, such as `/speckit.plan` instead of `/speckit-plan`, in check-prerequisites.sh and setup-tasks.sh. This is the same existence-vs-runtime pattern fixed for the feature.json parser in #3304's primary report; the reporter explicitly asked that other python3 call sites be checked. The two remaining sites (resolve_template, resolve_template_content) already fall through on failure and are unaffected. - Restructure the jq -> python3 chain to fall through on parse failure, gated on a `parsed` success flag rather than exclusive elif branches. - Make the python3 branch signal failure (sys.exit(1)) instead of printing "." so a stub failure falls through instead of being accepted. - Add an awk text fallback (portable, no gawk-only whole-file slurp) that reads the active integration key and its invoke_separator, handling both pretty-printed (the written form) and compact JSON. Malformed input safely defaults to ".". Regression test simulates the broken-stub environment (jq + python3 stubs that exit 49 on PATH) and asserts the `-` separator is still recovered. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/bash/common.sh | 63 ++++++++++++++++++++++++++++++++------- tests/test_setup_tasks.py | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 11 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 609729cbfa..0b23e7e2ce 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -235,21 +235,29 @@ get_invoke_separator() { local integration_json="$repo_root/.specify/integration.json" local separator="." - local parsed_with_jq=0 + local parsed=0 if [[ -f "$integration_json" ]]; then + # Try parsers in order (jq -> python3 -> awk), 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). An availability-gated branch would pick python3, swallow + # its failure, and — because this function historically had no text + # fallback — silently return "." even for `-`-separator integrations + # (e.g. forge, cline), yielding wrong command hints (issue #3304). if command -v jq >/dev/null 2>&1; then local jq_separator if jq_separator=$(jq -r '(.default_integration // .integration // "") as $k | if $k == "" then "." else (.integration_settings[$k].invoke_separator // ".") end' "$integration_json" 2>/dev/null); then - parsed_with_jq=1 case "$jq_separator" in - "."|"-") separator="$jq_separator" ;; + "."|"-") separator="$jq_separator"; parsed=1 ;; esac fi fi - if [[ "$parsed_with_jq" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then - if separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null + if [[ "$parsed" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then + local py_separator + if py_separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null import json import sys @@ -265,17 +273,50 @@ try: separator = entry["invoke_separator"] print(separator) except Exception: - print(".") + sys.exit(1) PY ); then - case "$separator" in - "."|"-") ;; - *) separator="." ;; + case "$py_separator" in + "."|"-") separator="$py_separator"; parsed=1 ;; esac - else - separator="." fi fi + + if [[ "$parsed" -eq 0 ]]; then + # Last-resort text fallback for environments with neither jq nor a + # working python3 (e.g. stock Windows + Git Bash). Reads the active + # integration key (default_integration, else integration) and its + # invoke_separator from within the integration_settings object. + # Handles both pretty-printed (the written form) and compact JSON. + # Accumulate all lines into one buffer in END rather than using + # gawk-only whole-file slurp (RS="^$"), so this stays portable to + # the BSD awk on macOS. + local awk_separator + awk_separator=$(awk ' + function keyval(d, name, v) { + if (match(d, "\"" name "\"[ \t\r\n]*:[ \t\r\n]*\"[^\"]*\"")) { + v=substr(d,RSTART,RLENGTH); sub(/^.*:[ \t\r\n]*"/,"",v); sub(/"$/,"",v); return v + } + return "" + } + { doc = doc $0 "\n" } + END { + key=keyval(doc,"default_integration"); if (key=="") key=keyval(doc,"integration") + sep="." + if (key!="" && match(doc, "\"" key "\"[ \t\r\n]*:[ \t\r\n]*[{]")) { + rest=substr(doc, RSTART+RLENGTH-1) + if (match(rest, /"invoke_separator"[ \t\r\n]*:[ \t\r\n]*"[.-]"/)) { + tok=substr(rest,RSTART,RLENGTH); s=substr(tok,length(tok)-1,1) + if (s=="." || s=="-") sep=s + } + } + print sep + } + ' "$integration_json" 2>/dev/null) + case "$awk_separator" in + "."|"-") separator="$awk_separator" ;; + esac + fi fi _SPECIFY_INVOKE_SEPARATOR_CACHE_REPO_ROOT="$repo_root" diff --git a/tests/test_setup_tasks.py b/tests/test_setup_tasks.py index 47a284f8a0..df86e5ece8 100644 --- a/tests/test_setup_tasks.py +++ b/tests/test_setup_tasks.py @@ -466,6 +466,68 @@ def test_bash_command_hint_preserves_hyphens_inside_segments(tasks_repo: Path) - assert result.stdout.strip() == "/speckit.jira.sync-status" +def _install_broken_json_tool_stubs(repo: Path) -> Path: + """Create a bin dir with `jq` and `python3` stubs that exist but fail. + + Mimics stock Windows + Git Bash, where `jq` is absent and `python3` + resolves to the Microsoft Store App Execution Alias stub: both satisfy + `command -v` yet fail at runtime (the alias exits 49). Prepending this to + PATH forces the invoke-separator parser past jq and python3 to its awk + text fallback (#3304). + """ + stub_dir = repo / "_broken_bin" + stub_dir.mkdir(exist_ok=True) + for name in ("jq", "python3"): + stub = stub_dir / name + stub.write_text( + "#!/bin/sh\n" + 'echo "simulated broken interpreter/tool" >&2\n' + "exit 49\n", + encoding="utf-8", + newline="\n", + ) + stub.chmod(0o755) + return stub_dir + + +@requires_bash +def test_bash_command_hint_falls_back_to_awk_when_jq_and_python3_broken( + tasks_repo: Path, +) -> None: + """Separator resolution survives a broken python3 stub with no jq (#3304). + + `get_invoke_separator` historically selected python3 by availability and + had no text fallback, so a Windows Store python3 stub made it silently + return "." even for `-`-separator integrations (e.g. forge), yielding a + wrong hint like `/speckit.plan`. The awk fallback must recover `-`. + """ + _write_integration_state(tasks_repo, "forge", "-") + stub_dir = _install_broken_json_tool_stubs(tasks_repo) + + script = tasks_repo / ".specify" / "scripts" / "bash" / "common.sh" + env = _clean_env() + env["PATH"] = f"{stub_dir}{os.pathsep}{env.get('PATH', '')}" + + result = subprocess.run( + [ + "bash", + "-c", + 'source "$1"; format_speckit_command "$2" "$PWD"', + "bash", + str(script), + "plan", + ], + cwd=tasks_repo, + capture_output=True, + text=True, + check=False, + env=env, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "/speckit-plan" + + @requires_bash def test_bash_command_hint_caches_invoke_separator_per_process(tasks_repo: Path) -> None: _write_integration_state(tasks_repo, "claude", "-")