Skip to content

Commit 8eadcd7

Browse files
Noor-ul-ain001claudeCopilot
authored
fix(scripts): resolve invoke_separator by parse success, not python3 availability (#3304) (#3320)
* fix(scripts): resolve invoke_separator by parse success, add awk fallback (#3304) `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) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(scripts): close awk END block so invoke_separator fallback works The awk text fallback in get_invoke_separator was missing the closing brace for its END block, so awk aborted with a syntax error on every invocation and the separator silently stayed at the default '.'. On environments with neither jq nor a working python3 (stock Windows + Git Bash, the exact case this fallback exists for) a '-'-separator integration like forge produced a wrong command hint. The bug escaped CI because the test that exercises it is gated on working bash and skips on the Windows dev shell. Also harden the fallback per review: use the portable '[-.]' character class (a leading '-' inside '[]' can be read as an ill-defined range on some awk builds), and correct the test docstring, which said 'with no jq' though the helper installs a present-but-failing jq stub. Addresses Copilot review feedback on PR #3320. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 892dd65 commit 8eadcd7

2 files changed

Lines changed: 128 additions & 11 deletions

File tree

scripts/bash/common.sh

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -244,21 +244,29 @@ get_invoke_separator() {
244244

245245
local integration_json="$repo_root/.specify/integration.json"
246246
local separator="."
247-
local parsed_with_jq=0
247+
local parsed=0
248248

249249
if [[ -f "$integration_json" ]]; then
250+
# Try parsers in order (jq -> python3 -> awk), falling through on
251+
# failure. Selection is by *parse success*, not mere availability: on
252+
# Windows `python3` commonly resolves to the Microsoft Store App
253+
# Execution Alias stub, which passes `command -v` but fails at runtime
254+
# (exit 49). An availability-gated branch would pick python3, swallow
255+
# its failure, and — because this function historically had no text
256+
# fallback — silently return "." even for `-`-separator integrations
257+
# (e.g. forge, cline), yielding wrong command hints (issue #3304).
250258
if command -v jq >/dev/null 2>&1; then
251259
local jq_separator
252260
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
253-
parsed_with_jq=1
254261
case "$jq_separator" in
255-
"."|"-") separator="$jq_separator" ;;
262+
"."|"-") separator="$jq_separator"; parsed=1 ;;
256263
esac
257264
fi
258265
fi
259266

260-
if [[ "$parsed_with_jq" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then
261-
if separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null
267+
if [[ "$parsed" -eq 0 ]] && command -v python3 >/dev/null 2>&1; then
268+
local py_separator
269+
if py_separator=$(python3 - "$integration_json" <<'PY' 2>/dev/null
262270
import json
263271
import sys
264272
@@ -274,17 +282,64 @@ try:
274282
separator = entry["invoke_separator"]
275283
print(separator)
276284
except Exception:
277-
print(".")
285+
sys.exit(1)
278286
PY
279287
); then
280-
case "$separator" in
281-
"."|"-") ;;
282-
*) separator="." ;;
288+
case "$py_separator" in
289+
"."|"-") separator="$py_separator"; parsed=1 ;;
283290
esac
284-
else
285-
separator="."
286291
fi
287292
fi
293+
294+
if [[ "$parsed" -eq 0 ]]; then
295+
# Last-resort text fallback for environments with neither jq nor a
296+
# working python3 (e.g. stock Windows + Git Bash). Reads the active
297+
# integration key (default_integration, else integration) and its
298+
# invoke_separator from within the integration_settings object.
299+
# Handles both pretty-printed (the written form) and compact JSON.
300+
# Accumulate all lines into one buffer in END rather than using
301+
# gawk-only whole-file slurp (RS="^$"), so this stays portable to
302+
# the BSD awk on macOS.
303+
local awk_separator
304+
awk_separator=$(awk '
305+
function keyval(d, name, v) {
306+
if (match(d, "\"" name "\"[ \t\r\n]*:[ \t\r\n]*\"[^\"]*\"")) {
307+
v=substr(d,RSTART,RLENGTH); sub(/^.*:[ \t\r\n]*"/,"",v); sub(/"$/,"",v); return v
308+
}
309+
return ""
310+
}
311+
{ doc = doc $0 "\n" }
312+
END {
313+
key=keyval(doc,"default_integration"); if (key=="") key=keyval(doc,"integration")
314+
sep="."
315+
if (key!="") {
316+
settings=doc
317+
if (match(doc, /"integration_settings"[ \t\r\n]*:[ \t\r\n]*[{]/)) {
318+
settings=substr(doc, RSTART+RLENGTH-1)
319+
}
320+
if (match(settings, "\"" key "\"[ \t\r\n]*:[ \t\r\n]*[{]")) {
321+
start=RSTART+RLENGTH-1
322+
depth=0
323+
obj=""
324+
for (i=start; i<=length(settings); i++) {
325+
c=substr(settings,i,1)
326+
obj=obj c
327+
if (c=="{") depth++
328+
else if (c=="}") { depth--; if (depth==0) break }
329+
}
330+
if (match(obj, /"invoke_separator"[ \t\r\n]*:[ \t\r\n]*"[-.]"/)) {
331+
tok=substr(obj,RSTART,RLENGTH); s=substr(tok,length(tok)-1,1)
332+
if (s=="." || s=="-") sep=s
333+
}
334+
}
335+
}
336+
print sep
337+
}
338+
' "$integration_json" 2>/dev/null)
339+
case "$awk_separator" in
340+
"."|"-") separator="$awk_separator" ;;
341+
esac
342+
fi
288343
fi
289344

290345
_SPECIFY_INVOKE_SEPARATOR_CACHE_REPO_ROOT="$repo_root"

tests/test_setup_tasks.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,68 @@ def test_bash_command_hint_preserves_hyphens_inside_segments(tasks_repo: Path) -
466466
assert result.stdout.strip() == "/speckit.jira.sync-status"
467467

468468

469+
def _install_broken_json_tool_stubs(repo: Path) -> Path:
470+
"""Create a bin dir with `jq` and `python3` stubs that exist but fail.
471+
472+
Mimics stock Windows + Git Bash, where a JSON tool may be missing or broken
473+
and `python3` resolves to the Microsoft Store App Execution Alias stub: both
474+
satisfy `command -v` yet fail at runtime (the alias exits 49). Prepending
475+
this to PATH forces the invoke-separator parser past jq and python3 to its
476+
awk text fallback (#3304).
477+
"""
478+
stub_dir = repo / "_broken_bin"
479+
stub_dir.mkdir(exist_ok=True)
480+
for name in ("jq", "python3"):
481+
stub = stub_dir / name
482+
stub.write_text(
483+
"#!/bin/sh\n"
484+
'echo "simulated broken interpreter/tool" >&2\n'
485+
"exit 49\n",
486+
encoding="utf-8",
487+
newline="\n",
488+
)
489+
stub.chmod(0o755)
490+
return stub_dir
491+
492+
493+
@requires_bash
494+
def test_bash_command_hint_falls_back_to_awk_when_jq_and_python3_broken(
495+
tasks_repo: Path,
496+
) -> None:
497+
"""Separator resolution survives broken jq and python3 stubs (#3304).
498+
499+
`get_invoke_separator` historically selected python3 by availability and
500+
had no text fallback, so a Windows Store python3 stub made it silently
501+
return "." even for `-`-separator integrations (e.g. forge), yielding a
502+
wrong hint like `/speckit.plan`. The awk fallback must recover `-`.
503+
"""
504+
_write_integration_state(tasks_repo, "forge", "-")
505+
stub_dir = _install_broken_json_tool_stubs(tasks_repo)
506+
507+
script = tasks_repo / ".specify" / "scripts" / "bash" / "common.sh"
508+
env = _clean_env()
509+
env["PATH"] = f"{stub_dir}{os.pathsep}{env.get('PATH', '')}"
510+
511+
result = subprocess.run(
512+
[
513+
"bash",
514+
"-c",
515+
'source "$1"; format_speckit_command "$2" "$PWD"',
516+
"bash",
517+
str(script),
518+
"plan",
519+
],
520+
cwd=tasks_repo,
521+
capture_output=True,
522+
text=True,
523+
check=False,
524+
env=env,
525+
)
526+
527+
assert result.returncode == 0, result.stderr
528+
assert result.stdout.strip() == "/speckit-plan"
529+
530+
469531
@requires_bash
470532
def test_bash_command_hint_caches_invoke_separator_per_process(tasks_repo: Path) -> None:
471533
_write_integration_state(tasks_repo, "claude", "-")

0 commit comments

Comments
 (0)